[tarantool-patches] Re: [PATCH v1 1/1] sql: do not replace the error with a syntax error

["n.pettik" <korablev@tarantool.org>]

> On 7 May 2019, at 19:03, imeevma@tarantool.org wrote:
> 
> Currently, it is possible to set a syntax error, even if there has
> already been another error before.
> 
> For example:
> box.execute("insert into not_exist values(1) a")
> 
> The first error is "Space 'NOT_EXIST' does not exist", but "Syntax
> error near 'a'" is displayed.
> 
> After this patch, all syntax errors will be set only if there have
> been no errors before.
> 
> Closes #3964

This patch doesn’t resolve issue completely (at least what issue’s
topic says): we just ignore further errors and continue parsing process.

> ---
> https://github.com/tarantool/tarantool/issues/3964
> https://github.com/tarantool/tarantool/tree/imeevma/gh-3964-stop-parser-on-error
> 
> src/box/sql/parse.y              | 12 ++++++++++++
> test/sql-tap/e_select1.test.lua  |  2 +-
> test/sql-tap/misc1.test.lua      |  2 +-
> test/sql-tap/sql-errors.test.lua | 12 +++++++++++-
> test/sql-tap/view.test.lua       |  2 +-
> 5 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
> index 3a443a0..ac3cce0 100644
> --- a/src/box/sql/parse.y
> +++ b/src/box/sql/parse.y
> @@ -32,6 +32,18 @@
> %syntax_error {
>   UNUSED_PARAMETER(yymajor);  /* Silence some compiler warnings */
>   assert( TOKEN.z[0] );  /* The tokenizer always gives us a token */
> +  /*
> +   * Do nothing if there was already an error. Before this patch,
> +   * a syntax error can replace an already set error when the
> +   * parser rule is violated.
> +   * For example:
> +   * INSERT INSERT not_exist VALUES(1) a
> +   *
> +   * In this case space NOT_EXIST do not exist, but this error has
> +   * been replaced by a syntax error.
> +   */
> +  if (pParse->is_aborted)

Let’s add assertion:

-  if (pParse->is_aborted)
+  if (pParse->is_aborted) {
+    assert(! diag_is_empty(diag_get()));
     return;
+  }

> +    return;

A bit re-phrased your comment:

/*
 * Do nothing if an error has been already set. Since we don't
 * stop parsing process, syntax error can replace an already
 * set error when the parser's rule is violated.
 * For example:
 * INSERT INSERT not_exist VALUES(1) a;
 *
 * In this case space NOT_EXIST doesn't exist, but this error
 * will be replaced by a error 'near "''a''": syntax error'
 */

diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index ac3cce0ce..44d002b4b 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -33,14 +33,14 @@
   UNUSED_PARAMETER(yymajor);  /* Silence some compiler warnings */
   assert( TOKEN.z[0] );  /* The tokenizer always gives us a token */
   /*
-   * Do nothing if there was already an error. Before this patch,
-   * a syntax error can replace an already set error when the
-   * parser rule is violated.
+   * Do nothing if an error has been already set. Since we don't
+   * stop parsing process, syntax error can replace an already
+   * set error when the parser's rule is violated.
    * For example:
-   * INSERT INSERT not_exist VALUES(1) a
+   * INSERT INSERT not_exist VALUES(1) a;
    *
-   * In this case space NOT_EXIST do not exist, but this error has
-   * been replaced by a syntax error.
+   * In this case space NOT_EXIST doesn't exist, but this error
+   * will be replaced by a error 'near "''a''": syntax error'
    */
   if (pParse->is_aborted)