Re: [Tarantool-patches] [PATCH v2 1/1] lua/serializer: encode Lua number -2^63 as integer

[Igor Munkin <imun@tarantool.org>]

Mergen,

Thanks, the changes are totally LGTM. However, please consider the side
note I left below (I expect no changes in patch regarding it).

Cced Kirill to proceed with with the patch.

On 11.12.19, imeevma@tarantool.org wrote:

<snipped>

> 
> New patch:
> 
> From 0e3de775c20f0e5776301a5b39fa1e22e0e8bdb7 Mon Sep 17 00:00:00 2001
> From: Mergen Imeev <imeevma@gmail.com>
> Date: Fri, 6 Dec 2019 19:22:40 +0300
> Subject: [PATCH] lua/serializer: encode Lua number -2^63 as integer
> 
> This patch fixes a bug that appeared after commit
> 3a13be1de38e79b6b7e0e7a6fa81f4c59c379f4e ('lua: fix lua_tofield()
> for 2**64 value') .
> 
> Due to this bug, -2^63 was serialized as double, although it
> should be serialized as integer.
> 
> Closes #4672
> 

<snipped>

> +
> +    -- gh-4672: Make sure that -2^63 encoded as INTEGER.
> +    test:ok(s.encode(-9223372036854775808LL) == s.encode(-2^63),
> +            '-2^63 encoded as INTEGER')

Side note: After applying the following diff on your patch, tests are
still fine.
| diff --git a/test/app-tap/lua/serializer_test.lua b/test/app-tap/lua/serializer_test.lua
| index 038559756..7d3f62109 100644
| --- a/test/app-tap/lua/serializer_test.lua
| +++ b/test/app-tap/lua/serializer_test.lua
| @@ -145,7 +145,7 @@ local function test_signed(test, s)
|      rt(test, s, ffi.new('int', -128), 'number')
|
|      -- gh-4672: Make sure that -2^63 encoded as INTEGER.
| -    test:ok(s.encode(-9223372036854775808LL) == s.encode(-2^63),
| +    test:ok(s.encode(-9223372036854775808LL) == s.encode(-2^63 - 1),
|              '-2^63 encoded as INTEGER')
|  end
Yep, it's double precision issue, and we can do nothing here, but I
guess it ought to be well-documented, taking into account the patch
you've made.

-- 
Best regards,
IM