<HTML><BODY><p style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;">Hi!<br><br>Thanks for your review!</p><p style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;">Guess we might need to confirm it with Pettik <br>or someone else who interacts with sql a lot,<br>but i am quite sure sqlRunParser doesn't need<br>to set diagnostics even in case of returning -1.</p><p style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;">So far, i think we need to set diagnostics here,<br>in field_def_decode, like we do it in<br>ck_constraint_new.</p><p style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;">Although it is not something i broke within<br>this patchset, i will add it to follow-up.</p><p style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;"><br data-mce-bogus="1"></p><p style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;">Sincerely,<br>Ilya Kosarev</p><br><br><br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">
Вторник, 12 ноября 2019, 14:00 +03:00 от Sergey Ostanevich <sergos@tarantool.org>:<br>
<br>
<div id="">
<div class="js-helper js-readmsg-msg">
<style type="text/css"></style>
<div>
<div id="style_15735564040568068792_BODY">Hi!<br>
<br>
Thanks for the patch. One follow-up below, LGTM .<br>
<br>
<br>
Sergos<br>
<br>
> const char *dv = field->default_value;<br>
> @@ -496,8 +507,9 @@ field_def_decode(struct field_def *field, const char **data,<br>
> field->default_value_expr = sql_expr_compile(sql_get(), dv,<br>
> strlen(dv));<br>
> if (field->default_value_expr == NULL)<br>
> - diag_raise();<br>
> + return -1;<br>
<br>
The sql_expr_compile() calls for sqlRunParser() that does not set diag<br>
in all exit paths. Please, put into followup.<br>
<br>
<br>
</div>
</div>
</div>
</div>
</blockquote>
<br>
<br>-- <br>Ilya Kosarev<br><div id="_mcePaste" class="mcePaste" data-mce-bogus="1" style="position: absolute; left: 0px; top: -25px; width: 1px; height: 1px; overflow: hidden;"><pre style="font-family: "DejaVu Sans Mono"; font-size: 9pt;">field_def_decode</pre></div></BODY></HTML>