Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

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

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

josiahcarlson opened this issue May 14, 2016 · 9 comments

Comments

@josiahcarlson
Copy link

@josiahcarlson josiahcarlson commented May 14, 2016

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
Copy link

@vitaliylag vitaliylag commented May 26, 2016

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
Copy link
Author

@josiahcarlson josiahcarlson commented May 26, 2016

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
Copy link

@vitaliylag vitaliylag commented May 26, 2016

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
Copy link
Author

@josiahcarlson josiahcarlson commented May 26, 2016

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
Copy link
Contributor

@itamarhaber itamarhaber commented May 27, 2016

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
Copy link
Author

@josiahcarlson josiahcarlson commented May 28, 2016

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

@dvirsky
Copy link
Contributor

@dvirsky 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
Copy link
Contributor

@antirez antirez commented May 31, 2016

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
Copy link
Author

@josiahcarlson josiahcarlson commented May 31, 2016

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

vstakhov added a commit to rspamd/rspamd that referenced this issue Jan 14, 2020
[Minor] bayes_expiry: Fix type check (redis/redis#3231)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.