From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 26F5C43040E for ; Fri, 21 Aug 2020 12:19:54 +0300 (MSK) From: imeevma@tarantool.org Date: Fri, 21 Aug 2020 12:19:53 +0300 Message-Id: <96a8dec05827608e00503fd33d2cb0d2a1076052.1598000242.git.imeevma@gmail.com> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH v5 3/6] sql: change comparison between numbers using index List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: korablev@tarantool.org, tsafin@tarantool.org Cc: tarantool-patches@dev.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( -- }) -test:do_execsql_test( +test:do_catchsql_test( "in4-2.8", [[ SELECT b FROM t2 WHERE a IN ('', '0.0.0', '2') ]], { -- - "two" + 1, "Type mismatch: can not convert to integer" -- }) 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( -- }) -test:do_execsql_test( +test:do_catchsql_test( "join-11.9", [[ SELECT * FROM t1 NATURAL JOIN t2 ]], { -- - "one", "1", "two", "2" + 1, "Type mismatch: can not convert 1 to integer" -- }) 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( -- }) -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" -- }) @@ -213,13 +213,13 @@ test:do_execsql_test( -- }) -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" -- }) 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