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

[n.pettik]

> On 9 Jul 2019, at 12:14, Imeev Mergen <imeevma at tarantool.org> wrote:
> 
> Hi! Thank you for review. My answer below.
> 
> On 7/8/19 4:32 PM, n.pettik wrote:
>> 
>>> Diff:
>>> 
>>> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
>>> index f8cf1af..79232de 100644
>>> --- a/src/box/sql/vdbe.c
>>> +++ b/src/box/sql/vdbe.c
>>> @@ -351,19 +351,22 @@ mem_apply_type(struct Mem *record, enum field_type type)
>>> 		return 0;
>>> 	case FIELD_TYPE_SCALAR:
>>> 		/* Can't cast MAP and ARRAY to scalar types. */
>>> -		if (record->subtype == SQL_SUBTYPE_MSGPACK) {
>>> +		if ((record->flags & MEM_Blob) == MEM_Blob &&
>> 
>> Why do you need this additional check on MEM_Blob?
>> Is it possible that memory holds raw msgpack and its
>> type not blob? If so, please provide an example.
>> 
> I'm not sure what you describe is possible. But it is impossible
> to say what type MEM has when you look at the SUBTYPE. At the same
> time, the subtype has any meaning only for BLOBs. Any other type
> has any SUBTYPE, and it is not checked anywhere. So, to avoid an
> error, when something, for example INT, throws an error when
> casting to SCALAR, I added these checks.

Ok, but then I guess more appropriate solution would be
checking MEM_Subtype flag, instead of MEM_Blob
Anyway, I would add clear test which reaches this path
and record->flags != MEM_Blob, but ->subtype == msgpack.