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

[Sergey Ostanevich <sergos@tarantool.org>]

Hi!

Thanks, Follow-up is fine - the patch is LGTM.

Sergos

On 19 Nov 18:16, Ilya Kosarev wrote:
> 
> 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