Re: [Tarantool-patches] [tarantool-patches] Re: [PATCH v4 12/20] refactoring: remove exceptions from space_def_new_from_tuple

["Ilya Kosarev" <i.kosarev@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 1480 bytes --]


Hi!
After brief discussion with Nikita we found out that
sqlRunParser should set diagnostics for all error cases,
though it doesn't. Thus i submitted an issue:
https://github.com/tarantool/tarantool/issues/4633

Sincerely,
Ilya Kosarev


>Вторник, 12 ноября 2019, 18:32 +03:00 от Ilya Kosarev <i.kosarev@tarantool.org>:
>
>Hi!
>
>Thanks for your review!
>Guess we might need to confirm it with Pettik 
>or someone else who interacts with sql a lot,
>but i am quite sure sqlRunParser doesn't need
>to set diagnostics even in case of returning -1.
>So far, i think we need to set diagnostics here,
>in field_def_decode, like we do it in
>ck_constraint_new.
>Although it is not something i broke within
>this patchset, i will add it to follow-up.
>
>Sincerely,
>Ilya Kosarev
>
>
>>Вторник, 12 ноября 2019, 14:00 +03:00 от Sergey Ostanevich < sergos@tarantool.org >:
>>
>>Hi!
>>
>>Thanks for the patch. One follow-up below, LGTM .
>>
>>
>>Sergos
>>
>>>  	const char *dv = field->default_value;
>>> @@ -496,8 +507,9 @@ field_def_decode(struct field_def *field, const char **data,
>>>  		field->default_value_expr = sql_expr_compile(sql_get(), dv,
>>>  							     strlen(dv));
>>>  		if (field->default_value_expr == NULL)
>>> -			diag_raise();
>>> +			return -1;
>>
>>The sql_expr_compile() calls for sqlRunParser() that does not set diag
>>in all exit paths. Please, put into followup.
>>
>>
>
>
>-- 
>Ilya Kosarev
>field_def_decode

-- 
Ilya Kosarev

[-- Attachment #2: Type: text/html, Size: 4123 bytes --]