[Tarantool-patches] [PATCH v6 19/22] sql: change comparison between numbers using index
imeevma at tarantool.org
imeevma at tarantool.org
Thu Jul 16 17:47:13 MSK 2020
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 cd94616ca..7d6119904 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -2309,6 +2309,8 @@ struct Parse {
*/
/** OP_ApplyType: Treat BLOB as STRING. */
#define OPFLAG_BLOB_LIKE_STRING 0x01
+/** OP_ApplyType: Do not convert numbers. */
+#define OPFLAG_DO_NOT_CONVERT_NUMBERS 0x02
/**
* Prepare vdbe P5 flags for OP_{IdxInsert, IdxReplace, Update}
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index dd1ce97b7..96f02717c 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -3150,7 +3150,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 43cdffad0..7a4e9bca3 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -2775,3 +2775,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
More information about the Tarantool-patches
mailing list