From: Kirill Shcherbatov <kshcherbatov@tarantool.org> To: tarantool-patches@freelists.org, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Subject: [tarantool-patches] Re: [PATCH v2 3/7] sql: refactor sql_src_list_append to set diag Date: Wed, 20 Mar 2019 14:02:47 +0300 [thread overview] Message-ID: <a210dfb1-e718-cb53-5481-81d124ecd42f@tarantool.org> (raw) In-Reply-To: <2c6214c0-dc40-bb33-283b-781234ad3360@tarantool.org> > 1. Stray empty line. Please, do self-review before sending a patch. > Such a stray change says that you did not scan the patch before a > send. Fixed. > 2. This should be said in @retval sections. > >> + * The returned SrcList might be the same as the list that was >> + * input or it might be a new one. If an OOM error does occurs, >> + * then the prior value of list that is input to this routine is >> + * automatically freed. >> + * >> + * @param db The database connection. >> + * @param list Append to this SrcList. NULL creates a new SrcList. >> + * @param name_token Token representing table name. >> + * @retval Not NULL SrcList pointer on success. >> + * @retval NULL Otherwise. The diag message is set. >> + */ >> +struct SrcList * >> +sql_src_list_append(struct sql *db, struct SrcList *list, >> + struct Token *name_token); /** * Append a new table name to the given list. Create a new * SrcList if need be. A new entry is created in the list even * if name_token is NULL. * * @param db The database connection. * @param list Append to this SrcList. NULL creates a new SrcList. * @param name_token Token representing table name. * @retval Not NULL SrcList pointer is returned. The returned * SrcList might be the same as the list that was input * or it might be a new one. * @retval NULL Otherwise. The diag message is set. The prior * value of list that is input to this routine is * automatically freed. */ =================================================== 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 | 8 +++++--- src/box/sql/parse.y | 19 +++++++++++++++---- src/box/sql/sqlInt.h | 16 +++++++++++++++- 4 files changed, 56 insertions(+), 31 deletions(-) diff --git a/src/box/sql/build.c b/src/box/sql/build.c index ef38503da..dae582d1f 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -2516,30 +2516,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(struct IdList))) == NULL) { + diag_set(OutOfMemory, sizeof(struct 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 a2c70935e..0457ff833 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -1658,9 +1658,11 @@ sqlIdListDup(sql * db, IdList * p) sqlDbFree(db, pNew); 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 - * on the duplicate created by this function. + /* + * Note that because the size of the allocation for p->a[] + * is not 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++) { struct IdList_item *pNewItem = &pNew->a[i]; diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index ead71dfc0..daeb6e84b 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -804,10 +804,21 @@ 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) { + pParse->is_aborted = true; + return; + } +} +idlist(A) ::= nm(Y). { + /* A-overwrites-Y. */ + A = sql_id_list_append(pParse->db,0,&Y); + if (A == NULL) { + pParse->is_aborted = true; + return; + } +} /////////////////////////// Expression Processing ///////////////////////////// // diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index b6c89893a..8f56f3e63 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -3411,7 +3411,21 @@ 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 *list, + struct Token *name_token); + int sqlIdListIndex(IdList *, const char *); /** -- 2.21.0
next prev parent reply other threads:[~2019-03-20 11:02 UTC|newest] Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-02-27 11:13 [tarantool-patches] [PATCH v2 0/7] sql: store regular identifiers in case-normal form Kirill Shcherbatov 2019-02-27 11:13 ` [tarantool-patches] [PATCH v2 1/7] sql: refactor sql_alloc_src_list to set diag Kirill Shcherbatov 2019-03-07 17:34 ` [tarantool-patches] " Vladislav Shpilevoy 2019-03-11 15:04 ` Kirill Shcherbatov 2019-02-27 11:13 ` [tarantool-patches] [PATCH v2 2/7] sql: rework sql_src_list_enlarge " Kirill Shcherbatov 2019-03-07 17:34 ` [tarantool-patches] " Vladislav Shpilevoy 2019-03-11 15:04 ` Kirill Shcherbatov 2019-02-27 11:13 ` [tarantool-patches] [PATCH v2 3/7] sql: refactor sql_src_list_append " Kirill Shcherbatov 2019-03-07 17:34 ` [tarantool-patches] " Vladislav Shpilevoy 2019-03-11 15:04 ` Kirill Shcherbatov 2019-03-18 19:33 ` Vladislav Shpilevoy 2019-03-20 11:02 ` Kirill Shcherbatov [this message] 2019-03-26 17:08 ` Vladislav Shpilevoy 2019-03-26 18:07 ` Vladislav Shpilevoy 2019-02-27 11:13 ` [tarantool-patches] [PATCH v2 4/7] sql: refactor sql_name_from_token " Kirill Shcherbatov 2019-03-07 17:34 ` [tarantool-patches] " Vladislav Shpilevoy 2019-03-11 15:04 ` Kirill Shcherbatov 2019-03-18 19:33 ` Vladislav Shpilevoy 2019-03-20 11:02 ` Kirill Shcherbatov 2019-02-27 11:13 ` [tarantool-patches] [PATCH v2 5/7] sql: refactor sql_trigger_step_allocate " Kirill Shcherbatov 2019-03-07 17:34 ` [tarantool-patches] " Vladislav Shpilevoy 2019-03-11 15:04 ` Kirill Shcherbatov 2019-03-18 19:33 ` Vladislav Shpilevoy 2019-03-20 11:02 ` Kirill Shcherbatov 2019-03-26 17:08 ` Vladislav Shpilevoy 2019-02-27 11:13 ` [tarantool-patches] [PATCH v2 6/7] sql: refactor sql_expr_create " Kirill Shcherbatov 2019-03-07 17:34 ` [tarantool-patches] " Vladislav Shpilevoy 2019-03-11 15:04 ` Kirill Shcherbatov 2019-03-18 19:33 ` Vladislav Shpilevoy 2019-03-20 11:02 ` Kirill Shcherbatov 2019-03-26 17:08 ` Vladislav Shpilevoy 2019-02-27 11:13 ` [tarantool-patches] [PATCH v2 7/7] sql: store regular identifiers in case-normal form Kirill Shcherbatov 2019-03-07 17:34 ` [tarantool-patches] " Vladislav Shpilevoy 2019-03-11 15:04 ` Kirill Shcherbatov 2019-03-18 19:33 ` Vladislav Shpilevoy 2019-03-20 11:02 ` Kirill Shcherbatov 2019-03-26 17:08 ` Vladislav Shpilevoy 2019-03-18 19:33 ` [tarantool-patches] Re: [PATCH v2 0/7] " Vladislav Shpilevoy 2019-03-20 11:02 ` Kirill Shcherbatov 2019-03-26 17:09 ` Vladislav Shpilevoy 2019-03-27 14:06 ` 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=a210dfb1-e718-cb53-5481-81d124ecd42f@tarantool.org \ --to=kshcherbatov@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=v.shpilevoy@tarantool.org \ --subject='[tarantool-patches] Re: [PATCH v2 3/7] sql: refactor sql_src_list_append to set diag' \ /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