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 5527E24EEB for ; Thu, 24 May 2018 15:26:44 -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 NzbIdlvzPNVK for ; Thu, 24 May 2018 15:26:44 -0400 (EDT) Received: from smtp53.i.mail.ru (smtp53.i.mail.ru [94.100.177.113]) (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 D292324F23 for ; Thu, 24 May 2018 15:26:43 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v7 6/7] sql: export funcs defined on Expr, ExprList to sql.h References: <26d7bf8070e9e573eb61897c34bf027a52eb8d66.1527084287.git.kshcherbatov@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Thu, 24 May 2018 22:26:41 +0300 MIME-Version: 1.0 In-Reply-To: <26d7bf8070e9e573eb61897c34bf027a52eb8d66.1527084287.git.kshcherbatov@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed 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, Kirill Shcherbatov Thanks for the patch! See 17 comments below. On 23/05/2018 17:05, Kirill Shcherbatov wrote: > As we would like work with check in server, we need to export > - sqlite3ExprListAppend as sql_expr_list_append > - sqlite3ExprListDelete as sql_expr_list_delete > - sqlite3ExprListDup as sql_expr_list_dup. > > Part of #3272. > --- > src/box/sql.h | 57 ++++++++++++++++++++++++++++++ > src/box/sql/build.c | 39 +++++++++++---------- > src/box/sql/expr.c | 93 +++++++++++++++++++++---------------------------- > src/box/sql/fkey.c | 11 +++--- > src/box/sql/insert.c | 2 +- > src/box/sql/parse.y | 81 +++++++++++++++++++++--------------------- > src/box/sql/prepare.c | 2 +- > src/box/sql/resolve.c | 37 +++++++------------- > src/box/sql/select.c | 40 +++++++++++---------- > src/box/sql/sqliteInt.h | 4 --- > src/box/sql/trigger.c | 8 ++--- > src/box/sql/update.c | 2 +- > src/box/sql/wherecode.c | 12 +++---- > src/box/sql/whereexpr.c | 6 ++-- > 14 files changed, 214 insertions(+), 180 deletions(-) > > diff --git a/src/box/sql.h b/src/box/sql.h > index 20f0651..d031d9b 100644 > --- a/src/box/sql.h > +++ b/src/box/sql.h > @@ -178,6 +178,63 @@ int > sql_table_def_rebuild(struct sqlite3 *db, struct Table *table); > > /** > + * Duplicate Expr list. > + * The flags parameter contains a combination of the EXPRDUP_XXX flags. > + * If the EXPRDUP_REDUCE flag is set, then the structure returned is a > + * truncated version of the usual Expr structure that will be stored as > + * part of the in-memory representation of the database schema. > + * @param db The database connection. > + * @param p The ExprList to duplicate. > + * @param flags EXPRDUP_XXX flags. > + * @retval NULL on memory allocation error. > + * @retval not NULL on success. > + */ > +struct ExprList * > +sql_expr_list_dup(struct sqlite3 * db, struct ExprList * p, int flags); 1. Please, do not put extra white space after *. > + > +/** > + * Add a new element to the end of an expression list. If expr_list is > + * initially NULL, then create a new expression list. > + * > + * If a memory allocation error occurs, the entire list is freed and > + * NULL is returned. If non-NULL is returned, then it is guaranteed > + * that the new entry was successfully appended. > + * @param db SQL handle. > + * @param expr_list List to which to append. Might be NULL. > + * @param expr Expression to be appended. Might be NULL. > + * @retval NULL on memory allocation error. > + * @retval not NULL on success. > + */ > +struct ExprList * > +sql_expr_list_append(struct sqlite3 * db, struct ExprList *expr_list, > + struct Expr *expr); 2. Invalid indentation. > + > +/** > + * Resolve names in expressions that can only reference a single table: > + * * CHECK constraints > + * * WHERE clauses on partial indices > + * The Expr.iTable value for Expr.op==TK_COLUMN nodes of the expression > + * is set to -1 and the Expr.iColumn value is set to the column number. > + * Any errors cause an error message to be set in pParse. 3. No pParse. > + * @param parser Parsing context. > + * @param table The table being referenced. > + * @param type NC_IsCheck or NC_PartIdx or NC_IdxExpr. > + * @param expr Expression to resolve. May be NULL. > + * @param expr_list Expression list to resolve. May be NUL. > + */ > +void > +sql_resolve_self_reference(struct Parse *parser, struct Table *table, int type, > + struct Expr *expr, struct ExprList *expr_list); > + > diff --git a/src/box/sql/build.c b/src/box/sql/build.c > index afb1128..d00bb1d 100644 > --- a/src/box/sql/build.c > +++ b/src/box/sql/build.c > @@ -2952,7 +2952,7 @@ sql_create_index(struct Parse *parse, struct Token *token, > Token prevCol; > sqlite3TokenInit(&prevCol, pTab->def->fields[ > pTab->def->field_count - 1].name); > - col_list = sqlite3ExprListAppend(parse, 0, > + col_list = sql_expr_list_append(parse->db, NULL, > sqlite3ExprAlloc(db, TK_ID, 4. Wrong indentation. > &prevCol, 0)); > if (col_list == NULL) > @@ -3022,8 +3023,8 @@ sql_create_index(struct Parse *parse, struct Token *token, > for (i = 0, col_listItem = col_list->a; i < col_list->nExpr; > i++, col_listItem++) { > Expr *pCExpr; /* The i-th index expression */ > - sqlite3ResolveSelfReference(parse, pTab, NC_IdxExpr, > - col_listItem->pExpr, 0); > + sql_resolve_self_reference(parse, pTab, NC_IdxExpr, > + col_listItem->pExpr, NULL); 5. Same. > if (parse->nErr > 0) > goto exit_create_index; > pCExpr = sqlite3ExprSkipCollate(col_listItem->pExpr); > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > index 64991ee..ecb0915 100644 > --- a/src/box/sql/expr.c > +++ b/src/box/sql/expr.c > @@ -1444,8 +1443,8 @@ sqlite3ExprDup(sqlite3 * db, Expr * p, int flags) > return p ? sql_expr_dup(db, p, flags, 0) : 0; > } > > -ExprList * > -sqlite3ExprListDup(sqlite3 * db, ExprList * p, int flags) > +struct ExprList * > +sql_expr_list_dup(struct sqlite3 * db, struct ExprList *p, int flags) 6. Please, consider this diff: diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index ecb091554..fa43569a7 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -1446,38 +1446,37 @@ sqlite3ExprDup(sqlite3 * db, Expr * p, int flags) struct ExprList * sql_expr_list_dup(struct sqlite3 * db, struct ExprList *p, int flags) { - ExprList *pNew; struct ExprList_item *pItem, *pOldItem; int i; - Expr *pPriorSelectCol = 0; - assert(db != 0); - if (p == 0) - return 0; - pNew = sqlite3DbMallocRawNN(db, sizeof(*pNew)); - if (pNew == 0) - return 0; + Expr *pPriorSelectCol = NULL; + assert(db != NULL); + if (p == NULL) + return NULL; + ExprList *pNew = sqlite3DbMallocRawNN(db, sizeof(*pNew)); + if (pNew == NULL) + return NULL; pNew->nExpr = i = p->nExpr; - if ((flags & EXPRDUP_REDUCE) == 0) + if ((flags & EXPRDUP_REDUCE) == 0) { for (i = 1; i < p->nExpr; i += i) { } + } pNew->a = pItem = sqlite3DbMallocRawNN(db, i * sizeof(p->a[0])); - if (pItem == 0) { + if (pItem == NULL) { sqlite3DbFree(db, pNew); - return 0; + return NULL; } pOldItem = p->a; for (i = 0; i < p->nExpr; i++, pItem++, pOldItem++) { Expr *pOldExpr = pOldItem->pExpr; Expr *pNewExpr; pItem->pExpr = sqlite3ExprDup(db, pOldExpr, flags); - if (pOldExpr - && pOldExpr->op == TK_SELECT_COLUMN - && (pNewExpr = pItem->pExpr) != 0) { + if (pOldExpr != NULL && pOldExpr->op == TK_SELECT_COLUMN + && (pNewExpr = pItem->pExpr) != NULL) { assert(pNewExpr->iColumn == 0 || i > 0); if (pNewExpr->iColumn == 0) { assert(pOldExpr->pLeft == pOldExpr->pRight); pPriorSelectCol = pNewExpr->pLeft = - pNewExpr->pRight; + pNewExpr->pRight; } else { assert(i > 0); assert(pItem[-1].pExpr != 0); > { > ExprList *pNew; > struct ExprList_item *pItem, *pOldItem; > @@ -1618,51 +1617,39 @@ sqlite3SelectDup(sqlite3 * db, Select * p, int flags) > return pNew; > } > > -/* > - * Add a new element to the end of an expression list. If pList is > - * initially NULL, then create a new expression list. > - * > - * If a memory allocation error occurs, the entire list is freed and > - * NULL is returned. If non-NULL is returned, then it is guaranteed > - * that the new entry was successfully appended. > - */ > -ExprList * > -sqlite3ExprListAppend(Parse * pParse, /* Parsing context */ > - ExprList * pList, /* List to which to append. Might be NULL */ > - Expr * pExpr /* Expression to be appended. Might be NULL */ > - ) > +struct ExprList * > +sql_expr_list_append(struct sqlite3 * db, struct ExprList *expr_list, > + struct Expr *expr) 7. @@ -1618,8 +1617,8 @@ sqlite3SelectDup(sqlite3 * db, Select * p, int flags) } struct ExprList * -sql_expr_list_append(struct sqlite3 * db, struct ExprList *expr_list, - struct Expr *expr) +sql_expr_list_append(struct sqlite3 *db, struct ExprList *expr_list, + struct Expr *expr) { assert(db != NULL); if (expr_list == NULL) { @@ -1650,7 +1649,7 @@ sql_expr_list_append(struct sqlite3 * db, struct ExprList *expr_list, /* Avoid leaking memory if malloc has failed. */ sql_expr_delete(db, expr, false); sql_expr_list_delete(db, expr_list); - return 0; + return NULL; } > @@ -1847,10 +1834,10 @@ exprListDeleteNN(sqlite3 * db, ExprList * pList) > } > > void > -sqlite3ExprListDelete(sqlite3 * db, ExprList * pList) > +sql_expr_list_delete(sqlite3 *db, ExprList *expr_list) > { > - if (pList) > - exprListDeleteNN(db, pList); > + if (expr_list) > + exprListDeleteNN(db, expr_list); > } 8. @@ -1836,7 +1836,7 @@ exprListDeleteNN(sqlite3 * db, ExprList * pList) void sql_expr_list_delete(sqlite3 *db, ExprList *expr_list) { - if (expr_list) + if (expr_list != NULL) exprListDeleteNN(db, expr_list); } > diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c > index 2ab8fdd..3e30a44 100644 > --- a/src/box/sql/fkey.c > +++ b/src/box/sql/fkey.c > @@ -1376,7 +1377,7 @@ fkActionTrigger(Parse * pParse, /* Parse context */ > pRaise->affinity = ON_CONFLICT_ACTION_ABORT; > } > pSelect = sqlite3SelectNew(pParse, > - sqlite3ExprListAppend(pParse, > + sql_expr_list_append(pParse->db, > 0, > pRaise), > sqlite3SrcListAppend(db, 0, 9. @@ -1378,8 +1378,8 @@ fkActionTrigger(Parse * pParse, /* Parse context */ } pSelect = sqlite3SelectNew(pParse, sql_expr_list_append(pParse->db, - 0, - pRaise), + NULL, + pRaise), sqlite3SrcListAppend(db, 0, &tFrom), pWhere, 0, 0, 0, 0, 0, 0); > diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c > index 7f0858a..fd82119 100644 > --- a/src/box/sql/resolve.c > +++ b/src/box/sql/resolve.c > @@ -1578,40 +1578,27 @@ sqlite3ResolveSelectNames(Parse * pParse, /* The parser context */ > void > -sqlite3ResolveSelfReference(Parse * pParse, /* Parsing context */ > - Table * pTab, /* The table being referenced */ > - int type, /* NC_IsCheck or NC_PartIdx or NC_IdxExpr */ > - Expr * pExpr, /* Expression to resolve. May be NULL. */ > - ExprList * pList /* Expression list to resolve. May be NUL. */ > - ) > +sql_resolve_self_reference(struct Parse *parser, struct Table *table, int type, > + struct Expr *expr, struct ExprList *expr_list) 10. @@ -1582,9 +1582,9 @@ void sql_resolve_self_reference(struct Parse *parser, struct Table *table, int type, struct Expr *expr, struct ExprList *expr_list) { - /* Fake SrcList for pParse->pNewTable */ + /* Fake SrcList for parser->pNewTable */ SrcList sSrc; - /* Name context for pParse->pNewTable */ + /* Name context for parser->pNewTable */ NameContext sNC; assert(type == NC_IsCheck || type == NC_PartIdx || type == NC_IdxExpr); @@ -1597,7 +1597,7 @@ sql_resolve_self_reference(struct Parse *parser, struct Table *table, int type, sNC.pParse = parser; sNC.pSrcList = &sSrc; sNC.ncFlags = type; - if (sqlite3ResolveExprNames(&sNC, expr)) + if (sqlite3ResolveExprNames(&sNC, expr) != 0) return; if (expr_list != NULL) sqlite3ResolveExprListNames(&sNC, expr_list); > diff --git a/src/box/sql/select.c b/src/box/sql/select.c > index 75bd53d..cd305be 100644 > --- a/src/box/sql/select.c > +++ b/src/box/sql/select.c > @@ -151,7 +151,7 @@ sqlite3SelectNew(Parse * pParse, /* Parsing context */ > } > if (pEList == 0) { > pEList = > - sqlite3ExprListAppend(pParse, 0, > + sql_expr_list_append(pParse->db, NULL, > sqlite3Expr(db, TK_ASTERISK, 0)); 11. @@ -150,9 +150,8 @@ sqlite3SelectNew(Parse * pParse, /* Parsing context */ pNew = &standin; } if (pEList == 0) { - pEList = - sql_expr_list_append(pParse->db, NULL, - sqlite3Expr(db, TK_ASTERISK, 0)); + pEList = sql_expr_list_append(pParse->db, NULL, + sqlite3Expr(db, TK_ASTERISK, 0)); } struct session MAYBE_UNUSED *user_session; user_session = current_session(); > } > struct session MAYBE_UNUSED *user_session; > @@ -3191,7 +3191,7 @@ multiSelectOrderBy(Parse * pParse, /* Parsing context */ > pNew->flags |= EP_IntValue; > pNew->u.iValue = i; > pOrderBy = > - sqlite3ExprListAppend(pParse, pOrderBy, > + sql_expr_list_append(pParse->db, pOrderBy, > pNew); 12. @@ -3190,9 +3189,8 @@ multiSelectOrderBy(Parse * pParse, /* Parsing context */ return SQLITE_NOMEM_BKPT; pNew->flags |= EP_IntValue; pNew->u.iValue = i; - pOrderBy = - sql_expr_list_append(pParse->db, pOrderBy, - pNew); + pOrderBy = sql_expr_list_append(pParse->db, + pOrderBy, pNew); if (pOrderBy) pOrderBy->a[nOrderBy++].u.x. iOrderByCol = (u16) i; > @@ -4376,7 +4376,8 @@ convertCompoundSelectToSubquery(Walker * pWalker, Select * p) > *pNew = *p; > p->pSrc = pNewSrc; > p->pEList = > - sqlite3ExprListAppend(pParse, 0, sqlite3Expr(db, TK_ASTERISK, 0)); > + sql_expr_list_append(pParse->db, NULL, > + sqlite3Expr(db, TK_ASTERISK, 0)); 13. @@ -4375,9 +4373,8 @@ convertCompoundSelectToSubquery(Walker * pWalker, Select * p) return WRC_Abort; *pNew = *p; p->pSrc = pNewSrc; - p->pEList = - sql_expr_list_append(pParse->db, NULL, - sqlite3Expr(db, TK_ASTERISK, 0)); + p->pEList = sql_expr_list_append(pParse->db, NULL, + sqlite3Expr(db, TK_ASTERISK, 0)); p->op = TK_SELECT; p->pWhere = 0; pNew->pGroupBy = 0; > @@ -4817,7 +4818,7 @@ selectExpander(Walker * pWalker, Select * p) > /* This particular expression does not need to be expanded. > */ > pNew = > - sqlite3ExprListAppend(pParse, pNew, > + sql_expr_list_append(pParse->db, pNew, > a[k].pExpr); 14. @@ -4817,9 +4817,8 @@ selectExpander(Walker * pWalker, Select * p) ) { /* This particular expression does not need to be expanded. */ - pNew = - sql_expr_list_append(pParse->db, pNew, - a[k].pExpr); + pNew = sql_expr_list_append(pParse->db, pNew, + a[k].pExpr); if (pNew) { pNew->a[pNew->nExpr - 1].zName = a[k].zName; > @@ -4922,7 +4923,10 @@ selectExpander(Walker * pWalker, Select * p) > } else { > pExpr = pRight; > } > - pNew = sqlite3ExprListAppend(pParse, pNew, pExpr); > + pNew = > + sql_expr_list_append( > + pParse->db, > + pNew, pExpr); 15. @@ -4923,10 +4922,8 @@ selectExpander(Walker * pWalker, Select * p) } else { pExpr = pRight; } - pNew = - sql_expr_list_append( - pParse->db, - pNew, pExpr); + pNew = sql_expr_list_append( + pParse->db,pNew, pExpr); sqlite3TokenInit(&sColname, zColname); sqlite3ExprListSetName(pParse, pNew, > diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c > index 2704816..27902fd 100644 > --- a/src/box/sql/trigger.c > +++ b/src/box/sql/trigger.c > @@ -723,7 +723,7 @@ codeTriggerProgram(Parse * pParse, /* The parser context */ > case TK_UPDATE:{ > sqlite3Update(pParse, > targetSrcList(pParse, pStep), > - sqlite3ExprListDup(db, > + sql_expr_list_dup(db, > pStep-> > pExprList, 0), 16. @@ -724,8 +724,8 @@ codeTriggerProgram(Parse * pParse, /* The parser context */ sqlite3Update(pParse, targetSrcList(pParse, pStep), sql_expr_list_dup(db, - pStep-> - pExprList, 0), + pStep->pExprList, + 0), sqlite3ExprDup(db, pStep->pWhere, 0), pParse->eOrconf); > diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c > index 6da2762..d5a5b07 100644 > --- a/src/box/sql/whereexpr.c > +++ b/src/box/sql/whereexpr.c > @@ -784,8 +784,8 @@ exprAnalyzeOrTerm(SrcList * pSrc, /* the FROM clause */ > sqlite3ExprDup(db, pOrTerm->pExpr->pRight, > 0); > pList = > - sqlite3ExprListAppend(pWInfo->pParse, pList, > - pDup); > + sql_expr_list_append(pWInfo->pParse->db, > + pList, pDup); 17. @@ -783,9 +783,8 @@ exprAnalyzeOrTerm(SrcList * pSrc, /* the FROM clause */ pDup = sqlite3ExprDup(db, pOrTerm->pExpr->pRight, 0); - pList = - sql_expr_list_append(pWInfo->pParse->db, - pList, pDup); + pList = sql_expr_list_append(pWInfo->pParse->db, + pList, pDup); pLeft = pOrTerm->pExpr->pLeft;