[tarantool-patches] Re: [PATCH v2 4/7] sql: refactor sql_name_from_token to set diag
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Mon Mar 18 22:33:57 MSK 2019
The diff in your email has nothing to do with sql_name_from_token.
You've not sent a new version of that patch, so I did it below
with my 2 comments inlined.
> commit b052b1b0a43159ac79320d5dca43a201fffa6ab9
> Author: Kirill Shcherbatov <kshcherbatov at tarantool.org>
> Date: Wed Feb 13 15:15:22 2019 +0300
>
> sql: rework sqlNameFromToken to set diag
>
> Refactored sqlNameFromToken routine as sql_name_from_token and
> reworked it to use diag_set in case of memory allocation error.
> This change is necessary because the sql_name_from_token body has
> a sqlNameFromToken call that will be changed in subsequent
> patches.
1. Now, reread that paragraph and fix what is wrong. You do not call
sqlNameFromToken from sql_name_from_token.
>
> Part of #3931
>
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index d374acb47..1864bd0ad 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -2981,19 +3001,23 @@ sqlWithAdd(Parse * pParse, /* Parsing context */
> {
> sql *db = pParse->db;
> With *pNew;
> - char *zName;
>
> - /* Check that the CTE name is unique within this WITH clause. If
> - * not, store an error in the Parse structure.
> + /*
> + * Check that the CTE name is unique within this WITH
> + * clause. If not, store an error in the Parse structure.
> */
> - zName = sqlNameFromToken(pParse->db, pName);
> - if (zName && pWith) {
> + char *name = sql_name_from_token(db, pName);
> + if (name == NULL) {
> + sql_parser_error(pParse);
> + return NULL;
2. Leak. sqlWithAdd should delete some structures in a case of OOM.
Lines 3035 - 3038. Also, that function does not return NULL on an
error - it should return the old value. Otherwise you have a second
leak in parse.y:1494.
> + }
> + if (pWith != NULL) {
> int i;
> for (i = 0; i < pWith->nCte; i++) {
> - if (strcmp(zName, pWith->a[i].zName) == 0) {
> + if (strcmp(name, pWith->a[i].zName) == 0) {
> sqlErrorMsg(pParse,
> - "duplicate WITH table name: %s",
> - zName);
> + "duplicate WITH table name: %s",
> + name);
> }
> }
> }
More information about the Tarantool-patches
mailing list