tostring() non-strings for protocol encoding

[slact]

It's quite a nuisance to need to tostring() all the numbers and whatever other non-strings there may be in a command call when it can be done automatically, and the tostring() check is already present in the code.

[coveralls]

Coverage decreased (-0.5%) to 83.889% when pulling 59e17eaf850e42311c3b642ab9c34f374358754b on slact:protocol-encode-tostring into 677cb3ed27219b2cf7e08290926c1769d08ae3f2 on daurnimator:master.

[daurnimator]

tostring of a number has undefined format: you should always explicitly pick a serialisation.

[daurnimator]

That said, I'm happy for specific commands (that e.g. only take integers) to be implemented with a specific format specifier

[slact]

What do you mean by "underfined format"? It's guaranteed to be a parseable float, even +/-inf (except nan). The one possible concern is loss of precision. Is this the issue you have with opportunistic tostring()ing?

[daurnimator]

lua is free to make tostring(1) anything like: 1, 1.0, 1e0, 1.0e0, 0x1, etc. even stuff like 1,0 if you're e.g in a norwegian locale. Which is fine for debugging, but not great for a protocol

[slact]

I see your point. However, here's a list of lua clients that tostring() their multibulk values:

• redis-lua
• lua-hiredis
• openresty redis
• redis-luvit

So it looks like all the other (maintained) clients are tostring()ing their non-strings, and none error on on-strings.

[daurnimator]

I wouldn't take other lua clients as an example, I doubt they've fully considered it. More interesting would be to look at what the lua built into redis does.

[daurnimator]

Looks like redis internally uses %.17g for number => string converstions https://github.com/antirez/redis/blob/b44ad302d2337b45a02b6a3e4920089a48292c6d/src/scripting.c#L395

[slact]

their issue there was precision loss, not locale stuff. string.format is locale-dependent as well.

If you want, I can add the "%.17g" formatting for numbers.

[daurnimator]

their issue there was precision loss, not locale stuff. string.format is locale-dependent as well.

Right.... %.17g would fix the issues except for locale. We could use %a (i.e. hexfloats) if they weren't broken in lua 5.1 (they work in luajit, 5.2 and 5.3 though)

[slact]

The other issue with "%.17g" is there'd still need to be special cases for +/inf, whereas tostring() produces redis-compatible output. Hexfloats are a bad idea here because unless this is a value used for a zset score or an INCRBYFLOAT, it will be treated by Redis and stored as a string. It also means normal, human readable integers would be surprisingly stored in hex.

Basically, if you want to have convenience functions that accept tables for HMSET and such, it doesn't make sense to error out if some of the values in those tables are numbers -- just because some conversions may be lossy and floats are locale-dependent. It's quite standard to stringify arguments before passing them on to Redis (or MySQL or whatever db), rather than erroring out. But I'm guessing you're a strong-types guy and prefer correctness over convenience.