From: Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: v.shpilevoy@tarantool.org, tsafin@tarantool.org Cc: tarantool-patches@dev.tarantool.org Subject: [Tarantool-patches] [PATCH v5 44/52] sql: introduce mem_cast_implicit() Date: Fri, 9 Apr 2021 23:53:44 +0300 [thread overview] Message-ID: <1e61942aa006bb53457006df9b80fc9d0e5aab0a.1618000037.git.imeevma@gmail.com> (raw) In-Reply-To: <cover.1618000036.git.imeevma@gmail.com> Thank you for the review! My answer and new patch below. On 30.03.2021 02:08, Vladislav Shpilevoy wrote: > Thanks for the patch! > >> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c >> index 559bf6121..1baf4c482 100644 >> --- a/src/box/sql/mem.c >> +++ b/src/box/sql/mem.c >> @@ -1071,6 +1093,140 @@ mem_explicit_cast(struct Mem *mem, enum field_type type) > > <...> > >> + >> +int >> +mem_implicit_cast_old(struct Mem *mem, enum field_type type) > > What is this? > Last year there was a patch-set that should have removed implicit cast from SQL. However, only part of this patch-set was pushed to master, because second review wasn't completed. This lead to a situation where old and new implicit cast rules exist at the same time and applied to different parts SQL. Also, @tsafin wants to return old implicit cast rules. In short, the situation with implicit cast is a mess right now. Still, I hope that we will remove implicit cast as we wanted last year. >> +{ >> + if (mem_is_null(mem)) >> + return 0; >> + switch (type) { >> + case FIELD_TYPE_UNSIGNED: >> + if ((mem->flags & MEM_UInt) != 0) >> + return 0; >> + if ((mem->flags & MEM_Real) != 0) >> + return mem_convert_double_to_unsigned_lossless(mem); >> + if ((mem->flags & MEM_Str) != 0) >> + return mem_convert_varstring_to_unsigned(mem); >> + return -1; >> + case FIELD_TYPE_STRING: >> + if ((mem->flags & (MEM_Str | MEM_Blob)) != 0) >> + return 0; >> + if ((mem->flags & (MEM_Int | MEM_UInt)) != 0) >> + return mem_convert_integer_to_string(mem); >> + if ((mem->flags & MEM_Real) != 0) >> + return mem_convert_double_to_string(mem); >> + return -1; >> + case FIELD_TYPE_DOUBLE: >> + if ((mem->flags & MEM_Real) != 0) >> + return 0; >> + if ((mem->flags & (MEM_Int | MEM_UInt)) != 0) >> + return mem_convert_integer_to_double(mem); >> + if ((mem->flags & MEM_Str) != 0) >> + return mem_convert_varstring_to_double(mem); >> + return -1; >> + case FIELD_TYPE_INTEGER: >> + if ((mem->flags & (MEM_Int | MEM_UInt)) != 0) >> + return 0; >> + if ((mem->flags & MEM_Str) != 0) >> + return mem_convert_varstring_to_integer(mem); >> + if (mem_is_double(mem)) >> + return mem_convert_double_to_integer_lossless(mem); >> + return -1; >> + case FIELD_TYPE_BOOLEAN: >> + if ((mem->flags & MEM_Bool) != 0) >> + return 0; >> + return -1; >> + case FIELD_TYPE_VARBINARY: >> + if ((mem->flags & MEM_Blob) != 0) >> + return 0; >> + return -1; >> + case FIELD_TYPE_NUMBER: >> + if ((mem->flags & (MEM_Int | MEM_UInt | MEM_Real)) != 0) >> + return 0; >> + if ((mem->flags & MEM_Str) != 0) >> + return mem_convert_to_number(mem); >> + return -1; >> + case FIELD_TYPE_MAP: >> + if (mem_is_map(mem)) >> + return 0; >> + return -1; >> + case FIELD_TYPE_ARRAY: >> + if (mem_is_array(mem)) >> + return 0; >> + return -1; >> + case FIELD_TYPE_SCALAR: >> + if ((mem->flags & MEM_Blob) != 0 && >> + (mem->flags & MEM_Subtype) != 0) >> + return -1; >> + return 0; >> + default: >> + break; >> + } >> + return -1; >> +} New patch: commit 1e61942aa006bb53457006df9b80fc9d0e5aab0a Author: Mergen Imeev <imeevma@gmail.com> Date: Wed Mar 17 12:43:08 2021 +0300 sql: introduce mem_cast_implicit() This patch introduces mem_cast_implicit(). This function is used to convert a MEM to given type according to implicit cast rules. Part of #5818 diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c index 45d2d5fe3..64c183411 100644 --- a/src/box/sql/mem.c +++ b/src/box/sql/mem.c @@ -946,6 +946,32 @@ mem_to_str(struct Mem *mem) return -1; } +static inline int +double_to_uint(struct Mem *mem) +{ + double d = mem->u.r; + if (d >= 0 && d < (double)UINT64_MAX) { + mem->u.u = (uint64_t)d; + mem->flags = MEM_UInt; + mem->field_type = FIELD_TYPE_UNSIGNED; + return 0; + } + return -1; +} + +static inline int +double_to_uint_precise(struct Mem *mem) +{ + double d = mem->u.r; + if (d >= 0 && d < (double)UINT64_MAX && (double)(uint64_t)d == d) { + mem->u.u = (uint64_t)d; + mem->flags = MEM_UInt; + mem->field_type = FIELD_TYPE_UNSIGNED; + return 0; + } + return -1; +} + static inline int bytes_to_uint(struct Mem *mem) { @@ -1071,6 +1097,140 @@ mem_cast_explicit(struct Mem *mem, enum field_type type) return -1; } +int +mem_cast_implicit(struct Mem *mem, enum field_type type) +{ + if ((mem->flags & MEM_Null) != 0) { + mem->field_type = type; + return 0; + } + switch (type) { + case FIELD_TYPE_UNSIGNED: + if ((mem->flags & MEM_UInt) != 0) + return 0; + if ((mem->flags & MEM_Real) != 0) + return double_to_uint(mem); + return -1; + case FIELD_TYPE_STRING: + if ((mem->flags & MEM_Str) != 0) + return 0; + return -1; + case FIELD_TYPE_DOUBLE: + if ((mem->flags & MEM_Real) != 0) + return 0; + if ((mem->flags & (MEM_Int | MEM_UInt)) != 0) + return int_to_double(mem); + return -1; + case FIELD_TYPE_INTEGER: + if ((mem->flags & (MEM_Int | MEM_UInt)) != 0) + return 0; + if ((mem->flags & MEM_Real) != 0) + return double_to_int(mem); + return -1; + case FIELD_TYPE_BOOLEAN: + if ((mem->flags & MEM_Bool) != 0) + return 0; + return -1; + case FIELD_TYPE_VARBINARY: + if ((mem->flags & MEM_Blob) != 0) + return 0; + return -1; + case FIELD_TYPE_NUMBER: + if ((mem->flags & (MEM_Int | MEM_UInt | MEM_Real)) != 0) + return 0; + return -1; + case FIELD_TYPE_MAP: + if (mem_is_map(mem)) + return 0; + return -1; + case FIELD_TYPE_ARRAY: + if (mem_is_array(mem)) + return 0; + return -1; + case FIELD_TYPE_SCALAR: + if ((mem->flags & MEM_Blob) != 0 && + (mem->flags & MEM_Subtype) != 0) + return -1; + return 0; + case FIELD_TYPE_ANY: + return 0; + default: + break; + } + return -1; +} + +int +mem_cast_implicit_old(struct Mem *mem, enum field_type type) +{ + if (mem_is_null(mem)) + return 0; + switch (type) { + case FIELD_TYPE_UNSIGNED: + if ((mem->flags & MEM_UInt) != 0) + return 0; + if ((mem->flags & MEM_Real) != 0) + return double_to_uint_precise(mem); + if ((mem->flags & MEM_Str) != 0) + return bytes_to_uint(mem); + return -1; + case FIELD_TYPE_STRING: + if ((mem->flags & (MEM_Str | MEM_Blob)) != 0) + return 0; + if ((mem->flags & (MEM_Int | MEM_UInt)) != 0) + return int_to_str0(mem); + if ((mem->flags & MEM_Real) != 0) + return double_to_str0(mem); + return -1; + case FIELD_TYPE_DOUBLE: + if ((mem->flags & MEM_Real) != 0) + return 0; + if ((mem->flags & (MEM_Int | MEM_UInt)) != 0) + return int_to_double(mem); + if ((mem->flags & MEM_Str) != 0) + return bin_to_str(mem); + return -1; + case FIELD_TYPE_INTEGER: + if ((mem->flags & (MEM_Int | MEM_UInt)) != 0) + return 0; + if ((mem->flags & MEM_Str) != 0) + return bytes_to_int(mem); + if ((mem->flags & MEM_Real) != 0) + return double_to_int_precise(mem); + return -1; + case FIELD_TYPE_BOOLEAN: + if ((mem->flags & MEM_Bool) != 0) + return 0; + return -1; + case FIELD_TYPE_VARBINARY: + if ((mem->flags & MEM_Blob) != 0) + return 0; + return -1; + case FIELD_TYPE_NUMBER: + if ((mem->flags & (MEM_Int | MEM_UInt | MEM_Real)) != 0) + return 0; + if ((mem->flags & MEM_Str) != 0) + return mem_to_number(mem); + return -1; + case FIELD_TYPE_MAP: + if (mem_is_map(mem)) + return 0; + return -1; + case FIELD_TYPE_ARRAY: + if (mem_is_array(mem)) + return 0; + return -1; + case FIELD_TYPE_SCALAR: + if ((mem->flags & MEM_Blob) != 0 && + (mem->flags & MEM_Subtype) != 0) + return -1; + return 0; + default: + break; + } + return -1; +} + int mem_copy(struct Mem *to, const struct Mem *from) { @@ -2148,122 +2308,6 @@ sqlVdbeMemExpandBlob(Mem * pMem) return 0; } -/* - * Exported version of mem_apply_type(). This one works on sql_value*, - * not the internal Mem* type. - */ -void -sql_value_apply_type( - sql_value *pVal, - enum field_type type) -{ - mem_apply_type((Mem *) pVal, type); -} - -int -mem_apply_type(struct Mem *record, enum field_type type) -{ - if ((record->flags & MEM_Null) != 0) - return 0; - assert(type < field_type_MAX); - switch (type) { - case FIELD_TYPE_INTEGER: - case FIELD_TYPE_UNSIGNED: - if ((record->flags & (MEM_Bool | MEM_Blob)) != 0) - return -1; - if ((record->flags & MEM_UInt) == MEM_UInt) - return 0; - if ((record->flags & MEM_Real) == MEM_Real) { - double d = record->u.r; - if (d >= 0) { - if (double_compare_uint64(d, UINT64_MAX, - 1) > 0) - return 0; - if ((double)(uint64_t)d == d) { - record->u.u = (uint64_t)d; - record->flags = MEM_UInt; - record->field_type = - FIELD_TYPE_UNSIGNED; - } - } else { - if (double_compare_nint64(d, INT64_MIN, 1) < 0) - return 0; - if ((double)(int64_t)d == d) { - record->u.i = (int64_t)d; - record->flags = MEM_Int; - record->field_type = FIELD_TYPE_INTEGER; - } - } - return 0; - } - if ((record->flags & MEM_Str) != 0) { - bool is_neg; - int64_t i; - if (sql_atoi64(record->z, &i, &is_neg, record->n) != 0) - return -1; - mem_set_int(record, i, is_neg); - } - if ((record->flags & MEM_Int) == MEM_Int) { - if (type == FIELD_TYPE_UNSIGNED) - return -1; - return 0; - } - return 0; - case FIELD_TYPE_BOOLEAN: - if ((record->flags & MEM_Bool) == MEM_Bool) - return 0; - return -1; - case FIELD_TYPE_NUMBER: - if ((record->flags & (MEM_Real | MEM_Int | MEM_UInt)) != 0) - return 0; - return mem_to_double(record); - case FIELD_TYPE_DOUBLE: - if ((record->flags & MEM_Real) != 0) - return 0; - return mem_to_double(record); - case FIELD_TYPE_STRING: - /* - * Only attempt the conversion to TEXT if there is - * an integer or real representation (BLOB and - * NULL do not get converted). - */ - if ((record->flags & MEM_Str) == 0 && - (record->flags & (MEM_Real | MEM_Int | MEM_UInt)) != 0) - mem_to_str(record); - record->flags &= ~(MEM_Real | MEM_Int | MEM_UInt); - return 0; - case FIELD_TYPE_VARBINARY: - if ((record->flags & MEM_Blob) == 0) - return -1; - return 0; - case FIELD_TYPE_SCALAR: - /* Can't cast MAP and ARRAY to scalar types. */ - if ((record->flags & MEM_Subtype) != 0 && - record->subtype == SQL_SUBTYPE_MSGPACK) { - assert(mp_typeof(*record->z) == MP_MAP || - mp_typeof(*record->z) == MP_ARRAY); - return -1; - } - return 0; - case FIELD_TYPE_MAP: - if ((record->flags & MEM_Subtype) != 0 && - record->subtype == SQL_SUBTYPE_MSGPACK && - mp_typeof(*record->z) == MP_MAP) - return 0; - return -1; - case FIELD_TYPE_ARRAY: - if ((record->flags & MEM_Subtype) != 0 && - record->subtype == SQL_SUBTYPE_MSGPACK && - mp_typeof(*record->z) == MP_ARRAY) - return 0; - return -1; - case FIELD_TYPE_ANY: - return 0; - default: - return -1; - } -} - static int sqlVdbeMemGrow(struct Mem *pMem, int n, int bPreserve) { @@ -2683,14 +2727,6 @@ sqlMemCompare(const Mem * pMem1, const Mem * pMem2, const struct coll * pColl) return res; } -bool -mem_is_type_compatible(struct Mem *mem, enum field_type type) -{ - enum mp_type mp_type = mem_mp_type(mem); - assert(mp_type < MP_EXT); - return field_mp_plain_type_is_compatible(type, mp_type, true); -} - int sql_vdbemem_finalize(struct Mem *mem, struct func *func) { diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h index 73d2ffd6a..dbd58e1a5 100644 --- a/src/box/sql/mem.h +++ b/src/box/sql/mem.h @@ -526,6 +526,16 @@ mem_to_str0(struct Mem *mem); int mem_cast_explicit(struct Mem *mem, enum field_type type); +/** Convert the given MEM to given type according to implicit cast rules. */ +int +mem_cast_implicit(struct Mem *mem, enum field_type type); + +/** + * Convert the given MEM to given type according to legacy implicit cast rules. + */ +int +mem_cast_implicit_old(struct Mem *mem, enum field_type type); + /** * Simple type to str convertor. It is used to simplify * error reporting. @@ -562,44 +572,6 @@ registerTrace(int iReg, Mem *p); int sqlVdbeMemNulTerminate(struct Mem *); int sqlVdbeMemExpandBlob(struct Mem *); #define ExpandBlob(P) (mem_is_zerobin(P)? sqlVdbeMemExpandBlob(P) : 0) -void sql_value_apply_type(struct Mem *val, enum field_type type); - - -/** - * Processing is determined by the field type parameter: - * - * INTEGER: - * If memory holds floating point value and it can be - * converted without loss (2.0 - > 2), it's type is - * changed to INT. Otherwise, simply return success status. - * - * NUMBER: - * If memory holds INT or floating point value, - * no actions take place. - * - * STRING: - * Convert mem to a string representation. - * - * SCALAR: - * Mem is unchanged, but flag is set to BLOB in case of - * scalar-like type. Otherwise, (MAP, ARRAY) conversion - * is impossible. - * - * BOOLEAN: - * If memory holds BOOLEAN no actions take place. - * - * ANY: - * Mem is unchanged, no actions take place. - * - * MAP/ARRAY: - * These types can't be casted to scalar ones, or to each - * other. So the only valid conversion is to type itself. - * - * @param record The value to apply type to. - * @param type The type to be applied. - */ -int -mem_apply_type(struct Mem *record, enum field_type type); /** Setters = Change MEM value. */ @@ -657,17 +629,6 @@ int sqlVdbeMemTooBig(Mem *); int sqlMemCompare(const Mem *, const Mem *, const struct coll *); -/** - * Check that MEM_type of the mem is compatible with given type. - * - * @param mem The MEM that contains the value to check. - * @param type The type to check. - * @retval TRUE if the MEM_type of the value and the given type - * are compatible, FALSE otherwise. - */ -bool -mem_is_type_compatible(struct Mem *mem, enum field_type type); - /** MEM manipulate functions. */ /** diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 29634841a..72a84e80d 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -2092,23 +2092,12 @@ case OP_ApplyType: { while((type = *(types++)) != field_type_MAX) { assert(pIn1 <= &p->aMem[(p->nMem+1 - p->nCursor)]); assert(memIsValid(pIn1)); - if (!mem_is_type_compatible(pIn1, type)) { - /* Implicit cast is allowed only to numeric type. */ - if (!sql_type_is_numeric(type)) - goto type_mismatch; - /* Implicit cast is allowed only from numeric type. */ - if (!mem_is_num(pIn1)) - goto type_mismatch; - /* Try to convert numeric-to-numeric. */ - if (mem_cast_explicit(pIn1, type) != 0) - goto type_mismatch; + if (mem_cast_implicit(pIn1, type) != 0) { + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, + mem_str(pIn1), field_type_strs[type]); + goto abort_due_to_error; } pIn1++; - continue; -type_mismatch: - diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - mem_str(pIn1), field_type_strs[type]); - goto abort_due_to_error; } break; } @@ -2167,7 +2156,7 @@ case OP_MakeRecord: { if (types != NULL) { pRec = pData0; do { - mem_apply_type(pRec++, *(types++)); + mem_cast_implicit_old(pRec++, *(types++)); } while(types[0] != field_type_MAX); } diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index a5e78be06..672213775 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -2334,7 +2334,7 @@ sqlVdbeGetBoundValue(Vdbe * v, int iVar, u8 aff) sql_value *pRet = sqlValueNew(v->db); if (pRet) { mem_copy(pRet, pMem); - sql_value_apply_type(pRet, aff); + mem_cast_implicit_old(pRet, aff); } return pRet; }
next prev parent reply other threads:[~2021-04-09 20:55 UTC|newest] Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <cover.1618000036.git.imeevma@gmail.com> 2021-04-09 20:53 ` [Tarantool-patches] [PATCH v5 41/52] sql: introduce mem_to_number() Mergen Imeev via Tarantool-patches 2021-04-13 23:25 ` Mergen Imeev via Tarantool-patches 2021-04-09 20:53 ` [Tarantool-patches] [PATCH v5 42/52] sql: introduce mem_to_str() and mem_to_str0() Mergen Imeev via Tarantool-patches 2021-04-13 22:58 ` Vladislav Shpilevoy via Tarantool-patches 2021-04-13 23:41 ` Mergen Imeev via Tarantool-patches 2021-04-09 20:53 ` [Tarantool-patches] [PATCH v5 43/52] sql: introduce mem_cast_explicit() Mergen Imeev via Tarantool-patches 2021-04-13 22:59 ` Vladislav Shpilevoy via Tarantool-patches 2021-04-14 0:01 ` Mergen Imeev via Tarantool-patches 2021-04-09 20:53 ` Mergen Imeev via Tarantool-patches [this message] 2021-04-13 22:59 ` [Tarantool-patches] [PATCH v5 44/52] sql: introduce mem_cast_implicit() Vladislav Shpilevoy via Tarantool-patches 2021-04-14 0:05 ` Mergen Imeev via Tarantool-patches 2021-04-09 20:53 ` [Tarantool-patches] [PATCH v5 45/52] sql: introduce mem_get_int() Mergen Imeev via Tarantool-patches 2021-04-13 23:01 ` Vladislav Shpilevoy via Tarantool-patches 2021-04-14 0:28 ` Mergen Imeev via Tarantool-patches 2021-04-14 1:17 ` Mergen Imeev via Tarantool-patches 2021-04-09 21:08 ` [Tarantool-patches] [PATCH v5 46/52] sql: introduce mem_get_uint() Mergen Imeev via Tarantool-patches 2021-04-13 23:04 ` Vladislav Shpilevoy via Tarantool-patches 2021-04-14 0:39 ` Mergen Imeev via Tarantool-patches 2021-04-14 1:21 ` Mergen Imeev via Tarantool-patches 2021-04-09 21:08 ` [Tarantool-patches] [PATCH v5 47/52] sql: introduce mem_get_double() Mergen Imeev via Tarantool-patches 2021-04-13 23:04 ` Vladislav Shpilevoy via Tarantool-patches 2021-04-14 1:00 ` Mergen Imeev via Tarantool-patches 2021-04-15 0:17 ` Vladislav Shpilevoy via Tarantool-patches 2021-04-15 0:46 ` Mergen Imeev via Tarantool-patches 2021-04-09 21:08 ` [Tarantool-patches] [PATCH v5 48/52] sql: introduce mem_get_bool() Mergen Imeev via Tarantool-patches 2021-04-13 23:04 ` Vladislav Shpilevoy via Tarantool-patches 2021-04-14 1:29 ` Mergen Imeev via Tarantool-patches 2021-04-09 21:08 ` [Tarantool-patches] [PATCH v5 49/52] sql: introduce mem_get_str0() and mem_as_str0() Mergen Imeev via Tarantool-patches 2021-04-13 23:06 ` Vladislav Shpilevoy via Tarantool-patches 2021-04-14 1:43 ` Mergen Imeev via Tarantool-patches 2021-04-09 21:08 ` [Tarantool-patches] [PATCH v5 50/52] sql: introduce mem_get_bin() Mergen Imeev via Tarantool-patches 2021-04-09 21:08 ` [Tarantool-patches] [PATCH v5 51/52] sql: introduce mem_get_bytes_len() Mergen Imeev via Tarantool-patches 2021-04-13 23:06 ` Vladislav Shpilevoy via Tarantool-patches 2021-04-14 1:55 ` Mergen Imeev via Tarantool-patches 2021-04-15 0:21 ` Vladislav Shpilevoy via Tarantool-patches 2021-04-15 0:51 ` Mergen Imeev via Tarantool-patches 2021-04-09 21:08 ` [Tarantool-patches] [PATCH v5 52/52] sql: introduce mem_get_agg() Mergen Imeev via Tarantool-patches
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=1e61942aa006bb53457006df9b80fc9d0e5aab0a.1618000037.git.imeevma@gmail.com \ --to=tarantool-patches@dev.tarantool.org \ --cc=imeevma@tarantool.org \ --cc=tsafin@tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v5 44/52] sql: introduce mem_cast_implicit()' \ /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