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