Re: [Tarantool-patches] [PATCH v4 4/5] sql: change implicit cast for assignment

[Nikita Pettik <korablev@tarantool.org>]

On 13 Jul 08:33, imeevma@tarantool.org wrote:
> +
>   * Synopsis: type(r[P1@P2])
>   *
> - * Apply types to a range of P2 registers starting with P1.
> - *
> - * P4 is a string that is P2 characters long. The nth character of the
> - * string indicates the column type that should be used for the nth
> - * memory cell in the range.
> + * Check that types of P2 registers starting from register P1 are
> + * compatible with given field types in P4. If the MEM_type of the
> + * value and the given type are incompatible according to
> + * field_mp_plain_type_is_compatible(), but both are numeric,
> + * this opcode attempts to convert the value to the type.
>   */
>  case OP_ApplyType: {
>  	enum field_type *types = pOp->p4.types;
> @@ -2762,7 +2900,13 @@ case OP_ApplyType: {
>  	while((type = *(types++)) != field_type_MAX) {
>  		assert(pIn1 <= &p->aMem[(p->nMem+1 - p->nCursor)]);
>  		assert(memIsValid(pIn1));
> -		if (mem_apply_type(pIn1, type) != 0) {
> +		if (mem_is_type_compatible(pIn1, type)) {
> +			pIn1++;
> +			continue;
> +		}
> +		if (!mp_type_is_numeric(mem_mp_type(pIn1)) ||
> +		    !sql_type_is_numeric(type) ||
> +		    mem_convert_to_numeric(pIn1, type, false) != 0) {

Consider refactoring:

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 863f38f5d..41a4750da 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2900,19 +2900,23 @@ 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)) {
-                       pIn1++;
-                       continue;
-               }
-               if (!mp_type_is_numeric(mem_mp_type(pIn1)) ||
-                   !sql_type_is_numeric(type) ||
-                   mem_convert_to_numeric(pIn1, type, false) != 0) {
-                       diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-                                sql_value_to_diag_str(pIn1),
-                                field_type_strs[type]);
-                       goto abort_due_to_error;
+               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 (!mp_type_is_numeric(mem_mp_type(pIn1)))
+                               goto type_mismatch;
+                       /* Try to convert numeric-to-numeric. */
+                       if (mem_convert_to_numeric(pIn1, type, false) != 0)
+                               goto type_mismatch;
                }
                pIn1++;
+               continue;
+type_mismatch:
+               diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
+                        sql_value_to_diag_str(pIn1), field_type_strs[type]);
+               goto abort_due_to_error;
        }
        break;
 }

Otherwise LGTM

>  			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
>  				 sql_value_to_diag_str(pIn1),
>  				 field_type_strs[type]);
> diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
> index 44c27bdb7..ad46ab129 100644
> --- a/src/box/sql/vdbeInt.h
> +++ b/src/box/sql/vdbeInt.h
> @@ -566,6 +566,10 @@ mem_mp_type(struct Mem *mem);
>   */
>  #define mp_type_is_bloblike(X) ((X) == MP_BIN || (X) == MP_ARRAY || (X) == MP_MAP)
>  
> +/** Return TRUE if MP_type of X is numeric, FALSE otherwise. */
> +#define mp_type_is_numeric(X) ((X) == MP_INT || (X) == MP_UINT ||\
> +			       (X) == MP_DOUBLE)