From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <imun@tarantool.org>
Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id 86FBA46970E
 for <tarantool-patches@dev.tarantool.org>;
 Thu, 19 Dec 2019 00:44:21 +0300 (MSK)
Date: Thu, 19 Dec 2019 00:42:10 +0300
From: Igor Munkin <imun@tarantool.org>
Message-ID: <20191218214210.GX1214@tarantool.org>
References: <0e3de775c20f0e5776301a5b39fa1e22e0e8bdb7.1576060409.git.imeevma@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <0e3de775c20f0e5776301a5b39fa1e22e0e8bdb7.1576060409.git.imeevma@gmail.com>
Subject: Re: [Tarantool-patches] [PATCH v2 1/1] lua/serializer: encode Lua
 number -2^63 as integer
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: imeevma@tarantool.org
Cc: tarantool-patches@dev.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