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