[Tarantool-patches] [PATCH v5 44/52] sql: introduce mem_cast_implicit()
Mergen Imeev
imeevma at tarantool.org
Wed Apr 14 03:05:53 MSK 2021
Thank you for the review! My answer below. I also included patch since there
were some changes due to conflicts with patches 'sql: introduce mem_cmp_*()
functions' and patch 'sql: introduce mem_ecast_explicit()'.
On Wed, Apr 14, 2021 at 12:59:58AM +0200, Vladislav Shpilevoy wrote:
> Good job on the fixes!
>
> > 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;
>
> I think it would be better both for explicit and implicit casts
> to have the diag_set() inside.
I am not sure about this. There are some cases when result of execution is not
checked, which means that diag_set() will be set without actual error. Is this
fine? If so, I have no problem with moving diag_set() to mem_cast_*() functions
and mem_to_*() functions.
New patch:
commit 10dd667c04574a05fe0e75f0d4fd9b0327df76b1
Author: Mergen Imeev <imeevma at 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 4b619b032..537288c14 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -670,6 +670,32 @@ double_to_int_precise(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
double_to_str0(struct Mem *mem)
{
@@ -882,6 +908,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)
{
@@ -1924,122 +2084,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)
{
@@ -2459,14 +2503,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 7cced5537..91c1c464f 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -736,6 +736,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.
@@ -772,44 +782,6 @@ registerTrace(int iReg, Mem *p);
int sqlVdbeMemNulTerminate(struct Mem *);
int sqlVdbeMemExpandBlob(struct Mem *);
#define ExpandBlob(P) (((P)->flags&MEM_Zero)?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. */
@@ -874,17 +846,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 049965bd0..f7b6df0d9 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1645,8 +1645,9 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */
}
} else if (type == FIELD_TYPE_STRING) {
if (mem_cmp_str(pIn3, pIn1, &res, pOp->p4.pColl) != 0) {
- const char *str = mem_apply_type(pIn3, type) != 0 ?
- mem_str(pIn3) : mem_str(pIn1);
+ const char *str =
+ mem_cast_implicit_old(pIn3, type) != 0 ?
+ mem_str(pIn3) : mem_str(pIn1);
diag_set(ClientError, ER_SQL_TYPE_MISMATCH, str,
"string");
goto abort_due_to_error;
@@ -1655,8 +1656,9 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */
mem_is_num(pIn1)) {
type = FIELD_TYPE_NUMBER;
if (mem_cmp_num(pIn3, pIn1, &res) != 0) {
- const char *str = mem_apply_type(pIn3, type) != 0 ?
- mem_str(pIn3) : mem_str(pIn1);
+ const char *str =
+ mem_cast_implicit_old(pIn3, type) != 0 ?
+ mem_str(pIn3) : mem_str(pIn1);
diag_set(ClientError, ER_SQL_TYPE_MISMATCH, str,
"numeric");
goto abort_due_to_error;
@@ -1665,8 +1667,9 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */
type = FIELD_TYPE_STRING;
assert(mem_is_str(pIn3) && mem_is_same_type(pIn3, pIn1));
if (mem_cmp_str(pIn3, pIn1, &res, pOp->p4.pColl) != 0) {
- const char *str = mem_apply_type(pIn3, type) != 0 ?
- mem_str(pIn3) : mem_str(pIn1);
+ const char *str =
+ mem_cast_implicit_old(pIn3, type) != 0 ?
+ mem_str(pIn3) : mem_str(pIn1);
diag_set(ClientError, ER_SQL_TYPE_MISMATCH, str,
"string");
goto abort_due_to_error;
@@ -2137,23 +2140,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;
}
@@ -2212,7 +2204,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 749a3b455..ec413feb8 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;
}
More information about the Tarantool-patches
mailing list