[tarantool-patches] Re: [PATCH v1 1/1] sql: Fix UPDATE for types unknown to SQL.

[n.pettik]

> New patch:
> 
> From 1d6abdeb18652890c1819349f24d8b450f6ed6c7 Mon Sep 17 00:00:00 2001
> Date: Fri, 28 Jun 2019 18:16:16 +0300
> Subject: [PATCH] sql: add ARRAY, MAP and ANY types to mem_apply_type()
> 
> Function mem_apply_type() implements implicit type conversion. As
> a rule, tuple to be inserted to the space is exposed to this
> conversion which is invoked during execution of OP_MakeRecord
> opcode (which in turn forms tuple). This function was not adjusted
> to operate on ARRAY, MAP and ANY field types since they are poorly
> supported in current SQL implementation. Hence, when tuple to be
> inserted in space having mentioned field types reaches this
> function, it results in error. Note that we can't set ARRAY or MAP
> types in SQL, but such situation may appear during UPDATE
> operation on space created via Lua interface. This problem is
> solved by extending implicit type conversions with obvious casts:
> array field can be casted to array, map to map and any to any.
> 
> Closes #4189
> 
> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> index 9f4ee7a..cf4715d 100644
> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -292,7 +292,19 @@ mem_apply_numeric_type(struct Mem *record)
>  *    Convert mem to a string representation.
>  *
>  * SCALAR:
> - *    Mem is unchanged, but flag is set to BLOB.
> + *    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.
> @@ -338,6 +350,27 @@ mem_apply_type(struct Mem *record, enum field_type type)
> 		record->flags &= ~(MEM_Real | MEM_Int);
> 		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;
> @@ -2662,8 +2695,17 @@ case OP_ApplyType: {
> 		assert(pIn1 <= &p->aMem[(p->nMem+1 - p->nCursor)]);
> 		assert(memIsValid(pIn1));
> 		if (mem_apply_type(pIn1, type) != 0) {
> -			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> -				 sql_value_text(pIn1),
> +			const char *value;
> +			if ((pIn1->flags & MEM_Subtype) != 0 &&
> +			    pIn1->subtype == SQL_SUBTYPE_MSGPACK) {
> +				if (mp_typeof(*pIn1->z) == MP_MAP)
> +					value = "map”;

Please move this fix alongside with test to a separate patch.

> +				else
> +					value = "array";
> +			} else {
> +				value = (const char *)sql_value_text(pIn1);
> +			}
> +			diag_set(ClientError, ER_SQL_TYPE_MISMATCH, value,
> 				 field_type_strs[type]);
> 			goto abort_due_to_error;
> 		}

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20190718/417e2910/attachment.html>