From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id DCC546EC56; Wed, 28 Jul 2021 23:53:08 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org DCC546EC56 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1627505589; bh=GCbWuq8Kyv2qbKw+rUHVI2LbtZyAz+vFVN0WyYhkKMA=; h=To:Cc:Date:In-Reply-To:References:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=vQZrsGU8O6txNE+w34cIfPz4klB/X0gomoGgNsgmEk1C5ly1plixOrkpipfkskucd 5JwkdNw0dNKN1IfbukEcQCKnG0OVkZI79QHK/emOE0RuI3Ll4n2WNOndIWKrZRXFz/ Z4U9ZzPlLiCSrR3QNHGBTP35PoHQ0ZR1HBHtEWss= Received: from smtp17.mail.ru (smtp17.mail.ru [94.100.176.154]) (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 6A5166EC6E for ; Wed, 28 Jul 2021 23:51:15 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 6A5166EC6E Received: by smtp17.mail.ru with esmtpa (envelope-from ) id 1m8qWI-00060m-LD; Wed, 28 Jul 2021 23:51:15 +0300 To: v.shpilevoy@tarantool.org Cc: tarantool-patches@dev.tarantool.org Date: Wed, 28 Jul 2021 23:51:14 +0300 Message-Id: <58a114d408cdfb9f82f2f11005a93981d392de38.1627504973.git.imeevma@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD941C43E597735A9C366EE405EC28A2001F8302D8429E0DE58182A05F5380850407648E790A9511CA3498F646FD8BA9B314398E0F87DC311127C6AD8A8591AF00B X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE77F5852B248C7AFEDEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637CD693CC314794D958638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8740F3530B41CFD775A4B659E75397AE0117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC8C7ADC89C2F0B2A5A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD182CC0D3CB04F14752D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EEC24E1E72F37C03A06E0066C2D8992A164AD6D5ED66289B52698AB9A7B718F8C46E0066C2D8992A16725E5C173C3A84C3056D5A8E4C6B598EBA3038C0950A5D36B5C8C57E37DE458B0BC6067A898B09E46D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE75A9E79F66F1C28F3731C566533BA786AA5CC5B56E945C8DA X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A213B5FB47DCBC3458834459D11680B5052A0D2D9740CDC953B4C319C08D8AFBF4 X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C414F749A5E30D975C7BEA09003D200E0811F814C0DF91520FB4E6796EA6F0E43E9C2B6934AE262D3EE7EAB7254005DCED7532B743992DF240BDC6A1CF3F042BAD6DF99611D93F60EF795D7D556640A06E699F904B3F4130E343918A1A30D5E7FCCB5012B2E24CD356 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3475AE8838CC1BF58E9E2AAEDDE3D96F3896C4F86F3A0D1B061134FAA868B3C7A93009A0796CAD39DD1D7E09C32AA3244C405CCC703EAF550488065598626BA8ED725D5B54B2FE4575729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojbL9S8ysBdXi4QUX63t/5tjnMEqoMgGRw X-Mailru-Sender: 3A338A78718AEC5ABE350BD77BF82E157112D09727C6D563F3ABC26383D6CF42881F9B83824648CCA3E7B4BFDCAD2EFE027D9DD7AE851095A2E8D17B49942DB0CBEE3F9BE14373499437F6177E88F7363CDA0F3B3F5B9367 X-Mras: Ok Subject: [Tarantool-patches] [PATCH v1 4/7] sql: remove unnecessary calls of OP_ApplyType X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Mergen Imeev via Tarantool-patches Reply-To: imeevma@tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Since the OP_Seek* opcodes now work using the new implicit casting rules, we don't need to call OP_ApplyType before them. This patch removes such calls. Part of #4230 Part of #4470 --- src/box/sql/vdbe.c | 20 +-- src/box/sql/wherecode.c | 202 +-------------------------- test/sql-tap/cast.test.lua | 14 +- test/sql-tap/in4.test.lua | 4 +- test/sql-tap/join.test.lua | 4 +- test/sql-tap/tkt-9a8b09f8e6.test.lua | 22 +-- 6 files changed, 24 insertions(+), 242 deletions(-) diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index a69402720..c24265a1d 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -2506,7 +2506,7 @@ case OP_Close: { break; } -/* Opcode: SeekLT P1 P2 P3 P4 P5 +/* Opcode: SeekLT P1 P2 P3 P4 * * Synopsis: key=r[P3@P4] * * If cursor P1 refers to an SQL table (B-Tree that uses integer keys), @@ -2522,8 +2522,6 @@ case OP_Close: { * from the end toward the beginning. In other words, the cursor is * configured to use Prev, not Next. * - * P5 has the same meaning as for SeekGE. - * * See also: Found, NotFound, SeekGt, SeekGe, SeekLe */ case OP_SeekLT: { /* jump, in3 */ @@ -2582,7 +2580,7 @@ case OP_SeekLT: { /* jump, in3 */ break; } -/* Opcode: SeekLE P1 P2 P3 P4 P5 +/* Opcode: SeekLE P1 P2 P3 P4 * * Synopsis: key=r[P3@P4] * * If cursor P1 refers to an SQL table (B-Tree that uses integer keys), @@ -2605,8 +2603,6 @@ case OP_SeekLT: { /* jump, in3 */ * The IdxGE opcode will be skipped if this opcode succeeds, but the * IdxGE opcode will be used on subsequent loop iterations. * - * P5 has the same meaning as for SeekGE. - * * See also: Found, NotFound, SeekGt, SeekGe, SeekLt */ case OP_SeekLE: { /* jump, in3 */ @@ -2672,7 +2668,7 @@ case OP_SeekLE: { /* jump, in3 */ break; } -/* Opcode: SeekGE P1 P2 P3 P4 P5 +/* Opcode: SeekGE P1 P2 P3 P4 * * Synopsis: key=r[P3@P4] * * If cursor P1 refers to an SQL table (B-Tree that uses integer keys), @@ -2695,11 +2691,6 @@ case OP_SeekLE: { /* jump, in3 */ * from the beginning toward the end. In other words, the cursor is * configured to use Next, not Prev. * - * If P5 is not zero, than it is offset of integer fields in input - * vector. Force corresponding value to be INTEGER, in case it - * is floating point value. Alongside with that, type of - * iterator may be changed: a > 1.5 -> a >= 2. - * * See also: Found, NotFound, SeekLt, SeekGt, SeekLe */ case OP_SeekGE: { /* jump, in3 */ @@ -2765,7 +2756,7 @@ case OP_SeekGE: { /* jump, in3 */ break; } -/* Opcode: SeekGT P1 P2 P3 P4 P5 +/* Opcode: SeekGT P1 P2 P3 P4 * * Synopsis: key=r[P3@P4] * * If cursor P1 refers to an SQL table (B-Tree that uses integer keys), @@ -2781,9 +2772,6 @@ case OP_SeekGE: { /* jump, in3 */ * from the beginning toward the end. In other words, the cursor is * configured to use Next, not Prev. * - * If P5 is not zero, than it is offset of integer fields in input - * vector. Force corresponding value to be INTEGER. - * * P5 has the same meaning as for SeekGE. */ case OP_SeekGT: { /* jump, in3 */ diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c index 96bcab110..92d374200 100644 --- a/src/box/sql/wherecode.c +++ b/src/box/sql/wherecode.c @@ -335,72 +335,6 @@ disableTerm(WhereLevel * pLevel, WhereTerm * pTerm) } } -/** - * Code an OP_ApplyType opcode to apply the column type string - * @types to the n registers starting at @base. - * - * As an optimization, SCALAR entries (which are no-ops) at the - * beginning and end of @types are ignored. If all entries in - * @types are SCALAR, then no code gets generated. - * - * This routine makes its own copy of @types so that the caller is - * free to modify @types after this routine returns. - */ -static void -emit_apply_type(Parse *pParse, int base, int n, enum field_type *types) -{ - Vdbe *v = pParse->pVdbe; - if (types == NULL) { - assert(pParse->db->mallocFailed); - return; - } - assert(v != 0); - - /* - * Adjust base and n to skip over SCALAR entries at the - * beginning and end of the type sequence. - */ - while (n > 0 && types[0] == FIELD_TYPE_SCALAR) { - n--; - base++; - types++; - } - while (n > 1 && types[n - 1] == FIELD_TYPE_SCALAR) { - n--; - } - - if (n > 0) { - enum field_type *types_dup = field_type_sequence_dup(pParse, - types, n); - sqlVdbeAddOp4(v, OP_ApplyType, base, n, 0, - (char *) types_dup, P4_DYNAMIC); - sql_expr_type_cache_change(pParse, base, n); - } -} - -/** - * Expression @rhs, which is the RHS of a comparison operation, is - * either a vector of n elements or, if n==1, a scalar expression. - * Before the comparison operation, types @types are to be applied - * to the @rhs values. This function modifies entries within the - * field sequence to SCALAR if either: - * - * * the comparison will be performed with no type, or - * * the type change in @types is guaranteed not to change the value. - */ -static void -expr_cmp_update_rhs_type(struct Expr *rhs, int n, enum field_type *types) -{ - for (int i = 0; i < n; i++) { - Expr *p = sqlVectorFieldSubexpr(rhs, i); - enum field_type expr_type = sql_expr_type(p); - if (sql_type_result(expr_type, types[i]) == FIELD_TYPE_SCALAR || - sql_expr_needs_no_type_change(p, types[i])) { - types[i] = FIELD_TYPE_SCALAR; - } - } -} - /* * Generate code for a single equality term of the WHERE clause. An equality * term can be either X=expr or X IN (...). pTerm is the term to be @@ -644,8 +578,7 @@ static int codeAllEqualityTerms(Parse * pParse, /* Parsing context */ WhereLevel * pLevel, /* Which nested loop of the FROM we are coding */ int bRev, /* Reverse the order of IN operators */ - int nExtraReg, /* Number of extra registers to allocate */ - enum field_type **res_type) + int nExtraReg) /* Number of extra registers to allocate */ { u16 nEq; /* The number of == or IN constraints to code */ u16 nSkip; /* Number of left-most columns to skip */ @@ -733,7 +666,6 @@ codeAllEqualityTerms(Parse * pParse, /* Parsing context */ } } } - *res_type = type; return regBase; } @@ -905,16 +837,8 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about the W int iIdxCur; /* The VDBE cursor for the index */ int nExtraReg = 0; /* Number of extra registers needed */ int op; /* Instruction opcode */ - /* Types for start of range constraint. */ - enum field_type *start_types; - /* Types for end of range constraint */ - enum field_type *end_types = NULL; u8 bSeekPastNull = 0; /* True to seek past initial nulls */ u8 bStopAtNull = 0; /* Add condition to terminate at NULLs */ - int force_integer_reg = -1; /* If non-negative: number of - * column which must be converted - * to integer type, used for IPK. - */ struct index_def *idx_def = pLoop->index_def; assert(idx_def != NULL); @@ -996,16 +920,7 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about the W * starting at regBase. */ regBase = - codeAllEqualityTerms(pParse, pLevel, bRev, nExtraReg, - &start_types); - if (start_types != NULL && nTop) { - uint32_t len = 0; - for (enum field_type *tmp = &start_types[nEq]; - *tmp != field_type_MAX; tmp++, len++); - uint32_t sz = len * sizeof(enum field_type); - end_types = sqlDbMallocRaw(db, sz); - memcpy(end_types, &start_types[nEq], sz); - } + codeAllEqualityTerms(pParse, pLevel, bRev, nExtraReg); addrNxt = pLevel->addrNxt; testcase(pRangeStart && (pRangeStart->eOperator & WO_LE) != 0); @@ -1030,10 +945,6 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about the W VdbeCoverage(v); } - if (start_types) { - expr_cmp_update_rhs_type(pRight, nBtm, - &start_types[nEq]); - } nConstraint += nBtm; testcase(pRangeStart->wtFlags & TERM_VIRTUAL); if (sqlExprIsVector(pRight) == 0) { @@ -1048,94 +959,6 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about the W startEq = 0; start_constraints = 1; } - /* - * 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); - } - } - /* Inequality constraint comes always at the end of list. */ - part_count = idx_def->key_def->part_count; - if (pRangeStart != NULL) { - /* - * nEq == 0 means that filter condition - * contains only inequality. - */ - uint32_t ineq_idx = nEq == 0 ? 0 : nEq - 1; - assert(ineq_idx < part_count); - enum field_type ineq_type = - idx_def->key_def->parts[ineq_idx].type; - if (ineq_type == FIELD_TYPE_INTEGER || - ineq_type == FIELD_TYPE_UNSIGNED) - force_integer_reg = regBase + nEq; - } - emit_apply_type(pParse, regBase, nConstraint - bSeekPastNull, - start_types); if (pLoop->nSkip > 0 && nConstraint == pLoop->nSkip) { /* The skip-scan logic inside the call to codeAllEqualityConstraints() * above has already left the cursor sitting on the correct row, @@ -1145,20 +968,8 @@ 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); - /* If this is Seek* opcode, and IPK is detected in the - * constraints vector: force it to be integer. - */ - if ((op == OP_SeekGE || op == OP_SeekGT - || op == OP_SeekLE || op == OP_SeekLT) - && force_integer_reg > 0) { - sqlVdbeChangeP5(v, force_integer_reg); - } VdbeCoverage(v); VdbeCoverageIf(v, op == OP_Rewind); testcase(op == OP_Rewind); @@ -1188,13 +999,6 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about the W addrNxt); VdbeCoverage(v); } - if (end_types) { - expr_cmp_update_rhs_type(pRight, nTop, end_types); - emit_apply_type(pParse, regBase + nEq, nTop, - end_types); - } else { - assert(pParse->db->mallocFailed); - } nConstraint += nTop; testcase(pRangeEnd->wtFlags & TERM_VIRTUAL); @@ -1208,8 +1012,6 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about the W endEq = 0; nConstraint++; } - sqlDbFree(db, start_types); - sqlDbFree(db, end_types); /* Top of the loop body */ pLevel->p2 = sqlVdbeCurrentAddr(v); diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua index b8ad23317..b570fc878 100755 --- a/test/sql-tap/cast.test.lua +++ b/test/sql-tap/cast.test.lua @@ -1,6 +1,6 @@ #!/usr/bin/env tarantool local test = require("sqltester") -test:plan(107) +test:plan(108) --!./tcltestrunner.lua -- 2005 June 25 @@ -1144,4 +1144,16 @@ test:do_catchsql_test( 1, "Type mismatch: can not convert double(2.5) to string" }) +-- Make sure that search using index in field type number work right. +test:do_execsql_test( + "cast-11", + [[ + CREATE TABLE t6(d DOUBLE PRIMARY KEY); + INSERT INTO t6 VALUES(10000000000000000); + SELECT d FROM t6 WHERE d < 10000000000000001 and d > 9999999999999999; + DROP TABLE t6; + ]], { + 10000000000000000 + }) + test:finish_test() diff --git a/test/sql-tap/in4.test.lua b/test/sql-tap/in4.test.lua index 8442944b9..aa6483697 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 string('') to integer" -- }) diff --git a/test/sql-tap/join.test.lua b/test/sql-tap/join.test.lua index 48639a7b3..abfabfacf 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 string('1') to integer" -- }) diff --git a/test/sql-tap/tkt-9a8b09f8e6.test.lua b/test/sql-tap/tkt-9a8b09f8e6.test.lua index 43322468d..cc321c2f6 100755 --- a/test/sql-tap/tkt-9a8b09f8e6.test.lua +++ b/test/sql-tap/tkt-9a8b09f8e6.test.lua @@ -1,6 +1,6 @@ #!/usr/bin/env tarantool local test = require("sqltester") -test:plan(47) +test:plan(45) --!./tcltestrunner.lua -- 2014 June 26 @@ -179,16 +179,6 @@ test:do_execsql_test( -- }) -test:do_execsql_test( - 3.3, - [[ - SELECT x FROM t2 WHERE x IN ('1'); - ]], { - -- <3.3> - 1 - -- - }) - test:do_execsql_test( 3.5, [[ @@ -209,16 +199,6 @@ test:do_execsql_test( -- }) -test:do_execsql_test( - 3.7, - [[ - SELECT x FROM t2 WHERE '1' IN (x); - ]], { - -- <3.7> - 1 - -- - }) - test:do_execsql_test( 4.1, [[ -- 2.25.1