From: imeevma@tarantool.org To: korablev@tarantool.org, tsafin@tarantool.org, tarantool-patches@dev.tarantool.org Subject: [Tarantool-patches] [PATCH v6 17/22] sql: remove unused DOUBLE to INTEGER conversion Date: Thu, 16 Jul 2020 17:47:10 +0300 [thread overview] Message-ID: <a25ad9c5dadec9a13cc35b10fcdcf16476dbc202.1594909974.git.imeevma@gmail.com> (raw) In-Reply-To: <cover.1594909974.git.imeevma@gmail.com> This patch removes the unused DOUBLE to INTEGER conversion from OP_Seek* opcodes. This transformation is not used due to changes in the ApplyType opcode. The next few patches will introduce new rules for converting numbers (not just DOUBLE to INTEGER), and the implicit conversion within the ApplyType opcode will be disabled for this case. Part of #4230 --- src/box/sql/vdbe.c | 106 ++-------------------------------------- src/box/sql/wherecode.c | 27 ---------- 2 files changed, 5 insertions(+), 128 deletions(-) diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 97e09cac4..d853a0edb 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -3366,7 +3366,7 @@ case OP_Close: { 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), @@ -3389,14 +3389,9 @@ case OP_Close: { * 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 */ -/* 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), @@ -3412,12 +3407,9 @@ case OP_Close: { * 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. + * See also: Found, NotFound, SeekLt, SeekGe, SeekLe */ -/* 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), @@ -3433,11 +3425,9 @@ 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 */ -/* 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), @@ -3460,8 +3450,6 @@ case OP_Close: { * 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_SeekLT: /* jump, in3 */ @@ -3473,7 +3461,6 @@ case OP_SeekGT: { /* jump, in3 */ VdbeCursor *pC; /* The cursor to seek */ UnpackedRecord r; /* The key to seek for */ int nField; /* Number of columns or fields in the key */ - i64 iKey; /* The id we are to seek to */ int eqOnly; /* Only interested in == results */ assert(pOp->p1>=0 && pOp->p1<p->nCursor); @@ -3491,86 +3478,6 @@ case OP_SeekGT: { /* jump, in3 */ #ifdef SQL_DEBUG pC->seekOp = pOp->opcode; #endif - iKey = 0; - /* - * In case floating value is intended to be passed to - * iterator over integer field, we must truncate it to - * integer value and change type of iterator: - * a > 1.5 -> a >= 2 - */ - int int_field = pOp->p5; - bool is_neg = false; - - if (int_field > 0) { - /* The input value in P3 might be of any type: integer, real, string, - * blob, or NULL. But it needs to be an integer before we can do - * the seek, so convert it. - */ - pIn3 = &aMem[int_field]; - if ((pIn3->flags & MEM_Null) != 0) - goto skip_truncate; - if ((pIn3->flags & MEM_Str) != 0) - mem_apply_numeric_type(pIn3); - int64_t i; - if ((pIn3->flags & MEM_Int) == MEM_Int) { - i = pIn3->u.i; - is_neg = true; - } else if ((pIn3->flags & MEM_UInt) == MEM_UInt) { - i = pIn3->u.u; - is_neg = false; - } else if ((pIn3->flags & MEM_Real) == MEM_Real) { - if (pIn3->u.r > INT64_MAX) - i = INT64_MAX; - else if (pIn3->u.r < INT64_MIN) - i = INT64_MIN; - else - i = pIn3->u.r; - is_neg = i < 0; - } else { - diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn3), "integer"); - goto abort_due_to_error; - } - iKey = i; - - /* If the P3 value could not be converted into an integer without - * loss of information, then special processing is required... - */ - if ((pIn3->flags & (MEM_Int | MEM_UInt)) == 0) { - if ((pIn3->flags & MEM_Real)==0) { - /* If the P3 value cannot be converted into any kind of a number, - * then the seek is not possible, so jump to P2 - */ - VdbeBranchTaken(1,2); goto jump_to_p2; - break; - } - - /* If the approximation iKey is larger than the actual real search - * term, substitute >= for > and < for <=. e.g. if the search term - * is 4.9 and the integer approximation 5: - * - * (x > 4.9) -> (x >= 5) - * (x <= 4.9) -> (x < 5) - */ - if (pIn3->u.r<(double)iKey) { - assert(OP_SeekGE==(OP_SeekGT-1)); - assert(OP_SeekLT==(OP_SeekLE-1)); - assert((OP_SeekLE & 0x0001)==(OP_SeekGT & 0x0001)); - if ((oc & 0x0001)==(OP_SeekGT & 0x0001)) oc--; - } - - /* If the approximation iKey is smaller than the actual real search - * term, substitute <= for < and > for >=. - */ - else if (pIn3->u.r>(double)iKey) { - assert(OP_SeekLE==(OP_SeekLT+1)); - assert(OP_SeekGT==(OP_SeekGE+1)); - assert((OP_SeekLT & 0x0001)==(OP_SeekGE & 0x0001)); - if ((oc & 0x0001)==(OP_SeekLT & 0x0001)) oc++; - } - } - } -skip_truncate: /* * For a cursor with the OPFLAG_SEEKEQ hint, only the * OP_SeekGE and OP_SeekLE opcodes are allowed, and these @@ -3593,9 +3500,6 @@ skip_truncate: r.key_def = pC->key_def; r.nField = (u16)nField; - if (int_field > 0) - mem_set_int(&aMem[int_field], iKey, is_neg); - r.default_rc = ((1 & (oc - OP_SeekLT)) ? -1 : +1); assert(oc!=OP_SeekGT || r.default_rc==-1); assert(oc!=OP_SeekLE || r.default_rc==-1); diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c index 6d8768865..97ba59151 100644 --- a/src/box/sql/wherecode.c +++ b/src/box/sql/wherecode.c @@ -910,10 +910,6 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about the W 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); @@ -1120,21 +1116,6 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about the W 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) { @@ -1152,14 +1133,6 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about the W } 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); -- 2.25.1
next prev parent reply other threads:[~2020-07-16 14:47 UTC|newest] Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-16 14:45 [Tarantool-patches] [PATCH v6 00/22] sql: change implicit cast imeevma 2020-07-16 14:45 ` [Tarantool-patches] [PATCH v6 01/22] sql: change implicit cast for assignment imeevma 2020-07-16 14:45 ` [Tarantool-patches] [PATCH v6 02/22] sql: use ApplyType to check function arguments imeevma 2020-07-16 14:45 ` [Tarantool-patches] [PATCH v6 03/22] sql: check args of abs() imeevma 2020-07-16 16:12 ` Nikita Pettik 2020-07-16 14:45 ` [Tarantool-patches] [PATCH v6 04/22] sql: check args of avg(), sum() and total() imeevma 2020-07-16 16:21 ` Nikita Pettik 2020-07-16 14:45 ` [Tarantool-patches] [PATCH v6 05/22] sql: check args of char() imeevma 2020-07-16 16:27 ` Nikita Pettik 2020-07-16 14:45 ` [Tarantool-patches] [PATCH v6 06/22] sql: check args of length() imeevma 2020-07-16 16:31 ` Nikita Pettik 2020-07-16 14:46 ` [Tarantool-patches] [PATCH v6 07/22] sql: check operands of LIKE imeevma 2020-07-16 17:03 ` Nikita Pettik 2020-07-16 14:46 ` [Tarantool-patches] [PATCH v6 08/22] sql: check args of lower() and upper() imeevma 2020-07-16 17:09 ` Nikita Pettik 2020-07-16 14:46 ` [Tarantool-patches] [PATCH v6 09/22] sql: check args of position() imeevma 2020-07-16 17:28 ` Nikita Pettik 2020-07-16 14:46 ` [Tarantool-patches] [PATCH v6 10/22] sql: check args of randomblob() imeevma 2020-07-16 17:28 ` Nikita Pettik 2020-07-16 14:46 ` [Tarantool-patches] [PATCH v6 11/22] sql: check args of replace() imeevma 2020-07-16 14:46 ` [Tarantool-patches] [PATCH v6 12/22] sql: check args of round() imeevma 2020-07-16 14:46 ` [Tarantool-patches] [PATCH v6 13/22] sql: check args of soundex() imeevma 2020-07-16 14:47 ` [Tarantool-patches] [PATCH v6 14/22] sql: check args of substr() imeevma 2020-07-16 14:47 ` [Tarantool-patches] [PATCH v6 15/22] sql: check args of unicode() imeevma 2020-07-16 14:47 ` [Tarantool-patches] [PATCH v6 16/22] sql: check args of zeroblob() imeevma 2020-07-16 14:47 ` imeevma [this message] 2020-07-16 14:47 ` [Tarantool-patches] [PATCH v6 18/22] sql: add implicit cast between numbers in OP_Seek* imeevma 2020-07-16 14:47 ` [Tarantool-patches] [PATCH v6 19/22] sql: change comparison between numbers using index imeevma 2020-07-16 14:47 ` [Tarantool-patches] [PATCH v6 20/22] sql: remove implicit cast from comparison opcodes imeevma 2020-07-16 14:47 ` [Tarantool-patches] [PATCH v6 21/22] sql: fix implicit cast in opcode MustBeInt imeevma 2020-07-16 14:47 ` [Tarantool-patches] [PATCH v6 22/22] sql: remove implicit cast from MakeRecord opcode imeevma
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=a25ad9c5dadec9a13cc35b10fcdcf16476dbc202.1594909974.git.imeevma@gmail.com \ --to=imeevma@tarantool.org \ --cc=korablev@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=tsafin@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v6 17/22] sql: remove unused DOUBLE to INTEGER conversion' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox