From: "n.pettik" <korablev@tarantool.org> To: tarantool-patches@freelists.org Cc: Imeev Mergen <imeevma@tarantool.org> Subject: [tarantool-patches] Re: [PATCH v4 2/8] sql: set SQL parser errors via diag_set() Date: Thu, 14 Mar 2019 22:26:16 +0300 [thread overview] Message-ID: <21543710-C2CB-4818-AE87-FA1BE3AEB0A4@tarantool.org> (raw) In-Reply-To: <3be89ad14633e9b03c01200ee3d1b3c6776fccc5.1552494059.git.imeevma@gmail.com> > On 13 Mar 2019, at 20:03, imeevma@tarantool.org wrote: > > 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”); This function doesn’t set mallocFailed flag. A lot of callers of this function don’d check its return value. So I guess this could result in installed diag error, but it would be ignored. Can we set here at least mallocFailed? > } > 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”); The same is here. > } > 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”); I would add to error message max possible length. > + 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”); What does it mean? AFAIR it is dead code (i.e. everything connected with “interrupt”). > + pParse->rc = SQL_TARANTOOL_ERROR; > break; > } > if (tokenType == TK_ILLEGAL) {
next prev parent reply other threads:[~2019-03-14 19:26 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 ` [tarantool-patches] [PATCH v4 2/8] sql: set SQL parser errors via diag_set() imeevma 2019-03-14 19:26 ` n.pettik [this message] 2019-03-14 19:36 ` [tarantool-patches] " 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=21543710-C2CB-4818-AE87-FA1BE3AEB0A4@tarantool.org \ --to=korablev@tarantool.org \ --cc=imeevma@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [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