From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 3C9D828FA9 for ; Mon, 11 Mar 2019 11:04:51 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id SsJ3jgTxpGvM for ; Mon, 11 Mar 2019 11:04:51 -0400 (EDT) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id B5D3428F8C for ; Mon, 11 Mar 2019 11:04:50 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v2 4/7] sql: refactor sql_name_from_token to set diag References: <97dde83dd0ad392cfe47a315ce7ad931cb750a4f.1551265819.git.kshcherbatov@tarantool.org> From: Kirill Shcherbatov Message-ID: <4bb2ebb3-dfc7-b95c-cd76-6992baa92128@tarantool.org> Date: Mon, 11 Mar 2019 18:04:48 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org, Vladislav Shpilevoy On 07.03.2019 20:34, Vladislav Shpilevoy wrote: > See 5 comments.First of all, I've split this patch on two patches. > 1. You already have the same assertion in sql_name_from_token. > Why do you need so many new equal assertions? Check other places > like build.c:1792 by yourself, please. I've dropped them all. > 2. Double check for zName == NULL. Fixed. > 3. You can make it shorter writing > > } else if (pValue != NULL) { > ... > } > > Instead of > > } else { > if (pValue != NULL) { > ... > } > } Ok. > 4. The last sentence is exactly what you wrote after @retvals. > Please, do not duplicate it. I've fixed it here. > 5. Sentences are started from capital letters. I will not write > that again. Please, find and fix other places. I've really looked over the patch for this. ========================================================= Refactored sqlIdListAppend routine as sql_id_list_append and reworked it to use diag_set in case of memory allocation error. This change is necessary because the sql_id_list_append body has a sqlNameFromToken call that will be changed in subsequent patches. This patch refers to a series of preparatory patches that provide the use of Tarantool errors in the call tree that includes sqlNormalizeName, since this call can later return errors. This patch is not self-sufficient, its sqlNameFromToken call remained to be non-Tarantool (for now). It means, that if sqlNameFromToken fails in sql_id_list_append there is no diag message created. Part of #3931 --- src/box/sql/build.c | 44 +++++++++++++++++++++----------------------- src/box/sql/expr.c | 2 +- src/box/sql/parse.y | 15 +++++++++++---- src/box/sql/sqlInt.h | 15 ++++++++++++++- 4 files changed, 47 insertions(+), 29 deletions(-) diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 04f65f165..d374acb47 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -2538,30 +2538,28 @@ sqlArrayAllocate(sql * db, /* Connection to notify of malloc failures */ return pArray; } -/* - * Append a new element to the given IdList. Create a new IdList if - * need be. - * - * A new IdList is returned, or NULL if malloc() fails. - */ -IdList * -sqlIdListAppend(sql * db, IdList * pList, Token * pToken) -{ - int i; - if (pList == 0) { - pList = sqlDbMallocZero(db, sizeof(IdList)); - if (pList == 0) - return 0; - } - pList->a = sqlArrayAllocate(db, - pList->a, - sizeof(pList->a[0]), &pList->nId, &i); - if (i < 0) { - sqlIdListDelete(db, pList); - return 0; +struct IdList * +sql_id_list_append(struct sql *db, struct IdList *list, + struct Token *name_token) +{ + if (list == NULL && + (list = sqlDbMallocZero(db, sizeof(IdList))) == NULL) { + diag_set(OutOfMemory, sizeof(IdList), "sqlDbMallocZero", + "list"); + return NULL; } - pList->a[i].zName = sqlNameFromToken(db, pToken); - return pList; + int i; + list->a = sqlArrayAllocate(db, list->a, sizeof(list->a[0]), + &list->nId, &i); + if (i < 0) + goto error; + list->a[i].zName = sqlNameFromToken(db, name_token); + if (list->a[i].zName == NULL) + goto error; + return list; +error: + sqlIdListDelete(db, list); + return NULL; } /* diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index a75f23756..44b6ce11f 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -1638,7 +1638,7 @@ sqlIdListDup(sql * db, IdList * p) return 0; } /* Note that because the size of the allocation for p->a[] is not - * necessarily a power of two, sqlIdListAppend() may not be called + * necessarily a power of two, sql_id_list_append() may not be called * on the duplicate created by this function. */ for (i = 0; i < p->nId; i++) { diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index 122fda2f0..8a8f1b82f 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -797,10 +797,17 @@ insert_cmd(A) ::= REPLACE. {A = ON_CONFLICT_ACTION_REPLACE;} idlist_opt(A) ::= . {A = 0;} idlist_opt(A) ::= LP idlist(X) RP. {A = X;} -idlist(A) ::= idlist(A) COMMA nm(Y). - {A = sqlIdListAppend(pParse->db,A,&Y);} -idlist(A) ::= nm(Y). - {A = sqlIdListAppend(pParse->db,0,&Y); /*A-overwrites-Y*/} +idlist(A) ::= idlist(A) COMMA nm(Y). { + A = sql_id_list_append(pParse->db,A,&Y); + if (A == NULL) + sql_parser_error(pParse); +} +idlist(A) ::= nm(Y). { + /* A-overwrites-Y. */ + A = sql_id_list_append(pParse->db,0,&Y); + if (A == NULL) + sql_parser_error(pParse); +} /////////////////////////// Expression Processing ///////////////////////////// // diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 2df1830d6..56ae90a95 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -3388,7 +3388,20 @@ sql_drop_table(struct Parse *, struct SrcList *, bool, bool); void sqlInsert(Parse *, SrcList *, Select *, IdList *, enum on_conflict_action); void *sqlArrayAllocate(sql *, void *, int, int *, int *); -IdList *sqlIdListAppend(sql *, IdList *, Token *); + +/** + * Append a new element to the given IdList. Create a new IdList + * if need be. + * + * @param db The database connection. + * @param list The pointer to existent Id list if exists. + * @param name_token The token containing name. + * @retval Not NULL IdList pointer is returned on success. + * @retval NULL otherwise. Diag message is set. + */ +struct IdList * +sql_id_list_append(struct sql *db, struct IdList *pList, struct Token *pToken); + int sqlIdListIndex(IdList *, const char *); /** -- 2.21.0 =========================================================== 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. Part of #3931 --- src/box/sql/alter.c | 7 +- src/box/sql/analyze.c | 47 +++++----- src/box/sql/build.c | 206 +++++++++++++++++++++++------------------- src/box/sql/pragma.c | 24 +++-- src/box/sql/sqlInt.h | 21 ++++- src/box/sql/trigger.c | 4 +- 6 files changed, 183 insertions(+), 126 deletions(-) diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c index d49ebb8df..3a9e9095e 100644 --- a/src/box/sql/alter.c +++ b/src/box/sql/alter.c @@ -43,9 +43,9 @@ sql_alter_table_rename(struct Parse *parse, struct SrcList *src_tab, { assert(src_tab->nSrc == 1); struct sql *db = parse->db; - char *new_name = sqlNameFromToken(db, new_name_tk); + char *new_name = sql_name_from_token(db, new_name_tk); if (new_name == NULL) - goto exit_rename_table; + goto tnt_error; /* Check that new name isn't occupied by another table. */ if (space_by_name(new_name) != NULL) { diag_set(ClientError, ER_SPACE_EXISTS, new_name); @@ -72,8 +72,7 @@ exit_rename_table: return; tnt_error: sqlDbFree(db, new_name); - parse->rc = SQL_TARANTOOL_ERROR; - parse->nErr++; + sql_parser_error(parse); goto exit_rename_table; } diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c index 8c83288e6..cf5af7a8b 100644 --- a/src/box/sql/analyze.c +++ b/src/box/sql/analyze.c @@ -1111,34 +1111,39 @@ vdbe_emit_analyze_table(struct Parse *parse, struct space *space) void sqlAnalyze(Parse * pParse, Token * pName) { - sql *db = pParse->db; + char *name = NULL; + struct sql *db = pParse->db; + struct Vdbe *v = sqlGetVdbe(pParse); + if (v == NULL) + return; if (pName == NULL) { /* Form 1: Analyze everything */ sql_analyze_database(pParse); } else { /* Form 2: Analyze table named */ - char *z = sqlNameFromToken(db, pName); - if (z != NULL) { - struct space *sp = space_by_name(z); - if (sp != NULL) { - if (sp->def->opts.is_view) { - sqlErrorMsg(pParse, "VIEW isn't "\ - "allowed to be "\ - "analyzed"); - } else { - vdbe_emit_analyze_table(pParse, sp); - } - } else { - diag_set(ClientError, ER_NO_SUCH_SPACE, z); - pParse->rc = SQL_TARANTOOL_ERROR; - pParse->nErr++; - } - sqlDbFree(db, z); + name = sql_name_from_token(db, pName); + if (name == NULL) + goto tnt_error; + struct space *sp = space_by_name(name); + if (sp == NULL) { + diag_set(ClientError, ER_NO_SUCH_SPACE, name); + goto tnt_error; + } + if (sp->def->opts.is_view) { + sqlErrorMsg(pParse, + "VIEW isn't allowed to be analyzed"); + goto cleanup; + } else { + vdbe_emit_analyze_table(pParse, sp); } } - Vdbe *v = sqlGetVdbe(pParse); - if (v != NULL) - sqlVdbeAddOp0(v, OP_Expire); + sqlVdbeAddOp0(v, OP_Expire); +cleanup: + sqlDbFree(db, name); + return; +tnt_error: + sql_parser_error(pParse); + goto cleanup; } ssize_t 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 @@ -232,30 +232,18 @@ sql_space_column_is_in_pk(struct space *space, uint32_t column) return false; } -/* - * Given a token, return a string that consists of the text of that - * token. Space to hold the returned string - * is obtained from sqlMalloc() and must be freed by the calling - * function. - * - * Any quotation marks (ex: "name", 'name', [name], or `name`) that - * surround the body of the token are removed. - * - * Tokens are often just pointers into the original SQL text and so - * are not \000 terminated and are not persistent. The returned string - * is \000 terminated and is persistent. - */ char * -sqlNameFromToken(sql * db, Token * pName) +sql_name_from_token(struct sql *db, struct Token *name_token) { - char *zName; - if (pName) { - zName = sqlDbStrNDup(db, (char *)pName->z, pName->n); - sqlNormalizeName(zName); - } else { - zName = 0; + assert(name_token != NULL && name_token->z != NULL); + char *name = sqlDbStrNDup(db, (char *)name_token->z, name_token->n); + if (name == NULL) { + diag_set(OutOfMemory, name_token->n + 1, "sqlDbStrNDup", + "name"); + return NULL; } - return zName; + sqlNormalizeName(name); + return name; } /* @@ -332,11 +320,11 @@ sqlStartTable(Parse *pParse, Token *pName, int noErr) goto cleanup; sqlVdbeCountChanges(v); - zName = sqlNameFromToken(db, pName); + zName = sql_name_from_token(db, pName); + if (zName == NULL) + goto tnt_error; pParse->sNameToken = *pName; - if (zName == 0) - return; if (sqlCheckIdentifierName(pParse, zName) != SQL_OK) goto cleanup; @@ -344,10 +332,9 @@ sqlStartTable(Parse *pParse, Token *pName, int noErr) if (space != NULL) { if (!noErr) { diag_set(ClientError, ER_SPACE_EXISTS, zName); - sql_parser_error(pParse); - } else { - assert(!db->init.busy || CORRUPT_DB); + goto tnt_error; } + assert(!db->init.busy || CORRUPT_DB); goto cleanup; } @@ -367,6 +354,9 @@ sqlStartTable(Parse *pParse, Token *pName, int noErr) cleanup: sqlDbFree(db, zName); return; +tnt_error: + sql_parser_error(pParse); + goto cleanup; } /** @@ -708,11 +698,13 @@ sqlAddCollateType(Parse * pParse, Token * pToken) struct space *space = pParse->new_space; uint32_t i = space->def->field_count - 1; sql *db = pParse->db; - char *zColl = sqlNameFromToken(db, pToken); - if (!zColl) + char *coll_name = sql_name_from_token(db, pToken); + if (coll_name == NULL) { + sql_parser_error(pParse); return; + } uint32_t *coll_id = &space->def->fields[i].coll_id; - if (sql_get_coll_seq(pParse, zColl, coll_id) != NULL) { + if (sql_get_coll_seq(pParse, coll_name, coll_id) != NULL) { /* If the column is declared as " PRIMARY KEY COLLATE ", * then an index may have been created on this column before the * collation type was added. Correct this if it is the case. @@ -726,7 +718,7 @@ sqlAddCollateType(Parse * pParse, Token * pToken) } } } - sqlDbFree(db, zColl); + sqlDbFree(db, coll_name); } struct coll * @@ -1749,10 +1741,9 @@ sql_create_foreign_key(struct Parse *parse_context, struct SrcList *child, memset(fk_parse, 0, sizeof(*fk_parse)); rlist_add_entry(&parse_context->new_fk_constraint, fk_parse, link); } - assert(parent != NULL); - parent_name = sqlNameFromToken(db, parent); + parent_name = sql_name_from_token(db, parent); if (parent_name == NULL) - goto exit_create_fk; + goto tnt_error; /* * Within ALTER TABLE ADD CONSTRAINT FK also can be * self-referenced, but in this case parent (which is @@ -1785,15 +1776,19 @@ sql_create_foreign_key(struct Parse *parse_context, struct SrcList *child, sqlMPrintf(db, "FK_CONSTRAINT_%d_%s", ++parse_context->fk_constraint_count, space->def->name); + if (constraint_name == NULL) + goto exit_create_fk; } else { struct Token *cnstr_nm = &parse_context->constraintName; - constraint_name = sqlNameFromToken(db, cnstr_nm); + constraint_name = sql_name_from_token(db, cnstr_nm); + if (constraint_name == NULL) + goto tnt_error; } } else { - constraint_name = sqlNameFromToken(db, constraint); + constraint_name = sql_name_from_token(db, constraint); + if (constraint_name == NULL) + goto tnt_error; } - if (constraint_name == NULL) - goto exit_create_fk; const char *error_msg = "number of columns in foreign key does not " "match the number of columns in the primary " "index of referenced table"; @@ -1922,15 +1917,17 @@ sql_drop_foreign_key(struct Parse *parse_context, struct SrcList *table, struct space *child = space_by_name(table_name); if (child == NULL) { diag_set(ClientError, ER_NO_SUCH_SPACE, table_name); - parse_context->rc = SQL_TARANTOOL_ERROR; - parse_context->nErr++; + sql_parser_error(parse_context); + return; + } + char *constraint_name = + sql_name_from_token(parse_context->db, constraint); + if (constraint_name == NULL) { + sql_parser_error(parse_context); return; } - char *constraint_name = sqlNameFromToken(parse_context->db, - constraint); - if (constraint_name != NULL) - vdbe_emit_fk_constraint_drop(parse_context, constraint_name, - child->def->id); + vdbe_emit_fk_constraint_drop(parse_context, constraint_name, + child->def->id); /* * We account changes to row count only if drop of * foreign keys take place in a separate @@ -2192,9 +2189,9 @@ sql_create_index(struct Parse *parse, struct Token *token, */ if (token != NULL) { assert(token->z != NULL); - name = sqlNameFromToken(db, token); + name = sql_name_from_token(db, token); if (name == NULL) - goto exit_create_index; + goto tnt_error; if (sql_space_index_by_name(space, name) != NULL) { if (!if_not_exist) { diag_set(ClientError, ER_INDEX_EXISTS_IN_SPACE, @@ -2205,10 +2202,13 @@ sql_create_index(struct Parse *parse, struct Token *token, } } else { char *constraint_name = NULL; - if (parse->constraintName.z != NULL) + if (parse->constraintName.z != NULL) { constraint_name = - sqlNameFromToken(db, - &parse->constraintName); + sql_name_from_token(db, + &parse->constraintName); + if (constraint_name == NULL) + goto tnt_error; + } /* * This naming is temporary. Now it's not @@ -2246,9 +2246,7 @@ sql_create_index(struct Parse *parse, struct Token *token, if (tbl_name != NULL && space_is_system(space)) { diag_set(ClientError, ER_MODIFY_INDEX, name, def->name, "can't create index on system space"); - parse->nErr++; - parse->rc = SQL_TARANTOOL_ERROR; - goto exit_create_index; + goto tnt_error; } /* @@ -2275,9 +2273,7 @@ sql_create_index(struct Parse *parse, struct Token *token, index = (struct index *) region_alloc(&parse->region, sizeof(*index)); if (index == NULL) { diag_set(OutOfMemory, sizeof(*index), "region", "index"); - parse->rc = SQL_TARANTOOL_ERROR; - parse->nErr++; - goto exit_create_index; + goto tnt_error; } memset(index, 0, sizeof(*index)); @@ -2436,6 +2432,10 @@ sql_create_index(struct Parse *parse, struct Token *token, sql_expr_list_delete(db, col_list); sqlSrcListDelete(db, tbl_name); sqlDbFree(db, name); + return; +tnt_error: + sql_parser_error(parse); + goto exit_create_index; } void @@ -2447,32 +2447,28 @@ sql_drop_index(struct Parse *parse_context, struct SrcList *index_name_list, struct sql *db = parse_context->db; /* Never called with prior errors. */ assert(parse_context->nErr == 0); - assert(table_token != NULL); - const char *table_name = sqlNameFromToken(db, table_token); - if (db->mallocFailed) { - goto exit_drop_index; - } + const char *table_name = sql_name_from_token(db, table_token); + if (table_name == NULL) + goto tnt_error; sqlVdbeCountChanges(v); assert(index_name_list->nSrc == 1); assert(table_token->n > 0); struct space *space = space_by_name(table_name); if (space == NULL) { - if (!if_exists) { - diag_set(ClientError, ER_NO_SUCH_SPACE, table_name); - sql_parser_error(parse_context); - } - goto exit_drop_index; + if (if_exists) + goto exit_drop_index; + diag_set(ClientError, ER_NO_SUCH_SPACE, table_name); + goto tnt_error; } const char *index_name = index_name_list->a[0].zName; uint32_t index_id = box_index_id_by_name(space->def->id, index_name, strlen(index_name)); if (index_id == BOX_ID_NIL) { - if (!if_exists) { - diag_set(ClientError, ER_NO_SUCH_INDEX_NAME, - index_name, table_name); - sql_parser_error(parse_context); - } - goto exit_drop_index; + if (if_exists) + goto exit_drop_index; + diag_set(ClientError, ER_NO_SUCH_INDEX_NAME, index_name, + table_name); + goto tnt_error; } struct index *index = space_index(space, index_id); assert(index != NULL); @@ -2493,6 +2489,10 @@ sql_drop_index(struct Parse *parse_context, struct SrcList *index_name_list, exit_drop_index: sqlSrcListDelete(db, index_name_list); sqlDbFree(db, (void *) table_name); + return; +tnt_error: + sql_parser_error(parse_context); + goto exit_drop_index; } /* @@ -2553,7 +2553,7 @@ sql_id_list_append(struct sql *db, struct IdList *list, &list->nId, &i); if (i < 0) goto error; - list->a[i].zName = sqlNameFromToken(db, name_token); + list->a[i].zName = sql_name_from_token(db, name_token); if (list->a[i].zName == NULL) goto error; return list; @@ -2668,7 +2668,13 @@ sql_src_list_append(struct sql *db, struct SrcList *list, list = new_list; } struct SrcList_item *item = &list->a[list->nSrc - 1]; - item->zName = sqlNameFromToken(db, name_token); + if (name_token != NULL) { + item->zName = sql_name_from_token(db, name_token); + if (item->zName == NULL) { + sqlSrcListDelete(db, list); + return NULL; + } + } return list; } @@ -2769,8 +2775,12 @@ sqlSrcListAppendFromTerm(Parse * pParse, /* Parsing context */ assert(p->nSrc != 0); pItem = &p->a[p->nSrc - 1]; assert(pAlias != 0); - if (pAlias->n) { - pItem->zAlias = sqlNameFromToken(db, pAlias); + if (pAlias->n != 0) { + pItem->zAlias = sql_name_from_token(db, pAlias); + if (pItem->zAlias == NULL) { + sql_parser_error(pParse); + goto append_from_error; + } } pItem->pSelect = pSubquery; pItem->pOn = pOn; @@ -2805,8 +2815,15 @@ sqlSrcListIndexedBy(Parse * pParse, SrcList * p, Token * pIndexedBy) */ pItem->fg.notIndexed = 1; } else { - pItem->u1.zIndexedBy = - sqlNameFromToken(pParse->db, pIndexedBy); + if (pIndexedBy->z != NULL) { + pItem->u1.zIndexedBy = + sql_name_from_token(pParse->db, + pIndexedBy); + if (pItem->u1.zIndexedBy == NULL) { + sql_parser_error(pParse); + return; + } + } pItem->fg.isIndexedBy = (pItem->u1.zIndexedBy != 0); } } @@ -2892,11 +2909,12 @@ sql_transaction_rollback(Parse *pParse) void sqlSavepoint(Parse * pParse, int op, Token * pName) { - char *zName = sqlNameFromToken(pParse->db, pName); + struct sql *db = pParse->db; + char *zName = sql_name_from_token(db, pName); if (zName) { Vdbe *v = sqlGetVdbe(pParse); if (!v) { - sqlDbFree(pParse->db, zName); + sqlDbFree(db, zName); return; } if (op == SAVEPOINT_BEGIN && @@ -2906,6 +2924,8 @@ sqlSavepoint(Parse * pParse, int op, Token * pName) return; } sqlVdbeAddOp4(v, OP_Savepoint, op, 0, 0, zName, P4_DYNAMIC); + } else { + sql_parser_error(pParse); } } @@ -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; + } + 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); } } } @@ -3005,17 +3029,17 @@ sqlWithAdd(Parse * pParse, /* Parsing context */ } else { pNew = sqlDbMallocZero(db, sizeof(*pWith)); } - assert((pNew != 0 && zName != 0) || db->mallocFailed); + assert((pNew != NULL && name != NULL) || db->mallocFailed); if (db->mallocFailed) { sql_expr_list_delete(db, pArglist); sql_select_delete(db, pQuery); - sqlDbFree(db, zName); + sqlDbFree(db, name); pNew = pWith; } else { pNew->a[pNew->nCte].pSelect = pQuery; pNew->a[pNew->nCte].pCols = pArglist; - pNew->a[pNew->nCte].zName = zName; + pNew->a[pNew->nCte].zName = name; pNew->a[pNew->nCte].zCteErr = 0; pNew->nCte++; } diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c index 8b909f2e1..97b716d47 100644 --- a/src/box/sql/pragma.c +++ b/src/box/sql/pragma.c @@ -423,19 +423,25 @@ sqlPragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */ sqlVdbeRunOnlyOnce(v); pParse->nMem = 2; - zLeft = sqlNameFromToken(db, pId); - if (!zLeft) { + if (pId == NULL) { vdbe_emit_pragma_status(pParse); return; } - + zLeft = sql_name_from_token(db, pId); + if (zLeft == NULL) + goto tnt_error; if (minusFlag) { zRight = sqlMPrintf(db, "-%T", pValue); - } else { - zRight = sqlNameFromToken(db, pValue); + } else if (pValue != NULL) { + zRight = sql_name_from_token(db, pValue); + if (zRight == NULL) + goto tnt_error; + } + if (pValue2 != NULL) { + zTable = sql_name_from_token(db, pValue2); + if (zTable == NULL) + goto tnt_error; } - zTable = sqlNameFromToken(db, pValue2); - /* Locate the pragma in the lookup table */ pPragma = pragmaLocate(zLeft); if (pPragma == 0) { @@ -615,4 +621,8 @@ sqlPragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */ sqlDbFree(db, zLeft); sqlDbFree(db, zRight); sqlDbFree(db, zTable); + return; +tnt_error: + sql_parser_error(pParse); + goto pragma_out; } diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 56ae90a95..b49229b26 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -3644,7 +3644,26 @@ int sqlExprCodeExprList(Parse *, ExprList *, int, int, u8); void sqlExprIfTrue(Parse *, Expr *, int, int); void sqlExprIfFalse(Parse *, Expr *, int, int); -char *sqlNameFromToken(sql *, Token *); +/** + * Given a token, return a string that consists of the text of + * that token. Space to hold the returned string is obtained + * from sqlMalloc() and must be freed by the calling function. + * + * Any quotation marks (ex: "name", 'name', [name], or `name`) + * that surround the body of the token are removed. + * + * Tokens are often just pointers into the original SQL text and + * so are not \000 terminated and are not persistent. The returned + * string is \000 terminated and is persistent. + * + * @param db The database connection. + * @param name_token The source token with text. + * @retval Not NULL string pointer on success. + * @retval NULL Otherwise. Diag message is set. + */ +char * +sql_name_from_token(struct sql *db, struct Token *name_token); + int sqlExprCompare(Expr *, Expr *, int); int sqlExprListCompare(ExprList *, ExprList *, int); int sqlExprImpliesExpr(Expr *, Expr *, int); diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c index 20d41e6a1..65218121b 100644 --- a/src/box/sql/trigger.c +++ b/src/box/sql/trigger.c @@ -82,9 +82,9 @@ sql_trigger_begin(struct Parse *parse, struct Token *name, int tr_tm, goto trigger_cleanup; assert(table->nSrc == 1); - trigger_name = sqlNameFromToken(db, name); + trigger_name = sql_name_from_token(db, name); if (trigger_name == NULL) - goto trigger_cleanup; + goto set_tarantool_error_and_cleanup; if (sqlCheckIdentifierName(parse, trigger_name) != SQL_OK) goto trigger_cleanup; -- 2.21.0