feat(lua): add legacy-float flag for Lua 5.1 number compatibility

[vyavdoshenko]

Introduces a legacy-float script flag that makes cjson.decode compatible with Lua 5.1 behavior.

The core incompatibility:

redis-cli EVAL "local data = cjson.decode(ARGV[1]); return 'foo_'..data.id" 0 '{"id": 101}'

• Legacy (Lua 5.1): returns "foo_101"
• Dragonfly (Lua 5.4): returns "foo_101.0"

Libraries like django-cacheops depend on this behavior.

Solution:

Patched lua_cjson.c to check a Lua global variable __dfly_legacy_float__. When enabled, cjson.decode pushes integers instead of floats for whole numbers.

Usage via script header:

--!df flags=legacy-float
local obj = cjson.decode('{"id": 42}')
return tostring(obj.id)  -- returns "42" instead of "42.0"

Via command line (for all scripts):

dragonfly --default_lua_flags=legacy-float

Fixes: #6147

[augmentcode]

Review completed. 1 suggestion posted.

Comment augment review to trigger a new review at any time.

[romange]

looks good. @dranikpg PTAL as well

[romange]

it's a weird assert, what does it check?

[dranikpg]

the default values of parameters, but yeah its a little weird 😅

[romange]

do we have multiple scripts? I saw you changed only one.

[vyavdoshenko]

I reused the same list of django-cacheops mentioned above. Only one of them has the same hash as the master branch. I guess the hashes can be used from previous versions, and my assumption is that we have to make these scripts work.

[romange]

I think it's an ok solution. The core problem is around the following incompatibility against valkey:

redis-cli EVAL "local data = cjson.decode(ARGV[1]); return 'foo_'..data.id" 0 '{"id": 101}'

and I think we can solve it directly in cjson.decode

[romange]

specifically, I think you can do a fix in json_process_value in lua_cjson.c and do the similar check of floor(token->value.number) to decide either to push an integer or the number. This way cjson.decode will be fully compatible on integers.

[dranikpg]

the default values of parameters, but yeah its a little weird 😅

[dranikpg]

lets store the state and run it only if it changes

[dranikpg]

because this will decrease performance even for the normal path

[vyavdoshenko]

I changed the implementation. Please take a look.

[dranikpg]

maybe it would've been easier if it was a global flag, as I assume that if someones uses it, he'll use it for all scripts... but this is cleaner of course this way

[vyavdoshenko]

I think it's better to use it per script

[dranikpg]

It should have a much smaller performance impact now, and is cleaner

[romange]

I agree that now it's much cleaner. But I must ask @dranikpg @vyavdoshenko why not use lua_pushinteger unconditionally if the number is integer ?

[romange]

in other words, why do we need __dfly_legacy_float__ mode at all?

[vyavdoshenko]

The flag preserves user choice. Scripts migrated from legacy Lua 4.1 can opt into legacy behavior, while new scripts keep Lua 5.4 semantics. Making it unconditional removes that flexibility.

If you insist on making it unconditionally, I will fix it, but in my opinion, it is better to save float behavior for future scripts.