[tarantool-patches] Re: [PATCH v2 6/7] sql: refactor sql_expr_create to set diag

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

> 1. Now you have a leak - parse.y:935 expects that sqlExprAddCollateToken on
> an error returns the old value, that is freed later, but after your patch
> the old value is lost.
You are right. Return old value in case of error.
	
	struct Expr *new_expr =
		sql_expr_new(pParse->db, TK_COLLATE, pCollName, dequote);
	if (new_expr == NULL) {
		pParse->is_aborted = true;
		return pExpr;
	}

>> +	struct Token name_token;
>> +	name_token.z = name;
>> +	name_token.n = name != NULL ? strlen(name) : 0;
> 
> 2. sqlTokenInit.
Ok.

>>    * the data for given space.  regBase+1 holds the first column.
>>    * regBase+2 holds the second column, and so forth.
>>    *
> 
> 3. Now the names above are outdated.
Fixed.
>>   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);
> 
> 4. The same as about other similar places. Why not 'return'?
Already answered in previous patch. Don't mind of return here.

> +	return;
> 
> 5. Why do you need a return at the end of a 'void' function?
Dropped here and in other places. It's a peace of dropped goto

>> +/*
>> + * 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.
> 
> 6. I am still not sure, if I said that earlier, but start
> sentences with a capital letter. It is a simple rule. Here
> and in all other places below and above.
"Not NULL expression"... 
okay
"Not NULL Expression"

=========================================

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         |  42 ++++--
 src/box/sql/expr.c          | 248 ++++++++++++++----------------------
 src/box/sql/fk_constraint.c | 190 +++++++++++++++++----------
 src/box/sql/parse.y         |  51 ++++++--
 src/box/sql/resolve.c       |  46 ++++---
 src/box/sql/select.c        |  86 +++++++++----
 src/box/sql/sqlInt.h        |  84 +++++++++++-
 src/box/sql/wherecode.c     |   8 +-
 src/box/sql/whereexpr.c     |  21 +--
 9 files changed, 471 insertions(+), 305 deletions(-)

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index f52cfd7dd..f527928bf 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -616,9 +616,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) {
+			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,
@@ -1355,13 +1358,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_new(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_op_expr_new(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);
@@ -1383,13 +1388,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)
+				parse->is_aborted = true;
+		}
 	}
 	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)
+				parse->is_aborted = true;
+		}
 	}
 	/**
 	 * On memory allocation error sql_table delete_from
@@ -2245,9 +2256,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, false);
+		if (expr == NULL) {
+			parse->is_aborted = true;
+			goto exit_create_index;
+		}
+		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 0457ff833..dad4ce3a6 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -149,17 +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;
-		}
-	}
-	return pExpr;
+	if (pCollName->n == 0)
+		return pExpr;
+	struct Expr *new_expr =
+		sql_expr_new(pParse->db, TK_COLLATE, pCollName, dequote);
+	if (new_expr == NULL) {
+		pParse->is_aborted = true;
+		return pExpr;
+	}
+	new_expr->pLeft = pExpr;
+	new_expr->flags |= EP_Collate | EP_Skip;
+	return new_expr;
 }
 
 Expr *
@@ -875,113 +875,61 @@ 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;
+	sqlTokenInit(&name_token, (char *)name);
+	return sql_expr_new(db, op, &name_token, false);
 }
 
 /*
@@ -1027,8 +975,15 @@ 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.
+		 */
+		if (pLeft == NULL && pRight == NULL)
+			return NULL;
+		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) {
@@ -1097,30 +1052,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;
 	}
 }
 
@@ -1131,18 +1080,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);
+		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;
 }
 
 /*
@@ -2953,10 +2902,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) {
+					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 cc4dc6448..3b27b2d4f 100644
--- a/src/box/sql/fk_constraint.c
+++ b/src/box/sql/fk_constraint.c
@@ -303,37 +303,36 @@ 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.
+ * corresponding to column 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.
+ * reg_base is the first of an array of register that contains
+ * the data for given space. reg_base+1 holds the first column.
+ * reg_base+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) {
+		parser->is_aborted = true;
+		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)
+			parser->is_aborted = true;
 		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)
+				parser->is_aborted = true;
+		}
 	}
 
 	/*
@@ -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)
+				parser->is_aborted = true;
 			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)
+					parser->is_aborted = true;
+			}
 		}
 		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)
+				parser->is_aborted = true;
+		}
 	}
 
 	/* 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)
+			pParse->is_aborted = true;
+		expr = sql_expr_new(db, TK_ID, &t_to_col, false);
+		if (expr == NULL)
+			pParse->is_aborted = true;
+		struct Expr *from_col_expr =
+			sql_expr_new(db, TK_ID, &t_from_col, false);
+		if (from_col_expr == NULL)
+			pParse->is_aborted = true;
+		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)
+				pParse->is_aborted = true;
+		}
 
 		/*
 		 * 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)
+				pParse->is_aborted = true;
+			expr = sql_expr_new(db, TK_ID, &t_to_col, false);
+			if (expr == NULL)
+				pParse->is_aborted = true;
 			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)
+				pParse->is_aborted = true;
+			expr = sql_expr_new(db, TK_ID, &t_to_col, false);
+			if (expr == NULL)
+				pParse->is_aborted = true;
 			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)
+					pParse->is_aborted = true;
+			}
 		}
 
 		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)
+					pParse->is_aborted = true;
+				expr = sql_expr_new(db, TK_ID, &t_to_col,
+						    false);
+				if (expr == NULL)
+					pParse->is_aborted = true;
+				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)
+						pParse->is_aborted = true;
 				}
 			} else {
-				new = sqlExprAlloc(db, TK_NULL, NULL, 0);
+				new = sql_expr_new(db, TK_NULL, NULL, false);
+				if (new == NULL)
+					pParse->is_aborted = true;
 			}
 			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)
+			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 194af99b7..c48cbd6f9 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -536,12 +536,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_op_expr_new(pParse->db, TK_ASTERISK, NULL);
+  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(pParse->db, TK_ID, &X, true);
+  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);
 }
@@ -898,15 +906,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(pParse->db, TK_ID, &X, true);
+  if (temp1 == NULL) {
+    pParse->is_aborted = true;
+    return;
+  }
+  struct Expr *temp2 = sql_expr_new(pParse->db, TK_ID, &Y, true);
+  if (temp2 == NULL) {
+    sql_expr_delete(pParse->db, temp2, 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(pParse->db, TK_INTEGER, &X, true);
+  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;
@@ -942,7 +963,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(pParse->db, TK_CAST, NULL, true);
+  if (A.pExpr == NULL) {
+    pParse->is_aborted = true;
+    return;
+  }
   A.pExpr->type = T.type;
   sqlExprAttachSubtrees(pParse->db, A.pExpr, E.pExpr, 0);
 }
@@ -1136,7 +1161,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(pParse->db, TK_INTEGER, &sqlIntTokens[N], true);
+    if (A.pExpr == NULL) {
+      pParse->is_aborted = true;
+      return;
+    }
   }else if( Y->nExpr==1 ){
     /* Expressions of the form:
     **
@@ -1477,10 +1506,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(pParse->db, TK_RAISE, &Z, true);
+  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 94bb0affc..eb74d024c 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)
+				struct Expr *pNew =
+					sql_op_expr_new(db, TK_INTEGER, NULL);
+				if (pNew == NULL) {
+					pParse->is_aborted = true;
 					return 1;
+				}
 				pNew->flags |= EP_IntValue;
 				pNew->u.iValue = iCol;
 				if (pItem->pExpr == pE) {
@@ -1351,9 +1348,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)
+				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 1b7d52b68..5bba0b5d0 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)
+			pParse->is_aborted = true;
+		pEList = sql_expr_list_append(pParse->db, NULL, expr);
 	}
 	struct session MAYBE_UNUSED *user_session;
 	user_session = current_session();
@@ -487,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;
@@ -497,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_new(pParse->db, pSrc, iLeft, iColLeft);
+	if (pE1 == NULL)
+		pParse->is_aborted = true;
+	pE2 = sql_column_expr_new(pParse->db, pSrc, iRight, iColRight);
+	if (pE2 == NULL)
+		pParse->is_aborted = true;
 
 	pEq = sqlPExpr(pParse, TK_EQ, pE1, pE2);
 	if (pEq && isOuterJoin) {
@@ -507,7 +512,11 @@ 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)
+			pParse->is_aborted = true;
+	}
 }
 
 /*
@@ -628,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)
+					pParse->is_aborted = true;
+			}
 			pRight->pOn = 0;
 		}
 
@@ -3334,9 +3348,12 @@ multiSelectOrderBy(Parse * pParse,	/* Parsing context */
 					break;
 			}
 			if (j == nOrderBy) {
-				Expr *pNew = sqlExpr(db, TK_INTEGER, 0);
-				if (pNew == 0)
+				struct Expr *pNew =
+					sql_op_expr_new(db, TK_INTEGER, NULL);
+				if (pNew == NULL) {
+					pParse->is_aborted = true;
 					return SQL_NOMEM;
+				}
 				pNew->flags |= EP_IntValue;
 				pNew->u.iValue = i;
 				pOrderBy = sql_expr_list_append(pParse->db,
@@ -4209,17 +4226,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)
+					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);
 
@@ -4324,8 +4346,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)
+					pParse->is_aborted = true;
+			}
 			pSubq = pSubq->pPrior;
 		}
 	}
@@ -4506,8 +4533,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)
+		pParse->is_aborted = true;
+	p->pEList = sql_expr_list_append(pParse->db, NULL, expr);
 	p->op = TK_SELECT;
 	p->pWhere = 0;
 	pNew->pGroupBy = 0;
@@ -4991,18 +5020,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)
+							pParse->is_aborted = true;
 						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) {
+								pParse->
+								is_aborted = true;
+							}
 							pExpr =
 							    sqlPExpr(pParse,
 									 TK_DOT,
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index d3ffa9d9b..13d0b16f8 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -3223,13 +3223,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 *);
@@ -4750,7 +4810,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 be30a40c0..036540d61 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -1372,7 +1372,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)
+						pParse->is_aborted = true;
+				}
 			}
 			if (pAndExpr) {
 				pAndExpr =
diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c
index 6df28ad8a..a9dec8e99 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)
+				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(db, TK_NULL, NULL, false);
+		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 */
 					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) {
+			pParse->is_aborted = true;
 			return;
+		}
 		pColRef->iTable = pItem->iCursor;
 		pColRef->iColumn = k++;
 		pColRef->space_def = space_def;
-- 
2.21.0