Describe the bug

We are upgrading our infrastructure to redis 7.2 from 6.2 and our integration tests found an issue which we were able to narrow down to a behavior change in lua scripts.

We have a lua script that does the equivalent of redis.call("HINCRBY", "key", "field", tonumber(ARGV[1]))

This works fine in redis 6.2 for all values. Under redis 7.2, the HINCRBY throws ERR value is not an integer or out of range if and only if ARGV[1] is a value that matches the form n * 100,000,000.

To reproduce

The following node script can be run in node 20.x or above with the command line: node <redisUrl>.

When run, it will call one of two lua scripts that are tiny wrappers around HINCRBY. The first wrapper uses tonumber, the second does not.

import redis from '@redis/client'

async function main(argv) {
  const client = redis.createClient({
    url: argv[0],
    scripts: {
      badScript: redis.defineScript({
        NUMBER_OF_KEYS: 0,
        SCRIPT: 'redis.call("HINCRBY", "hash", "field", tonumber(ARGV[1]))',
        transformArguments: (delta) => [delta.toString()],
      }),
      goodScript: redis.defineScript({
        NUMBER_OF_KEYS: 0,
        SCRIPT: 'redis.call("HINCRBY", "hash", "field", ARGV[1])',
        transformArguments: (delta) => [delta.toString()],
      }),
    },
  })
  await client.connect()

  for (let i = 0; i <= 2_000_000_000; i += 10_000_000) {
    try {
      await client.goodScript(i)
      console.log('goodScript succeeded', i)
    } catch (e) {
      console.error('goodScript failed', i, e)
    }

    try {
      await client.badScript(i)
      console.log('badScript succeeded', i)
    } catch (e) {
      console.error('badScript failed', i, e)
    }
  }

  await client.quit()
}

await main(process.argv.slice(2))

Expected behavior

When run with redis 6.2, the script logs:

goodScript succeeded 1000000000
badScript succeeded 1000000000
goodScript succeeded 1050000000
badScript succeeded 1050000000
goodScript succeeded 1100000000
badScript succeeded 1100000000
goodScript succeeded 1150000000
badScript succeeded 1150000000
goodScript succeeded 1200000000
badScript succeeded 1200000000
goodScript succeeded 1250000000
badScript succeeded 1250000000
goodScript succeeded 1300000000
badScript succeeded 1300000000
goodScript succeeded 1350000000
badScript succeeded 1350000000
goodScript succeeded 1400000000
badScript succeeded 1400000000
goodScript succeeded 1450000000
badScript succeeded 1450000000
goodScript succeeded 1500000000
badScript succeeded 1500000000
goodScript succeeded 1550000000
badScript succeeded 1550000000
goodScript succeeded 1600000000
badScript succeeded 1600000000
goodScript succeeded 1650000000
badScript succeeded 1650000000
goodScript succeeded 1700000000
badScript succeeded 1700000000
goodScript succeeded 1750000000
badScript succeeded 1750000000
goodScript succeeded 1800000000
badScript succeeded 1800000000
goodScript succeeded 1850000000
badScript succeeded 1850000000
goodScript succeeded 1900000000
badScript succeeded 1900000000
goodScript succeeded 1950000000
badScript succeeded 1950000000
goodScript succeeded 2000000000
badScript succeeded 2000000000

Actual behavior

When run with redis 7.2, the script logs:

goodScript succeeded 1000000000
badScript failed 1000000000 [ErrorReply: ERR value is not an integer or out of range script: e3e7126938f9468dab2f183f7b9ced79ed09b16f, on @user_script:1.]
goodScript succeeded 1050000000
badScript succeeded 1050000000
goodScript succeeded 1100000000
badScript failed 1100000000 [ErrorReply: ERR value is not an integer or out of range script: e3e7126938f9468dab2f183f7b9ced79ed09b16f, on @user_script:1.]
goodScript succeeded 1150000000
badScript succeeded 1150000000
goodScript succeeded 1200000000
badScript failed 1200000000 [ErrorReply: ERR value is not an integer or out of range script: e3e7126938f9468dab2f183f7b9ced79ed09b16f, on @user_script:1.]
goodScript succeeded 1250000000
badScript succeeded 1250000000
goodScript succeeded 1300000000
badScript failed 1300000000 [ErrorReply: ERR value is not an integer or out of range script: e3e7126938f9468dab2f183f7b9ced79ed09b16f, on @user_script:1.]
goodScript succeeded 1350000000
badScript succeeded 1350000000
goodScript succeeded 1400000000
badScript failed 1400000000 [ErrorReply: ERR value is not an integer or out of range script: e3e7126938f9468dab2f183f7b9ced79ed09b16f, on @user_script:1.]
goodScript succeeded 1450000000
badScript succeeded 1450000000
goodScript succeeded 1500000000
badScript failed 1500000000 [ErrorReply: ERR value is not an integer or out of range script: e3e7126938f9468dab2f183f7b9ced79ed09b16f, on @user_script:1.]
goodScript succeeded 1550000000
badScript succeeded 1550000000
goodScript succeeded 1600000000
badScript failed 1600000000 [ErrorReply: ERR value is not an integer or out of range script: e3e7126938f9468dab2f183f7b9ced79ed09b16f, on @user_script:1.]
goodScript succeeded 1650000000
badScript succeeded 1650000000
goodScript succeeded 1700000000
badScript failed 1700000000 [ErrorReply: ERR value is not an integer or out of range script: e3e7126938f9468dab2f183f7b9ced79ed09b16f, on @user_script:1.]
goodScript succeeded 1750000000
badScript succeeded 1750000000
goodScript succeeded 1800000000
badScript failed 1800000000 [ErrorReply: ERR value is not an integer or out of range script: e3e7126938f9468dab2f183f7b9ced79ed09b16f, on @user_script:1.]
goodScript succeeded 1850000000
badScript succeeded 1850000000
goodScript succeeded 1900000000
badScript failed 1900000000 [ErrorReply: ERR value is not an integer or out of range script: e3e7126938f9468dab2f183f7b9ced79ed09b16f, on @user_script:1.]
goodScript succeeded 1950000000
badScript succeeded 1950000000
goodScript succeeded 2000000000
badScript failed 2000000000 [ErrorReply: ERR value is not an integer or out of range script: e3e7126938f9468dab2f183f7b9ced79ed09b16f, on @user_script:1.]

Additional information

Any additional information that is relevant to the problem.

Comment From: mdouglass

redis 7.0 behaves the same as redis 6.2, the error only appears in redis 7.2. For reference, these are the redis versions (though I'm just pulling 7.0 and 7.2 tags from docker).

good

redis_version:7.0.13
redis_build_id:8a3b90fcd3d0bc72
os:Linux 6.7.6-200.fc39.x86_64 x86_64

bad

redis_version:7.2.4
redis_build_id:30468499a8bc54fe
os:Linux 6.7.6-200.fc39.x86_64 x86_64

Comment From: sundb

@mdouglass thanks, ths is a bug introducted by #10587. fpconv_dtoa uses exponential expressions to convert 1000000000 (2e+8) and lead to convert failed in getLongLongFromObjectOrReply(). do you wanna make a PR to fix it?

Comment From: enjoy-binbin

yes, it is a bug introduced in #10587. so it looks like we should revert the changes in script_lua.c in #10587? or should we find a way to support converting it in string2ll (i feel it is a bit too much)

Comment From: sundb

@enjoy-binbin i don't like to put it in string2ll. may be we can check if the lua_Number is a integer by (long long)num == num, then decide whether to use fpconv_dtoa or ll2string.

Comment From: mdouglass

I think that was enough guidance that I was able to figure out what to do. Let me know if there is anything else that I can do to make this work better.