Tarantool development patches archive
 help / color / mirror / Atom feed
From: imeevma@tarantool.org
To: korablev@tarantool.org
Cc: tarantool-patches@freelists.org
Subject: [tarantool-patches] [PATCH v4 2/8] sql: set SQL parser errors via diag_set()
Date: Wed, 13 Mar 2019 20:03:08 +0300	[thread overview]
Message-ID: <3be89ad14633e9b03c01200ee3d1b3c6776fccc5.1552494059.git.imeevma@gmail.com> (raw)
In-Reply-To: <cover.1552494059.git.imeevma@gmail.com>

Hi! Thank you for review! Diff between versions and new version of
patch below.

Diff between patches:

commit 61bc67e61298129d66a436d58957bb411b6c9b81
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Wed Mar 6 21:27:51 2019 +0300

    Temporary: Review fix

diff --git a/src/box/sql/malloc.c b/src/box/sql/malloc.c
index e0d2ec8..8812298 100644
--- a/src/box/sql/malloc.c
+++ b/src/box/sql/malloc.c
@@ -55,9 +55,7 @@ sql_sized_malloc(int nByte)
    p[0] = nByte;
    p++;
  } else {
-   testcase(sqlGlobalConfig.xLog != 0);
-   sql_log(SQL_NOMEM,
-         "failed to allocate %u bytes of memory", nByte);
+   diag_set(OutOfMemory, nByte, "realloc", "p");
  }
  return (void *)p;
 }
@@ -115,10 +113,7 @@ sql_sized_realloc(void *pPrior, int nByte)
    p[0] = nByte;
    p++;
  } else {
-   testcase(sqlGlobalConfig.xLog != 0);
-   sql_log(SQL_NOMEM,
-         "failed memory resize %u to %u bytes",
-         sql_sized_sizeof(pPrior), nByte);
+   diag_set(OutOfMemory, nByte, "malloc", "p");
  }
  return (void *)p;
 }
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index 0c6296d..828a1ae 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -102,9 +102,8 @@ sqlPrepare(sql * db,  /* Database handle. */
 
  if (sParse.rc == SQL_DONE)
    sParse.rc = SQL_OK;
- if (db->mallocFailed) {
-   sParse.rc = SQL_NOMEM;
- }
+ if (db->mallocFailed)
+   sParse.rc = SQL_TARANTOOL_ERROR;
  if (pzTail) {
    *pzTail = sParse.zTail;
  }
diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
index 58685c4..834c165 100644
--- a/src/box/sql/tokenize.c
+++ b/src/box/sql/tokenize.c
@@ -483,7 +483,9 @@ sqlRunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
              &pParse->sLastToken.isReserved);
      i += pParse->sLastToken.n;
      if (i > mxSqlLen) {
-       pParse->rc = SQL_TOOBIG;
+       diag_set(ClientError, ER_SQL_PARSER_GENERIC,
+          "string or blob too big");
+       pParse->rc = SQL_TARANTOOL_ERROR;
        break;
      }
    } else {
@@ -502,7 +504,9 @@ sqlRunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
      assert(tokenType == TK_SPACE
             || tokenType == TK_ILLEGAL);
      if (db->u1.isInterrupted) {
-       pParse->rc = SQL_INTERRUPT;
+       diag_set(ClientError, ER_SQL_PARSER_GENERIC,
+          "interrupted");
+       pParse->rc = SQL_TARANTOOL_ERROR;
        break;
      }
      if (tokenType == TK_ILLEGAL) {
@@ -529,7 +533,7 @@ sqlRunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
 #endif       /* YYDEBUG */
  sqlParserFree(pEngine, sql_free);
  if (db->mallocFailed) {
-   pParse->rc = SQL_NOMEM;
+   pParse->rc = SQL_TARANTOOL_ERROR;
  }
  if (pParse->rc != SQL_OK && pParse->rc != SQL_DONE
      && pParse->zErrMsg == 0) {
diff --git a/src/box/sql/util.c b/src/box/sql/util.c
index e4c93cb..a6d1f5c 100644
--- a/src/box/sql/util.c
+++ b/src/box/sql/util.c
@@ -211,7 +211,8 @@ sqlErrorWithMsg(sql * db, int err_code, const char *zFormat, ...)
 }
 
 /*
- * Add an error message to pParse->zErrMsg and increment pParse->nErr.
+ * Add an error to the diagnostics area, increment pParse->nErr
+ * and set pParse->rc.
  * The following formatting characters are allowed:
  *
  *      %s      Insert a string


New version:

commit 3be89ad14633e9b03c01200ee3d1b3c6776fccc5
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Sun Feb 24 12:02:49 2019 +0300

    sql: set SQL parser errors via diag_set()
    
    After this patch all SQL parser errors will be set via diag_set().
    They were saved in field zErrMsg of struct Parse before this
    patch.
    
    Part of #3965

diff --git a/src/box/errcode.h b/src/box/errcode.h
index f2f47c0..d234d26 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -238,6 +238,7 @@ struct errcode_record {
  /*183 */_(ER_SQL_KEYWORD_IS_RESERVED, "Keyword '%.*s' is reserved. Please use double quotes if '%.*s' is an identifier.") \
  /*184 */_(ER_SQL_UNRECOGNIZED_SYNTAX, "Syntax error near '%.*s'") \
  /*185 */_(ER_SQL_UNKNOWN_TOKEN,   "Syntax error: unrecognized token: '%.*s'") \
+ /*186 */_(ER_SQL_PARSER_GENERIC,  "%s") \
 
 /*
  * !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index f112c9f..deb5b89 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -194,7 +194,8 @@ sql_finish_coding(struct Parse *parse_context)
    sqlVdbeMakeReady(v, parse_context);
    parse_context->rc = SQL_DONE;
  } else {
-   parse_context->rc = SQL_ERROR;
+   if (parse_context->rc != SQL_TARANTOOL_ERROR)
+     parse_context->rc = SQL_ERROR;
  }
 }
 /**
diff --git a/src/box/sql/malloc.c b/src/box/sql/malloc.c
index e0d2ec8..8812298 100644
--- a/src/box/sql/malloc.c
+++ b/src/box/sql/malloc.c
@@ -55,9 +55,7 @@ sql_sized_malloc(int nByte)
    p[0] = nByte;
    p++;
  } else {
-   testcase(sqlGlobalConfig.xLog != 0);
-   sql_log(SQL_NOMEM,
-         "failed to allocate %u bytes of memory", nByte);
+   diag_set(OutOfMemory, nByte, "realloc", "p");
  }
  return (void *)p;
 }
@@ -115,10 +113,7 @@ sql_sized_realloc(void *pPrior, int nByte)
    p[0] = nByte;
    p++;
  } else {
-   testcase(sqlGlobalConfig.xLog != 0);
-   sql_log(SQL_NOMEM,
-         "failed memory resize %u to %u bytes",
-         sql_sized_sizeof(pPrior), nByte);
+   diag_set(OutOfMemory, nByte, "malloc", "p");
  }
  return (void *)p;
 }
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index 0c6296d..828a1ae 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -102,9 +102,8 @@ sqlPrepare(sql * db,  /* Database handle. */
 
  if (sParse.rc == SQL_DONE)
    sParse.rc = SQL_OK;
- if (db->mallocFailed) {
-   sParse.rc = SQL_NOMEM;
- }
+ if (db->mallocFailed)
+   sParse.rc = SQL_TARANTOOL_ERROR;
  if (pzTail) {
    *pzTail = sParse.zTail;
  }
diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
index 58685c4..834c165 100644
--- a/src/box/sql/tokenize.c
+++ b/src/box/sql/tokenize.c
@@ -483,7 +483,9 @@ sqlRunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
              &pParse->sLastToken.isReserved);
      i += pParse->sLastToken.n;
      if (i > mxSqlLen) {
-       pParse->rc = SQL_TOOBIG;
+       diag_set(ClientError, ER_SQL_PARSER_GENERIC,
+          "string or blob too big");
+       pParse->rc = SQL_TARANTOOL_ERROR;
        break;
      }
    } else {
@@ -502,7 +504,9 @@ sqlRunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
      assert(tokenType == TK_SPACE
             || tokenType == TK_ILLEGAL);
      if (db->u1.isInterrupted) {
-       pParse->rc = SQL_INTERRUPT;
+       diag_set(ClientError, ER_SQL_PARSER_GENERIC,
+          "interrupted");
+       pParse->rc = SQL_TARANTOOL_ERROR;
        break;
      }
      if (tokenType == TK_ILLEGAL) {
@@ -529,7 +533,7 @@ sqlRunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
 #endif       /* YYDEBUG */
  sqlParserFree(pEngine, sql_free);
  if (db->mallocFailed) {
-   pParse->rc = SQL_NOMEM;
+   pParse->rc = SQL_TARANTOOL_ERROR;
  }
  if (pParse->rc != SQL_OK && pParse->rc != SQL_DONE
      && pParse->zErrMsg == 0) {
diff --git a/src/box/sql/util.c b/src/box/sql/util.c
index c89e2e8..a6d1f5c 100644
--- a/src/box/sql/util.c
+++ b/src/box/sql/util.c
@@ -211,7 +211,8 @@ sqlErrorWithMsg(sql * db, int err_code, const char *zFormat, ...)
 }
 
 /*
- * Add an error message to pParse->zErrMsg and increment pParse->nErr.
+ * Add an error to the diagnostics area, increment pParse->nErr
+ * and set pParse->rc.
  * The following formatting characters are allowed:
  *
  *      %s      Insert a string
@@ -236,10 +237,10 @@ sqlErrorMsg(Parse * pParse, const char *zFormat, ...)
  va_start(ap, zFormat);
  zMsg = sqlVMPrintf(db, zFormat, ap);
  va_end(ap);
+ diag_set(ClientError, ER_SQL_PARSER_GENERIC, zMsg);
+ sqlDbFree(db, zMsg);
  pParse->nErr++;
- sqlDbFree(db, pParse->zErrMsg);
- pParse->zErrMsg = zMsg;
- pParse->rc = SQL_ERROR;
+ pParse->rc = SQL_TARANTOOL_ERROR;
 }
 
 void
diff --git a/test/box/misc.result b/test/box/misc.result
index 27579c6..9f0b2c7 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -514,6 +514,7 @@ t;
   182: box.error.SQL_STATEMENT_EMPTY
   184: box.error.SQL_UNRECOGNIZED_SYNTAX
   185: box.error.SQL_UNKNOWN_TOKEN
+  186: box.error.SQL_PARSER_GENERIC
 ...
 test_run:cmd("setopt delimiter ''");
 ---
diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua
index b6b7fc7..bab6493 100755
--- a/test/sql-tap/check.test.lua
+++ b/test/sql-tap/check.test.lua
@@ -319,7 +319,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <check-3.1>
-        1, "Failed to create space 'T3': SQL error: subqueries prohibited in CHECK constraints"
+        1, "Failed to create space 'T3': subqueries prohibited in CHECK constraints"
         -- </check-3.1>
     })
 
diff --git a/test/sql/triggers.result b/test/sql/triggers.result
index 2f5b148..826e998 100644
--- a/test/sql/triggers.result
+++ b/test/sql/triggers.result
@@ -398,14 +398,14 @@ space_id = box.space.T1.id
 ...
 box.sql.execute("CREATE TRIGGER tr1 AFTER INSERT ON t1 WHEN new.a = ? BEGIN SELECT 1; END;")
 ---
-- error: 'SQL error: bindings are not allowed in DDL'
+- error: bindings are not allowed in DDL
 ...
 tuple = {"TR1", space_id, {sql = [[CREATE TRIGGER tr1 AFTER INSERT ON t1 WHEN new.a = ? BEGIN SELECT 1; END;]]}}
 ---
 ...
 box.space._trigger:insert(tuple)
 ---
-- error: 'SQL error: bindings are not allowed in DDL'
+- error: bindings are not allowed in DDL
 ...
 box.sql.execute("DROP TABLE t1;")
 ---
-- 
2.7.4

  parent reply	other threads:[~2019-03-13 17:03 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-13 17:03 [tarantool-patches] [PATCH v4 0/8] sql: use diag_set() for errors in SQL imeevma
2019-03-13 17:03 ` [tarantool-patches] [PATCH v4 1/8] sql: rework syntax errors imeevma
2019-03-14 18:24   ` [tarantool-patches] " n.pettik
2019-03-14 18:28     ` Imeev Mergen
2019-03-15 14:09   ` Kirill Yukhin
2019-03-13 17:03 ` imeevma [this message]
2019-03-14 19:26   ` [tarantool-patches] Re: [PATCH v4 2/8] sql: set SQL parser errors via diag_set() n.pettik
2019-03-14 19:36     ` n.pettik
2019-03-18 15:06     ` Mergen Imeev
2019-03-19  9:41       ` n.pettik
2019-03-19 11:24   ` Kirill Yukhin
2019-03-13 17:03 ` [tarantool-patches] [PATCH v4 3/8] sql: replace rc with is_aborted status in struct Parse imeevma
2019-03-14 19:53   ` [tarantool-patches] " n.pettik
2019-03-18 15:28     ` Mergen Imeev
2019-03-19  9:54       ` n.pettik
2019-03-19 13:17   ` Kirill Yukhin
2019-03-13 17:03 ` [tarantool-patches] [PATCH v4 4/8] sql: remove field nErr from " imeevma
2019-03-14 19:58   ` [tarantool-patches] " n.pettik
2019-03-19 13:27   ` Kirill Yukhin
2019-03-13 17:03 ` [tarantool-patches] [PATCH v4 5/8] sql: remove field zErrMsg " imeevma
2019-03-14 22:15   ` [tarantool-patches] " n.pettik
2019-03-19 13:20   ` Kirill Yukhin
2019-03-13 17:03 ` [tarantool-patches] [PATCH v4 6/8] sql: rework three errors of "unsupported" type imeevma
2019-03-14 22:15   ` [tarantool-patches] " n.pettik
2019-03-19 13:30   ` Kirill Yukhin
2019-03-13 17:03 ` [tarantool-patches] [PATCH v4 7/8] sql: rework semantic errors imeevma
2019-03-15 15:49   ` [tarantool-patches] " n.pettik
2019-03-22 12:48     ` Mergen Imeev
2019-03-26 14:14       ` n.pettik
2019-03-26 16:56         ` Mergen Imeev
2019-03-26 18:16           ` n.pettik
2019-03-26 19:20             ` Mergen Imeev
2019-03-26 21:36               ` n.pettik
2019-03-27  6:48   ` Kirill Yukhin
2019-03-13 17:03 ` [tarantool-patches] [PATCH v4 8/8] sql: remove sqlErrorMsg() imeevma
2019-03-15 13:36   ` [tarantool-patches] " n.pettik
2019-03-25 18:47     ` Mergen Imeev
2019-03-26 13:34       ` n.pettik
2019-03-26 17:52         ` Mergen Imeev
2019-03-26 18:28           ` n.pettik
2019-03-26 19:21             ` Mergen Imeev
2019-03-26 21:36               ` n.pettik
2019-03-27  6:49   ` Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3be89ad14633e9b03c01200ee3d1b3c6776fccc5.1552494059.git.imeevma@gmail.com \
    --to=imeevma@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [PATCH v4 2/8] sql: set SQL parser errors via diag_set()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox