Invalid Lua script evaluation

[olizarevichroman]

Hi, i'm calling this simple Lua script passing Redis key to get string value, but key is passed to the ARGV collection. Also i've noticed that my key passed twice, first as an argument and then as a key. Profiler log provided below

var script = "return redis.call('get', @settingIdKey)");
var preparedScript = LuaScript.Prepare(script);

var redisResult = await database.ScriptEvaluateAsync(preparedScript, new
{
    settingIdKey = (RedisKey)"my key",
});

Profiler log:

"SCRIPT" "LOAD" "return redis.call('get', ARGV[1])"
"EVAL" "return redis.call('get', ARGV[1])" "1" "my key" "my key"
[0 lua] "get" "my key"

Based on the documentation it should treat it as a key

[mgravell]

Indeed, this is a side-effect of the current auto-parameterization implementation. If you want fine-grained control over the semantics, I suggest using the simple ScriptEvaluate API and passing the args/keys explicitly.

[olizarevichroman]

Is it supposed to be fixed? Or this "side-effect" is expected behavior?

[madelson]

Is this not just broken as-is?

[mgravell]

Suboptimal: yes. Broken: no. Unless I'm missing something, it is correctly routed and all values used are correct. Keys are generally short, so duplicating the keys isn't pathologically bad. But yes, it could be improved. Honestly, this whole API could be improved - I'll add to my backlog to review it.

[madelson]

@mgravell I had someone report that the scripts weren't working in their setup because it references all keys through ARGV instead of KEYS (see https://github.com/madelson/DistributedLock/issues/254). I think I've seen the same with an AWS Elasticache cluster but I'm not sure.

[mgravell]

OK, that's useful context. I'll see if we can take another look. A reliable workaround is to simply use naked Lua rather than the rewriter - but: it is an API that exists, so should either a: work as intended, or b: be deprecated. If you have a reliable repro (of it actually failing, rather than just being soboptimal), that would probably speed me up here, @madelson