From 20531034548e699b598d16ff6fabe7253db90747 Mon Sep 17 00:00:00 2001 From: Kirill Shcherbatov <kshcherbatov@tarantool.org> Date: Tue, 26 Feb 2019 19:37:17 +0300 Subject: [PATCH] sql: rework sqlExpr to set diag Refactored sqlExpr routine as sql_expr_new and reworked it to set diag message in case of memory allocation error. Also performed some additional name refactoring in adjacent places. This change is necessary because the sqlExpr body has a sqlNormalizeName call that will be changed in subsequent patches. After that patch there are basically 2 ways of errors processing and forwarding: - Use diag only. It works for all the places out of src/box/sql, and for some functions inside it. For example, sql_expr_new(); - Use global flags Parse.is_aborted, sql.mallocFailed. It is easy to see, that some places use both of them implicitly. For example, sql_expr_new() and every other place which uses SQLite memory allocators + diag. But it is ok until the former is removed. What is more important, is that at least one of these two methods should be consistent and finished in every functions. And match a declared behaviour. For example, it is incorrect to declare a function as setting flags on an error, but in fact set diag only. Or vice versa, that it throws a diag, but factually sets flags only. Part of #3931 --- src/box/sql/build.c | 43 ++++--- src/box/sql/expr.c | 244 ++++++++++++++---------------------- src/box/sql/fk_constraint.c | 190 +++++++++++++++------------- src/box/sql/parse.y | 51 ++++++-- src/box/sql/resolve.c | 46 ++++--- src/box/sql/select.c | 83 +++++++----- src/box/sql/sqlInt.h | 91 +++++++++++++- src/box/sql/wherecode.c | 5 +- src/box/sql/whereexpr.c | 21 ++-- 9 files changed, 442 insertions(+), 332 deletions(-) diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 2c1b1613fd..4070630086 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -630,9 +630,12 @@ sqlAddPrimaryKey(Parse * pParse, /* Parsing context */ struct ExprList *list; struct Token token; sqlTokenInit(&token, space->def->fields[iCol].name); - list = sql_expr_list_append(db, NULL, - sqlExprAlloc(db, TK_ID, - &token, 0)); + struct Expr *expr = sql_expr_new(db, TK_ID, &token); + if (expr == NULL) { + pParse->is_aborted = true; + goto primary_key_exit; + } + list = sql_expr_list_append(db, NULL, expr); if (list == NULL) goto primary_key_exit; sql_create_index(pParse, 0, 0, list, 0, SORT_ORDER_ASC, @@ -1374,13 +1377,16 @@ sql_id_eq_str_expr(struct Parse *parse, const char *col_name, const char *col_value) { struct sql *db = parse->db; - - struct Expr *col_name_expr = sqlExpr(db, TK_ID, col_name); - if (col_name_expr == NULL) + struct Expr *col_name_expr = sql_expr_new_named(db, TK_ID, col_name); + if (col_name_expr == NULL) { + parse->is_aborted = true; return NULL; - struct Expr *col_value_expr = sqlExpr(db, TK_STRING, col_value); + } + struct Expr *col_value_expr = + sql_expr_new_named(db, TK_STRING, col_value); if (col_value_expr == NULL) { sql_expr_delete(db, col_name_expr, false); + parse->is_aborted = true; return NULL; } return sqlPExpr(parse, TK_EQ, col_name_expr, col_value_expr); @@ -1399,17 +1405,17 @@ vdbe_emit_stat_space_clear(struct Parse *parse, const char *stat_table_name, return; } src_list->a[0].zName = sqlDbStrDup(db, stat_table_name); - struct Expr *where = NULL; + struct Expr *expr, *where = NULL; if (idx_name != NULL) { - struct Expr *expr = sql_id_eq_str_expr(parse, "idx", idx_name); - if (expr != NULL) - where = sqlExprAnd(db, expr, where); + expr = sql_id_eq_str_expr(parse, "idx", idx_name); + where = sql_and_expr_new(db, expr, where); } if (table_name != NULL) { - struct Expr *expr = sql_id_eq_str_expr(parse, "tbl", table_name); - if (expr != NULL) - where = sqlExprAnd(db, expr, where); + expr = sql_id_eq_str_expr(parse, "tbl", table_name); + where = sql_and_expr_new(db, expr, where); } + if (where == NULL) + parse->is_aborted = true; /** * On memory allocation error sql_table delete_from * releases memory for its own. @@ -2270,9 +2276,12 @@ sql_create_index(struct Parse *parse, struct Token *token, struct Token prev_col; uint32_t last_field = def->field_count - 1; sqlTokenInit(&prev_col, def->fields[last_field].name); - col_list = sql_expr_list_append(parse->db, NULL, - sqlExprAlloc(db, TK_ID, - &prev_col, 0)); + struct Expr *expr = sql_expr_new(db, TK_ID, &prev_col); + if (expr == NULL) { + parse->is_aborted = true; + goto exit_create_index; + } + col_list = sql_expr_list_append(db, NULL, expr); if (col_list == NULL) goto exit_create_index; assert(col_list->nExpr == 1); diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 8b703f3e45..48db9b45dc 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -149,17 +149,21 @@ sqlExprAddCollateToken(Parse * pParse, /* Parsing context */ int dequote /* True to dequote pCollName */ ) { - if (pCollName->n > 0) { - Expr *pNew = - sqlExprAlloc(pParse->db, TK_COLLATE, pCollName, - dequote); - if (pNew) { - pNew->pLeft = pExpr; - pNew->flags |= EP_Collate | EP_Skip; - pExpr = pNew; - } + if (pCollName->n == 0) + return pExpr; + struct Expr *new_expr; + struct sql *db = pParse->db; + if (dequote) + new_expr = sql_expr_new_dequoted(db, TK_COLLATE, pCollName); + else + new_expr = sql_expr_new(db, TK_COLLATE, pCollName); + if (new_expr == NULL) { + pParse->is_aborted = true; + return pExpr; } - return pExpr; + new_expr->pLeft = pExpr; + new_expr->flags |= EP_Collate | EP_Skip; + return new_expr; } Expr * @@ -882,113 +886,59 @@ sqlExprSetHeightAndFlags(Parse * pParse, Expr * p) #define exprSetHeight(y) #endif /* SQL_MAX_EXPR_DEPTH>0 */ -/* - * This routine is the core allocator for Expr nodes. - * - * Construct a new expression node and return a pointer to it. Memory - * for this node and for the pToken argument is a single allocation - * obtained from sqlDbMalloc(). The calling function - * is responsible for making sure the node eventually gets freed. - * - * If dequote is true, then the token (if it exists) is dequoted. - * If dequote is false, no dequoting is performed. The deQuote - * parameter is ignored if pToken is NULL or if the token does not - * appear to be quoted. If the quotes were of the form "..." (double-quotes) - * then the EP_DblQuoted flag is set on the expression node. - * - * Special case: If op==TK_INTEGER and pToken points to a string that - * can be translated into a 32-bit integer, then the token is not - * stored in u.zToken. Instead, the integer values is written - * into u.iValue and the EP_IntValue flag is set. No extra storage - * is allocated to hold the integer text and the dequote flag is ignored. - */ -Expr * -sqlExprAlloc(sql * db, /* Handle for sqlDbMallocRawNN() */ - int op, /* Expression opcode */ - const Token * pToken, /* Token argument. Might be NULL */ - int dequote /* True to dequote */ - ) +struct Expr * +sql_expr_new(struct sql *db, int op, const struct Token *token) { - Expr *pNew; - int nExtra = 0; - int iValue = 0; - - assert(db != 0); - if (pToken) { - if (op != TK_INTEGER || pToken->z == 0 - || sqlGetInt32(pToken->z, &iValue) == 0) { - nExtra = pToken->n + 1; - assert(iValue >= 0); + int extra_sz = 0; + int val = 0; + if (token != NULL) { + if (op != TK_INTEGER || token->z == NULL || + sqlGetInt32(token->z, &val) == 0) { + extra_sz = token->n + 1; + assert(val >= 0); } } - pNew = sqlDbMallocRawNN(db, sizeof(Expr) + nExtra); - if (pNew) { - memset(pNew, 0, sizeof(Expr)); - pNew->op = (u8) op; - pNew->iAgg = -1; - if (pToken) { - if (nExtra == 0) { - pNew->flags |= EP_IntValue; - pNew->u.iValue = iValue; - } else { - pNew->u.zToken = (char *)&pNew[1]; - assert(pToken->z != 0 || pToken->n == 0); - if (pToken->n) - memcpy(pNew->u.zToken, pToken->z, - pToken->n); - pNew->u.zToken[pToken->n] = 0; - if (dequote){ - if (pNew->u.zToken[0] == '"') - pNew->flags |= EP_DblQuoted; - if (pNew->op == TK_ID || - pNew->op == TK_COLLATE || - pNew->op == TK_FUNCTION){ - sqlNormalizeName(pNew->u.zToken); - }else{ - sqlDequote(pNew->u.zToken); - } - } - } - } -#if SQL_MAX_EXPR_DEPTH>0 - pNew->nHeight = 1; -#endif + struct Expr *expr = sqlDbMallocRawNN(db, sizeof(*expr) + extra_sz); + if (expr == NULL) { + diag_set(OutOfMemory, sizeof(*expr), "sqlDbMallocRawNN", + "expr"); + return NULL; } - return pNew; -} -/* - * Allocate a new expression node from a zero-terminated token that has - * already been dequoted. - */ -Expr * -sqlExpr(sql * db, /* Handle for sqlDbMallocZero() (may be null) */ - int op, /* Expression opcode */ - const char *zToken /* Token argument. Might be NULL */ - ) -{ - Token x; - x.z = zToken; - x.n = zToken ? sqlStrlen30(zToken) : 0; - return sqlExprAlloc(db, op, &x, 0); + memset(expr, 0, sizeof(*expr)); + expr->op = (u8)op; + expr->iAgg = -1; +#if SQL_MAX_EXPR_DEPTH > 0 + expr->nHeight = 1; +#endif + if (token == NULL) + return expr; + + if (extra_sz == 0) { + expr->flags |= EP_IntValue; + expr->u.iValue = val; + } else { + expr->u.zToken = (char *)&expr[1]; + assert(token->z != NULL || token->n == 0); + memcpy(expr->u.zToken, token->z, token->n); + expr->u.zToken[token->n] = '\0'; + } + return expr; } -/* Allocate a new expression and initialize it as integer. - * @param db sql engine. - * @param value Value to initialize by. - * - * @retval not NULL Allocated and initialized expr. - * @retval NULL Memory error. - */ -Expr * -sqlExprInteger(sql * db, int value) +struct Expr * +sql_expr_new_dequoted(struct sql *db, int op, const struct Token *token) { - Expr *ret = sqlExpr(db, TK_INTEGER, NULL); - if (ret != NULL) { - ret->flags = EP_IntValue; - ret->u.iValue = value; - } - return ret; + struct Expr *e = sql_expr_new(db, op, token); + if (e == NULL || (e->flags & EP_IntValue) != 0 || e->u.zToken == NULL) + return e; + if (e->u.zToken[0] == '"') + e->flags |= EP_DblQuoted; + if (e->op == TK_ID || e->op == TK_COLLATE || e->op == TK_FUNCTION) + sqlNormalizeName(e->u.zToken); + else + sqlDequote(e->u.zToken); + return e; } /* @@ -1034,8 +984,13 @@ sqlPExpr(Parse * pParse, /* Parsing context */ { Expr *p; if (op == TK_AND && !pParse->is_aborted) { - /* Take advantage of short-circuit false optimization for AND */ - p = sqlExprAnd(pParse->db, pLeft, pRight); + /* + * Take advantage of short-circuit false + * optimization for AND. + */ + p = sql_and_expr_new(pParse->db, pLeft, pRight); + if (p == NULL) + pParse->is_aborted = true; } else { p = sqlDbMallocRawNN(pParse->db, sizeof(Expr)); if (p) { @@ -1104,30 +1059,22 @@ exprAlwaysFalse(Expr * p) return v == 0; } -/* - * Join two expressions using an AND operator. If either expression is - * NULL, then just return the other expression. - * - * If one side or the other of the AND is known to be false, then instead - * of returning an AND expression, just return a constant expression with - * a value of false. - */ -Expr * -sqlExprAnd(sql * db, Expr * pLeft, Expr * pRight) +struct Expr * +sql_and_expr_new(struct sql *db, struct Expr *left_expr, + struct Expr *right_expr) { - if (pLeft == 0) { - return pRight; - } else if (pRight == 0) { - return pLeft; - } else if (exprAlwaysFalse(pLeft) || exprAlwaysFalse(pRight)) { - sql_expr_delete(db, pLeft, false); - sql_expr_delete(db, pRight, false); - return sqlExprAlloc(db, TK_INTEGER, &sqlIntTokens[0], - 0); + if (left_expr == NULL) { + return right_expr; + } else if (right_expr == NULL) { + return left_expr; + } else if (exprAlwaysFalse(left_expr) || exprAlwaysFalse(right_expr)) { + sql_expr_delete(db, left_expr, false); + sql_expr_delete(db, right_expr, false); + return sql_expr_new(db, TK_INTEGER, &sqlIntTokens[0]); } else { - Expr *pNew = sqlExprAlloc(db, TK_AND, 0, 0); - sqlExprAttachSubtrees(db, pNew, pLeft, pRight); - return pNew; + struct Expr *new_expr = sql_expr_new_anon(db, TK_AND); + sqlExprAttachSubtrees(db, new_expr, left_expr, right_expr); + return new_expr; } } @@ -1138,18 +1085,18 @@ sqlExprAnd(sql * db, Expr * pLeft, Expr * pRight) Expr * sqlExprFunction(Parse * pParse, ExprList * pList, Token * pToken) { - Expr *pNew; - sql *db = pParse->db; - assert(pToken); - pNew = sqlExprAlloc(db, TK_FUNCTION, pToken, 1); - if (pNew == 0) { - sql_expr_list_delete(db, pList); /* Avoid memory leak when malloc fails */ - return 0; + struct sql *db = pParse->db; + assert(pToken != NULL); + struct Expr *new_expr = sql_expr_new_dequoted(db, TK_FUNCTION, pToken); + if (new_expr == NULL) { + sql_expr_list_delete(db, pList); + pParse->is_aborted = true; + return NULL; } - pNew->x.pList = pList; - assert(!ExprHasProperty(pNew, EP_xIsSelect)); - sqlExprSetHeightAndFlags(pParse, pNew); - return pNew; + new_expr->x.pList = pList; + assert(!ExprHasProperty(new_expr, EP_xIsSelect)); + sqlExprSetHeightAndFlags(pParse, new_expr); + return new_expr; } /* @@ -2920,10 +2867,11 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */ } if (pSel->pLimit == NULL) { pSel->pLimit = - sqlExprAlloc(pParse->db, TK_INTEGER, - &sqlIntTokens[1], - 0); - if (pSel->pLimit != NULL) { + sql_expr_new(pParse->db, TK_INTEGER, + &sqlIntTokens[1]); + if (pSel->pLimit == NULL) { + pParse->is_aborted = true; + } else { ExprSetProperty(pSel->pLimit, EP_System); } diff --git a/src/box/sql/fk_constraint.c b/src/box/sql/fk_constraint.c index cc4dc64482..8151c66e55 100644 --- a/src/box/sql/fk_constraint.c +++ b/src/box/sql/fk_constraint.c @@ -302,38 +302,29 @@ fk_constraint_lookup_parent(struct Parse *parse_context, struct space *parent, } /** - * Return an Expr object that refers to a memory register - * corresponding to column iCol of given space. - * - * regBase is the first of an array of register that contains - * the data for given space. regBase+1 holds the first column. - * regBase+2 holds the second column, and so forth. - * - * @param pParse Parsing and code generating context. - * @param def Definition of space whose content is at r[regBase]... - * @param regBase Contents of table defined by def. - * @param iCol Which column of space is desired. - * @return an Expr object that refers to a memory register - * corresponding to column iCol of given space. + * Build an expression that refers to a memory register + * corresponding to @a column of given space. + * + * @param db SQL context. + * @param def Definition of space whose content starts from + * @a reg_base register. + * @param reg_base Index of a first element in an array of + * registers, containing data of a space. Register + * reg_base + i holds an i-th column, i >= 1. + * @param column Index of a first table column to point at. + * @retval Not NULL Success. An expression representing register. + * @retval NULL Error. A diag message is set. */ -static Expr * -space_field_register(Parse *pParse, struct space_def *def, int regBase, - i16 iCol) +static struct Expr * +sql_expr_new_register(struct sql *db, struct space_def *def, int reg_base, + uint32_t column) { - Expr *pExpr; - sql *db = pParse->db; - - pExpr = sqlExpr(db, TK_REGISTER, 0); - if (pExpr) { - if (iCol >= 0) { - pExpr->iTable = regBase + iCol + 1; - pExpr->type = def->fields[iCol].type; - } else { - pExpr->iTable = regBase; - pExpr->type = FIELD_TYPE_INTEGER; - } - } - return pExpr; + struct Expr *expr = sql_expr_new_anon(db, TK_REGISTER); + if (expr == NULL) + return NULL; + expr->iTable = reg_base + column + 1; + expr->type = def->fields[column].type; + return expr; } /** @@ -346,16 +337,17 @@ space_field_register(Parse *pParse, struct space_def *def, int regBase, * @retval not NULL on success. * @retval NULL on error. */ -static Expr * -exprTableColumn(sql * db, struct space_def *def, int cursor, i16 column) +static struct Expr * +sql_expr_new_column_by_cursor(struct sql *db, struct space_def *def, + int cursor, int column) { - Expr *pExpr = sqlExpr(db, TK_COLUMN, 0); - if (pExpr) { - pExpr->space_def = def; - pExpr->iTable = cursor; - pExpr->iColumn = column; - } - return pExpr; + struct Expr *expr = sql_expr_new_anon(db, TK_COLUMN); + if (expr == NULL) + return NULL; + expr->space_def = def; + expr->iTable = cursor; + expr->iColumn = column; + return expr; } /* @@ -435,12 +427,14 @@ fk_constraint_scan_children(struct Parse *parser, struct SrcList *src, for (uint32_t i = 0; i < fk_def->field_count; i++) { uint32_t fieldno = fk_def->links[i].parent_field; struct Expr *pexpr = - space_field_register(parser, def, reg_data, fieldno); + sql_expr_new_register(db, def, reg_data, fieldno); fieldno = fk_def->links[i].child_field; const char *field_name = child_space->def->fields[fieldno].name; - struct Expr *chexpr = sqlExpr(db, TK_ID, field_name); + struct Expr *chexpr = sql_expr_new_named(db, TK_ID, field_name); struct Expr *eq = sqlPExpr(parser, TK_EQ, pexpr, chexpr); - where = sqlExprAnd(db, where, eq); + where = sql_and_expr_new(db, where, eq); + if (where == NULL || chexpr == NULL || pexpr == NULL) + parser->is_aborted = true; } /* @@ -456,15 +450,20 @@ fk_constraint_scan_children(struct Parse *parser, struct SrcList *src, struct Expr *expr = NULL, *pexpr, *chexpr, *eq; for (uint32_t i = 0; i < fk_def->field_count; i++) { uint32_t fieldno = fk_def->links[i].parent_field; - pexpr = space_field_register(parser, def, reg_data, - fieldno); - chexpr = exprTableColumn(db, def, src->a[0].iCursor, - fieldno); + pexpr = sql_expr_new_register(db, def, reg_data, + fieldno); + int cursor = src->a[0].iCursor; + chexpr = sql_expr_new_column_by_cursor(db, def, cursor, + fieldno); eq = sqlPExpr(parser, TK_EQ, pexpr, chexpr); - expr = sqlExprAnd(db, expr, eq); + expr = sql_and_expr_new(db, expr, eq); + if (expr == NULL || chexpr == NULL || pexpr == NULL) + parser->is_aborted = true; } struct Expr *pNe = sqlPExpr(parser, TK_NOT, expr, 0); - where = sqlExprAnd(db, where, pNe); + where = sql_and_expr_new(db, where, pNe); + if (where == NULL) + parser->is_aborted = true; } /* Resolve the references in the WHERE clause. */ @@ -703,6 +702,28 @@ fk_constraint_is_required(struct space *space, const int *changes) return false; } +/** + * Create a new expression representing two-part path + * '<main>.<sub>'. + * @param parser SQL Parser. + * @param main First and main part of a result path. For example, + * a table name. + * @param sub Second and last part of a result path. For example, + * a column name. + * @retval Not NULL Success. An expression with two-part path. + * @retval NULL Error. A diag message is set. + */ +static inline struct Expr * +sql_expr_new_2part_id(struct Parse *parser, const struct Token *main, + const struct Token *sub) +{ + struct Expr *emain = sql_expr_new(parser->db, TK_ID, main); + struct Expr *esub = sql_expr_new(parser->db, TK_ID, sub); + if (emain == NULL || esub == NULL) + parser->is_aborted = true; + return sqlPExpr(parser, TK_DOT, emain, esub); +} + /** * This function is called when an UPDATE or DELETE operation is * being compiled on table pTab, which is the parent table of @@ -785,15 +806,13 @@ fk_constraint_action_trigger(struct Parse *pParse, struct space_def *def, * type and collation sequence associated with * the parent table are used for the comparison. */ - struct Expr *to_col = - sqlPExpr(pParse, TK_DOT, - sqlExprAlloc(db, TK_ID, &t_old, 0), - sqlExprAlloc(db, TK_ID, &t_to_col, 0)); - struct Expr *from_col = - sqlExprAlloc(db, TK_ID, &t_from_col, 0); - struct Expr *eq = sqlPExpr(pParse, TK_EQ, to_col, from_col); - where = sqlExprAnd(db, where, eq); - + struct Expr *new, *old = + sql_expr_new_2part_id(pParse, &t_old, &t_to_col); + struct Expr *from = sql_expr_new(db, TK_ID, &t_from_col); + struct Expr *eq = sqlPExpr(pParse, TK_EQ, old, from); + where = sql_and_expr_new(db, where, eq); + if (where == NULL || from == NULL) + pParse->is_aborted = true; /* * For ON UPDATE, construct the next term of the * WHEN clause, which should return false in case @@ -810,47 +829,38 @@ fk_constraint_action_trigger(struct Parse *pParse, struct space_def *def, * no_action_needed(colN)) */ if (is_update) { - struct Expr *old_val = sqlPExpr(pParse, TK_DOT, - sqlExprAlloc(db, TK_ID, &t_old, 0), - sqlExprAlloc(db, TK_ID, &t_to_col, 0)); - struct Expr *new_val = sqlPExpr(pParse, TK_DOT, - sqlExprAlloc(db, TK_ID, &t_new, 0), - sqlExprAlloc(db, TK_ID, &t_to_col, 0)); - struct Expr *old_is_null = sqlPExpr( - pParse, TK_ISNULL, - sqlExprDup(db, old_val, 0), NULL); - eq = sqlPExpr(pParse, TK_EQ, old_val, - sqlExprDup(db, new_val, 0)); + old = sql_expr_new_2part_id(pParse, &t_old, &t_to_col); + new = sql_expr_new_2part_id(pParse, &t_new, &t_to_col); + struct Expr *old_is_null = + sqlPExpr(pParse, TK_ISNULL, + sqlExprDup(db, old, 0), NULL); + eq = sqlPExpr(pParse, TK_EQ, old, + sqlExprDup(db, new, 0)); struct Expr *new_non_null = - sqlPExpr(pParse, TK_NOTNULL, new_val, NULL); + sqlPExpr(pParse, TK_NOTNULL, new, NULL); struct Expr *non_null_eq = sqlPExpr(pParse, TK_AND, new_non_null, eq); struct Expr *no_action_needed = sqlPExpr(pParse, TK_OR, old_is_null, non_null_eq); - when = sqlExprAnd(db, when, no_action_needed); + when = sql_and_expr_new(db, when, no_action_needed); + if (when == NULL) + pParse->is_aborted = true; } if (action != FKEY_ACTION_RESTRICT && (action != FKEY_ACTION_CASCADE || is_update)) { - struct Expr *new, *d; + struct Expr *d = child_fields[chcol].default_value_expr; if (action == FKEY_ACTION_CASCADE) { - new = sqlPExpr(pParse, TK_DOT, - sqlExprAlloc(db, TK_ID, - &t_new, 0), - sqlExprAlloc(db, TK_ID, - &t_to_col, - 0)); - } else if (action == FKEY_ACTION_SET_DEFAULT) { - d = child_fields[chcol].default_value_expr; - if (d != NULL) { - new = sqlExprDup(db, d, 0); - } else { - new = sqlExprAlloc(db, TK_NULL, - NULL, 0); - } + new = sql_expr_new_2part_id(pParse, &t_new, + &t_to_col); + } else if (action == FKEY_ACTION_SET_DEFAULT && + d != NULL) { + new = sqlExprDup(db, d, 0); } else { - new = sqlExprAlloc(db, TK_NULL, NULL, 0); + new = sql_expr_new_anon(db, TK_NULL); + if (new == NULL) + pParse->is_aborted = true; } list = sql_expr_list_append(db, list, new); sqlExprListSetName(pParse, list, &t_from_col, 0); @@ -864,9 +874,11 @@ fk_constraint_action_trigger(struct Parse *pParse, struct space_def *def, struct Token err; err.z = space_name; err.n = name_len; - struct Expr *r = sqlExpr(db, TK_RAISE, "FOREIGN KEY "\ - "constraint failed"); - if (r != NULL) + struct Expr *r = sql_expr_new_named(db, TK_RAISE, "FOREIGN "\ + "KEY constraint failed"); + if (r == NULL) + pParse->is_aborted = true; + else r->on_conflict_action = ON_CONFLICT_ACTION_ABORT; struct SrcList *src_list = sql_src_list_append(db, NULL, &err); if (src_list == NULL) diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index 4b88e57b07..8c1d1983aa 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -537,12 +537,20 @@ selcollist(A) ::= sclp(A) expr(X) as(Y). { sqlExprListSetSpan(pParse,A,&X); } selcollist(A) ::= sclp(A) STAR. { - Expr *p = sqlExpr(pParse->db, TK_ASTERISK, 0); + struct Expr *p = sql_expr_new_anon(pParse->db, TK_ASTERISK); + if (p == NULL) { + pParse->is_aborted = true; + return; + } A = sql_expr_list_append(pParse->db, A, p); } selcollist(A) ::= sclp(A) nm(X) DOT STAR. { + struct Expr *pLeft = sql_expr_new_dequoted(pParse->db, TK_ID, &X); + if (pLeft == NULL) { + pParse->is_aborted = true; + return; + } Expr *pRight = sqlPExpr(pParse, TK_ASTERISK, 0, 0); - Expr *pLeft = sqlExprAlloc(pParse->db, TK_ID, &X, 1); Expr *pDot = sqlPExpr(pParse, TK_DOT, pLeft, pRight); A = sql_expr_list_append(pParse->db,A, pDot); } @@ -903,15 +911,28 @@ term(A) ::= NULL(X). {spanExpr(&A,pParse,@X,X);/*A-overwrites-X*/} expr(A) ::= id(X). {spanExpr(&A,pParse,TK_ID,X); /*A-overwrites-X*/} expr(A) ::= JOIN_KW(X). {spanExpr(&A,pParse,TK_ID,X); /*A-overwrites-X*/} expr(A) ::= nm(X) DOT nm(Y). { - Expr *temp1 = sqlExprAlloc(pParse->db, TK_ID, &X, 1); - Expr *temp2 = sqlExprAlloc(pParse->db, TK_ID, &Y, 1); + struct Expr *temp1 = sql_expr_new_dequoted(pParse->db, TK_ID, &X); + if (temp1 == NULL) { + pParse->is_aborted = true; + return; + } + struct Expr *temp2 = sql_expr_new_dequoted(pParse->db, TK_ID, &Y); + if (temp2 == NULL) { + sql_expr_delete(pParse->db, temp1, false); + pParse->is_aborted = true; + return; + } spanSet(&A,&X,&Y); /*A-overwrites-X*/ A.pExpr = sqlPExpr(pParse, TK_DOT, temp1, temp2); } term(A) ::= FLOAT|BLOB(X). {spanExpr(&A,pParse,@X,X);/*A-overwrites-X*/} term(A) ::= STRING(X). {spanExpr(&A,pParse,@X,X);/*A-overwrites-X*/} term(A) ::= INTEGER(X). { - A.pExpr = sqlExprAlloc(pParse->db, TK_INTEGER, &X, 1); + A.pExpr = sql_expr_new_dequoted(pParse->db, TK_INTEGER, &X); + if (A.pExpr == NULL) { + pParse->is_aborted = true; + return; + } A.pExpr->type = FIELD_TYPE_INTEGER; A.zStart = X.z; A.zEnd = X.z + X.n; @@ -949,7 +970,11 @@ expr(A) ::= expr(A) COLLATE id(C). { %ifndef SQL_OMIT_CAST expr(A) ::= CAST(X) LP expr(E) AS typedef(T) RP(Y). { spanSet(&A,&X,&Y); /*A-overwrites-X*/ - A.pExpr = sqlExprAlloc(pParse->db, TK_CAST, 0, 1); + A.pExpr = sql_expr_new_dequoted(pParse->db, TK_CAST, NULL); + if (A.pExpr == NULL) { + pParse->is_aborted = true; + return; + } A.pExpr->type = T.type; sqlExprAttachSubtrees(pParse->db, A.pExpr, E.pExpr, 0); } @@ -1151,7 +1176,11 @@ expr(A) ::= expr(A) in_op(N) LP exprlist(Y) RP(E). [IN] { ** regardless of the value of expr1. */ sql_expr_delete(pParse->db, A.pExpr, false); - A.pExpr = sqlExprAlloc(pParse->db, TK_INTEGER,&sqlIntTokens[N],1); + A.pExpr = sql_expr_new_dequoted(pParse->db, TK_INTEGER, &sqlIntTokens[N]); + if (A.pExpr == NULL) { + pParse->is_aborted = true; + return; + } }else if( Y->nExpr==1 ){ /* Expressions of the form: ** @@ -1492,10 +1521,12 @@ expr(A) ::= RAISE(X) LP IGNORE RP(Y). { } expr(A) ::= RAISE(X) LP raisetype(T) COMMA STRING(Z) RP(Y). { spanSet(&A,&X,&Y); /*A-overwrites-X*/ - A.pExpr = sqlExprAlloc(pParse->db, TK_RAISE, &Z, 1); - if( A.pExpr ) { - A.pExpr->on_conflict_action = (enum on_conflict_action) T; + A.pExpr = sql_expr_new_dequoted(pParse->db, TK_RAISE, &Z); + if(A.pExpr == NULL) { + pParse->is_aborted = true; + return; } + A.pExpr->on_conflict_action = (enum on_conflict_action) T; } %type raisetype {int} diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index 84e2376de2..6f0bccb469 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -488,26 +488,20 @@ lookupName(Parse * pParse, /* The parsing context */ } } -/* - * Allocate and return a pointer to an expression to load the column iCol - * from datasource iSrc in SrcList pSrc. - */ -Expr * -sqlCreateColumnExpr(sql * db, SrcList * pSrc, int iSrc, int iCol) +struct Expr * +sql_expr_new_column(struct sql *db, struct SrcList *src_list, int src_idx, + int column) { - Expr *p = sqlExprAlloc(db, TK_COLUMN, 0, 0); - if (p) { - struct SrcList_item *pItem = &pSrc->a[iSrc]; - p->space_def = pItem->space->def; - p->iTable = pItem->iCursor; - p->iColumn = (ynVar) iCol; - testcase(iCol == BMS); - testcase(iCol == BMS - 1); - pItem->colUsed |= - ((Bitmask) 1) << (iCol >= BMS ? BMS - 1 : iCol); - ExprSetProperty(p, EP_Resolved); - } - return p; + struct Expr *expr = sql_expr_new_anon(db, TK_COLUMN); + if (expr == NULL) + return NULL; + struct SrcList_item *item = &src_list->a[src_idx]; + expr->space_def = item->space->def; + expr->iTable = item->iCursor; + expr->iColumn = column; + item->colUsed |= ((Bitmask) 1) << (column >= BMS ? BMS - 1 : column); + ExprSetProperty(expr, EP_Resolved); + return expr; } /* @@ -993,9 +987,12 @@ resolveCompoundOrderBy(Parse * pParse, /* Parsing context. Leave error messages /* Convert the ORDER BY term into an integer column number iCol, * taking care to preserve the COLLATE clause if it exists */ - Expr *pNew = sqlExpr(db, TK_INTEGER, 0); - if (pNew == 0) + struct Expr *pNew = + sql_expr_new_anon(db, TK_INTEGER); + if (pNew == NULL) { + pParse->is_aborted = true; return 1; + } pNew->flags |= EP_IntValue; pNew->u.iValue = iCol; if (pItem->pExpr == pE) { @@ -1358,9 +1355,10 @@ resolveSelectStep(Walker * pWalker, Select * p) * restrict it directly). */ sql_expr_delete(db, p->pLimit, false); - p->pLimit = - sqlExprAlloc(db, TK_INTEGER, - &sqlIntTokens[1], 0); + p->pLimit = sql_expr_new(db, TK_INTEGER, + &sqlIntTokens[1]); + if (p->pLimit == NULL) + pParse->is_aborted = true; } else { if (sqlResolveExprNames(&sNC, p->pHaving)) return WRC_Abort; diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 8c87f30ec3..190578f754 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -164,8 +164,10 @@ sqlSelectNew(Parse * pParse, /* Parsing context */ pNew = &standin; } if (pEList == 0) { - pEList = sql_expr_list_append(pParse->db, NULL, - sqlExpr(db, TK_ASTERISK, 0)); + struct Expr *expr = sql_expr_new_anon(db, TK_ASTERISK); + if (expr == NULL) + pParse->is_aborted = true; + pEList = sql_expr_list_append(db, NULL, expr); } struct session MAYBE_UNUSED *user_session; user_session = current_session(); @@ -493,9 +495,7 @@ addWhereTerm(Parse * pParse, /* Parsing context */ int isOuterJoin, /* True if this is an OUTER join */ Expr ** ppWhere) /* IN/OUT: The WHERE clause to add to */ { - sql *db = pParse->db; - Expr *pE1; - Expr *pE2; + struct sql *db = pParse->db; Expr *pEq; assert(iLeft < iRight); @@ -503,9 +503,10 @@ addWhereTerm(Parse * pParse, /* Parsing context */ assert(pSrc->a[iLeft].space != NULL); assert(pSrc->a[iRight].space != NULL); - pE1 = sqlCreateColumnExpr(db, pSrc, iLeft, iColLeft); - pE2 = sqlCreateColumnExpr(db, pSrc, iRight, iColRight); - + struct Expr *pE1 = sql_expr_new_column(db, pSrc, iLeft, iColLeft); + struct Expr *pE2 = sql_expr_new_column(db, pSrc, iRight, iColRight); + if (pE1 == NULL || pE2 == NULL) + pParse->is_aborted = true; pEq = sqlPExpr(pParse, TK_EQ, pE1, pE2); if (pEq && isOuterJoin) { ExprSetProperty(pEq, EP_FromJoin); @@ -513,7 +514,9 @@ addWhereTerm(Parse * pParse, /* Parsing context */ ExprSetVVAProperty(pEq, EP_NoReduce); pEq->iRightJoinTable = (i16) pE2->iTable; } - *ppWhere = sqlExprAnd(db, *ppWhere, pEq); + *ppWhere = sql_and_expr_new(db, *ppWhere, pEq); + if (*ppWhere == NULL) + pParse->is_aborted = true; } /* @@ -637,8 +640,10 @@ sqlProcessJoin(Parse * pParse, Select * p) if (pRight->pOn) { if (isOuter) setJoinExpr(pRight->pOn, pRight->iCursor); - p->pWhere = - sqlExprAnd(pParse->db, p->pWhere, pRight->pOn); + p->pWhere = sql_and_expr_new(pParse->db, p->pWhere, + pRight->pOn); + if (p->pWhere == NULL) + pParse->is_aborted = true; pRight->pOn = 0; } @@ -3333,9 +3338,12 @@ multiSelectOrderBy(Parse * pParse, /* Parsing context */ break; } if (j == nOrderBy) { - Expr *pNew = sqlExpr(db, TK_INTEGER, 0); - if (pNew == 0) - return SQL_NOMEM; + struct Expr *pNew = + sql_expr_new_anon(db, TK_INTEGER); + if (pNew == NULL) { + pParse->is_aborted = true; + return 1; + } pNew->flags |= EP_IntValue; pNew->u.iValue = i; pOrderBy = sql_expr_list_append(pParse->db, @@ -4215,17 +4223,23 @@ flattenSubquery(Parse * pParse, /* Parsing context */ assert(pParent->pHaving == 0); pParent->pHaving = pParent->pWhere; pParent->pWhere = pWhere; - pParent->pHaving = sqlExprAnd(db, - sqlExprDup(db, - pSub->pHaving, - 0), - pParent->pHaving); + struct Expr *sub_having = + sqlExprDup(db, pSub->pHaving, 0); + if (sub_having != NULL || pParent->pHaving != NULL) { + pParent->pHaving = + sql_and_expr_new(db, sub_having, + pParent->pHaving); + if (pParent->pHaving == NULL) + pParse->is_aborted = true; + } assert(pParent->pGroupBy == 0); pParent->pGroupBy = sql_expr_list_dup(db, pSub->pGroupBy, 0); - } else { + } else if (pWhere != NULL || pParent->pWhere != NULL) { pParent->pWhere = - sqlExprAnd(db, pWhere, pParent->pWhere); + sql_and_expr_new(db, pWhere, pParent->pWhere); + if (pParent->pWhere == NULL) + pParse->is_aborted = true; } substSelect(pParse, pParent, iParent, pSub->pEList, 0); @@ -4330,8 +4344,10 @@ pushDownWhereTerms(Parse * pParse, /* Parse context (for malloc() and error repo while (pSubq) { pNew = sqlExprDup(pParse->db, pWhere, 0); pNew = substExpr(pParse, pNew, iCursor, pSubq->pEList); - pSubq->pWhere = - sqlExprAnd(pParse->db, pSubq->pWhere, pNew); + pSubq->pWhere = sql_and_expr_new(pParse->db, + pSubq->pWhere, pNew); + if (pSubq->pWhere == NULL) + pParse->is_aborted = true; pSubq = pSubq->pPrior; } } @@ -4512,8 +4528,10 @@ convertCompoundSelectToSubquery(Walker * pWalker, Select * p) return WRC_Abort; *pNew = *p; p->pSrc = pNewSrc; - p->pEList = sql_expr_list_append(pParse->db, NULL, - sqlExpr(db, TK_ASTERISK, 0)); + struct Expr *expr = sql_expr_new_anon(db, TK_ASTERISK); + if (expr == NULL) + pParse->is_aborted = true; + p->pEList = sql_expr_list_append(pParse->db, NULL, expr); p->op = TK_SELECT; p->pWhere = 0; pNew->pGroupBy = 0; @@ -5001,18 +5019,23 @@ selectExpander(Walker * pWalker, Select * p) continue; } } - pRight = - sqlExpr(db, TK_ID, - zName); + pRight = sql_expr_new_named(db, + TK_ID, zName); + if (pRight == NULL) + pParse->is_aborted = true; zColname = zName; zToFree = 0; if (longNames || pTabList->nSrc > 1) { Expr *pLeft; - pLeft = - sqlExpr(db, + pLeft = sql_expr_new_named( + db, TK_ID, zTabName); + if (pLeft == NULL) { + pParse-> + is_aborted = true; + } pExpr = sqlPExpr(pParse, TK_DOT, diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index e602abcc27..049d5aeada 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -3228,13 +3228,80 @@ void sqlClearTempRegCache(Parse *); #ifdef SQL_DEBUG int sqlNoTempsInRange(Parse *, int, int); #endif -Expr *sqlExprAlloc(sql *, int, const Token *, int); -Expr *sqlExpr(sql *, int, const char *); -Expr *sqlExprInteger(sql *, int); + +/** + * Construct a new expression. Memory for this node and for the + * token argument is a single allocation obtained from + * sqlDbMalloc(). The calling function is responsible for making + * sure the node eventually gets freed. + * + * Special case: If op==TK_INTEGER and token points to a string + * that can be translated into a 32-bit integer, then the token is + * not stored in u.zToken. Instead, the integer values is written + * into u.iValue and the EP_IntValue flag is set. No extra storage + * is allocated to hold the integer text. + * + * @param db The database connection. + * @param op Expression opcode (TK_*). + * @param token Source token. Might be NULL. + * @retval Not NULL New expression object on success. + * @retval NULL Otherwise. The diag message is set. + */ +struct Expr * +sql_expr_new(struct sql *db, int op, const struct Token *token); + +/** + * The same as @sa sql_expr_new, but normalizes name, stored in + * @a token. Quotes are removed if they are presented. + */ +struct Expr * +sql_expr_new_dequoted(struct sql *db, int op, const struct Token *token); + +/** + * The same as @a sql_expr_new, but takes const char instead of + * Token. Just sugar to do not touch tokens in many places. + */ +static inline struct Expr * +sql_expr_new_named(struct sql *db, int op, const char *name) +{ + struct Token name_token; + sqlTokenInit(&name_token, (char *)name); + return sql_expr_new(db, op, &name_token); +} + +/** + * The same as @a sql_expr_new, but a result expression has no + * name. + */ +static inline struct Expr * +sql_expr_new_anon(struct sql *db, int op) +{ + return sql_expr_new_named(db, op, NULL); +} + void sqlExprAttachSubtrees(sql *, Expr *, Expr *, Expr *); Expr *sqlPExpr(Parse *, int, Expr *, Expr *); void sqlPExprAddSelect(Parse *, Expr *, Select *); -Expr *sqlExprAnd(sql *, Expr *, Expr *); + +/** + * Join two expressions using an AND operator. If either + * expression is NULL, then just return the other expression. + * + * If one side or the other of the AND is known to be false, then + * instead of returning an AND expression, just return a constant + * expression with a value of false. + * + * @param db The database connection. + * @param left_expr The left-branch expresion to join. + * @param right_expr The right-branch expression to join. + * @retval Not NULL New expression root node pointer on success. + * @retval NULL Error. A diag message is set. + * @retval NULL Not an error. Both arguments were NULL. + */ +struct Expr * +sql_and_expr_new(struct sql *db, struct Expr *left_expr, + struct Expr *right_expr); + Expr *sqlExprFunction(Parse *, ExprList *, Token *); void sqlExprAssignVarNumber(Parse *, Expr *, u32); ExprList *sqlExprListAppendVector(Parse *, ExprList *, IdList *, Expr *); @@ -4751,7 +4818,21 @@ void sqlAppendChar(StrAccum *, int, char); char *sqlStrAccumFinish(StrAccum *); void sqlStrAccumReset(StrAccum *); void sqlSelectDestInit(SelectDest *, int, int, int); -Expr *sqlCreateColumnExpr(sql *, SrcList *, int, int); + +/* + * Create an expression to load @a column from datasource + * @a src_idx in @a src_list. + * + * @param db The database connection. + * @param src_list The source list described with FROM clause. + * @param src_idx The resource index to use in src_list. + * @param column The column index. + * @retval Not NULL Success. An expression to load @a column. + * @retval NULL Error. A diag message is set. + */ +struct Expr * +sql_expr_new_column(struct sql *db, struct SrcList *src_list, int src_idx, + int column); int sqlExprCheckIN(Parse *, Expr *); diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c index be30a40c0a..a453fe9790 100644 --- a/src/box/sql/wherecode.c +++ b/src/box/sql/wherecode.c @@ -1372,7 +1372,10 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about the W continue; testcase(pWC->a[iTerm].wtFlags & TERM_ORINFO); pExpr = sqlExprDup(db, pExpr, 0); - pAndExpr = sqlExprAnd(db, pAndExpr, pExpr); + pAndExpr = sql_and_expr_new(db, pAndExpr, + pExpr); + if (pAndExpr == NULL) + pParse->is_aborted = true; } if (pAndExpr) { pAndExpr = diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c index ec08889e77..1fb8fd29f2 100644 --- a/src/box/sql/whereexpr.c +++ b/src/box/sql/whereexpr.c @@ -307,8 +307,10 @@ like_optimization_is_valid(Parse *pParse, Expr *pExpr, Expr **ppPrefix, Expr *pPrefix; *pisComplete = c == MATCH_ALL_WILDCARD && z[cnt + 1] == 0; - pPrefix = sqlExpr(db, TK_STRING, z); - if (pPrefix) + pPrefix = sql_expr_new_named(db, TK_STRING, z); + if (pPrefix == NULL) + pParse->is_aborted = true; + else pPrefix->u.zToken[cnt] = 0; *ppPrefix = pPrefix; if (op == TK_VARIABLE) { @@ -1306,10 +1308,11 @@ exprAnalyze(SrcList * pSrc, /* the FROM clause */ Expr *pLeft = pExpr->pLeft; int idxNew; WhereTerm *pNewTerm; - - pNewExpr = sqlPExpr(pParse, TK_GT, - sqlExprDup(db, pLeft, 0), - sqlExprAlloc(db, TK_NULL, 0, 0)); + struct Expr *expr = sql_expr_new_anon(db, TK_NULL); + if (expr == NULL) + pParse->is_aborted = true; + pNewExpr = sqlPExpr(pParse, TK_GT, sqlExprDup(db, pLeft, 0), + expr); idxNew = whereClauseInsert(pWC, pNewExpr, TERM_VIRTUAL | TERM_DYNAMIC | @@ -1502,9 +1505,11 @@ sqlWhereTabFuncArgs(Parse * pParse, /* Parsing context */ * unused. */ assert(k < (int)space_def->field_count); - pColRef = sqlExprAlloc(pParse->db, TK_COLUMN, 0, 0); - if (pColRef == 0) + pColRef = sql_expr_new_anon(pParse->db, TK_COLUMN); + if (pColRef == NULL) { + pParse->is_aborted = true; return; + } pColRef->iTable = pItem->iCursor; pColRef->iColumn = k++; pColRef->space_def = space_def; -- GitLab