* [Tarantool-patches] [PATCH v5 0/6] sql; remove implicit cast for comparison
@ 2020-08-21 9:19 imeevma
2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 1/6] sql: remove unused DOUBLE to INTEGER conversion imeevma
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: imeevma @ 2020-08-21 9:19 UTC (permalink / raw)
To: korablev, tsafin; +Cc: tarantool-patches
This patch-set removes implicit cast from STRING to NUMBER and vice versa for
comparison.
https://github.com/tarantool/tarantool/issues/4230
https://github.com/tarantool/tarantool/tree/imeevma/gh-4230-remove-implicit-cast-for-comparison
@ChangeLog
- Implicit cast from STRING to number and vice versa for comparison removed (gh-4230).
Changes in v5:
- Patches were moved in a new patch-set.
- Patch-set was simplified since implicit cast for assignment was removed.
Mergen Imeev (6):
sql: remove unused DOUBLE to INTEGER conversion
sql: add implicit cast between numbers in OP_Seek*
sql: change comparison between numbers using index
sql: remove implicit cast from comparison opcodes
sql: fix implicit cast in opcode MustBeInt
sql: remove implicit cast from MakeRecord opcode
src/box/sql/analyze.c | 6 +-
src/box/sql/delete.c | 15 +-
src/box/sql/expr.c | 17 +-
src/box/sql/fk_constraint.c | 12 +-
src/box/sql/select.c | 26 +-
src/box/sql/sqlInt.h | 2 +
src/box/sql/update.c | 23 +-
src/box/sql/vdbe.c | 536 ++++++++++++++++++--------
src/box/sql/wherecode.c | 103 +----
test/sql-tap/identifier_case.test.lua | 6 +-
test/sql-tap/in1.test.lua | 4 +-
test/sql-tap/in3.test.lua | 26 +-
test/sql-tap/in4.test.lua | 4 +-
test/sql-tap/insert3.test.lua | 2 +-
test/sql-tap/join.test.lua | 8 +-
test/sql-tap/misc1.test.lua | 32 +-
test/sql-tap/select1.test.lua | 4 +-
test/sql-tap/select7.test.lua | 2 +-
test/sql-tap/subquery.test.lua | 4 +-
test/sql-tap/tkt-9a8b09f8e6.test.lua | 508 ------------------------
test/sql-tap/tkt3493.test.lua | 40 +-
test/sql-tap/transitive1.test.lua | 12 +-
test/sql-tap/where2.test.lua | 183 +--------
test/sql-tap/where5.test.lua | 12 +-
test/sql/boolean.result | 76 +---
test/sql/types.result | 286 +++++++++++++-
test/sql/types.test.lua | 63 +++
27 files changed, 875 insertions(+), 1137 deletions(-)
delete mode 100755 test/sql-tap/tkt-9a8b09f8e6.test.lua
--
2.25.1
^ permalink raw reply [flat|nested] 15+ messages in thread* [Tarantool-patches] [PATCH v5 1/6] sql: remove unused DOUBLE to INTEGER conversion 2020-08-21 9:19 [Tarantool-patches] [PATCH v5 0/6] sql; remove implicit cast for comparison imeevma @ 2020-08-21 9:19 ` imeevma 2020-09-17 14:48 ` Nikita Pettik 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 2/6] sql: add implicit cast between numbers in OP_Seek* imeevma ` (4 subsequent siblings) 5 siblings, 1 reply; 15+ messages in thread From: imeevma @ 2020-08-21 9:19 UTC (permalink / raw) To: korablev, tsafin; +Cc: tarantool-patches 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 14ddb5160..7405009a7 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -3350,7 +3350,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), @@ -3373,14 +3373,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), @@ -3396,12 +3391,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), @@ -3417,11 +3409,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), @@ -3444,8 +3434,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 */ @@ -3457,7 +3445,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); @@ -3475,86 +3462,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 @@ -3577,9 +3484,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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Tarantool-patches] [PATCH v5 1/6] sql: remove unused DOUBLE to INTEGER conversion 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 1/6] sql: remove unused DOUBLE to INTEGER conversion imeevma @ 2020-09-17 14:48 ` Nikita Pettik 2020-09-28 15:50 ` Mergen Imeev 0 siblings, 1 reply; 15+ messages in thread From: Nikita Pettik @ 2020-09-17 14:48 UTC (permalink / raw) To: imeevma; +Cc: tarantool-patches On 21 Aug 12:19, imeevma@tarantool.org wrote: > This patch removes the unused DOUBLE to INTEGER conversion from OP_Seek* > opcodes. This transformation is not used due to changes in the ApplyType What changes are..? > 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 LGTM ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Tarantool-patches] [PATCH v5 1/6] sql: remove unused DOUBLE to INTEGER conversion 2020-09-17 14:48 ` Nikita Pettik @ 2020-09-28 15:50 ` Mergen Imeev 0 siblings, 0 replies; 15+ messages in thread From: Mergen Imeev @ 2020-09-28 15:50 UTC (permalink / raw) To: Nikita Pettik; +Cc: tarantool-patches Hi! Thank you for the review. My answer below. On Thu, Sep 17, 2020 at 02:48:08PM +0000, Nikita Pettik wrote: > On 21 Aug 12:19, imeevma@tarantool.org wrote: > > This patch removes the unused DOUBLE to INTEGER conversion from OP_Seek* > > opcodes. This transformation is not used due to changes in the ApplyType > > What changes are..? Changed commit-message a bit: This patch removes the unused DOUBLE to INTEGER conversion from OP_Seek* opcodes. This conversion is not used due to changes in the ApplyType opcode after #3809. 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. > > > 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 > > LGTM ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Tarantool-patches] [PATCH v5 2/6] sql: add implicit cast between numbers in OP_Seek* 2020-08-21 9:19 [Tarantool-patches] [PATCH v5 0/6] sql; remove implicit cast for comparison imeevma 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 1/6] sql: remove unused DOUBLE to INTEGER conversion imeevma @ 2020-08-21 9:19 ` imeevma 2020-09-17 15:34 ` Nikita Pettik 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 3/6] sql: change comparison between numbers using index imeevma ` (3 subsequent siblings) 5 siblings, 1 reply; 15+ messages in thread From: imeevma @ 2020-08-21 9:19 UTC (permalink / raw) To: korablev, tsafin; +Cc: tarantool-patches This patch adds new rules for implicit casting between numbers in OP_Seek * opcodes. They are still not used because the ApplyType opcode is converting numbers, but this will be changed in the next patch. Conversion within the ApplyType opcode can affect the result of comparison operations. Part of #4230 --- src/box/sql/vdbe.c | 332 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 332 insertions(+) diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 7405009a7..822d7e177 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -522,6 +522,268 @@ mem_convert_to_numeric(struct Mem *mem, enum field_type type) return mem_convert_to_integer(mem); } +/** + * Convert the numeric value contained in the MEM to UNSIGNED. + * @see mem_prepare_for_cmp() for more info. + * + * @param mem The MEM that contains the numeric value. + * @param[in][out] oc Operation code. + * @retval 0 if the conversion was successful, -1 otherwise. + */ +static int +mem_prepare_for_cmp_to_uint(struct Mem *mem, int *oc, int eqOnly) +{ + if ((mem->flags & MEM_Int) != 0) { + if (eqOnly == 1) + return 0; + if (*oc == OP_SeekGT) { + mem_set_u64(mem, 0); + *oc = OP_SeekGE; + return 0; + } + if (*oc == OP_SeekGE) { + mem_set_u64(mem, 0); + return 0; + } + if (*oc == OP_SeekLT) + return 0; + assert(*oc == OP_SeekLE); + return 0; + } + assert((mem->flags & MEM_Real) != 0); + double d = mem->u.r; + if (d >= 0.0 && d < (double)UINT64_MAX && d == (double)(uint64_t)d) + return mem_convert_to_unsigned(mem); + if (eqOnly == 1) + return 0; + if (*oc == OP_SeekGT) { + if (d >= (double)UINT64_MAX) + return 0; + if (d < 0) { + mem_set_u64(mem, 0); + *oc = OP_SeekGE; + return 0; + } + assert((double)(uint64_t)d < d); + return mem_convert_to_unsigned(mem); + } + if (*oc == OP_SeekGE) { + if (d >= (double)UINT64_MAX) + return 0; + if (d < 0) { + mem_set_u64(mem, 0); + return 0; + } + assert((double)(uint64_t)d < d); + *oc = OP_SeekGT; + return mem_convert_to_unsigned(mem); + } + if (*oc == OP_SeekLT) { + if (d >= (double)UINT64_MAX) { + *oc = OP_SeekLE; + mem_set_u64(mem, UINT64_MAX); + return 0; + } + if (d < 0) + return 0; + assert((double)(uint64_t)d < d); + *oc = OP_SeekLE; + return mem_convert_to_unsigned(mem); + } + assert(*oc == OP_SeekLE); + if (d >= (double)UINT64_MAX) { + mem_set_u64(mem, UINT64_MAX); + return 0; + } + if (d < 0) + return 0; + assert((double)(uint64_t)d < d); + return mem_convert_to_unsigned(mem); +} + +/** + * Convert the numeric value contained in the MEM to INTEGER. + * @see mem_prepare_for_cmp() for more info. + * + * @param mem The MEM that contains the numeric value. + * @param[in][out] oc Operation code. + * @retval 0 if the conversion was successful, -1 otherwise. + */ +static int +mem_prepare_for_cmp_to_int(struct Mem *mem, int *oc, int eqOnly) +{ + assert((mem->flags & MEM_Real) != 0); + double d = mem->u.r; + if (d >= 0.0 && d < (double)UINT64_MAX && d == (double)(uint64_t)d) + return mem_convert_to_integer(mem); + if (d >= (double)INT64_MIN && d < (double)INT64_MAX && + d == (double)(uint64_t)d) + return mem_convert_to_integer(mem); + if (eqOnly == 1) + return 0; + if (*oc == OP_SeekGT) { + if (d >= (double)UINT64_MAX) + return 0; + if (d < (double)INT64_MIN) { + mem_set_i64(mem, INT64_MIN); + *oc = OP_SeekGE; + return 0; + } + if (d > 0 || (double)(int64_t)d < d) + return mem_convert_to_integer(mem); + *oc = OP_SeekGE; + return mem_convert_to_integer(mem); + } + if (*oc == OP_SeekGE) { + if (d >= (double)UINT64_MAX) + return 0; + if (d < (double)INT64_MIN) { + mem_set_i64(mem, INT64_MIN); + return 0; + } + if (d > 0 || (double)(int64_t)d < d) { + *oc = OP_SeekGT; + return mem_convert_to_integer(mem); + } + return mem_convert_to_integer(mem); + } + if (*oc == OP_SeekLT) { + if (d >= (double)UINT64_MAX) { + *oc = OP_SeekLE; + mem_set_int(mem, UINT64_MAX, false); + return 0; + } + if (d < (double)INT64_MIN) + return 0; + if (d > 0 || (double)(int64_t)d < d) { + *oc = OP_SeekLE; + return mem_convert_to_integer(mem); + } + return mem_convert_to_integer(mem); + } + assert(*oc == OP_SeekLE); + if (d >= (double)UINT64_MAX) { + mem_set_int(mem, UINT64_MAX, false); + return 0; + } + if (d > 0 || (double)(int64_t)d < d) + return mem_convert_to_integer(mem); + *oc = OP_SeekLT; + return mem_convert_to_integer(mem); +} + +/** + * Convert the numeric value contained in the MEM to DOUBLE. + * @see mem_prepare_for_cmp() for more info. + * + * @param mem The MEM that contains the numeric value. + * @param[in][out] oc Operation code. + * @retval 0 if the conversion was successful, -1 otherwise. + */ +static int +mem_prepare_for_cmp_to_double(struct Mem *mem, int *oc, int eqOnly) +{ + if ((mem->flags & MEM_Int) != 0) { + int64_t i = mem->u.i; + if (i == (int64_t)(double)i) + return mem_convert_to_double(mem); + if (eqOnly == 1) + return 0; + double d = (double)i; + if (*oc == OP_SeekGT) { + if (d == (double)INT64_MAX || i < (int64_t)d) + *oc = OP_SeekGE; + return mem_convert_to_double(mem); + } + if (*oc == OP_SeekGE) { + if (d != (double)INT64_MAX && i > (int64_t)d) + *oc = OP_SeekGT; + return mem_convert_to_double(mem); + } + if (*oc == OP_SeekLT) { + if (d != (double)INT64_MAX && i > (int64_t)d) + *oc = OP_SeekLE; + return mem_convert_to_double(mem); + } + assert(*oc == OP_SeekLE); + if (d == (double)INT64_MAX || i < (int64_t)d) + *oc = OP_SeekLT; + return mem_convert_to_double(mem); + } + assert((mem->flags & MEM_UInt) != 0); + uint64_t u = mem->u.u; + if (u == (uint64_t)(double)u) + return mem_convert_to_double(mem); + if (eqOnly == 1) + return 0; + double d = (double)u; + if (*oc == OP_SeekGT) { + if (d == (double)UINT64_MAX || u < (uint64_t)d) + *oc = OP_SeekGE; + return mem_convert_to_double(mem); + } + if (*oc == OP_SeekGE) { + if (d != (double)UINT64_MAX && u > (uint64_t)d) + *oc = OP_SeekGT; + return mem_convert_to_double(mem); + } + if (*oc == OP_SeekLT) { + if (d != (double)UINT64_MAX && u > (uint64_t)d) + *oc = OP_SeekLE; + return mem_convert_to_double(mem); + } + assert(*oc == OP_SeekLE); + if (d == (double)UINT64_MAX || u < (uint64_t)d) + *oc = OP_SeekLT; + return mem_convert_to_double(mem); +} + +/** + * Convert the numeric value contained in the MEM to another + * numeric type according to the specified operation. If the + * conversion is successful, we will get the converted MEM. If the + * conversion fails, the MEM will not be changed. + * + * There are two reasons why the MEM might not convert. + * 1) MEM conversion affects the result of the operation. + * For example: + * CREATE TABLE t (i INT PRIMARY KEY); + * ... + * SELECT * FROM t WHERE i = 1.5; + * + * 2) After conversion, nothing will be found as a result of the + * operation. + * For example: + * CREATE TABLE t (i INT PRIMARY KEY); + * ... + * SELECT * FROM t WHERE i > 2^100; + * + * + * If the conversion is successful, the operation can also change. + * For example: + * CREATE TABLE t (i INT PRIMARY KEY); + * ... + * SELECT * FROM t WHERE i > -(2^100); + * + * The value becomes INT64_MIN after conversion and the operation + * becomes '> ='. + * + * @param mem The MEM that contains the numeric value. + * @param type The type to convert to. + * @param[in][out] oc Operation code. + * @retval 0 if the conversion was successful, -1 otherwise. + */ +static int +mem_prepare_for_cmp(struct Mem *mem, enum field_type type, int *oc, int eqOnly) +{ + if (type == FIELD_TYPE_UNSIGNED) + return mem_prepare_for_cmp_to_uint(mem, oc, eqOnly); + if (type == FIELD_TYPE_INTEGER) + return mem_prepare_for_cmp_to_int(mem, oc, eqOnly); + assert(type == FIELD_TYPE_DOUBLE); + return mem_prepare_for_cmp_to_double(mem, oc, eqOnly); +} + /* * pMem currently only holds a string type (or maybe a BLOB that we can * interpret as a string if we want to). Compute its corresponding @@ -3491,6 +3753,74 @@ case OP_SeekGT: { /* jump, in3 */ assert(oc!=OP_SeekLT || r.default_rc==+1); r.aMem = &aMem[pOp->p3]; + bool is_on_region = false; + struct region *region = &fiber()->gc; + size_t region_svp = 0; + for (int i = 0; i < r.nField; ++i) { + struct Mem *mem = &r.aMem[i]; + enum field_type type = r.key_def->parts[i].type; + /* + * We already know that MEM_type and field type + * are comparable. If they are not compatible, we + * should try to convert MEM to field type. + */ + if (!mem_is_type_compatible(mem, type)) { + /* + * We cannot make changes to original MEMs + * since they will be used in OP_Idx*. So + * we should copy them and make changes to + * the copies. + */ + if (!is_on_region) { + region_svp = region_used(region); + uint32_t size = sizeof(struct Mem) * r.nField; + r.aMem = region_aligned_alloc(region, size, + alignof(struct Mem)); + if (r.aMem == NULL) { + diag_set(OutOfMemory, size, + "region_aligned_alloc", + "r.aMem"); + goto abort_due_to_error; + } + memcpy(r.aMem, &aMem[pOp->p3], size); + is_on_region = true; + mem = &r.aMem[i]; + } + /* + * In cases where we can change the MEM + * according to the field type and opcode, + * we will get the converted MEM after + * this function. + * + * There are two cases where MEM does not + * change: + * 1) any resultof conversion can affect + * the result of the operation; + * 2) after conversion nothing will be + * found as a result of the operation. + * + * Examples can be found in description of + * the function. + */ + if (mem_prepare_for_cmp(mem, type, &oc, eqOnly) != 0) { + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, + sql_value_to_diag_str(mem), + field_type_strs[type]); + goto abort_due_to_error; + } + /* + * if the MEM type and the field type are + * still not compatible, then the + * conversion failed and we won't find + * anything. + */ + if (!mem_is_type_compatible(mem, type)) { + res = 1; + goto seek_not_found; + } + } + } + #ifdef SQL_DEBUG { int i; for(i=0; i<r.nField; i++) assert(memIsValid(&r.aMem[i])); } #endif @@ -3528,6 +3858,8 @@ case OP_SeekGT: { /* jump, in3 */ } } seek_not_found: + if (is_on_region) + region_truncate(region, region_svp); assert(pOp->p2>0); VdbeBranchTaken(res!=0,2); if (res) { -- 2.25.1 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Tarantool-patches] [PATCH v5 2/6] sql: add implicit cast between numbers in OP_Seek* 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 2/6] sql: add implicit cast between numbers in OP_Seek* imeevma @ 2020-09-17 15:34 ` Nikita Pettik 2020-09-28 15:55 ` Mergen Imeev 0 siblings, 1 reply; 15+ messages in thread From: Nikita Pettik @ 2020-09-17 15:34 UTC (permalink / raw) To: imeevma; +Cc: tarantool-patches On 21 Aug 12:19, imeevma@tarantool.org wrote: > This patch adds new rules for implicit casting between numbers in > OP_Seek * opcodes. They are still not used because the ApplyType > opcode is converting numbers, but this will be changed in the next > patch. Conversion within the ApplyType opcode can affect the > result of comparison operations. > > Part of #4230 > --- > src/box/sql/vdbe.c | 332 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 332 insertions(+) > > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c > index 7405009a7..822d7e177 100644 > --- a/src/box/sql/vdbe.c > +++ b/src/box/sql/vdbe.c > @@ -522,6 +522,268 @@ mem_convert_to_numeric(struct Mem *mem, enum field_type type) > return mem_convert_to_integer(mem); > } > > +/** > + * Convert the numeric value contained in the MEM to UNSIGNED. > + * @see mem_prepare_for_cmp() for more info. > + * > + * @param mem The MEM that contains the numeric value. > + * @param[in][out] oc Operation code. > + * @retval 0 if the conversion was successful, -1 otherwise. > + */ > +static int Would be nice to see sort of table containing value/action columns, i.e. what should be done in each seprate case. These three functions are quite complicated and can contain bugs with ease. At first sight I find no obvious mistakes. > +/** > + * Convert the numeric value contained in the MEM to another > + * numeric type according to the specified operation. If the > + * conversion is successful, we will get the converted MEM. If the > + * conversion fails, the MEM will not be changed. > + * > + * @param mem The MEM that contains the numeric value. > + * @param type The type to convert to. > + * @param[in][out] oc Operation code. Just [out] - all parameters are in > + * @retval 0 if the conversion was successful, -1 otherwise. > + */ > +static int > +mem_prepare_for_cmp(struct Mem *mem, enum field_type type, int *oc, int eqOnly) Please rename according to guides eqOnly param and mention it in the comment. > +{ > + if (type == FIELD_TYPE_UNSIGNED) > + return mem_prepare_for_cmp_to_uint(mem, oc, eqOnly); > + if (type == FIELD_TYPE_INTEGER) > + return mem_prepare_for_cmp_to_int(mem, oc, eqOnly); > + assert(type == FIELD_TYPE_DOUBLE); > + return mem_prepare_for_cmp_to_double(mem, oc, eqOnly); > +} > + > /* > * pMem currently only holds a string type (or maybe a BLOB that we can > * interpret as a string if we want to). Compute its corresponding ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Tarantool-patches] [PATCH v5 2/6] sql: add implicit cast between numbers in OP_Seek* 2020-09-17 15:34 ` Nikita Pettik @ 2020-09-28 15:55 ` Mergen Imeev 0 siblings, 0 replies; 15+ messages in thread From: Mergen Imeev @ 2020-09-28 15:55 UTC (permalink / raw) To: Nikita Pettik; +Cc: tarantool-patches Thank you for the review. I reworked this patch. I think it become a lot simpler. My answers and new patch below. On Thu, Sep 17, 2020 at 03:34:09PM +0000, Nikita Pettik wrote: > On 21 Aug 12:19, imeevma@tarantool.org wrote: > > This patch adds new rules for implicit casting between numbers in > > OP_Seek * opcodes. They are still not used because the ApplyType > > opcode is converting numbers, but this will be changed in the next > > patch. Conversion within the ApplyType opcode can affect the > > result of comparison operations. > > > > Part of #4230 > > --- > > src/box/sql/vdbe.c | 332 +++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 332 insertions(+) > > > > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c > > index 7405009a7..822d7e177 100644 > > --- a/src/box/sql/vdbe.c > > +++ b/src/box/sql/vdbe.c > > @@ -522,6 +522,268 @@ mem_convert_to_numeric(struct Mem *mem, enum field_type type) > > return mem_convert_to_integer(mem); > > } > > > > +/** > > + * Convert the numeric value contained in the MEM to UNSIGNED. > > + * @see mem_prepare_for_cmp() for more info. > > + * > > + * @param mem The MEM that contains the numeric value. > > + * @param[in][out] oc Operation code. > > + * @retval 0 if the conversion was successful, -1 otherwise. > > + */ > > +static int > > Would be nice to see sort of table containing value/action columns, > i.e. what should be done in each seprate case. These three functions are > quite complicated and can contain bugs with ease. At first sight I find > no obvious mistakes. There was a letter: "Rules for comparing numbers using Tarantool indexes in SQL". However, after I reworked this patch it become a lot easier to understand. Rules of the convertion can be seen in description of the functions. > > > +/** > > + * Convert the numeric value contained in the MEM to another > > + * numeric type according to the specified operation. If the > > + * conversion is successful, we will get the converted MEM. If the > > + * conversion fails, the MEM will not be changed. > > + * > > + * @param mem The MEM that contains the numeric value. > > + * @param type The type to convert to. > > + * @param[in][out] oc Operation code. > > Just [out] - all parameters are in Fixed. > > > + * @retval 0 if the conversion was successful, -1 otherwise. > > + */ > > +static int > > +mem_prepare_for_cmp(struct Mem *mem, enum field_type type, int *oc, int eqOnly) > > Please rename according to guides eqOnly param and mention it in the comment. Fixed, since this argument is removed. > > > +{ > > + if (type == FIELD_TYPE_UNSIGNED) > > + return mem_prepare_for_cmp_to_uint(mem, oc, eqOnly); > > + if (type == FIELD_TYPE_INTEGER) > > + return mem_prepare_for_cmp_to_int(mem, oc, eqOnly); > > + assert(type == FIELD_TYPE_DOUBLE); > > + return mem_prepare_for_cmp_to_double(mem, oc, eqOnly); > > +} > > + > > /* > > * pMem currently only holds a string type (or maybe a BLOB that we can > > * interpret as a string if we want to). Compute its corresponding New patch: commit a6e7ecf454905741bfaa5c4dea9b58f3f5f647ca Author: Mergen Imeev <imeevma@gmail.com> Date: Wed Jul 15 20:00:05 2020 +0300 sql: add implicit cast between numbers in OP_Seek* This patch adds new rules for implicit casting between numbers in OP_Seek * opcodes. They are still not used because the ApplyType opcode is converting numbers, but this will be changed in the next patch. Conversion within the ApplyType opcode can affect the result of comparison operations. Part of #4230 diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 7405009a7..bf753429f 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -522,6 +522,235 @@ mem_convert_to_numeric(struct Mem *mem, enum field_type type) return mem_convert_to_integer(mem); } +/** + * Check if it is possible to convert numeric value to given numeric type + * without precision loss. + * + * @param mem The MEM that contains the numeric value. + * @param type Field type. + * @retval true if conversion can be performed without precision loss, false + * otherwise. + */ +static bool +mem_is_convertion_lossless(struct Mem *mem, enum field_type type) +{ + enum mp_type mp_type = mem_mp_type(mem); + assert(sql_type_is_numeric(type) && mp_type_is_numeric(mp_type)); + assert(!mem_is_type_compatible(mem, type)); + if (type == FIELD_TYPE_UNSIGNED) { + if (mp_type == MP_INT) + return false; + assert(mp_type == MP_DOUBLE); + double d = mem->u.r; + if (d >= 0.0 && d < (double)UINT64_MAX && + d == (double)(uint64_t)d) + return true; + return false; + } + if (type == FIELD_TYPE_INTEGER) { + assert(mp_type == MP_DOUBLE); + double d = mem->u.r; + if (d < (double)INT64_MIN || d > (double)UINT64_MAX) + return false; + if (d < 0.0 && d == (double)(int64_t)d) + return true; + if (d >= 0.0 && d == (double)(uint64_t)d) + return true; + return false; + } + assert(type == FIELD_TYPE_DOUBLE); + if (mp_type == MP_INT && mem->u.i == (int64_t)(double)mem->u.i) + return true; + if (mp_type == MP_UINT && mem->u.u == (uint64_t)(double)mem->u.u) + return true; + return false; +} + +static inline int +new_comparison_operation(int oc, bool has_value_increased) +{ + if (has_value_increased) { + if (oc == OP_SeekGT) + return OP_SeekGE; + if (oc == OP_SeekLE) + return OP_SeekLT; + return oc; + } + if (oc == OP_SeekGE) + return OP_SeekGT; + if (oc == OP_SeekLT) + return OP_SeekLE; + return oc; +} + +/** + * Convert the numeric value contained in the MEM to UNSIGNED. If successful + * mem is always of field type UNSIGNED after this function. Operation given to + * this function is one of four: '>', '>=', '<', '<='. Operations '==' and '!=' + * are not included. Due to conversion operation may change. + * + * Main rules of the conversion: + * a) If conversion can be done without precision loss, it is done. Nothing else + * changes. + * b) If after conversion value becomes less than before, operation '>=' changed + * to '>' and operation '<' changed to '<='. + * c) If after conversion value becomes more than before, operation '>' changed + * to '>=' and operation '<=' changed to '<'. + * + * Additional rules: + * - If value is less than 0 it becomes 0 and rule 'c' applied. + * - If value is more than UINT64_MAX it becomes UINT64_MAX and rule 'b' + * applied. + * + * @param mem The MEM that contains the numeric value. + * @param[out] oc Operation code. + * @retval 0 if the conversion was successful, -1 otherwise. + */ +static int +mem_prepare_for_cmp_to_uint(struct Mem *mem, int *oc) +{ + enum mp_type type = mem_mp_type(mem); + if (type == MP_INT) { + mem_set_u64(mem, 0); + *oc = new_comparison_operation(*oc, true); + return 0; + } + assert(type == MP_DOUBLE); + double d = mem->u.r; + if (d < 0.0) { + mem_set_u64(mem, 0); + *oc = new_comparison_operation(*oc, true); + return 0; + } + if (d >= (double)UINT64_MAX) { + mem_set_u64(mem, UINT64_MAX); + *oc = new_comparison_operation(*oc, false); + return 0; + } + if (d == (double)(uint64_t)d) + return mem_convert_to_unsigned(mem); + assert(d > (double)(uint64_t)d); + *oc = new_comparison_operation(*oc, false); + return mem_convert_to_unsigned(mem); +} + +/** + * Convert the numeric value contained in the MEM to INTEGER. If successful + * mem is always of field type INTEGER after this function. Operation given to + * this function is one of four: '>', '>=', '<', '<='. Operations '==' and '!=' + * are not included. Due to conversion operation may change. + * + * Main rules of the conversion: + * a) If conversion can be done without precision loss, it is done. Nothing else + * changes. + * b) If after conversion value becomes less than before, operation '>=' changed + * to '>' and operation '<' changed to '<='. + * c) If after conversion value becomes more than before, operation '>' changed + * to '>=' and operation '<=' changed to '<'. + * + * Additional rules: + * - If value is less than INT64_MIN it becomes INT64_MIN and rule 'c' applied. + * - If value is more than UINT64_MAX it becomes UINT64_MAX and rule 'b' + * applied. + * + * @param mem The MEM that contains the numeric value. + * @param[out] oc Operation code. + * @retval 0 if the conversion was successful, -1 otherwise. + */ +static int +mem_prepare_for_cmp_to_int(struct Mem *mem, int *oc) +{ + assert(mem_mp_type(mem) == MP_DOUBLE); + double d = mem->u.r; + if (d < (double)INT64_MIN) { + mem_set_i64(mem, INT64_MIN); + *oc = new_comparison_operation(*oc, true); + return 0; + } + if (d >= (double)UINT64_MAX) { + mem_set_u64(mem, UINT64_MAX); + *oc = new_comparison_operation(*oc, false); + return 0; + } + if (d >= 0.0 && d == (double)(uint64_t)d) + return mem_convert_to_integer(mem); + if (d < 0.0 && d == (double)(int64_t)d) + return mem_convert_to_integer(mem); + bool has_increased = d < 0.0 && (double)(int64_t)d < d; + *oc = new_comparison_operation(*oc, has_increased); + return mem_convert_to_integer(mem); +} + +/** + * Convert the numeric value contained in the MEM to DOUBLE. If successful + * mem is always of field type DOUBLE after this procedure. Operation given to + * this function is one of four: '>', '>=', '<', '<='. Operations '==' and '!=' + * are not included. Due to conversion operation may change. + * + * Rules of the conversion: + * a) If conversion can be done without precision loss, it is done. Nothing else + * changes. + * b) If after conversion value becomes less than before, operation '>=' changed + * to '>' and operation '<' changed to '<='. + * c) If after conversion value becomes more than before, operation '>' changed + * to '>=' and operation '<=' changed to '<'. + * + * @param mem The MEM that contains the numeric value. + * @param[out] oc Operation code. + * @retval 0 if the conversion was successful, -1 otherwise. + */ +static int +mem_prepare_for_cmp_to_double(struct Mem *mem, int *oc) +{ + enum mp_type type = mem_mp_type(mem); + bool has_increased; + if (type == MP_INT) { + int64_t i = mem->u.i; + if (i == (int64_t)(double)i) + return mem_convert_to_double(mem); + double d = (double)i; + has_increased = d == (double)INT64_MAX || i < (int64_t)d; + *oc = new_comparison_operation(*oc, has_increased); + } else { + uint64_t u = mem->u.u; + if (u == (uint64_t)(double)u) + return mem_convert_to_double(mem); + double d = (double)u; + has_increased = d == (double)UINT64_MAX || u < (uint64_t)d; + } + *oc = new_comparison_operation(*oc, has_increased); + return mem_convert_to_double(mem); +} + +/** + * Convert the numeric value contained in the MEM to another numeric type + * according to the specified operation. If successful, we will get the MEM of + * given field type. Due to conversion operation may change. + * + * Rules of the conversion: + * a) If conversion can be done without precision loss, it is done. Nothing else + * changes. + * b) If after conversion value becomes less than before, operation '>=' changed + * to '>' and operation '<' changed to '<='. + * c) If after conversion value becomes more than before, operation '>' changed + * to '>=' and operation '<=' changed to '<'. + * + * @param mem The MEM that contains the numeric value. + * @param type The field type to convert to. + * @param[out] oc Operation code. + * @retval 0 if the conversion was successful, -1 otherwise. + */ +static int +mem_prepare_for_cmp(struct Mem *mem, enum field_type type, int *oc) +{ + if (type == FIELD_TYPE_UNSIGNED) + return mem_prepare_for_cmp_to_uint(mem, oc); + if (type == FIELD_TYPE_INTEGER) + return mem_prepare_for_cmp_to_int(mem, oc); + assert(type == FIELD_TYPE_DOUBLE); + return mem_prepare_for_cmp_to_double(mem, oc); +} + /* * pMem currently only holds a string type (or maybe a BLOB that we can * interpret as a string if we want to). Compute its corresponding @@ -3491,6 +3720,66 @@ case OP_SeekGT: { /* jump, in3 */ assert(oc!=OP_SeekLT || r.default_rc==+1); r.aMem = &aMem[pOp->p3]; + bool is_on_region = false; + for (int i = 0; i < r.nField; ++i) { + struct Mem *mem = &r.aMem[i]; + enum field_type type = r.key_def->parts[i].type; + /* + * Comparison possible only if MEM_type and field type are + * compatible or both are numeric. In the second case we need to + * convert MEM to field type. + */ + if (mem_is_type_compatible(mem, type)) + continue; + /* + * Since OP_ApplyType was applied prior to this opcode, we can + * say that the only case where MEM_type and type incompatible, + * but comparable, is when MEM contains a numeric value and type + * is a numeric type. + */ + assert(mp_type_is_numeric(mem_mp_type(mem)) && + sql_type_is_numeric(type)); + /* + * We cannot make changes to original MEMs since they will be + * used in OP_Idx*. We should copy them and make changes to the + * copies. + */ + if (!is_on_region) { + uint32_t size = sizeof(struct Mem) * r.nField; + r.aMem = region_aligned_alloc(&fiber()->gc, size, + alignof(struct Mem)); + if (r.aMem == NULL) { + diag_set(OutOfMemory, size, + "region_aligned_alloc", "r.aMem"); + goto abort_due_to_error; + } + memcpy(r.aMem, &aMem[pOp->p3], size); + is_on_region = true; + mem = &r.aMem[i]; + } + /* + * In case our operation is EQ we may convert value only if + * convertion is lossless. In any other case we may receive + * wrong result. If convertion is not lossless we may say that + * we will find nothing. For example 1.5 != any integer. + */ + if (eqOnly == 1) { + if (!mem_is_convertion_lossless(mem, type)) { + res = 1; + goto seek_not_found; + } + mem_convert_to_numeric(mem, type); + } else { + if (mem_prepare_for_cmp(mem, type, &oc) != 0) { + diag_set(ClientError, + ER_SQL_TYPE_MISMATCH, + sql_value_to_diag_str(mem), + field_type_strs[type]); + goto abort_due_to_error; + } + } + } + #ifdef SQL_DEBUG { int i; for(i=0; i<r.nField; i++) assert(memIsValid(&r.aMem[i])); } #endif ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Tarantool-patches] [PATCH v5 3/6] sql: change comparison between numbers using index 2020-08-21 9:19 [Tarantool-patches] [PATCH v5 0/6] sql; remove implicit cast for comparison imeevma 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 1/6] sql: remove unused DOUBLE to INTEGER conversion imeevma 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 2/6] sql: add implicit cast between numbers in OP_Seek* imeevma @ 2020-08-21 9:19 ` imeevma 2020-09-18 8:08 ` Nikita Pettik 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 4/6] sql: remove implicit cast from comparison opcodes imeevma ` (2 subsequent siblings) 5 siblings, 1 reply; 15+ messages in thread From: imeevma @ 2020-08-21 9:19 UTC (permalink / raw) To: korablev, tsafin; +Cc: tarantool-patches 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Tarantool-patches] [PATCH v5 3/6] sql: change comparison between numbers using index 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 3/6] sql: change comparison between numbers using index imeevma @ 2020-09-18 8:08 ` Nikita Pettik 2020-09-28 16:10 ` Mergen Imeev 0 siblings, 1 reply; 15+ messages in thread From: Nikita Pettik @ 2020-09-18 8:08 UTC (permalink / raw) To: imeevma; +Cc: tarantool-patches On 21 Aug 12:19, imeevma@tarantool.org wrote: > 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. */ Useless comment. > +#define OPFLAG_DO_NOT_CONVERT_NUMBERS 0x01 > 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}) I'd place also eqp statements to make sure that index is really used. > +--- > +- row_count: 1 > +... ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Tarantool-patches] [PATCH v5 3/6] sql: change comparison between numbers using index 2020-09-18 8:08 ` Nikita Pettik @ 2020-09-28 16:10 ` Mergen Imeev 0 siblings, 0 replies; 15+ messages in thread From: Mergen Imeev @ 2020-09-28 16:10 UTC (permalink / raw) To: Nikita Pettik; +Cc: tarantool-patches Thank you for review! My answers and diff below. On Fri, Sep 18, 2020 at 08:08:21AM +0000, Nikita Pettik wrote: > On 21 Aug 12:19, imeevma@tarantool.org wrote: > > 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. */ > > Useless comment. I do not think so, since it shows that this MACRO should be used only in OP_ApplyType. > > > +#define OPFLAG_DO_NOT_CONVERT_NUMBERS 0x01 > > 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}) > > I'd place also eqp statements to make sure that index is really used. Done. It helped to find that there wasn't an index in column 'a'. Also fixed. > > > +--- > > +- row_count: 1 > > +... Diff: diff --git a/test/sql/types.result b/test/sql/types.result index 8810a9f82..9a7578d75 100644 --- a/test/sql/types.result +++ b/test/sql/types.result @@ -2799,7 +2796,7 @@ 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([[CREATE TABLE t (i INTEGER PRIMARY KEY, a DOUBLE UNIQUE);]]) --- - row_count: 1 ... @@ -2807,6 +2804,20 @@ box.execute([[INSERT INTO t VALUES (1, ?);]], {2^60}) --- - row_count: 1 ... +box.execute([[EXPLAIN QUERY PLAN SELECT * FROM t WHERE i > ?]], {2^70}) +--- +- metadata: + - name: selectid + type: integer + - name: order + type: integer + - name: from + type: integer + - name: detail + type: text + rows: + - [0, 0, 0, 'SEARCH TABLE T USING PRIMARY KEY (I>?) (~262144 rows)'] +... box.execute([[SELECT * FROM t WHERE i > ?]], {2^70}) --- - metadata: @@ -2816,6 +2827,20 @@ box.execute([[SELECT * FROM t WHERE i > ?]], {2^70}) type: double rows: [] ... +box.execute([[EXPLAIN QUERY PLAN SELECT * FROM t WHERE i > ?]], {-2^70}) +--- +- metadata: + - name: selectid + type: integer + - name: order + type: integer + - name: from + type: integer + - name: detail + type: text + rows: + - [0, 0, 0, 'SEARCH TABLE T USING PRIMARY KEY (I>?) (~262144 rows)'] +... box.execute([[SELECT * FROM t WHERE i > ?]], {-2^70}) --- - metadata: @@ -2826,6 +2851,20 @@ box.execute([[SELECT * FROM t WHERE i > ?]], {-2^70}) rows: - [1, 1152921504606846976] ... +box.execute([[EXPLAIN QUERY PLAN SELECT * FROM t WHERE a = ?]], {2ULL^60ULL - 1ULL}) +--- +- metadata: + - name: selectid + type: integer + - name: order + type: integer + - name: from + type: integer + - name: detail + type: text + rows: + - [0, 0, 0, 'SEARCH TABLE T USING COVERING INDEX unique_unnamed_T_2 (A=?) (~1 row)'] +... box.execute([[SELECT * FROM t WHERE a = ?]], {2ULL^60ULL - 1ULL}) --- - metadata: @@ -2835,6 +2874,21 @@ box.execute([[SELECT * FROM t WHERE a = ?]], {2ULL^60ULL - 1ULL}) type: double rows: [] ... +box.execute([[EXPLAIN QUERY PLAN SELECT * FROM t WHERE a > ?]], {2ULL^60ULL - 1ULL}) +--- +- metadata: + - name: selectid + type: integer + - name: order + type: integer + - name: from + type: integer + - name: detail + type: text + rows: + - [0, 0, 0, 'SEARCH TABLE T USING COVERING INDEX unique_unnamed_T_2 (A>?) (~262144 + rows)'] +... box.execute([[SELECT * FROM t WHERE a > ?]], {2ULL^60ULL - 1ULL}) --- - metadata: diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua index a23b12801..ae19167bd 100644 --- a/test/sql/types.test.lua +++ b/test/sql/types.test.lua @@ -628,10 +628,14 @@ 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([[CREATE TABLE t (i INTEGER PRIMARY KEY, a DOUBLE UNIQUE);]]) box.execute([[INSERT INTO t VALUES (1, ?);]], {2^60}) +box.execute([[EXPLAIN QUERY PLAN SELECT * FROM t WHERE i > ?]], {2^70}) box.execute([[SELECT * FROM t WHERE i > ?]], {2^70}) +box.execute([[EXPLAIN QUERY PLAN SELECT * FROM t WHERE i > ?]], {-2^70}) box.execute([[SELECT * FROM t WHERE i > ?]], {-2^70}) +box.execute([[EXPLAIN QUERY PLAN SELECT * FROM t WHERE a = ?]], {2ULL^60ULL - 1ULL}) box.execute([[SELECT * FROM t WHERE a = ?]], {2ULL^60ULL - 1ULL}) +box.execute([[EXPLAIN QUERY PLAN SELECT * FROM t WHERE a > ?]], {2ULL^60ULL - 1ULL}) box.execute([[SELECT * FROM t WHERE a > ?]], {2ULL^60ULL - 1ULL}) box.execute([[DROP TABLE t;]]) ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Tarantool-patches] [PATCH v5 4/6] sql: remove implicit cast from comparison opcodes 2020-08-21 9:19 [Tarantool-patches] [PATCH v5 0/6] sql; remove implicit cast for comparison imeevma ` (2 preceding siblings ...) 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 3/6] sql: change comparison between numbers using index imeevma @ 2020-08-21 9:19 ` imeevma 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 5/6] sql: fix implicit cast in opcode MustBeInt imeevma 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 6/6] sql: remove implicit cast from MakeRecord opcode imeevma 5 siblings, 0 replies; 15+ messages in thread From: imeevma @ 2020-08-21 9:19 UTC (permalink / raw) To: korablev, tsafin; +Cc: tarantool-patches This patch removes implicit casting from STRING to number and vice versa from comparison opcodes. Part of #4230 --- src/box/sql/vdbe.c | 52 ++- test/sql-tap/identifier_case.test.lua | 6 +- test/sql-tap/in1.test.lua | 4 +- test/sql-tap/insert3.test.lua | 2 +- test/sql-tap/join.test.lua | 4 +- test/sql-tap/misc1.test.lua | 32 +- test/sql-tap/select1.test.lua | 4 +- test/sql-tap/select7.test.lua | 2 +- test/sql-tap/subquery.test.lua | 4 +- test/sql-tap/tkt-9a8b09f8e6.test.lua | 508 -------------------------- test/sql-tap/tkt3493.test.lua | 40 +- test/sql-tap/transitive1.test.lua | 12 +- test/sql-tap/where2.test.lua | 183 +--------- test/sql-tap/where5.test.lua | 12 +- test/sql/types.result | 7 +- 15 files changed, 86 insertions(+), 786 deletions(-) delete mode 100755 test/sql-tap/tkt-9a8b09f8e6.test.lua diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index a377ceae7..b326c4ba4 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -2577,22 +2577,17 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ } else { enum field_type type = pOp->p5 & FIELD_TYPE_MASK; if (sql_type_is_numeric(type)) { - if ((flags1 | flags3)&MEM_Str) { - if ((flags1 & MEM_Str) == MEM_Str) { - mem_apply_numeric_type(pIn1); - testcase( flags3!=pIn3->flags); /* Possible if pIn1==pIn3 */ - flags3 = pIn3->flags; - } - if ((flags3 & MEM_Str) == MEM_Str) { - if (mem_apply_numeric_type(pIn3) != 0) { - diag_set(ClientError, - ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn3), - "numeric"); - goto abort_due_to_error; - } - - } + if ((flags1 & MEM_Str) == MEM_Str) { + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, + sql_value_to_diag_str(pIn1), + "numeric"); + goto abort_due_to_error; + } + if ((flags3 & MEM_Str) == MEM_Str) { + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, + sql_value_to_diag_str(pIn3), + "numeric"); + goto abort_due_to_error; } /* Handle the common case of integer comparison here, as an * optimization, to avoid a call to sqlMemCompare() @@ -2625,22 +2620,17 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ goto compare_op; } } else if (type == FIELD_TYPE_STRING) { - if ((flags1 & MEM_Str) == 0 && - (flags1 & (MEM_Int | MEM_UInt | MEM_Real)) != 0) { - testcase( pIn1->flags & MEM_Int); - testcase( pIn1->flags & MEM_Real); - sqlVdbeMemStringify(pIn1); - testcase( (flags1&MEM_Dyn) != (pIn1->flags&MEM_Dyn)); - flags1 = (pIn1->flags & ~MEM_TypeMask) | (flags1 & MEM_TypeMask); - assert(pIn1!=pIn3); + if ((flags1 & MEM_Str) == 0) { + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, + mem_type_to_str(pIn3), + mem_type_to_str(pIn1)); + goto abort_due_to_error; } - if ((flags3 & MEM_Str) == 0 && - (flags3 & (MEM_Int | MEM_UInt | MEM_Real)) != 0) { - testcase( pIn3->flags & MEM_Int); - testcase( pIn3->flags & MEM_Real); - sqlVdbeMemStringify(pIn3); - testcase( (flags3&MEM_Dyn) != (pIn3->flags&MEM_Dyn)); - flags3 = (pIn3->flags & ~MEM_TypeMask) | (flags3 & MEM_TypeMask); + if ((flags3 & MEM_Str) == 0) { + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, + mem_type_to_str(pIn1), + mem_type_to_str(pIn3)); + goto abort_due_to_error; } } assert(pOp->p4type==P4_COLLSEQ || pOp->p4.pColl==0); diff --git a/test/sql-tap/identifier_case.test.lua b/test/sql-tap/identifier_case.test.lua index 2a00626fc..1d56ffb44 100755 --- a/test/sql-tap/identifier_case.test.lua +++ b/test/sql-tap/identifier_case.test.lua @@ -242,11 +242,11 @@ data = { { 2, [[ 'a' < 'b' collate "binary" ]], {0, {true}}}, { 3, [[ 'a' < 'b' collate 'binary' ]], {1, [[Syntax error at line 1 near ''binary'']]}}, { 4, [[ 'a' < 'b' collate "unicode" ]], {0, {true}}}, - { 5, [[ 5 < 'b' collate "unicode" ]], {0, {true}}}, + { 5, [[ 5 < 'b' collate "unicode" ]], {1, "Type mismatch: can not convert b to numeric"}}, { 6, [[ 5 < 'b' collate unicode ]], {1,"Collation 'UNICODE' does not exist"}}, - { 7, [[ 5 < 'b' collate "unicode_ci" ]], {0, {true}}}, + { 7, [[ 5 < 'b' collate "unicode_ci" ]], {1, "Type mismatch: can not convert b to numeric"}}, { 8, [[ 5 < 'b' collate NONE ]], {1, "Collation 'NONE' does not exist"}}, - { 9, [[ 5 < 'b' collate "none" ]], {0, {true}}}, + { 9, [[ 5 < 'b' collate "none" ]], {1, "Type mismatch: can not convert b to numeric"}}, } for _, row in ipairs(data) do diff --git a/test/sql-tap/in1.test.lua b/test/sql-tap/in1.test.lua index 570cc1779..e2f498889 100755 --- a/test/sql-tap/in1.test.lua +++ b/test/sql-tap/in1.test.lua @@ -637,12 +637,12 @@ test:do_test( "in-11.2", function() -- The '2' should be coerced into 2 because t6.b is NUMERIC - return test:execsql [[ + return test:catchsql [[ SELECT * FROM t6 WHERE b IN ('2'); ]] end, { -- <in-11.2> - 1, 2 + 1, "Type mismatch: can not convert 2 to numeric" -- </in-11.2> }) diff --git a/test/sql-tap/insert3.test.lua b/test/sql-tap/insert3.test.lua index b92bc508e..3276f0db2 100755 --- a/test/sql-tap/insert3.test.lua +++ b/test/sql-tap/insert3.test.lua @@ -59,7 +59,7 @@ test:do_execsql_test( [[ CREATE TABLE log2(rowid INTEGER PRIMARY KEY AUTOINCREMENT, x TEXT UNIQUE,y INT ); CREATE TRIGGER r2 BEFORE INSERT ON t1 FOR EACH ROW BEGIN - UPDATE log2 SET y=y+1 WHERE x=new.b; + UPDATE log2 SET y=y+1 WHERE x=CAST(new.b AS STRING); INSERT OR IGNORE INTO log2(x, y) VALUES(CAST(new.b AS STRING),1); END; INSERT INTO t1(a, b) VALUES('hi', 453); diff --git a/test/sql-tap/join.test.lua b/test/sql-tap/join.test.lua index 7a1346094..a78913836 100755 --- a/test/sql-tap/join.test.lua +++ b/test/sql-tap/join.test.lua @@ -1038,13 +1038,13 @@ test:do_catchsql_test( -- </join-11.9> }) -test:do_execsql_test( +test:do_catchsql_test( "join-11.10", [[ SELECT * FROM t2 NATURAL JOIN t1 ]], { -- <join-11.10> - 1, "one", 2, "two" + 1, "Type mismatch: can not convert 1 to numeric" -- </join-11.10> }) diff --git a/test/sql-tap/misc1.test.lua b/test/sql-tap/misc1.test.lua index c0136d04c..66666878e 100755 --- a/test/sql-tap/misc1.test.lua +++ b/test/sql-tap/misc1.test.lua @@ -88,7 +88,7 @@ test:do_execsql_test( test:do_execsql_test( "misc1-1.4", [[ - SELECT x75 FROM manycol WHERE x50=350 + SELECT x75 FROM manycol WHERE x50='350' ]], { -- <misc1-1.4> "375" @@ -98,7 +98,7 @@ test:do_execsql_test( test:do_execsql_test( "misc1-1.5", [[ - SELECT x50 FROM manycol WHERE x99=599 + SELECT x50 FROM manycol WHERE x99='599' ]], { -- <misc1-1.5> "550" @@ -109,7 +109,7 @@ test:do_test( "misc1-1.6", function() test:execsql("CREATE INDEX manycol_idx1 ON manycol(x99)") - return test:execsql("SELECT x50 FROM manycol WHERE x99=899") + return test:execsql("SELECT x50 FROM manycol WHERE x99='899'") end, { -- <misc1-1.6> "850" @@ -129,7 +129,7 @@ test:do_execsql_test( test:do_test( "misc1-1.8", function() - test:execsql("DELETE FROM manycol WHERE x98=1234") + test:execsql("DELETE FROM manycol WHERE x98='1234'") return test:execsql("SELECT count(*) FROM manycol") end, { -- <misc1-1.8> @@ -140,7 +140,7 @@ test:do_test( test:do_test( "misc1-1.9", function() - test:execsql("DELETE FROM manycol WHERE x98=998") + test:execsql("DELETE FROM manycol WHERE x98='998'") return test:execsql("SELECT count(*) FROM manycol") end, { -- <misc1-1.9> @@ -151,7 +151,7 @@ test:do_test( test:do_test( "misc1-1.10", function() - test:execsql("DELETE FROM manycol WHERE x99=500") + test:execsql("DELETE FROM manycol WHERE x99='500'") return test:execsql("SELECT count(*) FROM manycol") end, { -- <misc1-1.10> @@ -162,7 +162,7 @@ test:do_test( test:do_test( "misc1-1.11", function() - test:execsql("DELETE FROM manycol WHERE x99=599") + test:execsql("DELETE FROM manycol WHERE x99='599'") return test:execsql("SELECT count(*) FROM manycol") end, { -- <misc1-1.11> @@ -479,9 +479,9 @@ local where = "" test:do_test( "misc1-10.1", function() - where = "WHERE x0>=0" + where = "WHERE x0>='0'" for i = 1, 99, 1 do - where = where .. " AND x"..i.."<>0" + where = where .. " AND x"..i.."<>'0'" end return test:catchsql("SELECT count(*) FROM manycol "..where.."") end, { @@ -496,7 +496,7 @@ test:do_test( test:do_test( "misc1-10.3", function() - where = string.gsub(where,"x0>=0", "x0=0") + where = string.gsub(where,"x0>='0'", "x0='0'") return test:catchsql("DELETE FROM manycol "..where.."") end, { -- <misc1-10.3> @@ -520,7 +520,7 @@ test:do_execsql_test( test:do_execsql_test( "misc1-10.6", [[ - SELECT x1 FROM manycol WHERE x0=100 + SELECT x1 FROM manycol WHERE x0='100' ]], { -- <misc1-10.6> "101" @@ -530,7 +530,7 @@ test:do_execsql_test( test:do_test( "misc1-10.7", function() - where = string.gsub(where, "x0=0", "x0=100") + where = string.gsub(where, "x0='0'", "x0='100'") return test:catchsql("UPDATE manycol SET x1=CAST(x1+1 AS STRING) "..where.."") end, { -- <misc1-10.7> @@ -541,7 +541,7 @@ test:do_test( test:do_execsql_test( "misc1-10.8", [[ - SELECT x1 FROM manycol WHERE x0=100 + SELECT x1 FROM manycol WHERE x0='100' ]], { -- <misc1-10.8> "102" @@ -563,7 +563,7 @@ test:do_execsql_test( test:do_execsql_test( "misc1-10.10", [[ - SELECT x1 FROM manycol WHERE x0=100 + SELECT x1 FROM manycol WHERE x0='100' ]], { -- <misc1-10.10> "103" @@ -619,13 +619,13 @@ test:do_execsql_test( -- </misc1-12.1> }) -test:do_execsql_test( +test:do_catchsql_test( "misc1-12.2", [[ SELECT '0'==0.0 ]], { -- <misc1-12.2> - true + 1, "Type mismatch: can not convert 0 to numeric" -- </misc1-12.2> }) diff --git a/test/sql-tap/select1.test.lua b/test/sql-tap/select1.test.lua index d8be38da6..8c91b4d31 100755 --- a/test/sql-tap/select1.test.lua +++ b/test/sql-tap/select1.test.lua @@ -1913,7 +1913,7 @@ test:do_execsql_test( test:do_execsql_test( "select1-12.7", [[ - SELECT * FROM t3 WHERE a=(SELECT 1); + SELECT * FROM t3 WHERE a=(SELECT '1'); ]], { -- <select1-12.7> 0, "1", "2" @@ -1923,7 +1923,7 @@ test:do_execsql_test( test:do_execsql_test( "select1-12.8", [[ - SELECT * FROM t3 WHERE a=(SELECT 2); + SELECT * FROM t3 WHERE a=(SELECT '2'); ]], { -- <select1-12.8> diff --git a/test/sql-tap/select7.test.lua b/test/sql-tap/select7.test.lua index e1e43c557..0d1390fd6 100755 --- a/test/sql-tap/select7.test.lua +++ b/test/sql-tap/select7.test.lua @@ -256,7 +256,7 @@ test:do_execsql_test( DROP TABLE IF EXISTS t5; CREATE TABLE t5(a TEXT primary key, b INT); INSERT INTO t5 VALUES('123', 456); - SELECT typeof(a), a FROM t5 GROUP BY a HAVING a<b; + SELECT typeof(a), a FROM t5 GROUP BY a HAVING CAST(a AS INTEGER)<b; ]], { -- <select7-7.7> "string", "123" diff --git a/test/sql-tap/subquery.test.lua b/test/sql-tap/subquery.test.lua index e0771825e..bad702de9 100755 --- a/test/sql-tap/subquery.test.lua +++ b/test/sql-tap/subquery.test.lua @@ -284,13 +284,13 @@ test:do_execsql_test( -- </subquery-2.3.1> }) -test:do_execsql_test( +test:do_catchsql_test( "subquery-2.3.2", [[ SELECT a IN (10.0, 20) FROM t3; ]], { -- <subquery-2.3.2> - false + 1, "Type mismatch: can not convert text to real" -- </subquery-2.3.2> }) diff --git a/test/sql-tap/tkt-9a8b09f8e6.test.lua b/test/sql-tap/tkt-9a8b09f8e6.test.lua deleted file mode 100755 index 1314d0aad..000000000 --- a/test/sql-tap/tkt-9a8b09f8e6.test.lua +++ /dev/null @@ -1,508 +0,0 @@ -#!/usr/bin/env tarantool -test = require("sqltester") -test:plan(47) - ---!./tcltestrunner.lua --- 2014 June 26 --- --- The author disclaims copyright to this source code. In place of --- a legal notice, here is a blessing: --- --- May you do good and not evil. --- May you find forgiveness for yourself and forgive others. --- May you share freely, never taking more than you give. --- -------------------------------------------------------------------------- --- This file implements regression tests for sql library. --- --- This file implements tests to verify that ticket [9a8b09f8e6] has been --- fixed. --- --- ["set","testdir",[["file","dirname",["argv0"]]]] --- ["source",[["testdir"],"\/tester.tcl"]] -testprefix = "tkt-9a8b09f8e6" --- MUST_WORK_TEST -if (0 > 0) - then -end -test:do_execsql_test( - 1.1, - [[ - CREATE TABLE t1(x TEXT primary key); - INSERT INTO t1 VALUES('1'); - ]], { - -- <1.1> - - -- </1.1> - }) - -test:do_execsql_test( - 1.2, - [[ - CREATE TABLE t2(x INTEGER primary key); - INSERT INTO t2 VALUES(1); - ]], { - -- <1.2> - - -- </1.2> - }) - -test:do_execsql_test( - 1.3, - [[ - CREATE TABLE t3(x NUMBER primary key); - INSERT INTO t3 VALUES(1.0); - ]], { - -- <1.3> - - -- </1.3> - }) - -test:do_execsql_test( - 1.4, - [[ - CREATE TABLE t4(x NUMBER primary key); - INSERT INTO t4 VALUES(1.11); - ]], { - -- <1.4> - - -- </1.4> - }) - -test:do_execsql_test( - 1.5, - [[ - CREATE TABLE t5(id INT primary key, x INT , y TEXT); - INSERT INTO t5 VALUES(1, 1, 'one'); - INSERT INTO t5 VALUES(2, 1, 'two'); - INSERT INTO t5 VALUES(3, 1.0, 'three'); - INSERT INTO t5 VALUES(4, 1.0, 'four'); - ]], { - -- <1.5> - - -- </1.5> - }) - -test:do_execsql_test( - 2.1, - [[ - SELECT x FROM t1 WHERE x IN (1); - ]], { - -- <2.1> - "1" - -- </2.1> - }) - -test:do_execsql_test( - 2.2, - [[ - SELECT x FROM t1 WHERE x IN (1.0); - ]], { - -- <2.2> - "1" - -- </2.2> - }) - -test:do_execsql_test( - 2.3, - [[ - SELECT x FROM t1 WHERE x IN ('1'); - ]], { - -- <2.3> - "1" - -- </2.3> - }) - -test:do_execsql_test( - 2.4, - [[ - SELECT x FROM t1 WHERE x IN ('1.0'); - ]], { - -- <2.4> - - -- </2.4> - }) - -test:do_execsql_test( - 2.5, - [[ - SELECT x FROM t1 WHERE 1 IN (x); - ]], { - -- <2.5> - "1" - -- </2.5> - }) - -test:do_execsql_test( - 2.6, - [[ - SELECT x FROM t1 WHERE 1.0 IN (x); - ]], { - -- <2.6> - "1" - -- </2.6> - }) - -test:do_execsql_test( - 2.7, - [[ - SELECT x FROM t1 WHERE '1' IN (x); - ]], { - -- <2.7> - "1" - -- </2.7> - }) - -test:do_execsql_test( - 2.8, - [[ - SELECT x FROM t1 WHERE '1.0' IN (x); - ]], { - -- <2.8> - - -- </2.8> - }) - -test:do_execsql_test( - 3.1, - [[ - SELECT x FROM t2 WHERE x IN (1); - ]], { - -- <3.1> - 1 - -- </3.1> - }) - -test:do_execsql_test( - 3.2, - [[ - SELECT x FROM t2 WHERE x IN (1.0); - ]], { - -- <3.2> - 1 - -- </3.2> - }) - -test:do_catchsql_test( - 3.3, - [[ - SELECT x FROM t2 WHERE x IN ('1'); - ]], { - -- <3.3> - 1, "Type mismatch: can not convert 1 to integer" - -- </3.3> - }) - -test:do_execsql_test( - 3.5, - [[ - SELECT x FROM t2 WHERE 1 IN (x); - ]], { - -- <3.5> - 1 - -- </3.5> - }) - -test:do_execsql_test( - 3.6, - [[ - SELECT x FROM t2 WHERE 1.0 IN (x); - ]], { - -- <3.6> - 1 - -- </3.6> - }) - -test:do_catchsql_test( - 3.7, - [[ - SELECT x FROM t2 WHERE '1' IN (x); - ]], { - -- <3.7> - 1, "Type mismatch: can not convert 1 to integer" - -- </3.7> - }) - -test:do_execsql_test( - 4.1, - [[ - SELECT x FROM t3 WHERE x IN (1); - ]], { - -- <4.1> - 1.0 - -- </4.1> - }) - -test:do_execsql_test( - 4.2, - [[ - SELECT x FROM t3 WHERE x IN (1.0); - ]], { - -- <4.2> - 1.0 - -- </4.2> - }) - -test:do_catchsql_test( - 4.3, - [[ - SELECT x FROM t3 WHERE x IN ('1'); - ]], { - -- <4.3> - 1, "Type mismatch: can not convert 1 to number" - -- </4.3> - }) - -test:do_catchsql_test( - 4.4, - [[ - SELECT x FROM t3 WHERE x IN ('1.0'); - ]], { - -- <4.4> - 1, "Type mismatch: can not convert 1.0 to number" - -- </4.4> - }) - -test:do_execsql_test( - 4.5, - [[ - SELECT x FROM t3 WHERE 1 IN (x); - ]], { - -- <4.5> - 1.0 - -- </4.5> - }) - -test:do_execsql_test( - 4.6, - [[ - SELECT x FROM t3 WHERE 1.0 IN (x); - ]], { - -- <4.6> - 1.0 - -- </4.6> - }) - -test:do_catchsql_test( - 4.7, - [[ - SELECT x FROM t3 WHERE '1' IN (x); - ]], { - -- <4.7> - 1, "Type mismatch: can not convert 1 to number" - -- </4.7> - }) - -test:do_catchsql_test( - 4.8, - [[ - SELECT x FROM t3 WHERE '1.0' IN (x); - ]], { - -- <4.8> - 1, "Type mismatch: can not convert 1.0 to number" - -- </4.8> - }) - -test:do_execsql_test( - 5.1, - [[ - SELECT x FROM t4 WHERE x IN (1); - ]], { - -- <5.1> - - -- </5.1> - }) - -test:do_execsql_test( - 5.2, - [[ - SELECT x FROM t4 WHERE x IN (1.0); - ]], { - -- <5.2> - - -- </5.2> - }) - -test:do_catchsql_test( - 5.3, - [[ - SELECT x FROM t4 WHERE x IN ('1'); - ]], { - -- <5.3> - 1, "Type mismatch: can not convert 1 to number" - -- </5.3> - }) - -test:do_catchsql_test( - 5.4, - [[ - SELECT x FROM t4 WHERE x IN ('1.0'); - ]], { - -- <5.4> - 1, "Type mismatch: can not convert 1.0 to number" - -- </5.4> - }) - -test:do_execsql_test( - 5.5, - [[ - SELECT x FROM t4 WHERE x IN (1.11); - ]], { - -- <5.5> - 1.11 - -- </5.5> - }) - -test:do_catchsql_test( - 5.6, - [[ - SELECT x FROM t4 WHERE x IN ('1.11'); - ]], { - -- <5.6> - 1, "Type mismatch: can not convert 1.11 to number" - -- </5.6> - }) - -test:do_execsql_test( - 5.7, - [[ - SELECT x FROM t4 WHERE 1 IN (x); - ]], { - -- <5.7> - - -- </5.7> - }) - -test:do_execsql_test( - 5.8, - [[ - SELECT x FROM t4 WHERE 1.0 IN (x); - ]], { - -- <5.8> - - -- </5.8> - }) - -test:do_catchsql_test( - 5.9, - [[ - SELECT x FROM t4 WHERE '1' IN (x); - ]], { - -- <5.9> - 1, "Type mismatch: can not convert 1 to number" - -- </5.9> - }) - -test:do_catchsql_test( - 5.10, - [[ - SELECT x FROM t4 WHERE '1.0' IN (x); - ]], { - -- <5.10> - 1, "Type mismatch: can not convert 1.0 to number" - -- </5.10> - }) - -test:do_execsql_test( - 5.11, - [[ - SELECT x FROM t4 WHERE 1.11 IN (x); - ]], { - -- <5.11> - 1.11 - -- </5.11> - }) - -test:do_catchsql_test( - 5.12, - [[ - SELECT x FROM t4 WHERE '1.11' IN (x); - ]], { - -- <5.12> - 1, "Type mismatch: can not convert 1.11 to number" - -- </5.12> - }) - -test:do_execsql_test( - 6.1, - [[ - SELECT x, y FROM t5 WHERE x IN (1); - ]], { - -- <6.1> - 1, "one", 1, "two", 1, "three", 1.0, "four" - -- </6.1> - }) - -test:do_execsql_test( - 6.2, - [[ - SELECT x, y FROM t5 WHERE x IN (1.0); - ]], { - -- <6.2> - 1, "one", 1, "two", 1, "three", 1.0, "four" - -- </6.2> - }) - -test:do_execsql_test( - 6.3, - [[ - SELECT x, y FROM t5 WHERE x IN ('1'); - ]], { - -- <6.3> - 1, "one", 1, "two", 1, "three", 1.0, "four" - -- </6.3> - }) - -test:do_execsql_test( - 6.4, - [[ - SELECT x, y FROM t5 WHERE x IN ('1.0'); - ]], { - -- <6.4> - 1, "one", 1, "two", 1, "three", 1.0, "four" - -- </6.4> - }) - -test:do_execsql_test( - 6.5, - [[ - SELECT x, y FROM t5 WHERE 1 IN (x); - ]], { - -- <6.5> - 1, "one", 1, "two", 1, "three", 1.0, "four" - -- </6.5> - }) - -test:do_execsql_test( - 6.6, - [[ - SELECT x, y FROM t5 WHERE 1.0 IN (x); - ]], { - -- <6.6> - 1, "one", 1, "two", 1, "three", 1.0, "four" - -- </6.6> - }) - -test:do_execsql_test( - 6.7, - [[ - SELECT x, y FROM t5 WHERE '1' IN (x); - ]], { - -- <6.7> - 1, "one", 1, "two", 1, "three", 1.0, "four" - -- </6.7> - }) - -test:do_execsql_test( - 6.8, - [[ - SELECT x, y FROM t5 WHERE '1.0' IN (x); - ]], { - -- <6.8> - 1, "one", 1, "two", 1, "three", 1, "four" - -- </6.8> - }) - - - -test:finish_test() diff --git a/test/sql-tap/tkt3493.test.lua b/test/sql-tap/tkt3493.test.lua index de77e61e9..82ba828d0 100755 --- a/test/sql-tap/tkt3493.test.lua +++ b/test/sql-tap/tkt3493.test.lua @@ -1,6 +1,6 @@ #!/usr/bin/env tarantool test = require("sqltester") -test:plan(26) +test:plan(25) --!./tcltestrunner.lua -- 2008 October 13 @@ -45,7 +45,7 @@ test:do_execsql_test( [[ SELECT CASE - WHEN B.val = 1 THEN 'XYZ' + WHEN B.val = '1' THEN 'XYZ' ELSE A.val END AS Col1 FROM B @@ -63,7 +63,7 @@ test:do_execsql_test( [[ SELECT DISTINCT CASE - WHEN B.val = 1 THEN 'XYZ' + WHEN B.val = '1' THEN 'XYZ' ELSE A.val END AS Col1 FROM B @@ -79,7 +79,7 @@ test:do_execsql_test( test:do_execsql_test( "tkt3493-1.4", [[ - SELECT b.val, CASE WHEN b.val = 1 THEN 'xyz' ELSE b.val END AS col1 FROM b; + SELECT b.val, CASE WHEN b.val = '1' THEN 'xyz' ELSE b.val END AS col1 FROM b; ]], { -- <tkt3493-1.4> "1", "xyz", "2", "2" @@ -91,7 +91,7 @@ test:do_execsql_test( [[ SELECT DISTINCT b.val, - CASE WHEN b.val = 1 THEN 'xyz' ELSE b.val END AS col1 + CASE WHEN b.val = '1' THEN 'xyz' ELSE b.val END AS col1 FROM b; ]], { -- <tkt3493-1.5> @@ -126,23 +126,13 @@ test:do_execsql_test( test:do_execsql_test( "tkt3493-2.2.1", [[ - SELECT a=123 FROM t1 GROUP BY a + SELECT a='123' FROM t1 GROUP BY a ]], { -- <tkt3493-2.2.1> true -- </tkt3493-2.2.1> }) -test:do_execsql_test( - "tkt3493-2.2.2", - [[ - SELECT a=123 FROM t1 - ]], { - -- <tkt3493-2.2.2> - true - -- </tkt3493-2.2.2> - }) - test:do_execsql_test( "tkt3493-2.2.3", [[ @@ -156,7 +146,7 @@ test:do_execsql_test( test:do_execsql_test( "tkt3493-2.2.4", [[ - SELECT count(*), a=123 FROM t1 + SELECT count(*), a='123' FROM t1 ]], { -- <tkt3493-2.2.4> 1, true @@ -166,7 +156,7 @@ test:do_execsql_test( test:do_execsql_test( "tkt3493-2.2.5", [[ - SELECT count(*), +a=123 FROM t1 + SELECT count(*), +a='123' FROM t1 ]], { -- <tkt3493-2.2.5> 1, true @@ -176,7 +166,7 @@ test:do_execsql_test( test:do_execsql_test( "tkt3493-2.3.3", [[ - SELECT b='456' FROM t1 GROUP BY a + SELECT b = 456 FROM t1 GROUP BY a ]], { -- <tkt3493-2.3.3> true @@ -186,7 +176,7 @@ test:do_execsql_test( test:do_execsql_test( "tkt3493-2.3.1", [[ - SELECT b='456' FROM t1 GROUP BY b + SELECT b = 456 FROM t1 GROUP BY b ]], { -- <tkt3493-2.3.1> true @@ -196,7 +186,7 @@ test:do_execsql_test( test:do_execsql_test( "tkt3493-2.3.2", [[ - SELECT b='456' FROM t1 + SELECT b = 456 FROM t1 ]], { -- <tkt3493-2.3.2> true @@ -206,7 +196,7 @@ test:do_execsql_test( test:do_execsql_test( "tkt3493-2.4.1", [[ - SELECT typeof(a), a FROM t1 GROUP BY a HAVING a=123 + SELECT typeof(a), a FROM t1 GROUP BY a HAVING a='123' ]], { -- <tkt3493-2.4.1> "string", "123" @@ -216,7 +206,7 @@ test:do_execsql_test( test:do_execsql_test( "tkt3493-2.4.2", [[ - SELECT typeof(a), a FROM t1 GROUP BY b HAVING a=123 + SELECT typeof(a), a FROM t1 GROUP BY b HAVING a='123' ]], { -- <tkt3493-2.4.2> "string", "123" @@ -226,7 +216,7 @@ test:do_execsql_test( test:do_execsql_test( "tkt3493-2.5.1", [[ - SELECT typeof(b), b FROM t1 GROUP BY a HAVING b='456' + SELECT typeof(b), b FROM t1 GROUP BY a HAVING b=456 ]], { -- <tkt3493-2.5.1> "integer", 456 @@ -236,7 +226,7 @@ test:do_execsql_test( test:do_execsql_test( "tkt3493-2.5.2", [[ - SELECT typeof(b), b FROM t1 GROUP BY b HAVING b='456' + SELECT typeof(b), b FROM t1 GROUP BY b HAVING b=456 ]], { -- <tkt3493-2.5.2> "integer", 456 diff --git a/test/sql-tap/transitive1.test.lua b/test/sql-tap/transitive1.test.lua index 96895b4a7..cc7e066bf 100755 --- a/test/sql-tap/transitive1.test.lua +++ b/test/sql-tap/transitive1.test.lua @@ -63,7 +63,7 @@ test:do_execsql_test( INSERT INTO t2 VALUES(2, 20,20,'20'); INSERT INTO t2 VALUES(3, 3,3,'3'); - SELECT a,b,c FROM t2 WHERE a=b AND c=b AND c=20; + SELECT a,b,c FROM t2 WHERE a=b AND c=CAST(b AS STRING) AND c='20'; ]], { -- <transitive1-200> 20, 20, "20" @@ -73,7 +73,7 @@ test:do_execsql_test( test:do_execsql_test( "transitive1-210", [[ - SELECT a,b,c FROM t2 WHERE a=b AND c=b AND c>='20' ORDER BY +a; + SELECT a,b,c FROM t2 WHERE a=b AND c=CAST(b AS STRING) AND c>='20' ORDER BY +a; ]], { -- <transitive1-210> 3, 3, "3", 20, 20, "20" @@ -83,7 +83,7 @@ test:do_execsql_test( test:do_execsql_test( "transitive1-220", [[ - SELECT a,b,c FROM t2 WHERE a=b AND c=b AND c<='20' ORDER BY +a; + SELECT a,b,c FROM t2 WHERE a=b AND c=CAST(b AS STRING) AND c<='20' ORDER BY +a; ]], { -- <transitive1-220> 20, 20, "20", 100, 100, "100" @@ -402,7 +402,7 @@ test:do_execsql_test( [[ CREATE TABLE x(i INTEGER PRIMARY KEY, y TEXT); INSERT INTO x VALUES(10, '10'); - SELECT * FROM x WHERE x.y>='1' AND x.y<'2' AND x.i=x.y; + SELECT * FROM x WHERE x.y>='1' AND x.y<'2' AND CAST(x.i AS STRING)=x.y; ]], { -- <transitive1-500> 10, "10" @@ -430,7 +430,7 @@ test:do_execsql_test( [[ CREATE TABLE t3(i INTEGER PRIMARY KEY, t TEXT); INSERT INTO t3 VALUES(10, '10'); - SELECT * FROM t3 WHERE i=t AND t = '10 '; + SELECT * FROM t3 WHERE CAST(i AS STRING)=t AND t = '10 '; ]], { -- <transitive1-520> @@ -443,7 +443,7 @@ test:do_execsql_test( CREATE TABLE u1(x TEXT PRIMARY KEY, y INTEGER, z TEXT); CREATE INDEX i1 ON u1(x); INSERT INTO u1 VALUES('00013', 13, '013'); - SELECT * FROM u1 WHERE x=y AND y=z AND z='013'; + SELECT * FROM u1 WHERE CAST(x AS INTEGER)=y AND y=CAST(z AS INTEGER) AND z='013'; ]], { -- <transitive1-530> "00013",13,"013" diff --git a/test/sql-tap/where2.test.lua b/test/sql-tap/where2.test.lua index f267be8e6..7348a855a 100755 --- a/test/sql-tap/where2.test.lua +++ b/test/sql-tap/where2.test.lua @@ -4,7 +4,7 @@ yaml = require("yaml") fio = require("fio") ffi = require("ffi") -test:plan(74) +test:plan(62) ffi.cdef[[ int dup(int oldfd); @@ -622,181 +622,12 @@ test:do_test( -- </where2-6.6> }) --- if X(356, "X!cmd", [=[["expr","[permutation] != \"no_optimization\""]]=]) --- then - -- Ticket #2249. Make sure the OR optimization is not attempted if - -- comparisons between columns of different affinities are needed. - -- - test:do_test( - "where2-6.7", - function() - test:execsql [[ - CREATE TABLE t2249a(a TEXT PRIMARY KEY, x VARCHAR(100)); - CREATE TABLE t2249b(b INTEGER PRIMARY KEY); - INSERT INTO t2249a(a) VALUES('0123'); - INSERT INTO t2249b VALUES(123); - ]] - return queryplan([[ - -- Because a is type TEXT and b is type INTEGER, both a and b - -- will attempt to convert to NUMERIC before the comparison. - -- They will thus compare equal. - -- - SELECT b,a FROM t2249b CROSS JOIN t2249a WHERE a=b; - ]]) - end, { - -- <where2-6.7> - 123, '0123', "nosort", "T2249B", "*", "T2249A", "*" - -- </where2-6.7> - }) - - test:do_test( - "where2-6.9", - function() - return queryplan([[ - -- The + operator doesn't affect RHS. - -- - SELECT b,a FROM t2249b CROSS JOIN t2249a WHERE a=+b; - ]]) - end, { - -- <where2-6.9> - 123, "0123", "nosort", "T2249B", "*", "T2249A", "*" - -- </where2-6.9> - }) - - test:do_test( - "where2-6.9.2", - function() - -- The same thing but with the expression flipped around. - return queryplan([[ - SELECT b,a FROM t2249b CROSS JOIN t2249a WHERE +b=a - ]]) - end, { - -- <where2-6.9.2> - 123, "0123","nosort", "T2249B", "*", "T2249A", "*" - -- </where2-6.9.2> - }) - - test:do_test( - "where2-6.10", - function() - return queryplan([[ - SELECT b,a FROM t2249b CROSS JOIN t2249a WHERE +a=+b; - ]]) - end, { - -- <where2-6.10> - 123, "0123", "nosort", "T2249B", "*", "T2249A", "*" - -- </where2-6.10> - }) - - test:do_test( - "where2-6.11", - function() - -- This will not attempt the OR optimization because of the a=b - -- comparison. - return queryplan([[ - SELECT b,a FROM t2249b CROSS JOIN t2249a WHERE a=b OR a='hello'; - ]]) - end, { - -- <where2-6.11> - 123, '0123', "nosort", "T2249B", "*", "T2249A", "*" - -- </where2-6.11> - }) - - test:do_test( - "where2-6.11.2", - function() - -- Permutations of the expression terms. - return queryplan([[ - SELECT b,a FROM t2249b CROSS JOIN t2249a WHERE b=a OR a='hello'; - ]]) - end, { - -- <where2-6.11.2> - 123, '0123', "nosort", "T2249B", "*", "T2249A", "*" - -- </where2-6.11.2> - }) - - test:do_test( - "where2-6.11.3", - function() - -- Permutations of the expression terms. - return queryplan([[ - SELECT b,a FROM t2249b CROSS JOIN t2249a WHERE 'hello'=a OR b=a; - ]]) - end, { - -- <where2-6.11.3> - 123, '0123', "nosort", "T2249B", "*", "T2249A", "*" - -- </where2-6.11.3> - }) - - test:do_test( - "where2-6.11.4", - function() - -- Permutations of the expression terms. - return queryplan([[ - SELECT b,a FROM t2249b CROSS JOIN t2249a WHERE a='hello' OR b=a; - ]]) - end, { - -- <where2-6.11.4> - 123, '0123', "nosort", "T2249B", "*", "T2249A", "*" - -- </where2-6.11.4> - }) - - -- These tests are not run if subquery support is not included in the - -- build. This is because these tests test the "a = 1 OR a = 2" to - -- "a IN (1, 2)" optimisation transformation, which is not enabled if - -- subqueries and the IN operator is not available. - -- - test:do_test( - "where2-6.12", - function() - return queryplan([[ - SELECT b,a FROM t2249b CROSS JOIN t2249a WHERE a=+b OR a='hello'; - ]]) - end, { - -- <where2-6.12> - 123, "0123", "nosort", "T2249B", "*", "T2249A", "*" - -- </where2-6.12> - }) - - test:do_test( - "where2-6.12.2", - function() - return queryplan([[ - SELECT b,a FROM t2249b CROSS JOIN t2249a WHERE a='hello' OR +b=a; - ]]) - end, { - -- <where2-6.12.2> - 123, "0123", "nosort", "T2249B", "*", "T2249A", "*" - -- </where2-6.12.2> - }) - - test:do_test( - "where2-6.12.3", - function() - return queryplan([[ - SELECT b,a FROM t2249b CROSS JOIN t2249a WHERE +b=a OR a='hello'; - ]]) - end, { - -- <where2-6.12.3> - 123, "0123", "nosort", "T2249B", "*", "T2249A", "*" - -- </where2-6.12.3> - }) - - test:do_test( - "where2-6.13", - function() - -- The addition of +a on the second term disabled the OR optimization. - -- But we should still get the same empty-set result as in where2-6.9. - return queryplan([[ - SELECT b,a FROM t2249b CROSS JOIN t2249a WHERE a=+b OR +a='hello'; - ]]) - end, { - -- <where2-6.13> - 123, "0123", "nosort", "T2249B", "*", "T2249A", "*" - -- </where2-6.13> - }) - - + test:execsql [[ + CREATE TABLE t2249a(a TEXT PRIMARY KEY, x VARCHAR(100)); + CREATE TABLE t2249b(b INTEGER PRIMARY KEY); + INSERT INTO t2249a(a) VALUES('0123'); + INSERT INTO t2249b VALUES(123); + ]] -- Variations on the order of terms in a WHERE clause in order -- to make sure the OR optimizer can recognize them all. diff --git a/test/sql-tap/where5.test.lua b/test/sql-tap/where5.test.lua index 3aefcaca5..4a197aac2 100755 --- a/test/sql-tap/where5.test.lua +++ b/test/sql-tap/where5.test.lua @@ -34,7 +34,7 @@ test:do_test("where5-1.0", function() INSERT INTO t3 SELECT CAST(x AS INTEGER) FROM t1; ]] return test:execsql [[ - SELECT * FROM t1 WHERE x<0 + SELECT * FROM t1 WHERE x<'0' ]] end, { -- <where5-1.0> @@ -43,7 +43,7 @@ end, { }) test:do_execsql_test("where5-1.1", [[ - SELECT * FROM t1 WHERE x<=0 + SELECT * FROM t1 WHERE x<='0' ]], { -- <where5-1.1> '-1', '0' @@ -51,7 +51,7 @@ test:do_execsql_test("where5-1.1", [[ }) test:do_execsql_test("where5-1.2", [[ - SELECT * FROM t1 WHERE x=0 + SELECT * FROM t1 WHERE x='0' ]], { -- <where5-1.2> '0' @@ -59,7 +59,7 @@ test:do_execsql_test("where5-1.2", [[ }) test:do_execsql_test("where5-1.3", [[ - SELECT * FROM t1 WHERE x>=0 + SELECT * FROM t1 WHERE x>='0' ]], { -- <where5-1.3> '0', '1' @@ -67,7 +67,7 @@ test:do_execsql_test("where5-1.3", [[ }) test:do_execsql_test("where5-1.4", [[ - SELECT * FROM t1 WHERE x>0 + SELECT * FROM t1 WHERE x>'0' ]], { -- <where5-1.4> '1' @@ -75,7 +75,7 @@ test:do_execsql_test("where5-1.4", [[ }) test:do_execsql_test("where5-1.5", [[ - SELECT * FROM t1 WHERE x<>0 + SELECT * FROM t1 WHERE x<>'0' ]], { -- <where5-1.5> '-1', '1' diff --git a/test/sql/types.result b/test/sql/types.result index 8810a9f82..b40f45029 100644 --- a/test/sql/types.result +++ b/test/sql/types.result @@ -608,11 +608,8 @@ box.execute("SELECT 18446744073709551615.0 > 18446744073709551615") ... box.execute("SELECT 18446744073709551615 IN ('18446744073709551615', 18446744073709551615.0)") --- -- metadata: - - name: COLUMN_1 - type: boolean - rows: - - [true] +- null +- 'Type mismatch: can not convert 18446744073709551615 to numeric' ... box.execute("SELECT 1 LIMIT 18446744073709551615;") --- -- 2.25.1 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Tarantool-patches] [PATCH v5 5/6] sql: fix implicit cast in opcode MustBeInt 2020-08-21 9:19 [Tarantool-patches] [PATCH v5 0/6] sql; remove implicit cast for comparison imeevma ` (3 preceding siblings ...) 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 4/6] sql: remove implicit cast from comparison opcodes imeevma @ 2020-08-21 9:19 ` imeevma 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 6/6] sql: remove implicit cast from MakeRecord opcode imeevma 5 siblings, 0 replies; 15+ messages in thread From: imeevma @ 2020-08-21 9:19 UTC (permalink / raw) To: korablev, tsafin; +Cc: tarantool-patches This patch removes implicit casting from STRING to number and vice versa from MustBeInt opcode. Part of #4230 --- src/box/sql/vdbe.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index b326c4ba4..a0ddbaf60 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -2352,16 +2352,20 @@ case OP_AddImm: { /* in1 */ */ case OP_MustBeInt: { /* jump, in1 */ pIn1 = &aMem[pOp->p1]; - if ((pIn1->flags & (MEM_Int | MEM_UInt)) == 0) { - mem_apply_type(pIn1, FIELD_TYPE_INTEGER); - if ((pIn1->flags & (MEM_Int | MEM_UInt)) == 0) { - if (pOp->p2==0) { - diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn1), "integer"); - goto abort_due_to_error; - } else { - goto jump_to_p2; - } + if (mem_is_type_compatible(pIn1, FIELD_TYPE_INTEGER)) + break; + if ((pIn1->flags & MEM_Real) != 0) { + double d = pIn1->u.r; + if (d == (double)(int64_t)d || d == (double)(uint64_t)d) + mem_convert_to_integer(pIn1); + } + if (!mem_is_type_compatible(pIn1, FIELD_TYPE_INTEGER)) { + if (pOp->p2==0) { + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, + sql_value_to_diag_str(pIn1), "integer"); + goto abort_due_to_error; + } else { + goto jump_to_p2; } } break; -- 2.25.1 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Tarantool-patches] [PATCH v5 6/6] sql: remove implicit cast from MakeRecord opcode 2020-08-21 9:19 [Tarantool-patches] [PATCH v5 0/6] sql; remove implicit cast for comparison imeevma ` (4 preceding siblings ...) 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 5/6] sql: fix implicit cast in opcode MustBeInt imeevma @ 2020-08-21 9:19 ` imeevma 2020-09-18 12:28 ` Nikita Pettik 5 siblings, 1 reply; 15+ messages in thread From: imeevma @ 2020-08-21 9:19 UTC (permalink / raw) To: korablev, tsafin; +Cc: tarantool-patches This patch removes implicit casting from MakeRecord opcode. Closes #4230 @TarantoolBot document Title: remove implicit cast for comparison After this patch-set, there will be no implicit casts for comparison. This means that the values of the field types STRING, BOOLEAN and VARBINARY can be compared with the values of the same field type. Any numerical value can be compared with any other numerical value. Example: ``` tarantool> box.execute([[SELECT '1' > 0;]]) --- - null - 'Type mismatch: can not convert 1 to numeric' ... tarantool> box.execute([[SELECT true > X'33';]]) --- - null - 'Type mismatch: can not convert boolean to varbinary' ... tarantool> box.execute([[SELECT 1.23 > 123;]]) --- - metadata: - name: 1.23 > 123 type: boolean rows: - [false] ... ``` --- src/box/sql/analyze.c | 6 +- src/box/sql/delete.c | 15 ++- src/box/sql/expr.c | 17 ++- src/box/sql/fk_constraint.c | 12 +- src/box/sql/select.c | 26 +++-- src/box/sql/update.c | 23 ++-- src/box/sql/vdbe.c | 19 +-- test/sql-tap/in3.test.lua | 26 +---- test/sql/boolean.result | 76 ++++-------- test/sql/types.result | 225 ++++++++++++++++++++++++++++++++++++ test/sql/types.test.lua | 51 ++++++++ 11 files changed, 363 insertions(+), 133 deletions(-) diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c index f74f9b358..3efcd041a 100644 --- a/src/box/sql/analyze.c +++ b/src/box/sql/analyze.c @@ -969,8 +969,10 @@ vdbe_emit_analyze_space(struct Parse *parse, struct space *space) FIELD_TYPE_STRING, FIELD_TYPE_STRING, field_type_MAX }; - sqlVdbeAddOp4(v, OP_MakeRecord, tab_name_reg, 4, tmp_reg, - (char *)types, sizeof(types)); + sqlVdbeAddOp4(v, OP_ApplyType, tab_name_reg, 4, 0, + (char *)types, P4_STATIC); + sqlVdbeChangeP5(v, OPFLAG_DO_NOT_CONVERT_NUMBERS); + sqlVdbeAddOp3(v, OP_MakeRecord, tab_name_reg, 4, tmp_reg); sqlVdbeAddOp4(v, OP_IdxInsert, tmp_reg, 0, 0, (char *)stat1, P4_SPACEPTR); /* Add the entries to the stat4 table. */ diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c index 68abd1f58..40a52aadc 100644 --- a/src/box/sql/delete.c +++ b/src/box/sql/delete.c @@ -329,11 +329,16 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, */ key_len = 0; struct index *pk = space_index(space, 0); - enum field_type *types = is_view ? NULL : - sql_index_type_str(parse->db, - pk->def); - sqlVdbeAddOp4(v, OP_MakeRecord, reg_pk, pk_len, - reg_key, (char *)types, P4_DYNAMIC); + if (!is_view) { + enum field_type *types = + sql_index_type_str(parse->db, pk->def); + sqlVdbeAddOp4(v, OP_ApplyType, reg_pk, pk_len, + 0, (char *)types, P4_DYNAMIC); + sqlVdbeChangeP5(v, + OPFLAG_DO_NOT_CONVERT_NUMBERS); + } + sqlVdbeAddOp3(v, OP_MakeRecord, reg_pk, pk_len, + reg_key); /* Set flag to save memory allocating one * by malloc. */ diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index bc2182446..d553b80db 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -2886,11 +2886,18 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */ jmpIfDynamic = -1; } r3 = sqlExprCodeTarget(pParse, pE2, r1); - enum field_type types[2] = - { lhs_type, field_type_MAX }; - sqlVdbeAddOp4(v, OP_MakeRecord, r3, - 1, r2, (char *)types, - sizeof(types)); + uint32_t size = + 2 * sizeof(enum field_type); + enum field_type *types= + sqlDbMallocZero(pParse->db, + size); + types[0] = lhs_type; + types[1] = field_type_MAX; + sqlVdbeAddOp4(v, OP_ApplyType, r3, 1, 0, + (char *)types, P4_DYNAMIC); + sqlVdbeChangeP5(v, OPFLAG_DO_NOT_CONVERT_NUMBERS); + sqlVdbeAddOp3(v, OP_MakeRecord, r3, 1, + r2); sql_expr_type_cache_change(pParse, r3, 1); sqlVdbeAddOp2(v, OP_IdxInsert, r2, diff --git a/src/box/sql/fk_constraint.c b/src/box/sql/fk_constraint.c index 482220a95..50f9ebf74 100644 --- a/src/box/sql/fk_constraint.c +++ b/src/box/sql/fk_constraint.c @@ -264,11 +264,13 @@ fk_constraint_lookup_parent(struct Parse *parse_context, struct space *parent, } struct index *idx = space_index(parent, referenced_idx); assert(idx != NULL); - sqlVdbeAddOp4(v, OP_MakeRecord, temp_regs, field_count, - rec_reg, - (char *) sql_index_type_str(parse_context->db, - idx->def), - P4_DYNAMIC); + sqlVdbeAddOp4(v, OP_ApplyType, temp_regs, field_count, 0, + (char *) sql_index_type_str(parse_context->db, + idx->def), + P4_DYNAMIC); + sqlVdbeChangeP5(v, OPFLAG_DO_NOT_CONVERT_NUMBERS); + sqlVdbeAddOp3(v, OP_MakeRecord, temp_regs, field_count, + rec_reg); sqlVdbeAddOp4Int(v, OP_Found, cursor, ok_label, rec_reg, 0); sqlReleaseTempReg(parse_context, rec_reg); sqlReleaseTempRange(parse_context, temp_regs, field_count); diff --git a/src/box/sql/select.c b/src/box/sql/select.c index b0554a172..4c7d8001c 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -1274,9 +1274,13 @@ selectInnerLoop(Parse * pParse, /* The parser context */ field_type_sequence_dup(pParse, pDest->dest_type, nResultCol); - sqlVdbeAddOp4(v, OP_MakeRecord, regResult, - nResultCol, r1, (char *)types, - P4_DYNAMIC); + sqlVdbeAddOp4(v, OP_ApplyType, regResult, + nResultCol, 0, (char *)types, + P4_DYNAMIC); + sqlVdbeChangeP5(v, + OPFLAG_DO_NOT_CONVERT_NUMBERS); + sqlVdbeAddOp3(v, OP_MakeRecord, regResult, + nResultCol, r1); sql_expr_type_cache_change(pParse, regResult, nResultCol); @@ -1696,9 +1700,11 @@ generateSortTail(Parse * pParse, /* Parsing context */ enum field_type *types = field_type_sequence_dup(pParse, pDest->dest_type, nColumn); - sqlVdbeAddOp4(v, OP_MakeRecord, regRow, nColumn, - regTupleid, (char *)types, - P4_DYNAMIC); + sqlVdbeAddOp4(v, OP_ApplyType, regRow, nColumn, 0, + (char *)types, P4_DYNAMIC); + sqlVdbeChangeP5(v, OPFLAG_DO_NOT_CONVERT_NUMBERS); + sqlVdbeAddOp3(v, OP_MakeRecord, regRow, nColumn, + regTupleid); sql_expr_type_cache_change(pParse, regRow, nColumn); sqlVdbeAddOp2(v, OP_IdxInsert, regTupleid, pDest->reg_eph); break; @@ -3138,9 +3144,11 @@ generateOutputSubroutine(struct Parse *parse, struct Select *p, enum field_type *types = field_type_sequence_dup(parse, dest->dest_type, in->nSdst); - sqlVdbeAddOp4(v, OP_MakeRecord, in->iSdst, - in->nSdst, r1, (char *)types, - P4_DYNAMIC); + sqlVdbeAddOp4(v, OP_ApplyType, in->iSdst, in->nSdst, 0, + (char *)types, P4_DYNAMIC); + sqlVdbeChangeP5(v, OPFLAG_DO_NOT_CONVERT_NUMBERS); + sqlVdbeAddOp3(v, OP_MakeRecord, in->iSdst, in->nSdst, + r1); sql_expr_type_cache_change(parse, in->iSdst, in->nSdst); sqlVdbeAddOp2(v, OP_IdxInsert, r1, dest->reg_eph); diff --git a/src/box/sql/update.c b/src/box/sql/update.c index 24c7cfa27..7c80dcc4e 100644 --- a/src/box/sql/update.c +++ b/src/box/sql/update.c @@ -251,11 +251,14 @@ sqlUpdate(Parse * pParse, /* The parser context */ nKey = pk_part_count; regKey = iPk; } else { - enum field_type *types = is_view ? NULL : - sql_index_type_str(pParse->db, - pPk->def); - sqlVdbeAddOp4(v, OP_MakeRecord, iPk, pk_part_count, - regKey, (char *) types, P4_DYNAMIC); + if (!is_view) { + enum field_type *types = + sql_index_type_str(pParse->db, pPk->def); + sqlVdbeAddOp4(v, OP_ApplyType, iPk, pk_part_count, 0, + (char *)types, P4_DYNAMIC); + sqlVdbeChangeP5(v, OPFLAG_DO_NOT_CONVERT_NUMBERS); + } + sqlVdbeAddOp3(v, OP_MakeRecord, iPk, pk_part_count, regKey); /* * Set flag to save memory allocating one by * malloc. @@ -423,9 +426,13 @@ sqlUpdate(Parse * pParse, /* The parser context */ enum field_type *types = sql_index_type_str(pParse->db, pPk->def); - sqlVdbeAddOp4(v, OP_MakeRecord, iPk, - pk_part_count, key_reg, - (char *) types, P4_DYNAMIC); + sqlVdbeAddOp4(v, OP_ApplyType, iPk, + pk_part_count, 0, (char *)types, + P4_DYNAMIC); + sqlVdbeChangeP5(v, + OPFLAG_DO_NOT_CONVERT_NUMBERS); + sqlVdbeAddOp3(v, OP_MakeRecord, iPk, + pk_part_count, key_reg); } else { assert(nKey == 0); key_reg = regKey; diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index a0ddbaf60..544619b03 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -3145,24 +3145,17 @@ type_mismatch: break; } -/* Opcode: MakeRecord P1 P2 P3 P4 P5 +/* Opcode: MakeRecord P1 P2 P3 * P5 * Synopsis: r[P3]=mkrec(r[P1@P2]) * * Convert P2 registers beginning with P1 into the [record format] * use as a data record in a database table or as a key * in an index. The OP_Column opcode can decode the record later. * - * P4 may be a string that is P2 characters long. The nth character of the - * string indicates the column type that should be used for the nth - * field of the index key. - * - * If P4 is NULL then all index fields have type SCALAR. - * * If P5 is not NULL then record under construction is intended to be inserted * into ephemeral space. Thus, sort of memory optimization can be performed. */ case OP_MakeRecord: { - Mem *pRec; /* The new record */ Mem *pData0; /* First field to be combined into the record */ Mem MAYBE_UNUSED *pLast; /* Last field of the record */ int nField; /* Number of fields in the record */ @@ -3184,7 +3177,6 @@ case OP_MakeRecord: { * of the record to data0. */ nField = pOp->p1; - enum field_type *types = pOp->p4.types; bIsEphemeral = pOp->p5; assert(nField>0 && pOp->p2>0 && pOp->p2+nField<=(p->nMem+1 - p->nCursor)+1); pData0 = &aMem[nField]; @@ -3195,15 +3187,6 @@ case OP_MakeRecord: { assert(pOp->p3<pOp->p1 || pOp->p3>=pOp->p1+pOp->p2); pOut = vdbe_prepare_null_out(p, pOp->p3); - /* Apply the requested types to all inputs */ - assert(pData0<=pLast); - if (types != NULL) { - pRec = pData0; - do { - mem_apply_type(pRec++, *(types++)); - } while(types[0] != field_type_MAX); - } - struct region *region = &fiber()->gc; size_t used = region_used(region); uint32_t tuple_size; diff --git a/test/sql-tap/in3.test.lua b/test/sql-tap/in3.test.lua index a6d842962..7f3abbae0 100755 --- a/test/sql-tap/in3.test.lua +++ b/test/sql-tap/in3.test.lua @@ -1,6 +1,6 @@ #!/usr/bin/env tarantool test = require("sqltester") -test:plan(28) +test:plan(26) --!./tcltestrunner.lua -- 2007 November 29 @@ -334,18 +334,6 @@ test:do_test( -- </in3-3.4> }) -test:do_test( - "in3-3.5", - function() - -- Numeric affinity should be applied to each side before the comparison - -- takes place. Therefore we cannot use index t1_i1, which has no affinity. - return exec_neph(" SELECT y IN (SELECT a FROM t1) FROM t2 ") - end, { - -- <in3-3.5> - 1, true - -- </in3-3.5> - }) - test:do_test( "in3-3.6", function() @@ -358,18 +346,6 @@ test:do_test( -- </in3-3.6> }) -test:do_test( - "in3-3.7", - function() - -- Numeric affinity is applied before the comparison takes place. - -- Making it impossible to use index t1_i3. - return exec_neph(" SELECT y IN (SELECT c FROM t1) FROM t2 ") - end, { - -- <in3-3.7> - 1, true - -- </in3-3.7> - }) - ----------------------------------------------------------------------- -- -- Test using a multi-column index. diff --git a/test/sql/boolean.result b/test/sql/boolean.result index 51ec5820b..d3eca833c 100644 --- a/test/sql/boolean.result +++ b/test/sql/boolean.result @@ -2256,19 +2256,13 @@ SELECT false IN (SELECT a1 FROM t6 LIMIT 1); | ... SELECT true IN (1, 1.2, 'true', false); | --- - | - metadata: - | - name: COLUMN_1 - | type: boolean - | rows: - | - [false] + | - null + | - 'Type mismatch: can not convert 1 to boolean' | ... SELECT false IN (1, 1.2, 'true', false); | --- - | - metadata: - | - name: COLUMN_1 - | type: boolean - | rows: - | - [true] + | - null + | - 'Type mismatch: can not convert 1 to boolean' | ... SELECT a, a IN (true) FROM t; @@ -2328,14 +2322,8 @@ SELECT a, a IN (SELECT a1 FROM t6) FROM t; | ... SELECT a, a IN (1, 1.2, 'true', false) FROM t; | --- - | - metadata: - | - name: A - | type: boolean - | - name: COLUMN_1 - | type: boolean - | rows: - | - [false, true] - | - [true, false] + | - null + | - 'Type mismatch: can not convert 1 to boolean' | ... SELECT true BETWEEN true AND true; @@ -3860,19 +3848,13 @@ SELECT a2, b, b != a2 FROM t6, t7; SELECT true IN (0, 1, 2, 3); | --- - | - metadata: - | - name: COLUMN_1 - | type: boolean - | rows: - | - [false] + | - null + | - 'Type mismatch: can not convert 0 to boolean' | ... SELECT false IN (0, 1, 2, 3); | --- - | - metadata: - | - name: COLUMN_1 - | type: boolean - | rows: - | - [false] + | - null + | - 'Type mismatch: can not convert 0 to boolean' | ... SELECT true IN (SELECT b FROM t7); | --- @@ -3886,14 +3868,8 @@ SELECT false IN (SELECT b FROM t7); | ... SELECT a1, a1 IN (0, 1, 2, 3) FROM t6 | --- - | - metadata: - | - name: A1 - | type: boolean - | - name: COLUMN_1 - | type: boolean - | rows: - | - [false, false] - | - [true, false] + | - null + | - 'Type mismatch: can not convert 0 to boolean' | ... SELECT true BETWEEN 0 and 10; @@ -5005,35 +4981,23 @@ SELECT a2, c, c != a2 FROM t6, t8; SELECT true IN (0.1, 1.2, 2.3, 3.4); | --- - | - metadata: - | - name: COLUMN_1 - | type: boolean - | rows: - | - [false] + | - null + | - 'Type mismatch: can not convert 0.1 to boolean' | ... SELECT false IN (0.1, 1.2, 2.3, 3.4); | --- - | - metadata: - | - name: COLUMN_1 - | type: boolean - | rows: - | - [false] + | - null + | - 'Type mismatch: can not convert 0.1 to boolean' | ... SELECT a1 IN (0.1, 1.2, 2.3, 3.4) FROM t6 LIMIT 1; | --- - | - metadata: - | - name: COLUMN_1 - | type: boolean - | rows: - | - [false] + | - null + | - 'Type mismatch: can not convert 0.1 to boolean' | ... SELECT a2 IN (0.1, 1.2, 2.3, 3.4) FROM t6 LIMIT 1; | --- - | - metadata: - | - name: COLUMN_1 - | type: boolean - | rows: - | - [false] + | - null + | - 'Type mismatch: can not convert 0.1 to boolean' | ... SELECT true IN (SELECT c FROM t8); | --- diff --git a/test/sql/types.result b/test/sql/types.result index b40f45029..29aa90d7b 100644 --- a/test/sql/types.result +++ b/test/sql/types.result @@ -2846,3 +2846,228 @@ box.execute([[DROP TABLE t;]]) --- - row_count: 1 ... +-- +-- Make sure that there is no implicit cast between string and +-- number. +-- +box.execute([[SELECT '1' > 0;]]); +--- +- null +- 'Type mismatch: can not convert 1 to numeric' +... +box.execute([[SELECT 1 > '0';]]); +--- +- null +- 'Type mismatch: can not convert 0 to numeric' +... +box.execute([[CREATE TABLE t (i INT PRIMARY KEY, d DOUBLE, n NUMBER, s STRING);]]) +--- +- row_count: 1 +... +box.execute([[INSERT INTO t VALUES (1, 1.0, 1, '2'), (2, 2.0, 2.0, '2');]]) +--- +- row_count: 2 +... +box.execute([[SELECT * from t WHERE i > s;]]) +--- +- null +- 'Type mismatch: can not convert 2 to numeric' +... +box.execute([[SELECT * from t WHERE s > i;]]) +--- +- null +- 'Type mismatch: can not convert 2 to numeric' +... +box.execute([[SELECT * from t WHERE d > s;]]) +--- +- null +- 'Type mismatch: can not convert 2 to numeric' +... +box.execute([[SELECT * from t WHERE s > d;]]) +--- +- null +- 'Type mismatch: can not convert 2 to numeric' +... +box.execute([[SELECT * from t WHERE i = 1 and n > s;]]) +--- +- null +- 'Type mismatch: can not convert 2 to numeric' +... +box.execute([[SELECT * from t WHERE i = 2 and s > n;]]) +--- +- null +- 'Type mismatch: can not convert 2 to numeric' +... +box.execute([[SELECT i FROM t WHERE i in (1);]]) +--- +- metadata: + - name: I + type: integer + rows: + - [1] +... +box.execute([[SELECT i FROM t WHERE d in (1);]]) +--- +- metadata: + - name: I + type: integer + rows: + - [1] +... +box.execute([[SELECT i FROM t WHERE n in (1);]]) +--- +- metadata: + - name: I + type: integer + rows: + - [1] +... +box.execute([[SELECT i FROM t WHERE s in (1);]]) +--- +- null +- 'Type mismatch: can not convert 2 to numeric' +... +box.execute([[SELECT i FROM t WHERE i in (1.0);]]) +--- +- metadata: + - name: I + type: integer + rows: + - [1] +... +box.execute([[SELECT i FROM t WHERE d in (1.0);]]) +--- +- metadata: + - name: I + type: integer + rows: + - [1] +... +box.execute([[SELECT i FROM t WHERE n in (1.0);]]) +--- +- metadata: + - name: I + type: integer + rows: + - [1] +... +box.execute([[SELECT i FROM t WHERE s in (1.0);]]) +--- +- null +- 'Type mismatch: can not convert 2 to numeric' +... +box.execute([[SELECT i FROM t WHERE i in ('1');]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT i FROM t WHERE d in ('1');]]) +--- +- null +- 'Type mismatch: can not convert 1 to numeric' +... +box.execute([[SELECT i FROM t WHERE n in ('1');]]) +--- +- null +- 'Type mismatch: can not convert 1 to numeric' +... +box.execute([[SELECT i FROM t WHERE s in ('1');]]) +--- +- metadata: + - name: I + type: integer + rows: [] +... +box.execute([[SELECT i FROM t WHERE i in ('1.0');]]) +--- +- null +- 'Type mismatch: can not convert 1.0 to integer' +... +box.execute([[SELECT i FROM t WHERE d in ('1.0');]]) +--- +- null +- 'Type mismatch: can not convert 1.0 to numeric' +... +box.execute([[SELECT i FROM t WHERE n in ('1.0');]]) +--- +- null +- 'Type mismatch: can not convert 1.0 to numeric' +... +box.execute([[SELECT i FROM t WHERE s in ('1.0');]]) +--- +- metadata: + - name: I + type: integer + rows: [] +... +box.execute([[DROP TABLE t;]]) +--- +- row_count: 1 +... +-- Comparison with SCALAR. +box.execute([[CREATE TABLE t(a SCALAR PRIMARY KEY);]]) +--- +- row_count: 1 +... +box.execute([[INSERT INTO t VALUES (1), (2.2), ('3');]]); +--- +- row_count: 3 +... +box.execute([[SELECT a FROM t WHERE a > 1]]); +--- +- null +- 'Type mismatch: can not convert 3 to numeric' +... +box.execute([[SELECT a FROM t WHERE a > 1.0]]); +--- +- null +- 'Type mismatch: can not convert 3 to numeric' +... +box.execute([[SELECT a FROM t WHERE a > '1']]); +--- +- metadata: + - name: A + type: scalar + rows: + - ['3'] +... +box.execute([[SELECT a FROM t WHERE a < 1]]); +--- +- null +- 'Type mismatch: can not convert 3 to numeric' +... +box.execute([[SELECT a FROM t WHERE a < 1.0]]); +--- +- null +- 'Type mismatch: can not convert 3 to numeric' +... +box.execute([[SELECT a FROM t WHERE a < '1']]); +--- +- metadata: + - name: A + type: scalar + rows: + - [1] + - [2.2] +... +box.execute([[SELECT a FROM t WHERE a = 1]]); +--- +- null +- 'Type mismatch: can not convert 3 to numeric' +... +box.execute([[SELECT a FROM t WHERE a = 1.0]]); +--- +- null +- 'Type mismatch: can not convert 3 to numeric' +... +box.execute([[SELECT a FROM t WHERE a = '1']]); +--- +- metadata: + - name: A + type: scalar + rows: [] +... +box.execute([[DROP TABLE t;]]) +--- +- row_count: 1 +... diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua index a23b12801..09619653b 100644 --- a/test/sql/types.test.lua +++ b/test/sql/types.test.lua @@ -635,3 +635,54 @@ 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;]]) + +-- +-- Make sure that there is no implicit cast between string and +-- number. +-- +box.execute([[SELECT '1' > 0;]]); +box.execute([[SELECT 1 > '0';]]); +box.execute([[CREATE TABLE t (i INT PRIMARY KEY, d DOUBLE, n NUMBER, s STRING);]]) +box.execute([[INSERT INTO t VALUES (1, 1.0, 1, '2'), (2, 2.0, 2.0, '2');]]) +box.execute([[SELECT * from t WHERE i > s;]]) +box.execute([[SELECT * from t WHERE s > i;]]) +box.execute([[SELECT * from t WHERE d > s;]]) +box.execute([[SELECT * from t WHERE s > d;]]) +box.execute([[SELECT * from t WHERE i = 1 and n > s;]]) +box.execute([[SELECT * from t WHERE i = 2 and s > n;]]) + +box.execute([[SELECT i FROM t WHERE i in (1);]]) +box.execute([[SELECT i FROM t WHERE d in (1);]]) +box.execute([[SELECT i FROM t WHERE n in (1);]]) +box.execute([[SELECT i FROM t WHERE s in (1);]]) + +box.execute([[SELECT i FROM t WHERE i in (1.0);]]) +box.execute([[SELECT i FROM t WHERE d in (1.0);]]) +box.execute([[SELECT i FROM t WHERE n in (1.0);]]) +box.execute([[SELECT i FROM t WHERE s in (1.0);]]) + +box.execute([[SELECT i FROM t WHERE i in ('1');]]) +box.execute([[SELECT i FROM t WHERE d in ('1');]]) +box.execute([[SELECT i FROM t WHERE n in ('1');]]) +box.execute([[SELECT i FROM t WHERE s in ('1');]]) + +box.execute([[SELECT i FROM t WHERE i in ('1.0');]]) +box.execute([[SELECT i FROM t WHERE d in ('1.0');]]) +box.execute([[SELECT i FROM t WHERE n in ('1.0');]]) +box.execute([[SELECT i FROM t WHERE s in ('1.0');]]) + +box.execute([[DROP TABLE t;]]) + +-- Comparison with SCALAR. +box.execute([[CREATE TABLE t(a SCALAR PRIMARY KEY);]]) +box.execute([[INSERT INTO t VALUES (1), (2.2), ('3');]]); +box.execute([[SELECT a FROM t WHERE a > 1]]); +box.execute([[SELECT a FROM t WHERE a > 1.0]]); +box.execute([[SELECT a FROM t WHERE a > '1']]); +box.execute([[SELECT a FROM t WHERE a < 1]]); +box.execute([[SELECT a FROM t WHERE a < 1.0]]); +box.execute([[SELECT a FROM t WHERE a < '1']]); +box.execute([[SELECT a FROM t WHERE a = 1]]); +box.execute([[SELECT a FROM t WHERE a = 1.0]]); +box.execute([[SELECT a FROM t WHERE a = '1']]); +box.execute([[DROP TABLE t;]]) -- 2.25.1 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Tarantool-patches] [PATCH v5 6/6] sql: remove implicit cast from MakeRecord opcode 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 6/6] sql: remove implicit cast from MakeRecord opcode imeevma @ 2020-09-18 12:28 ` Nikita Pettik 2020-09-28 16:19 ` Mergen Imeev 0 siblings, 1 reply; 15+ messages in thread From: Nikita Pettik @ 2020-09-18 12:28 UTC (permalink / raw) To: imeevma; +Cc: tarantool-patches On 21 Aug 12:19, imeevma@tarantool.org wrote: > This patch removes implicit casting from MakeRecord opcode. > > Closes #4230 > > @TarantoolBot document > Title: remove implicit cast for comparison > > After this patch-set, there will be no implicit casts for comparison. > This means that the values of the field types STRING, BOOLEAN and Again these strange characters in commit msg.. > VARBINARY can be compared with the values of the same field type. Only. > Any numerical value can be compared with any other numerical value. > > Example: > > ``` > tarantool> box.execute([[SELECT '1' > 0;]]) > --- > - null > - 'Type mismatch: can not convert 1 to numeric' > ... > > tarantool> box.execute([[SELECT true > X'33';]]) > --- > - null > - 'Type mismatch: can not convert boolean to varbinary' > ... > > tarantool> box.execute([[SELECT 1.23 > 123;]]) > --- > - metadata: > - name: 1.23 > 123 > type: boolean > rows: > - [false] > ... > ``` > --- > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > index bc2182446..d553b80db 100644 > --- a/src/box/sql/expr.c > +++ b/src/box/sql/expr.c > @@ -2886,11 +2886,18 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */ > jmpIfDynamic = -1; > } > r3 = sqlExprCodeTarget(pParse, pE2, r1); > - enum field_type types[2] = > - { lhs_type, field_type_MAX }; > - sqlVdbeAddOp4(v, OP_MakeRecord, r3, > - 1, r2, (char *)types, > - sizeof(types)); > + uint32_t size = > + 2 * sizeof(enum field_type); > + enum field_type *types= > + sqlDbMallocZero(pParse->db, Why do you have manually alloc types? It is done in sqlVdbeAddOp4() if pass n > 0 (like it was before your refactoring). > + size); > + types[0] = lhs_type; > + types[1] = field_type_MAX; > + sqlVdbeAddOp4(v, OP_ApplyType, r3, 1, 0, > + (char *)types, P4_DYNAMIC); > + sqlVdbeChangeP5(v, OPFLAG_DO_NOT_CONVERT_NUMBERS); > + sqlVdbeAddOp3(v, OP_MakeRecord, r3, 1, > + r2); > sql_expr_type_cache_change(pParse, > r3, 1); > sqlVdbeAddOp2(v, OP_IdxInsert, r2, You also can remove now mem_apply_type(). > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c > index a0ddbaf60..544619b03 100644 > --- a/src/box/sql/vdbe.c > +++ b/src/box/sql/vdbe.c > @@ -3145,24 +3145,17 @@ type_mismatch: > break; > } > > diff --git a/test/sql-tap/in3.test.lua b/test/sql-tap/in3.test.lua > index a6d842962..7f3abbae0 100755 > --- a/test/sql-tap/in3.test.lua > +++ b/test/sql-tap/in3.test.lua > @@ -1,6 +1,6 @@ > #!/usr/bin/env tarantool > test = require("sqltester") > -test:plan(28) > +test:plan(26) > > --!./tcltestrunner.lua > -- 2007 November 29 > @@ -334,18 +334,6 @@ test:do_test( > -- </in3-3.4> > }) > > -test:do_test( > - "in3-3.5", > - function() > - -- Numeric affinity should be applied to each side before the comparison > - -- takes place. Therefore we cannot use index t1_i1, which has no affinity. > - return exec_neph(" SELECT y IN (SELECT a FROM t1) FROM t2 ") > - end, { > - -- <in3-3.5> > - 1, true > - -- </in3-3.5> > - }) > - > test:do_test( > "in3-3.6", > function() > @@ -358,18 +346,6 @@ test:do_test( > -- </in3-3.6> > }) > > -test:do_test( > - "in3-3.7", > - function() > - -- Numeric affinity is applied before the comparison takes place. > - -- Making it impossible to use index t1_i3. > - return exec_neph(" SELECT y IN (SELECT c FROM t1) FROM t2 ") > - end, { > - -- <in3-3.7> > - 1, true > - -- </in3-3.7> > - }) > - Why did you remove these tests? > ----------------------------------------------------------------------- > -- > -- Test using a multi-column index. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Tarantool-patches] [PATCH v5 6/6] sql: remove implicit cast from MakeRecord opcode 2020-09-18 12:28 ` Nikita Pettik @ 2020-09-28 16:19 ` Mergen Imeev 0 siblings, 0 replies; 15+ messages in thread From: Mergen Imeev @ 2020-09-28 16:19 UTC (permalink / raw) To: Nikita Pettik; +Cc: tarantool-patches Thank you for review! My answers and diff below. On Fri, Sep 18, 2020 at 12:28:10PM +0000, Nikita Pettik wrote: > On 21 Aug 12:19, imeevma@tarantool.org wrote: > > This patch removes implicit casting from MakeRecord opcode. > > > > Closes #4230 > > > > @TarantoolBot document > > Title: remove implicit cast for comparison > > > > After this patch-set, there will be no implicit casts for comparison. > > This means that the values of the field types STRING, BOOLEAN and > > Again these strange characters in commit msg.. Sorry, fixed. > > > VARBINARY can be compared with the values of the same field type. > > Only. Fixed: Title: remove implicit cast for comparison After this patch-set, there will be no implicit casts for comparison. This means that the values of field types STRING, BOOLEAN, and VARBINARY can only be compared with the values of the same field type. Any numeric value can be compared to any other numeric value. > > > Any numerical value can be compared with any other numerical value. > > > > Example: > > > > ``` > > tarantool> box.execute([[SELECT '1' > 0;]]) > > --- > > - null > > - 'Type mismatch: can not convert 1 to numeric' > > ... > > > > tarantool> box.execute([[SELECT true > X'33';]]) > > --- > > - null > > - 'Type mismatch: can not convert boolean to varbinary' > > ... > > > > tarantool> box.execute([[SELECT 1.23 > 123;]]) > > --- > > - metadata: > > - name: 1.23 > 123 > > type: boolean > > rows: > > - [false] > > ... > > ``` > > --- > > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > > index bc2182446..d553b80db 100644 > > --- a/src/box/sql/expr.c > > +++ b/src/box/sql/expr.c > > @@ -2886,11 +2886,18 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */ > > jmpIfDynamic = -1; > > } > > r3 = sqlExprCodeTarget(pParse, pE2, r1); > > - enum field_type types[2] = > > - { lhs_type, field_type_MAX }; > > - sqlVdbeAddOp4(v, OP_MakeRecord, r3, > > - 1, r2, (char *)types, > > - sizeof(types)); > > + uint32_t size = > > + 2 * sizeof(enum field_type); > > + enum field_type *types= > > + sqlDbMallocZero(pParse->db, > > Why do you have manually alloc types? It is done in sqlVdbeAddOp4() > if pass n > 0 (like it was before your refactoring). > You are right, fixed. > > + size); > > + types[0] = lhs_type; > > + types[1] = field_type_MAX; > > + sqlVdbeAddOp4(v, OP_ApplyType, r3, 1, 0, > > + (char *)types, P4_DYNAMIC); > > + sqlVdbeChangeP5(v, OPFLAG_DO_NOT_CONVERT_NUMBERS); > > + sqlVdbeAddOp3(v, OP_MakeRecord, r3, 1, > > + r2); > > sql_expr_type_cache_change(pParse, > > r3, 1); > > sqlVdbeAddOp2(v, OP_IdxInsert, r2, > > You also can remove now mem_apply_type(). > No, mem_apply_type() used in one place sql_value_apply_type(), however this function used in 8 more places. I think we need to do a bit more before we will be able to remove mem_apply_type(). > > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c > > index a0ddbaf60..544619b03 100644 > > --- a/src/box/sql/vdbe.c > > +++ b/src/box/sql/vdbe.c > > @@ -3145,24 +3145,17 @@ type_mismatch: > > break; > > } > > > > diff --git a/test/sql-tap/in3.test.lua b/test/sql-tap/in3.test.lua > > index a6d842962..7f3abbae0 100755 > > --- a/test/sql-tap/in3.test.lua > > +++ b/test/sql-tap/in3.test.lua > > @@ -1,6 +1,6 @@ > > #!/usr/bin/env tarantool > > test = require("sqltester") > > -test:plan(28) > > +test:plan(26) > > > > --!./tcltestrunner.lua > > -- 2007 November 29 > > @@ -334,18 +334,6 @@ test:do_test( > > -- </in3-3.4> > > }) > > > > -test:do_test( > > - "in3-3.5", > > - function() > > - -- Numeric affinity should be applied to each side before the comparison > > - -- takes place. Therefore we cannot use index t1_i1, which has no affinity. > > - return exec_neph(" SELECT y IN (SELECT a FROM t1) FROM t2 ") > > - end, { > > - -- <in3-3.5> > > - 1, true > > - -- </in3-3.5> > > - }) > > - > > test:do_test( > > "in3-3.6", > > function() > > @@ -358,18 +346,6 @@ test:do_test( > > -- </in3-3.6> > > }) > > > > -test:do_test( > > - "in3-3.7", > > - function() > > - -- Numeric affinity is applied before the comparison takes place. > > - -- Making it impossible to use index t1_i3. > > - return exec_neph(" SELECT y IN (SELECT c FROM t1) FROM t2 ") > > - end, { > > - -- <in3-3.7> > > - 1, true > > - -- </in3-3.7> > > - }) > > - > > Why did you remove these tests? Now they throw an error, so it is useless to have them here. We have other tests to check these errors and they don't check usage of ephemeral space anymore. > > > ----------------------------------------------------------------------- > > -- > > -- Test using a multi-column index. Diff: diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index d553b80db..764e68cb1 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -2886,15 +2886,11 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */ jmpIfDynamic = -1; } r3 = sqlExprCodeTarget(pParse, pE2, r1); - uint32_t size = - 2 * sizeof(enum field_type); - enum field_type *types= - sqlDbMallocZero(pParse->db, - size); - types[0] = lhs_type; - types[1] = field_type_MAX; + enum field_type types[2] = + { lhs_type, field_type_MAX }; sqlVdbeAddOp4(v, OP_ApplyType, r3, 1, 0, - (char *)types, P4_DYNAMIC); + (char *)types, + sizeof(types)); sqlVdbeChangeP5(v, OPFLAG_DO_NOT_CONVERT_NUMBERS); sqlVdbeAddOp3(v, OP_MakeRecord, r3, 1, r2); ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2020-09-28 16:19 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-08-21 9:19 [Tarantool-patches] [PATCH v5 0/6] sql; remove implicit cast for comparison imeevma 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 1/6] sql: remove unused DOUBLE to INTEGER conversion imeevma 2020-09-17 14:48 ` Nikita Pettik 2020-09-28 15:50 ` Mergen Imeev 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 2/6] sql: add implicit cast between numbers in OP_Seek* imeevma 2020-09-17 15:34 ` Nikita Pettik 2020-09-28 15:55 ` Mergen Imeev 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 3/6] sql: change comparison between numbers using index imeevma 2020-09-18 8:08 ` Nikita Pettik 2020-09-28 16:10 ` Mergen Imeev 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 4/6] sql: remove implicit cast from comparison opcodes imeevma 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 5/6] sql: fix implicit cast in opcode MustBeInt imeevma 2020-08-21 9:19 ` [Tarantool-patches] [PATCH v5 6/6] sql: remove implicit cast from MakeRecord opcode imeevma 2020-09-18 12:28 ` Nikita Pettik 2020-09-28 16:19 ` Mergen Imeev
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox