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

["n.pettik" <korablev@tarantool.org>]

> On 28 Jun 2019, at 18:34, imeevma@tarantool.org wrote:
> 
> Before this patch, UPDATE throwed an error when executed on a
> tuple with types unknown to SQL. This patch fixes the problem in
> case the fields with unknown types are not updated.

I’ve fixed commit message a bit:

sql: add ARRAY, MAP and ANY types to mem_apply_type()

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
> ---
> https://github.com/tarantool/tarantool/issues/4189
> https://github.com/tarantool/tarantool/tree/imeevma/gh-4189-field-type-conversion-error

I suggest follow refactoring:

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 8ddc29d14..f8cf1afc7 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -292,7 +292,9 @@ 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.
@@ -300,13 +302,9 @@ mem_apply_numeric_type(struct Mem *record)
  * ANY:
  *    Mem is unchanged, no actions take place.
  *
- * MAP:
- *    If memory holds value with SQL_SUBTYPE_MSGPACK subtype and
- *    data has MP_MAP type, no actions take place.
- *
- * ARRAY:
- *    If memory holds value with SQL_SUBTYPE_MSGPACK subtype and
- *    data has MP_ARRAY type, 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.
@@ -351,8 +349,13 @@ mem_apply_type(struct Mem *record, enum field_type type)
                }
                record->flags &= ~(MEM_Real | MEM_Int);
                return 0;
-       case FIELD_TYPE_ANY:
        case FIELD_TYPE_SCALAR:
+               /* Can't cast MAP and ARRAY to scalar types. */
+               if (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->subtype == SQL_SUBTYPE_MSGPACK &&
@@ -364,6 +367,8 @@ mem_apply_type(struct Mem *record, enum field_type type)
                    mp_typeof(*record->z) == MP_ARRAY)
                        return 0;
                return -1;
+       case FIELD_TYPE_ANY:
+               return 0;
        default:
                return -1;
        }
diff --git a/test/sql/types.result b/test/sql/types.result
index 5cc7f1659..32588d38c 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -967,8 +967,8 @@ box.execute('SELECT ?', {true})
   - [true]
 ...
 --
--- gh-4189: Update throws an error when executed on a tuple with
--- types unknown to SQL.
+-- gh-4189: make sure that update doesn't throw an error if format
+-- of table features map/array field types.
 --
 format = {}
 ---
diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua
index d33314a0b..69af421d2 100644
--- a/test/sql/types.test.lua
+++ b/test/sql/types.test.lua
@@ -236,8 +236,8 @@ box.execute('SELECT \'9223372036854\' + 1;')
 box.execute('SELECT ?', {true})
 
 --
--- gh-4189: Update throws an error when executed on a tuple with
--- types unknown to SQL.
+-- gh-4189: make sure that update doesn't throw an error if format
+-- of table features map/array field types.
 --
 format = {}
 format[1] = {type = 'integer', name = 'I'}