[tarantool-patches] [PATCH 2/2] sql: use double_compare_uint64() for int<->float cmp

[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