Adds RESP3 to protocol specification

[itamarhaber]

Fixes #1920

[netlify]

❌ Deploy Preview for redis-doc failed.

Name	Link
🔨 Latest commit	03e1905
🔍 Latest deploy log	https://app.netlify.com/sites/redis-doc/deploys/64db825b897628000816c4da

[nermiller]

LGTM with some minor edits

[zuiderkwast]

Nice! If it takes a long time to handle all the review comments, I suggest merging it as is and handling the comments afterwards in a separate PR. Btw, I do agree with most of the comments. But this specification is missing since forever and it'd be better to have something in the meantime IMO.

[nermiller]

@itamarhaber since this is an important change, mind resolving the conflicts and merging? We can take care of the feedback later, as suggested by @zuiderkwast.

[zuiderkwast]

@itamarhaber Ready to merge?

[itamarhaber]

@zuiderkwast this isn't ready to merge, please wait.

[zuiderkwast]

Just a few comments on the last commit about floats.

[itamarhaber]

@mgravell @zuiderkwast @leibale make this beefier if you can :)

[zuiderkwast]

Interesting. I've never used this. It seems somehow more strait-forward to use ZADD k 6.379e-5 foo; ZSCORE k foo to test doubles. The Lua way seems more complicated in the sense that it requires knowledge of how to return values from Lua (which is weird).

For this to be useful, perhaps include Lua examples for all types here?

Btw, the anchor link to the section within the page is lost due to redirects. The direct link nowdays seems to be https://redis.io/docs/manual/programmability/lua-api/#lua-to-resp3-type-conversion

[mgravell]

@itamarhaber first time I've found that one set of conversions (lua-to-resp3-type-conversion); very useful! but: is there some server version context needed in there? testing shows a few oddities:

• return true gives me integer with value 1 on both RESP2 and RESP3; I'm testing on a v6 server - is this a v7 change?
• return false acts the same as return nil on both RESP2 and RESP3; I'm testing on a v6 server - is this a v7 change?
• return { double = 42 } gives me double on RESP3, but gives me bulk string on RESP2; this seems odd, since in RESP3.md, double is defined as a variant of simple-string, not bulk-string (i.e. no length prefix) - intentional?

[itamarhaber]

@mgravell

first time I've found...

TBAH, same here 😆

return true, return false... is this a v7 change?

This is a conscious decision made to minimize immediate script breakage.
Once we feel most have moved to RESP3, we can debate making the breaking change.
It exists both in v6 as well as in v7.
One has to both use a RESP3 connection && instruct Lua to return RESP3 like so:

EVAL "redis.setresp(3) return true" 0

return { double = 42 }...since in RESP3.md...

I think this is the same thing, here's a comparison between old and new:

127.0.0.1:6379> HELLO 2
...
127.0.0.1:6379> ZADD z 42.1 foo
(integer) 1
127.0.0.1:6379> ZRANGE z 0 -1  WITHSCORES
1) "foo"
2) "42.100000000000001"
127.0.0.1:6379> EVAL "return redis.call('ZRANGE', 'z', 0, -1, 'WITHSCORES')" 0
1) "foo"
2) "42.100000000000001"

# No one should really do this because why?
127.0.0.1:6379> EVAL "redis.setresp(3) return redis.call('ZRANGE', 'z', 0, -1, 'WITHSCORES')" 0
1) 1) "foo"
   2) "42.100000000000001"

127.0.0.1:6379> HELLO 3
...
127.0.0.1:6379> ZRANGE z 0 -1  WITHSCORES
1) 1) "foo"
   2) (double) 42.100000000000001
127.0.0.1:6379> EVAL "return redis.call('ZRANGE', 'z', 0, -1, 'WITHSCORES')" 0
1) "foo"
2) "42.100000000000001"
127.0.0.1:6379> EVAL "redis.setresp(3) return redis.call('ZRANGE', 'z', 0, -1, 'WITHSCORES')" 0
1) 1) "foo"
   2) (double) 42.100000000000001

[mgravell]

EVAL "redis.setresp(3) return true" 0

ah! confirmed, that works; however, it wasn't at all clear to me that setresp(3) would change this behavior! thanks

[jobol]

I have not read all the text but I think that the parts related to ASCII can be improved