From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id B559822E24 for ; Tue, 6 Aug 2019 18:02:20 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id p5NNeOTu8fAj for ; Tue, 6 Aug 2019 18:02:20 -0400 (EDT) Received: from smtp32.i.mail.ru (smtp32.i.mail.ru [94.100.177.92]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id F01DF22DCA for ; Tue, 6 Aug 2019 18:02:19 -0400 (EDT) From: Nikita Pettik Subject: [tarantool-patches] [PATCH 2/2] sql: use double_compare_uint64() for int<->float cmp Date: Wed, 7 Aug 2019 01:02:15 +0300 Message-Id: In-Reply-To: References: In-Reply-To: References: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org Cc: v.shpilevoy@tarantool.org, Nikita Pettik To compare floating point values and integers in SQL functions compare_uint_float() and compare_int_float() are used. Unfortunately, they contain bug connected with checking border case: that's not correct to cast UINT64_MAX (2^64 - 1) to double. Proper way is to use exp2(2^64) or predefined floating point constant. To not bother fixing function which in turn may contain other tricky places, let's use instead already verified double_compare_uint64(). So that we have unified way of integer<->float comparison. --- src/box/sql/vdbeaux.c | 74 +++++++++++---------------------------------------- test/sql/types.result | 2 +- 2 files changed, 17 insertions(+), 59 deletions(-) diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index e7accc745..2654f5c7a 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -2886,52 +2886,6 @@ sqlBlobCompare(const Mem * pB1, const Mem * pB2) return n1 - n2; } -/** - * Do a comparison between a 64-bit unsigned/signed integer and a - * 64-bit floating-point number. Return negative, zero, or - * positive if the first (integer) is less than, equal to, or - * greater than the second (double). - */ -static int -compare_uint_float(uint64_t u, double r) -{ - if (r > (double) UINT64_MAX) - return -1; - if (r < 0.0) - return +1; - uint64_t y = (uint64_t) r; - if (u < y) - return -1; - if (u > y) - return +1; - double s = (double) u; - if (s < r) - return -1; - if (s > r) - return +1; - return 0; -} - -static int -compare_int_float(int64_t i, double r) -{ - if (r < (double) INT64_MIN) - return +1; - if (r >= 0.0) - return -1; - int64_t y = (int64_t) r; - if (i < y) - return -1; - if (i > y) - return +1; - double s = (double) i; - if (s < r) - return -1; - if (s > r) - return +1; - return 0; -} - /* * Compare the values contained by the two memory cells, returning * negative, zero or positive if pMem1 is less than, equal to, or greater @@ -2997,16 +2951,16 @@ sqlMemCompare(const Mem * pMem1, const Mem * pMem2, const struct coll * pColl) } if ((f1 & MEM_Int) != 0) { if ((f2 & MEM_Real) != 0) { - return compare_int_float(pMem1->u.i, - pMem2->u.r); + return double_compare_uint64(-pMem2->u.r, + -pMem1->u.i, 1); } else { return -1; } } if ((f1 & MEM_UInt) != 0) { if ((f2 & MEM_Real) != 0) { - return compare_uint_float(pMem1->u.u, - pMem2->u.r); + return double_compare_uint64(pMem2->u.r, + pMem1->u.u, -1); } else if ((f2 & MEM_Int) != 0) { return +1; } else { @@ -3015,11 +2969,11 @@ sqlMemCompare(const Mem * pMem1, const Mem * pMem2, const struct coll * pColl) } if ((f1 & MEM_Real) != 0) { if ((f2 & MEM_Int) != 0) { - return -compare_int_float(pMem2->u.i, - pMem1->u.r); + return double_compare_uint64(-pMem1->u.r, + -pMem2->u.i, -1); } else if ((f2 & MEM_UInt) != 0) { - return -compare_uint_float(pMem2->u.u, - pMem1->u.r); + return double_compare_uint64(pMem1->u.r, + pMem2->u.u, 1); } else { return -1; } @@ -3188,7 +3142,8 @@ sqlVdbeCompareMsgpack(const char **key1, else if (mem1.u.u > pKey2->u.u) rc = +1; } else if ((pKey2->flags & MEM_Real) != 0) { - rc = compare_uint_float(mem1.u.u, pKey2->u.r); + rc = double_compare_uint64(pKey2->u.r, + mem1.u.u, -1); } else { rc = (pKey2->flags & MEM_Null) ? +1 : -1; } @@ -3205,7 +3160,8 @@ sqlVdbeCompareMsgpack(const char **key1, rc = +1; } } else if (pKey2->flags & MEM_Real) { - rc = compare_int_float(mem1.u.i, pKey2->u.r); + rc = double_compare_uint64(-pKey2->u.r, + -mem1.u.i, 1); } else { rc = (pKey2->flags & MEM_Null) ? +1 : -1; } @@ -3219,9 +3175,11 @@ sqlVdbeCompareMsgpack(const char **key1, mem1.u.r = mp_decode_double(&aKey1); do_float: if ((pKey2->flags & MEM_Int) != 0) { - rc = -compare_int_float(pKey2->u.i, mem1.u.r); + rc = double_compare_uint64(-mem1.u.r, + -pKey2->u.i, -1); } else if (pKey2->flags & MEM_UInt) { - rc = -compare_uint_float(pKey2->u.u, mem1.u.r); + rc = double_compare_uint64(mem1.u.r, + pKey2->u.u, 1); } else if (pKey2->flags & MEM_Real) { if (mem1.u.r < pKey2->u.r) { rc = -1; diff --git a/test/sql/types.result b/test/sql/types.result index 83820af53..c95d21e0f 100644 --- a/test/sql/types.result +++ b/test/sql/types.result @@ -1286,7 +1286,7 @@ box.execute("SELECT 18446744073709551615.0 > 18446744073709551615") - name: 18446744073709551615.0 > 18446744073709551615 type: boolean rows: - - [false] + - [true] ... box.execute("SELECT 18446744073709551615 IN ('18446744073709551615', 18446744073709551615.0)") --- -- 2.15.1