From: Nikita Pettik <korablev@tarantool.org> To: tarantool-patches@freelists.org Cc: v.shpilevoy@tarantool.org, Nikita Pettik <korablev@tarantool.org> Subject: [tarantool-patches] [PATCH 2/2] sql: use double_compare_uint64() for int<->float cmp Date: Wed, 7 Aug 2019 01:02:15 +0300 [thread overview] Message-ID: <c89df31e03ba5e047f61864ebd65c9ee43b32592.1565120194.git.korablev@tarantool.org> (raw) In-Reply-To: <cover.1565120194.git.korablev@tarantool.org> In-Reply-To: <cover.1565120194.git.korablev@tarantool.org> 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
next prev parent reply other threads:[~2019-08-06 22:02 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-08-06 22:02 [tarantool-patches] [PATCH 0/2] Fix border case in float-int comparison in SQL Nikita Pettik 2019-08-06 22:02 ` [tarantool-patches] [PATCH 1/2] Move mp_compare_double_uint64() to trivia.h Nikita Pettik 2019-08-08 21:37 ` [tarantool-patches] ***UNCHECKED*** " Vladislav Shpilevoy 2019-08-14 10:36 ` [tarantool-patches] " n.pettik 2019-08-14 20:43 ` Vladislav Shpilevoy 2019-08-08 21:37 ` [tarantool-patches] ***UNCHECKED*** " Vladislav Shpilevoy 2019-08-06 22:02 ` Nikita Pettik [this message] 2019-08-22 12:31 ` [tarantool-patches] Re: [PATCH 0/2] Fix border case in float-int comparison in SQL Kirill Yukhin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=c89df31e03ba5e047f61864ebd65c9ee43b32592.1565120194.git.korablev@tarantool.org \ --to=korablev@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [tarantool-patches] [PATCH 2/2] sql: use double_compare_uint64() for int<->float cmp' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox