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

[BUG] TYPE command does not return a string in LUA #11413

Open
manast opened this issue Oct 20, 2022 · 6 comments
Open

[BUG] TYPE command does not return a string in LUA #11413

manast opened this issue Oct 20, 2022 · 6 comments

Comments

@manast
Copy link

manast commented Oct 20, 2022

Describe the bug
The TYPE command is supposed to return a string with the type of the key.

When called from a LUA script, the command does not return a string, it returns an empty table instead.

To reproduce

Just run this code and see the results:

127.0.0.1:6379> EVAL "redis.call(\"LPUSH\", \"dummy\", \"dummy\")\nlocal dummyType = redis.call(\"TYPE\", \"dummy\")\nredis.call(\"SET\", \"type\", type(dummyType))\nredis.call(\"SET\", \"len\", #dummyType)" 0
(nil)
127.0.0.1:6379> GET "type"
"table"
127.0.0.1:6379> GET "len"
"0"

Expected behavior

It should return a string, in this case the string "list"

Additional information

Works well if called directly on the redis-cli but not within a LUA script.

@enjoy-binbin
Copy link
Collaborator

This is indeed a very strange problem, and it look like it has always been there, i guess it is because TYPE did not return the "simple string" like GET

in this case, you can get the right result, by using dummyType['ok'] odd way

127.0.0.1:6379> EVAL "redis.call('LPUSH', 'dummy', 'dummy') local dummyType=redis.call('TYPE', 'dummy') redis.call('SET', 'type', dummyType['ok']) return dummyType" 0
list
127.0.0.1:6379> get type
"list"

let see what @MeirShpilraien would suggest, PTAL

@oranagra
Copy link
Member

i'd like to call @itamarhaber to take a look too

@manast
Copy link
Author

manast commented Oct 21, 2022

If the result is indeed in dummyType['ok'] and this issue has been there forever, maybe it is better to just document it instead of potentially breaking scripts?

I also found another inconsistent behavior, if you use redis.pcall on for example "ZADD", it returns an int if it succeeds and a table with 'err' if it fails. But this is not the same behavior as the example in the documentation where the command 'ECHO' is used, which always returns a table.

@MeirShpilraien
Copy link
Collaborator

As far as I understand, the idea was to distinguish between bulk string reply and simple string reply. I guess that bulk string reply are simply converted to Lua string because it is more common and simple string are returned as a Lua table with OK key. Maybe we can find a better way to distinguish them, but then we need to think about backward compatability.
I also believe this is already documented: https://redis.io/docs/manual/programmability/lua-api/#resp2-to-lua-type-conversion

@itamarhaber
Copy link
Member

Indeed - simple strings are converted from RESP to Lua as such, and TYPE's return is also documented.

I agree this isn't ideal, but it has been this way for a long time, so changing the behavior now isn't recommended.

@oranagra
Copy link
Member

@itamarhaber what do client libraries usually do with simple string return?
i understand that changing it is a breaking change, bug this feels a little awkward

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Needs triage
Development

No branches or pull requests

5 participants