[Tarantool-patches] [PATCH v5 3/6] sql: change comparison between numbers using index

[imeevma@tarantool.org]

This patch disables number conversions in ApplyType in wherecode.c. This
allows conversions between numbers introduced in previous commit to be
used.

Part of #4230
---
 src/box/sql/sqlInt.h                 |  2 +
 src/box/sql/vdbe.c                   |  3 +-
 src/box/sql/wherecode.c              | 76 +---------------------------
 test/sql-tap/in4.test.lua            |  4 +-
 test/sql-tap/join.test.lua           |  4 +-
 test/sql-tap/tkt-9a8b09f8e6.test.lua |  8 +--
 test/sql/types.result                | 54 ++++++++++++++++++++
 test/sql/types.test.lua              | 12 +++++
 8 files changed, 79 insertions(+), 84 deletions(-)

diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index adf90d824..1e6f0f41f 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -2309,6 +2309,8 @@ struct Parse {
 #define OPFLAG_SYSTEMSP      0x20	/* OP_Open**: set if space pointer
 					 * points to system space.
 					 */
+/** OP_ApplyType: Do not convert numbers. */
+#define OPFLAG_DO_NOT_CONVERT_NUMBERS	0x01
 
 /**
  * Prepare vdbe P5 flags for OP_{IdxInsert, IdxReplace, Update}
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 822d7e177..a377ceae7 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -3137,7 +3137,8 @@ case OP_ApplyType: {
 			if (!mp_type_is_numeric(mem_mp_type(pIn1)))
 				goto type_mismatch;
 			/* Try to convert numeric-to-numeric. */
-			if (mem_convert_to_numeric(pIn1, type) != 0)
+			if ((pOp->p5 & OPFLAG_DO_NOT_CONVERT_NUMBERS) == 0 &&
+			    mem_convert_to_numeric(pIn1, type) != 0)
 				goto type_mismatch;
 		}
 		pIn1++;
diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c
index 97ba59151..78d580801 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -374,6 +374,7 @@ emit_apply_type(Parse *pParse, int base, int n, enum field_type *types)
 								     types, n);
 		sqlVdbeAddOp4(v, OP_ApplyType, base, n, 0,
 				  (char *) types_dup, P4_DYNAMIC);
+		sqlVdbeChangeP5(v, OPFLAG_DO_NOT_CONVERT_NUMBERS);
 		sql_expr_type_cache_change(pParse, base, n);
 	}
 }
@@ -1045,77 +1046,6 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo,	/* Complete information about the W
 		}
 		struct index_def *idx_pk = space->index[0]->def;
 		uint32_t pk_part_count = idx_pk->key_def->part_count;
-		/*
-		 * Tarantool's iterator over integer fields doesn't
-		 * tolerate floating point values. Hence, if term
-		 * is equality comparison and value of operand is
-		 * not integer, we can skip it since it always
-		 * results in false: INT a == 0.5 -> false;
-		 * It is done using OP_MustBeInt facilities.
-		 * In case term is greater comparison (a > ?), we
-		 * should notify OP_SeekGT to process truncation of
-		 * floating point value: a > 0.5 -> a >= 1;
-		 * It is done by setting P5 flag for OP_Seek*.
-		 * It is worth mentioning that we do not need
-		 * this step when it comes for less (<) comparison
-		 * of nullable field. Key is NULL in this case:
-		 * values are ordered as  NULL, ... NULL, min_value,
-		 * so to fetch min value we pass NULL to GT iterator.
-		 * The only exception is less comparison in
-		 * conjunction with ORDER BY DESC clause:
-		 * in such situation we use LE iterator and
-		 * truncated value to compare. But then
-		 * pRangeStart == NULL.
-		 * This procedure is correct for compound index:
-		 * only one comparison of less/greater type can be
-		 * used at the same time. For instance,
-		 * a < 1.5 AND b > 0.5 is handled by SeekGT using
-		 * column a and fetching column b from tuple and
-		 * OP_Le comparison.
-		 *
-		 * Note that OP_ApplyType, which is emitted before
-		 * OP_Seek** doesn't truncate floating point to
-		 * integer. That's why we need this routine.
-		 * Also, note that terms are separated by OR
-		 * predicates, so we consider term as sequence
-		 * of AND'ed predicates.
-		 */
-		size_t addrs_sz;
-		int *seek_addrs = region_alloc_array(&pParse->region,
-						     typeof(seek_addrs[0]), nEq,
-						     &addrs_sz);
-		if (seek_addrs == NULL) {
-			diag_set(OutOfMemory, addrs_sz, "region_alloc_array",
-				 "seek_addrs");
-			pParse->is_aborted = true;
-			return 0;
-		}
-		memset(seek_addrs, 0, addrs_sz);
-		for (int i = 0; i < nEq; i++) {
-			enum field_type type = idx_def->key_def->parts[i].type;
-			if (type == FIELD_TYPE_INTEGER ||
-			    type == FIELD_TYPE_UNSIGNED) {
-				/*
-				 * OP_MustBeInt consider NULLs as
-				 * non-integer values, so firstly
-				 * check whether value is NULL or not.
-				 */
-				seek_addrs[i] = sqlVdbeAddOp1(v, OP_IsNull,
-							      regBase);
-				sqlVdbeAddOp2(v, OP_MustBeInt, regBase + i,
-					      addrNxt);
-				start_types[i] = FIELD_TYPE_SCALAR;
-				/*
-				 * We need to notify column cache
-				 * that type of value may change
-				 * so we should fetch value from
-				 * tuple again rather then copy
-				 * from register.
-				 */
-				sql_expr_type_cache_change(pParse, regBase + i,
-							   1);
-			}
-		}
 		emit_apply_type(pParse, regBase, nConstraint - bSeekPastNull,
 				start_types);
 		if (pLoop->nSkip > 0 && nConstraint == pLoop->nSkip) {
@@ -1127,10 +1057,6 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo,	/* Complete information about the W
 			op = aStartOp[(start_constraints << 2) +
 				      (startEq << 1) + bRev];
 			assert(op != 0);
-			for (uint32_t i = 0; i < nEq; ++i) {
-				if (seek_addrs[i] != 0)
-					sqlVdbeJumpHere(v, seek_addrs[i]);
-			}
 			sqlVdbeAddOp4Int(v, op, iIdxCur, addrNxt, regBase,
 					     nConstraint);
 			VdbeCoverage(v);
diff --git a/test/sql-tap/in4.test.lua b/test/sql-tap/in4.test.lua
index 5c01ccdab..e0bf671d9 100755
--- a/test/sql-tap/in4.test.lua
+++ b/test/sql-tap/in4.test.lua
@@ -147,13 +147,13 @@ test:do_execsql_test(
         -- </in4-2.7>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "in4-2.8",
     [[
         SELECT b FROM t2 WHERE a IN ('', '0.0.0', '2') 
     ]], {
         -- <in4-2.8>
-        "two"
+        1, "Type mismatch: can not convert  to integer"
         -- </in4-2.8>
     })
 
diff --git a/test/sql-tap/join.test.lua b/test/sql-tap/join.test.lua
index 840b780a3..7a1346094 100755
--- a/test/sql-tap/join.test.lua
+++ b/test/sql-tap/join.test.lua
@@ -1028,13 +1028,13 @@ test:do_test(
         -- </join-11.8>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "join-11.9",
     [[
         SELECT * FROM t1 NATURAL JOIN t2 
     ]], {
         -- <join-11.9>
-        "one", "1", "two", "2"
+        1, "Type mismatch: can not convert 1 to integer"
         -- </join-11.9>
     })
 
diff --git a/test/sql-tap/tkt-9a8b09f8e6.test.lua b/test/sql-tap/tkt-9a8b09f8e6.test.lua
index ca3a5427a..1314d0aad 100755
--- a/test/sql-tap/tkt-9a8b09f8e6.test.lua
+++ b/test/sql-tap/tkt-9a8b09f8e6.test.lua
@@ -183,13 +183,13 @@ test:do_execsql_test(
         -- </3.2>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     3.3,
     [[
         SELECT x FROM t2 WHERE x IN ('1');
     ]], {
         -- <3.3>
-        1
+        1, "Type mismatch: can not convert 1 to integer"
         -- </3.3>
     })
 
@@ -213,13 +213,13 @@ test:do_execsql_test(
         -- </3.6>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     3.7,
     [[
         SELECT x FROM t2 WHERE '1' IN (x);
     ]], {
         -- <3.7>
-        1
+        1, "Type mismatch: can not convert 1 to integer"
         -- </3.7>
     })
 
diff --git a/test/sql/types.result b/test/sql/types.result
index 442245186..8810a9f82 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -2795,3 +2795,57 @@ box.execute([[DROP TABLE ts;]])
 ---
 - row_count: 1
 ...
+--
+-- gh-4230: Make sure the comparison between numbers that use
+-- index is working correctly.
+--
+box.execute([[CREATE TABLE t (i INTEGER PRIMARY KEY, a DOUBLE);]])
+---
+- row_count: 1
+...
+box.execute([[INSERT INTO t VALUES (1, ?);]], {2^60})
+---
+- row_count: 1
+...
+box.execute([[SELECT * FROM t WHERE i > ?]], {2^70})
+---
+- metadata:
+  - name: I
+    type: integer
+  - name: A
+    type: double
+  rows: []
+...
+box.execute([[SELECT * FROM t WHERE i > ?]], {-2^70})
+---
+- metadata:
+  - name: I
+    type: integer
+  - name: A
+    type: double
+  rows:
+  - [1, 1152921504606846976]
+...
+box.execute([[SELECT * FROM t WHERE a = ?]], {2ULL^60ULL - 1ULL})
+---
+- metadata:
+  - name: I
+    type: integer
+  - name: A
+    type: double
+  rows: []
+...
+box.execute([[SELECT * FROM t WHERE a > ?]], {2ULL^60ULL - 1ULL})
+---
+- metadata:
+  - name: I
+    type: integer
+  - name: A
+    type: double
+  rows:
+  - [1, 1152921504606846976]
+...
+box.execute([[DROP TABLE t;]])
+---
+- row_count: 1
+...
diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua
index 0270d9f8a..a23b12801 100644
--- a/test/sql/types.test.lua
+++ b/test/sql/types.test.lua
@@ -623,3 +623,15 @@ box.execute([[DROP TABLE tb;]])
 box.execute([[DROP TABLE tt;]])
 box.execute([[DROP TABLE tv;]])
 box.execute([[DROP TABLE ts;]])
+
+--
+-- gh-4230: Make sure the comparison between numbers that use
+-- index is working correctly.
+--
+box.execute([[CREATE TABLE t (i INTEGER PRIMARY KEY, a DOUBLE);]])
+box.execute([[INSERT INTO t VALUES (1, ?);]], {2^60})
+box.execute([[SELECT * FROM t WHERE i > ?]], {2^70})
+box.execute([[SELECT * FROM t WHERE i > ?]], {-2^70})
+box.execute([[SELECT * FROM t WHERE a = ?]], {2ULL^60ULL - 1ULL})
+box.execute([[SELECT * FROM t WHERE a > ?]], {2ULL^60ULL - 1ULL})
+box.execute([[DROP TABLE t;]])
-- 
2.25.1