From: imeevma@tarantool.org To: korablev@tarantool.org, tsafin@tarantool.org Cc: tarantool-patches@dev.tarantool.org Subject: [Tarantool-patches] [PATCH v5 2/6] sql: add implicit cast between numbers in OP_Seek* Date: Fri, 21 Aug 2020 12:19:51 +0300 [thread overview] Message-ID: <dc9d1b3a51fcf5f53031e69c640643d432906fa2.1598000242.git.imeevma@gmail.com> (raw) In-Reply-To: <cover.1598000242.git.imeevma@gmail.com> 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
next prev parent reply other threads:[~2020-08-21 9:19 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 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 ` imeevma [this message] 2020-09-17 15:34 ` [Tarantool-patches] [PATCH v5 2/6] sql: add implicit cast between numbers in OP_Seek* 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
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=dc9d1b3a51fcf5f53031e69c640643d432906fa2.1598000242.git.imeevma@gmail.com \ --to=imeevma@tarantool.org \ --cc=korablev@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=tsafin@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v5 2/6] sql: add implicit cast between numbers in OP_Seek*' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox