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 3635E27D67 for ; Wed, 27 Feb 2019 06:13:26 -0500 (EST) 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 RVv0n7f9TStN for ; Wed, 27 Feb 2019 06:13:26 -0500 (EST) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 91CFE27DE9 for ; Wed, 27 Feb 2019 06:13:25 -0500 (EST) From: Kirill Shcherbatov Subject: [tarantool-patches] [PATCH v2 6/7] sql: refactor sql_expr_create to set diag Date: Wed, 27 Feb 2019 14:13:17 +0300 Message-Id: <645e4a4d0e2ddeaf747fe8a11affb52cb3157df3.1551265819.git.kshcherbatov@tarantool.org> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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, v.shpilevoy@tarantool.org Cc: Kirill Shcherbatov Refactored sql_expr_create routine to use diag_set in case of memory allocation error. This change is necessary because the sql_expr_create body has a sqlNameFromToken call that will be changed in subsequent patches. Needed for #3931 --- src/box/sql/build.c | 44 +++++-- src/box/sql/expr.c | 249 +++++++++++++++--------------------- src/box/sql/fk_constraint.c | 185 +++++++++++++++++---------- src/box/sql/parse.y | 35 +++-- src/box/sql/resolve.c | 47 ++++--- src/box/sql/select.c | 90 +++++++++---- src/box/sql/sqlInt.h | 80 +++++++++++- src/box/sql/wherecode.c | 9 +- src/box/sql/whereexpr.c | 22 ++-- 9 files changed, 462 insertions(+), 299 deletions(-) diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 2084dbfeb..24f20836b 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -627,9 +627,11 @@ 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_create(db, TK_ID, &token, false); + if (expr == NULL) + goto tnt_error; + 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, @@ -659,6 +661,9 @@ sqlAddPrimaryKey(Parse * pParse, /* Parsing context */ primary_key_exit: sql_expr_list_delete(pParse->db, pList); return; +tnt_error: + sql_parser_error(pParse); + goto primary_key_exit; } void @@ -1372,12 +1377,15 @@ 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_create(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_create(db, TK_STRING, col_value); if (col_value_expr == NULL) { + sql_parser_error(parse); sql_expr_delete(db, col_name_expr, false); return NULL; } @@ -1400,13 +1408,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_create(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_create(db, expr, where); + if (where == NULL) + sql_parser_error(parse); + } } /** * On memory allocation error sql_table delete_from @@ -2265,9 +2279,11 @@ 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_create(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 fa2ff1ab3..dd5e2c28d 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_create(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_create(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_create(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_create(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_create(pParse->db, pLeft, pRight); + if (p == NULL) + sql_parser_error(pParse); } else { p = sqlDbMallocRawNN(pParse->db, sizeof(Expr)); if (p) { @@ -1076,30 +1033,26 @@ 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_create(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_create(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_create(db, TK_AND, NULL, false); + sqlExprAttachSubtrees(db, new_expr, left_expr, right_expr); + return new_expr; } } @@ -1110,18 +1063,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_create(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 +2864,12 @@ 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_create(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 4f761d610..c81eb4ac1 100644 --- a/src/box/sql/fk_constraint.c +++ b/src/box/sql/fk_constraint.c @@ -308,32 +308,30 @@ fk_constraint_lookup_parent(struct Parse *parse_context, struct space *parent, * 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 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 no 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_create(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_create(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 +344,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_create(db, TK_COLUMN, NULL); + if (expr == NULL) + return NULL; + expr->space_def = def; + expr->iTable = cursor; + expr->iColumn = column; + return expr; } /* @@ -435,12 +434,20 @@ 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_create(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_create(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_create(db, where, eq); + if (where == NULL) + sql_parser_error(parser); + } } /* @@ -456,15 +463,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_create(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_create(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_create(db, where, pNe); + if (where == NULL) + sql_parser_error(parser); + } } /* Resolve the references in the WHERE clause. */ @@ -785,14 +803,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_create(db, TK_ID, &t_old, false); + if (old_expr == NULL) + sql_parser_error(pParse); + expr = sql_expr_create(db, TK_ID, &t_to_col, false); + if (expr == NULL) + sql_parser_error(pParse); + struct Expr *from_col_expr = + sql_expr_create(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_create(db, where, eq); + if (where == NULL) + sql_parser_error(pParse); + } /* * For ON UPDATE, construct the next term of the @@ -810,12 +840,22 @@ fk_constraint_action_trigger(struct Parse *pParse, struct space_def *def, * no_action_needed(colN)) */ if (is_update) { + old_expr = sql_expr_create(db, TK_ID, &t_old, false); + if (old_expr == NULL) + sql_parser_error(pParse); + expr = sql_expr_create(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_create(db, TK_ID, &t_new, false); + if (new_expr == NULL) + sql_parser_error(pParse); + expr = sql_expr_create(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 +868,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_create(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_create(db, TK_ID, &t_new, + false); + if (new_expr == NULL) + sql_parser_error(pParse); + expr = sql_expr_create(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_create(db, TK_NULL, + NULL, false); + if (new == NULL) + sql_parser_error(pParse); } } else { - new = sqlExprAlloc(db, TK_NULL, NULL, 0); + new = sql_expr_create(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 +916,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_create(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 db03714a9..d25995ec0 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_create(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_create(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_create(pParse->db, TK_ID, &X, true); + if (temp1 == NULL) + sql_parser_error(pParse); + struct Expr *temp2 = sql_expr_create(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_create(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_create(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); } @@ -1118,7 +1130,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_create(pParse->db, TK_INTEGER, &sqlIntTokens[N], true); + if (A.pExpr == NULL) + sql_parser_error(pParse); }else if( Y->nExpr==1 ){ /* Expressions of the form: ** @@ -1462,10 +1476,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_create(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..3731aee31 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_create(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_create(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,13 @@ 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_create(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 +1346,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_create(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 7eea45dd7..2ae27e6c4 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -164,8 +164,11 @@ 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_create(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 +489,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 +498,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_create(pParse->db, pSrc, iLeft, iColLeft); + if (pE1 == NULL) + sql_parser_error(pParse); + pE2 = sql_column_expr_create(pParse->db, pSrc, iRight, iColRight); + if (pE2 == NULL) + sql_parser_error(pParse); pEq = sqlPExpr(pParse, TK_EQ, pE1, pE2); if (pEq && isOuterJoin) { @@ -506,7 +512,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_create(pParse->db, *ppWhere, pEq); + if (*ppWhere == NULL) + sql_parser_error(pParse); + } + return; } /* @@ -627,8 +638,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_create(pParse->db, + p->pWhere, + pRight->pOn); + if (p->pWhere == NULL) + sql_parser_error(pParse); + } pRight->pOn = 0; } @@ -3332,9 +3348,13 @@ multiSelectOrderBy(Parse * pParse, /* Parsing context */ break; } if (j == nOrderBy) { - Expr *pNew = sqlExpr(db, TK_INTEGER, 0); - if (pNew == 0) + Expr *pNew = + sql_op_expr_create(db, TK_INTEGER, + NULL); + if (pNew == NULL) { + sql_parser_error(pParse); return SQL_NOMEM_BKPT; + } pNew->flags |= EP_IntValue; pNew->u.iValue = i; pOrderBy = sql_expr_list_append(pParse->db, @@ -4207,17 +4227,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_create(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_create(db, pWhere, + pParent->pWhere); + if (pParent->pWhere == NULL) + sql_parser_error(pParse); } substSelect(pParse, pParent, iParent, pSub->pEList, 0); @@ -4322,8 +4348,14 @@ 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_create(pParse->db, + pSubq->pWhere, + pNew); + if (pSubq->pWhere == NULL) + sql_parser_error(pParse); + } pSubq = pSubq->pPrior; } } @@ -4505,8 +4537,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_create(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; @@ -4990,18 +5024,24 @@ selectExpander(Walker * pWalker, Select * p) continue; } } - pRight = - sqlExpr(db, TK_ID, + pRight = sql_op_expr_create( + 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_create( + 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 531445f33..82aad0077 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -3254,13 +3254,70 @@ 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. + */ +struct Expr * +sql_expr_create(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. + */ +struct Expr * +sql_op_expr_create(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. + */ +struct Expr * +sql_and_expr_create(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 *); @@ -4724,7 +4781,20 @@ 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. + */ +struct Expr * +sql_column_expr_create(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 04b79ab36..01ecb9464 100644 --- a/src/box/sql/wherecode.c +++ b/src/box/sql/wherecode.c @@ -1437,7 +1437,14 @@ 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_create(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 fa906e305..c2213f229 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_create(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,12 @@ 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_create(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 +1506,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_create(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.20.1