[Tarantool-patches] [PATCH v5 44/52] sql: introduce mem_cast_implicit()

imeevma at tarantool.org imeevma at tarantool.org
Fri Apr 9 23:53:44 MSK 2021


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 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 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;
 		}


More information about the Tarantool-patches mailing list