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 AEFD428F84 for ; Mon, 11 Mar 2019 11:04:46 -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 aFIYIOpwBc6n for ; Mon, 11 Mar 2019 11:04:46 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 9E9D028783 for ; Mon, 11 Mar 2019 11:04:45 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v2 6/7] sql: refactor sql_expr_create to set diag References: <645e4a4d0e2ddeaf747fe8a11affb52cb3157df3.1551265819.git.kshcherbatov@tarantool.org> <30714d52-a753-0d00-b28d-d05a1fd2a623@tarantool.org> From: Kirill Shcherbatov Message-ID: <23e9426a-d4f3-7954-f342-6a5c738d1cca@tarantool.org> Date: Mon, 11 Mar 2019 18:04:42 +0300 MIME-Version: 1.0 In-Reply-To: <30714d52-a753-0d00-b28d-d05a1fd2a623@tarantool.org> 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 > Probably it was said several times, but we use _new suffix > for constructors, creating an object on their own memory. If > you decided to rename sqlExprAlloc, then use sql_expr_new. > >> This change is necessary because the sql_expr_create body has a >> sqlNameFromToken call that will be changed in subsequent patches. Accounted here and everywhere. ====================================================== 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. Part of #3931 --- src/box/sql/build.c | 40 ++++-- src/box/sql/expr.c | 246 +++++++++++++++--------------------- src/box/sql/fk_constraint.c | 182 ++++++++++++++++---------- src/box/sql/parse.y | 35 +++-- src/box/sql/resolve.c | 46 ++++--- src/box/sql/select.c | 87 +++++++++---- src/box/sql/sqlInt.h | 84 +++++++++++- src/box/sql/wherecode.c | 8 +- src/box/sql/whereexpr.c | 21 +-- 9 files changed, 450 insertions(+), 299 deletions(-) diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 1864bd0ad..8eac9fdb3 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -624,9 +624,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, false); + if (expr == NULL) { + sql_parser_error(pParse); + 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, @@ -1369,12 +1372,14 @@ 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_op_expr_new(db, TK_ID, col_name); + if (col_name_expr == NULL) { + sql_parser_error(parse); return NULL; - struct Expr *col_value_expr = sqlExpr(db, TK_STRING, col_value); + } + struct Expr *col_value_expr = sql_op_expr_new(db, TK_STRING, col_value); if (col_value_expr == NULL) { + sql_parser_error(parse); sql_expr_delete(db, col_name_expr, false); return NULL; } @@ -1397,13 +1402,19 @@ vdbe_emit_stat_space_clear(struct Parse *parse, const char *stat_table_name, struct 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); + if (expr != NULL && (expr != NULL || where != NULL)) { + where = sql_and_expr_new(db, expr, where); + if (where == NULL) + sql_parser_error(parse); + } } if (table_name != NULL) { struct Expr *expr = sql_id_eq_str_expr(parse, "tbl", table_name); - if (expr != NULL) - where = sqlExprAnd(db, expr, where); + if (expr != NULL && (expr != NULL || where != NULL)) { + where = sql_and_expr_new(db, expr, where); + if (where == NULL) + sql_parser_error(parse); + } } /** * On memory allocation error sql_table delete_from @@ -2259,9 +2270,10 @@ 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, false); + if (expr == NULL) + goto tnt_error; + col_list = sql_expr_list_append(parse->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 44b6ce11f..f5b2926c8 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -149,16 +149,17 @@ 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 = + sql_expr_new(pParse->db, TK_COLLATE, pCollName, dequote); + if (new_expr == NULL) { + sql_parser_error(pParse); + return NULL; } + new_expr->pLeft = pExpr; + new_expr->flags |= EP_Collate | EP_Skip; + pExpr = new_expr; return pExpr; } @@ -854,113 +855,62 @@ 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 Token *token, bool dequote) { - 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'; + if (dequote) { + if (expr->u.zToken[0] == '"') + expr->flags |= EP_DblQuoted; + if (expr->op == TK_ID || expr->op == TK_COLLATE || + expr->op == TK_FUNCTION) + sqlNormalizeName(expr->u.zToken); + else + sqlDequote(expr->u.zToken); + } + } + 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_op_expr_new(struct sql *db, int op, const char *name) { - Expr *ret = sqlExpr(db, TK_INTEGER, NULL); - if (ret != NULL) { - ret->flags = EP_IntValue; - ret->u.iValue = value; - } - return ret; + struct Token name_token; + name_token.z = name; + name_token.n = name != NULL ? strlen(name) : 0; + return sql_expr_new(db, op, &name_token, false); } /* @@ -1006,8 +956,15 @@ sqlPExpr(Parse * pParse, /* Parsing context */ { Expr *p; if (op == TK_AND && pParse->nErr == 0) { - /* Take advantage of short-circuit false optimization for AND */ - p = sqlExprAnd(pParse->db, pLeft, pRight); + /* + * Take advantage of short-circuit false + * optimization for AND. + */ + if (pLeft == NULL && pRight == NULL) + return NULL; + p = sql_and_expr_new(pParse->db, pLeft, pRight); + if (p == NULL) + sql_parser_error(pParse); } else { p = sqlDbMallocRawNN(pParse->db, sizeof(Expr)); if (p) { @@ -1076,30 +1033,24 @@ 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) { + assert(right_expr != NULL); + return right_expr; + } else if (right_expr == NULL) { + assert(left_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], false); } else { - Expr *pNew = sqlExprAlloc(db, TK_AND, 0, 0); - sqlExprAttachSubtrees(db, pNew, pLeft, pRight); - return pNew; + struct Expr *new_expr = sql_expr_new(db, TK_AND, NULL, false); + sqlExprAttachSubtrees(db, new_expr, left_expr, right_expr); + return new_expr; } } @@ -1110,18 +1061,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(db, TK_FUNCTION, pToken, true); + if (new_expr == NULL) { + sql_expr_list_delete(db, pList); + sql_parser_error(pParse); + 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; } /* @@ -2911,10 +2862,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], false); + if (pSel->pLimit == NULL) { + sql_parser_error(pParse); + } else { ExprSetProperty(pSel->pLimit, EP_System); } diff --git a/src/box/sql/fk_constraint.c b/src/box/sql/fk_constraint.c index d1eb6670a..ceba2db9f 100644 --- a/src/box/sql/fk_constraint.c +++ b/src/box/sql/fk_constraint.c @@ -309,31 +309,30 @@ fk_constraint_lookup_parent(struct Parse *parse_context, struct space *parent, * 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 parser The parsing 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. + * @param reg_base Contents of table table. + * @param column Index of table column is desired. + * @retval Not NULL expression pointer on success. + * @retval NULL Otherwise. */ -static Expr * -space_field_register(Parse *pParse, struct space_def *def, int regBase, - i16 iCol) +static struct Expr * +sql_register_expr_new(struct Parse *parser, struct space_def *def, int reg_base, + int 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; - } + struct Expr *expr = sql_op_expr_new(parser->db, TK_REGISTER, NULL); + if (expr == NULL) { + sql_parser_error(parser); + return NULL; + } + if (column >= 0) { + expr->iTable = reg_base + column + 1; + expr->type = def->fields[column].type; + } else { + expr->iTable = reg_base; + expr->type = FIELD_TYPE_INTEGER; } - return pExpr; + return expr; } /** @@ -346,16 +345,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_column_cursor_expr_create(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_op_expr_new(db, TK_COLUMN, NULL); + if (expr == NULL) + return NULL; + expr->space_def = def; + expr->iTable = cursor; + expr->iColumn = column; + return expr; } /* @@ -435,12 +435,18 @@ 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_register_expr_new(parser, 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_op_expr_new(db, TK_ID, field_name); + if (chexpr == NULL) + sql_parser_error(parser); struct Expr *eq = sqlPExpr(parser, TK_EQ, pexpr, chexpr); - where = sqlExprAnd(db, where, eq); + if (where != NULL || eq != NULL) { + where = sql_and_expr_new(db, where, eq); + if (where == NULL) + sql_parser_error(parser); + } } /* @@ -456,15 +462,26 @@ 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_register_expr_new(parser, def, reg_data, + fieldno); + int cursor = src->a[0].iCursor; + chexpr = sql_column_cursor_expr_create(db, def, cursor, + fieldno); + if (chexpr == NULL) + sql_parser_error(parser); eq = sqlPExpr(parser, TK_EQ, pexpr, chexpr); - expr = sqlExprAnd(db, expr, eq); + if (expr != NULL || eq != NULL) { + expr = sql_and_expr_new(db, expr, eq); + if (expr == NULL) + sql_parser_error(parser); + } } struct Expr *pNe = sqlPExpr(parser, TK_NOT, expr, 0); - where = sqlExprAnd(db, where, pNe); + if (where != NULL || pNe != NULL) { + where = sql_and_expr_new(db, where, pNe); + if (where == NULL) + sql_parser_error(parser); + } } /* Resolve the references in the WHERE clause. */ @@ -785,14 +802,26 @@ 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 *old_expr = NULL, *new_expr = NULL, *expr = NULL; + old_expr = sql_expr_new(db, TK_ID, &t_old, false); + if (old_expr == NULL) + sql_parser_error(pParse); + expr = sql_expr_new(db, TK_ID, &t_to_col, false); + if (expr == NULL) + sql_parser_error(pParse); + struct Expr *from_col_expr = + sql_expr_new(db, TK_ID, &t_from_col, false); + if (from_col_expr == NULL) + sql_parser_error(pParse); + struct Expr *to_col_expr = + sqlPExpr(pParse, TK_DOT, old_expr, expr); + struct Expr *eq = + sqlPExpr(pParse, TK_EQ, to_col_expr, from_col_expr); + if (where != NULL || eq != NULL) { + where = sql_and_expr_new(db, where, eq); + if (where == NULL) + sql_parser_error(pParse); + } /* * For ON UPDATE, construct the next term of the @@ -810,12 +839,22 @@ fk_constraint_action_trigger(struct Parse *pParse, struct space_def *def, * no_action_needed(colN)) */ if (is_update) { + old_expr = sql_expr_new(db, TK_ID, &t_old, false); + if (old_expr == NULL) + sql_parser_error(pParse); + expr = sql_expr_new(db, TK_ID, &t_to_col, false); + if (expr == NULL) + sql_parser_error(pParse); struct Expr *old_val = sqlPExpr(pParse, TK_DOT, - sqlExprAlloc(db, TK_ID, &t_old, 0), - sqlExprAlloc(db, TK_ID, &t_to_col, 0)); + old_expr, expr); + new_expr = sql_expr_new(db, TK_ID, &t_new, false); + if (new_expr == NULL) + sql_parser_error(pParse); + expr = sql_expr_new(db, TK_ID, &t_to_col, false); + if (expr == NULL) + sql_parser_error(pParse); struct Expr *new_val = sqlPExpr(pParse, TK_DOT, - sqlExprAlloc(db, TK_ID, &t_new, 0), - sqlExprAlloc(db, TK_ID, &t_to_col, 0)); + new_expr, expr); struct Expr *old_is_null = sqlPExpr( pParse, TK_ISNULL, sqlExprDup(db, old_val, 0), NULL); @@ -828,29 +867,41 @@ fk_constraint_action_trigger(struct Parse *pParse, struct space_def *def, struct Expr *no_action_needed = sqlPExpr(pParse, TK_OR, old_is_null, non_null_eq); - when = sqlExprAnd(db, when, no_action_needed); + if (when != NULL || no_action_needed != NULL) { + when = sql_and_expr_new(db, when, + no_action_needed); + if (when == NULL) + sql_parser_error(pParse); + } } if (action != FKEY_ACTION_RESTRICT && (action != FKEY_ACTION_CASCADE || is_update)) { struct Expr *new, *d; 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)); + new_expr = sql_expr_new(db, TK_ID, &t_new, + false); + if (new_expr == NULL) + sql_parser_error(pParse); + expr = sql_expr_new(db, TK_ID, &t_to_col, + false); + if (expr == NULL) + sql_parser_error(pParse); + new = sqlPExpr(pParse, TK_DOT, new_expr, expr); } 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(db, TK_NULL, NULL, + false); + if (new == NULL) + sql_parser_error(pParse); } } else { - new = sqlExprAlloc(db, TK_NULL, NULL, 0); + new = sql_expr_new(db, TK_NULL, NULL, false); + if (new == NULL) + sql_parser_error(pParse); } list = sql_expr_list_append(db, list, new); sqlExprListSetName(pParse, list, &t_from_col, 0); @@ -864,9 +915,12 @@ 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_op_expr_new(db, TK_RAISE, + "FOREIGN KEY constraint failed"); + if (r == NULL) + sql_parser_error(pParse); + 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 f0388b481..1a5882dda 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -529,12 +529,16 @@ 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_op_expr_new(pParse->db, TK_ASTERISK, NULL); + if (p == NULL) + sql_parser_error(pParse); A = sql_expr_list_append(pParse->db, A, p); } selcollist(A) ::= sclp(A) nm(X) DOT STAR. { Expr *pRight = sqlPExpr(pParse, TK_ASTERISK, 0, 0); - Expr *pLeft = sqlExprAlloc(pParse->db, TK_ID, &X, 1); + struct Expr *pLeft = sql_expr_new(pParse->db, TK_ID, &X, true); + if (pLeft == NULL) + sql_parser_error(pParse); Expr *pDot = sqlPExpr(pParse, TK_DOT, pLeft, pRight); A = sql_expr_list_append(pParse->db,A, pDot); } @@ -887,15 +891,21 @@ 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(pParse->db, TK_ID, &X, true); + if (temp1 == NULL) + sql_parser_error(pParse); + struct Expr *temp2 = sql_expr_new(pParse->db, TK_ID, &Y, true); + if (temp2 == NULL) + sql_parser_error(pParse); 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(pParse->db, TK_INTEGER, &X, true); + if (A.pExpr == NULL) + sql_parser_error(pParse); A.pExpr->type = FIELD_TYPE_INTEGER; A.zStart = X.z; A.zEnd = X.z + X.n; @@ -928,7 +938,9 @@ 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(pParse->db, TK_CAST, NULL, true); + if (A.pExpr == NULL) + sql_parser_error(pParse); A.pExpr->type = T.type; sqlExprAttachSubtrees(pParse->db, A.pExpr, E.pExpr, 0); } @@ -1122,7 +1134,9 @@ 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(pParse->db, TK_INTEGER, &sqlIntTokens[N], true); + if (A.pExpr == NULL) + sql_parser_error(pParse); }else if( Y->nExpr==1 ){ /* Expressions of the form: ** @@ -1454,10 +1468,11 @@ 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 = sql_expr_new(pParse->db, TK_RAISE, &Z, true); + if(A.pExpr == NULL) + sql_parser_error(pParse); + else 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 aed9e261d..66a795bb4 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -487,26 +487,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_column_expr_new(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(db, TK_COLUMN, NULL, false); + 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; } /* @@ -1000,9 +994,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) + Expr *pNew = + sql_op_expr_new(db, TK_INTEGER, NULL); + if (pNew == NULL) { + sql_parser_error(pParse); return 1; + } pNew->flags |= EP_IntValue; pNew->u.iValue = iCol; if (pItem->pExpr == pE) { @@ -1348,9 +1345,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], false); + if (p->pLimit == NULL) + sql_parser_error(pParse); } else { if (sqlResolveExprNames(&sNC, p->pHaving)) return WRC_Abort; diff --git a/src/box/sql/select.c b/src/box/sql/select.c index fda4296cc..5434a2ab0 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_op_expr_new(db, TK_ASTERISK, NULL); + if (expr == NULL) + sql_parser_error(pParse); + pEList = sql_expr_list_append(pParse->db, NULL, expr); } struct session MAYBE_UNUSED *user_session; user_session = current_session(); @@ -486,7 +488,6 @@ 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; Expr *pEq; @@ -496,8 +497,12 @@ 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); + pE1 = sql_column_expr_new(pParse->db, pSrc, iLeft, iColLeft); + if (pE1 == NULL) + sql_parser_error(pParse); + pE2 = sql_column_expr_new(pParse->db, pSrc, iRight, iColRight); + if (pE2 == NULL) + sql_parser_error(pParse); pEq = sqlPExpr(pParse, TK_EQ, pE1, pE2); if (pEq && isOuterJoin) { @@ -506,7 +511,12 @@ addWhereTerm(Parse * pParse, /* Parsing context */ ExprSetVVAProperty(pEq, EP_NoReduce); pEq->iRightJoinTable = (i16) pE2->iTable; } - *ppWhere = sqlExprAnd(db, *ppWhere, pEq); + if (*ppWhere != NULL || pEq != NULL) { + *ppWhere = sql_and_expr_new(pParse->db, *ppWhere, pEq); + if (*ppWhere == NULL) + sql_parser_error(pParse); + } + return; } /* @@ -627,8 +637,13 @@ sqlProcessJoin(Parse * pParse, Select * p) if (pRight->pOn) { if (isOuter) setJoinExpr(pRight->pOn, pRight->iCursor); - p->pWhere = - sqlExprAnd(pParse->db, p->pWhere, pRight->pOn); + if (p->pWhere != NULL || pRight->pOn != NULL) { + p->pWhere = + sql_and_expr_new(pParse->db, p->pWhere, + pRight->pOn); + if (p->pWhere == NULL) + sql_parser_error(pParse); + } pRight->pOn = 0; } @@ -3332,9 +3347,12 @@ multiSelectOrderBy(Parse * pParse, /* Parsing context */ break; } if (j == nOrderBy) { - Expr *pNew = sqlExpr(db, TK_INTEGER, 0); - if (pNew == 0) + Expr *pNew = + sql_op_expr_new(db, TK_INTEGER, NULL); + if (pNew == NULL) { + sql_parser_error(pParse); return SQL_NOMEM; + } pNew->flags |= EP_IntValue; pNew->u.iValue = i; pOrderBy = sql_expr_list_append(pParse->db, @@ -4207,17 +4225,22 @@ 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) + sql_parser_error(pParse); + } 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) + sql_parser_error(pParse); } substSelect(pParse, pParent, iParent, pSub->pEList, 0); @@ -4322,8 +4345,13 @@ 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); + if (pSubq->pWhere != NULL || pNew != NULL) { + pSubq->pWhere = + sql_and_expr_new(pParse->db, + pSubq->pWhere, pNew); + if (pSubq->pWhere == NULL) + sql_parser_error(pParse); + } pSubq = pSubq->pPrior; } } @@ -4504,8 +4532,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_op_expr_new(db, TK_ASTERISK, NULL); + if (expr == NULL) + sql_parser_error(pParse); + p->pEList = sql_expr_list_append(pParse->db, NULL, expr); p->op = TK_SELECT; p->pWhere = 0; pNew->pGroupBy = 0; @@ -4989,18 +5019,23 @@ selectExpander(Walker * pWalker, Select * p) continue; } } - pRight = - sqlExpr(db, TK_ID, - zName); + pRight = sql_op_expr_new(db, + TK_ID, zName); + if (pRight == NULL) + sql_parser_error(pParse); zColname = zName; zToFree = 0; if (longNames || pTabList->nSrc > 1) { Expr *pLeft; - pLeft = - sqlExpr(db, + pLeft = sql_op_expr_new( + db, TK_ID, zTabName); + if (pLeft == NULL) { + sql_parser_error( + pParse); + } pExpr = sqlPExpr(pParse, TK_DOT, diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 4e6c5717b..4401ea8b7 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -3218,13 +3218,73 @@ 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); + +/** + * 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 token 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 token 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 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 and the dequote flag is + * ignored. + * + * @param db The database connection. + * @param op Expression opcode (TK_*). + * @param token Source token. Might be NULL. + * @param dequote True to dequote string. + * @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 Token *token, bool dequote); + +/** + * Allocate a new expression node from a zero-terminated token + * that has already been dequoted. + * + * @param db The database connection. + * @param op Expression opcode. + * @param name The object name string. + * @retval Not NULL expression pointer on success. + * @retval NULL Otherwise. The diag message is set. + */ +struct Expr * +sql_op_expr_new(struct sql *db, int op, const char *name); + 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 Otherwise. The diag message is set. + */ +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 *); @@ -4732,7 +4792,21 @@ void sqlAppendChar(StrAccum *, int, char); char *sqlStrAccumFinish(StrAccum *); void sqlStrAccumReset(StrAccum *); void sqlSelectDestInit(SelectDest *, int, int, int); -Expr *sqlCreateColumnExpr(sql *, SrcList *, int, int); + +/* + * Allocate and return a pointer to an expression to load the + * column from datasource src_idx in SrcList 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 expression pointer on success. + * @retval NULL Otherwise. The diag message is set. + */ +struct Expr * +sql_column_expr_new(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 018fd8a28..9ea02970a 100644 --- a/src/box/sql/wherecode.c +++ b/src/box/sql/wherecode.c @@ -1436,7 +1436,13 @@ 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); + if (pAndExpr != NULL || pExpr != NULL) { + pAndExpr = + sql_and_expr_new(db, pAndExpr, + pExpr); + if (pAndExpr == NULL) + sql_parser_error(pParse); + } } if (pAndExpr) { pAndExpr = diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c index 6df28ad8a..fce2d1d54 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_op_expr_new(db, TK_STRING, z); + if (pPrefix == NULL) + sql_parser_error(pParse); + 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(db, TK_NULL, NULL, false); + if (expr == NULL) + sql_parser_error(pParse); + 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 */ space_def->name, j); return; } - pColRef = sqlExprAlloc(pParse->db, TK_COLUMN, 0, 0); - if (pColRef == 0) + pColRef = sql_expr_new(pParse->db, TK_COLUMN, NULL, false); + if (pColRef == NULL) { + sql_parser_error(pParse); return; + } pColRef->iTable = pItem->iCursor; pColRef->iColumn = k++; pColRef->space_def = space_def; -- 2.21.0