<HTML><BODY><p>Hi!</p><p>After brief discussion with Nikita we found out that<br><span style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;">sqlRunParser should set diagnostics for all error cases,<br></span>though it doesn't. Thus i submitted an issue:<br><a href="https://github.com/tarantool/tarantool/issues/4633">https://github.com/tarantool/tarantool/issues/4633</a><br><br><span style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;">Sincerely,</span><br style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;"><span style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;">Ilya Kosarev</span><a href="https://github.com/tarantool/tarantool/issues/4633"></a><br><br><br></p><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">
Вторник, 12 ноября 2019, 18:32 +03:00 от Ilya Kosarev <i.kosarev@tarantool.org>:<br>
<br>
<div id="">
<div class="js-helper js-readmsg-msg">
<style type="text/css"></style>
<div>
<div id="style_15735727661595278587_BODY"><div class="class_1574185592">
<p data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;" style="font-family: Arial, Tahoma, Verdana, sans-serif;">Hi!<br><br>Thanks for your review!</p><p data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;" 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 data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;" 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 data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;" 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 data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;" style="font-family: Arial, Tahoma, Verdana, sans-serif;"><br></p><p data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;" 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 <<a href="mailto:sergos@tarantool.org">sergos@tarantool.org</a>>:<br>
<br>
<div id="">
<div class="js-helper_mailru_css_attribute_postfix js-readmsg-msg_mailru_css_attribute_postfix">
<style></style>
<div>
<div id="style_15735564040568068792_BODY_mailru_css_attribute_postfix">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 style="width: 1px;height: 1px;overflow: hidden;" class="mcePaste_mailru_css_attribute_postfix" id="_mcePaste_mailru_css_attribute_postfix"><pre style="font-family: 'DejaVu Sans Mono';font-size: 9pt;">field_def_decode</pre></div>
</div></div>
</div>
</div>
</div>
</blockquote>
<br>
<br>-- <br>Ilya Kosarev<br></BODY></HTML>