Lua scripting call redis.call('type', key) returns table #3231

Closed
josiahcarlson opened this Issue May 14, 2016 · 9 comments

Projects

None yet

5 participants

@josiahcarlson

There seems to be a bug with redis.call() inside Lua scripting when calling the type function. Specifically, it returns a table as a result instead of a string.

127.0.0.1:6379> set foo 1
OK
127.0.0.1:6379> eval "return redis.call('type', KEYS[1]) == 'string'" 1 foo
(nil)
127.0.0.1:6379> eval "return redis.call('type', KEYS[1]).ok == 'string'" 1 foo
(integer) 1
127.0.0.1:6379> eval "return redis.pcall('type', KEYS[1]).ok == 'string'" 1 foo
(integer) 1

I had expected that redis.call('type', KEYS[1]) would return a string. It might have previously done so, I'm not sure. I haven't gone back to check previous versions of Redis.

This is using vanilla Redis 3.2.0 from the repository, freshly built:

127.0.0.1:6379> info server
# Server
redis_version:3.2.0
redis_git_sha1:7ca8fbab
redis_git_dirty:0
redis_build_id:44ae4cf0f61a3eb0
redis_mode:standalone
os:Linux 3.16.0-30-generic x86_64
arch_bits:64
multiplexing_api:epoll
gcc_version:4.8.4
process_id:31489
run_id:c9a2a4dcf046f9c0564d20662f0a3a90cbfe9e45
tcp_port:6379
uptime_in_seconds:86242
uptime_in_days:0
hz:10
lru_clock:3586382
executable:/usr/local/bin/redis-server
config_file:/etc/redis/6379.conf

Please close if this is expected behavior.

@vitaliylag
vitaliylag commented May 26, 2016 edited

I guess that is bug but you can use this code to convert it to string:

if type(t) == "table" then
    for k, v in pairs(t) do t = v; end;
end;
@josiahcarlson

I am not looking for a workaround for bugs. I already have a workaround that is better and faster in every way: use redis.pcall() - whose expected return value is a table.

local typ = redis.pcall('TYPE', idx).ok
@vitaliylag
vitaliylag commented May 26, 2016 edited

Thanks, buf I guess pcall should return table only on errors, so better is:

if type(t) == "table" then t = t.ok; end;

or just

t = t.ok or t;

(both are using redis.call without t.err checking)

@josiahcarlson

Except that redis.pcall('type', 'key') should only fail if Redis itself was failing. This is a type-less function that doesn't even require the key to exist. Aside from memory corruption or system out of memory conditions (can't malloc, not that Redis hit user-defined limits), this shouldn't ever fail. And failing here isn't likely to be recoverable.

@itamarhaber
Contributor

The same happens with PING, for example, because simple strings are interpreted as status replies when pushed back to the Lua script (https://github.com/antirez/redis/blob/unstable/src/scripting.c#L133).

@josiahcarlson

You've got the inside scoop @itamarhaber, bug or feature? ;)

@dvirsky
Contributor
dvirsky commented May 29, 2016

I assumed it was a feature when I came across it a while ago. I admit it's not nice or intuitive, but it will break backwards compatibility to change it now.

@antirez
Owner
antirez commented May 31, 2016 edited

Hello! THis is actually a feature... It's an odd one but the Lua type system did not offer many choices. It is also a very documented feature! Since day one of Lua scripting, but the current scripting documentation in the EVAL man page is not exactly the best in order to read it all.

So why it is like that? Because Lua scripts can return strings in three different forms:

  1. Bulk strings.
  2. Simple strings (+OK and similar).
  3. Errors.

Lua should be able to return them all in their different forms. Normal clients don't really need to distinguish between 1 and 2, but Lua must, because if you do:

return redis.call("ping");

You really want it to reply with a +PING in the protocol.
So what I did was to map all the Redis types that mapped into Lua types directly, but then I was left with this two that did not map, that are 2 and 3 of the list above. Simple strings (previously called status replies) and Errors. The only way to model them was to use a Lua table.

@antirez antirez closed this May 31, 2016
@josiahcarlson

Okay, feature. Have worked around it. Thank you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment