[tarantool-patches] Re: [PATCH v7 6/7] sql: export funcs defined on Expr, ExprList to sql.h
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Thu May 24 22:26:41 MSK 2018
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;
More information about the Tarantool-patches
mailing list