[tarantool-patches] [PATCH v1 3/4] sql: patch sql_expr_create routine to use Parser

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

The code was refactored so that the sql_expr_create function
would use the parser object instead of database connection. Also
performed some additional names refactoring in adjacent places.

Needed for #3931
---
 src/box/sql/build.c     |  18 ++--
 src/box/sql/expr.c      | 202 +++++++++++++++-------------------------
 src/box/sql/fkey.c      | 124 ++++++++++++------------
 src/box/sql/parse.y     |  16 ++--
 src/box/sql/resolve.c   |  41 ++++----
 src/box/sql/select.c    |  46 +++++----
 src/box/sql/sqlInt.h    |  79 +++++++++++++++-
 src/box/sql/wherecode.c |   3 +-
 src/box/sql/whereexpr.c |  10 +-
 9 files changed, 283 insertions(+), 256 deletions(-)

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 3f4530620..a08148a97 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -691,8 +691,8 @@ sqlAddPrimaryKey(Parse * pParse,	/* Parsing context */
 		struct Token token;
 		sqlTokenInit(&token, pTab->def->fields[iCol].name);
 		list = sql_expr_list_append(db, NULL,
-					    sqlExprAlloc(db, TK_ID,
-							     &token, 0));
+					    sql_expr_create(pParse, TK_ID,
+							    &token, false));
 		if (list == NULL)
 			goto primary_key_exit;
 		sql_create_index(pParse, 0, 0, list, 0, SORT_ORDER_ASC,
@@ -1455,10 +1455,11 @@ sql_id_eq_str_expr(struct Parse *parse, const char *col_name,
 {
 	struct sql *db = parse->db;
 
-	struct Expr *col_name_expr = sqlExpr(db, TK_ID, col_name);
+	struct Expr *col_name_expr = sql_op_expr_create(parse, TK_ID, col_name);
 	if (col_name_expr == NULL)
 		return NULL;
-	struct Expr *col_value_expr = sqlExpr(db, TK_STRING, col_value);
+	struct Expr *col_value_expr =
+		sql_op_expr_create(parse, TK_STRING, col_value);
 	if (col_value_expr == NULL) {
 		sql_expr_delete(db, col_name_expr, false);
 		return NULL;
@@ -1480,12 +1481,12 @@ vdbe_emit_stat_space_clear(struct Parse *parse, const char *stat_table_name,
 	if (idx_name != NULL) {
 		struct Expr *expr = sql_id_eq_str_expr(parse, "idx", idx_name);
 		if (expr != NULL)
-			where = sqlExprAnd(db, expr, where);
+			where = sql_and_expr_create(parse, expr, where);
 	}
 	if (table_name != NULL) {
 		struct Expr *expr = sql_id_eq_str_expr(parse, "tbl", table_name);
 		if (expr != NULL)
-			where = sqlExprAnd(db, expr, where);
+			where = sql_and_expr_create(parse, expr, where);
 	}
 	/**
 	 * On memory allocation error sql_table delete_from
@@ -2339,8 +2340,9 @@ sql_create_index(struct Parse *parse, struct Token *token,
 		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));
+						sql_expr_create(parse, TK_ID,
+								&prev_col,
+								false));
 		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 a6e35df6d..42531c107 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -151,8 +151,7 @@ sqlExprAddCollateToken(Parse * pParse,	/* Parsing context */
 {
 	if (pCollName->n > 0) {
 		Expr *pNew =
-		    sqlExprAlloc(pParse->db, TK_COLLATE, pCollName,
-				     dequote);
+		    sql_expr_create(pParse, TK_COLLATE, pCollName, dequote);
 		if (pNew) {
 			pNew->pLeft = pExpr;
 			pNew->flags |= EP_Collate | EP_Skip;
@@ -795,113 +794,60 @@ 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 Parse *parser, 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;
+	struct Expr *expr =
+		sqlDbMallocRawNN(parser->db, sizeof(*expr) + extra_sz);
+	if (expr == NULL)
+		return NULL;
+
+	memset(expr, 0, sizeof(*expr));
+	expr->op = (u8)op;
+	expr->iAgg = -1;
+#if SQL_MAX_EXPR_DEPTH > 0
+	expr->nHeight = 1;
 #endif
-	}
-	return pNew;
-}
+	if (token == NULL)
+		return expr;
 
-/*
- * 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);
+	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 Parse *parser, 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(parser, op, &name_token, false);
 }
 
 /*
@@ -947,8 +893,11 @@ 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.
+		 */
+		p = sql_and_expr_create(pParse, pLeft, pRight);
 	} else {
 		p = sqlDbMallocRawNN(pParse->db, sizeof(Expr));
 		if (p) {
@@ -1017,30 +966,25 @@ 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 Parse *parser, struct Expr *left_expr,
+		    struct Expr *right_expr)
 {
-	if (pLeft == 0) {
-		return pRight;
-	} else if (pRight == 0) {
-		return pLeft;
-	} else if (exprAlwaysFalse(pLeft) || exprAlwaysFalse(pRight)) {
-		sql_expr_delete(db, pLeft, false);
-		sql_expr_delete(db, pRight, false);
-		return sqlExprAlloc(db, TK_INTEGER, &sqlIntTokens[0],
-					0);
+	if (left_expr == NULL) {
+		return right_expr;
+	} else if (right_expr == NULL) {
+		return left_expr;
+	} else if (exprAlwaysFalse(left_expr) || exprAlwaysFalse(right_expr)) {
+		sql_expr_delete(parser->db, left_expr, false);
+		sql_expr_delete(parser->db, right_expr, false);
+		return sql_expr_create(parser, 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(parser, TK_AND, NULL, false);
+		sqlExprAttachSubtrees(parser->db, new_expr, left_expr,
+				      right_expr);
+		return new_expr;
 	}
 }
 
@@ -1054,7 +998,7 @@ sqlExprFunction(Parse * pParse, ExprList * pList, Token * pToken)
 	Expr *pNew;
 	sql *db = pParse->db;
 	assert(pToken);
-	pNew = sqlExprAlloc(db, TK_FUNCTION, pToken, 1);
+	pNew = sql_expr_create(pParse, TK_FUNCTION, pToken, true);
 	if (pNew == 0) {
 		sql_expr_list_delete(db, pList);	/* Avoid memory leak when malloc fails */
 		return 0;
@@ -2860,9 +2804,9 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
 			}
 			if (pSel->pLimit == NULL) {
 				pSel->pLimit =
-					sqlExprAlloc(pParse->db, TK_INTEGER,
-							 &sqlIntTokens[1],
-							 0);
+					sql_expr_create(pParse, TK_INTEGER,
+							&sqlIntTokens[1],
+							false);
 				if (pSel->pLimit != NULL) {
 					ExprSetProperty(pSel->pLimit,
 							EP_System);
diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c
index 69740b39f..8edafe149 100644
--- a/src/box/sql/fkey.c
+++ b/src/box/sql/fkey.c
@@ -308,50 +308,51 @@ fkey_lookup_parent(struct Parse *parse_context, struct space *parent,
  * regBase is the first of an array of register that contains the data
  * for pTab.  regBase+1 holds the first column.
  * regBase+2 holds the second column, and so forth.
+ * @param parser The parsing context.
+ * @param table The table whose content is at r[regBase]...
+ * @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 *
-exprTableRegister(Parse * pParse,	/* Parsing and code generating context */
-		  Table * pTab,	/* The table whose content is at r[regBase]... */
-		  int regBase,	/* Contents of table pTab */
-		  i16 iCol	/* Which column of pTab is desired */
-    )
+static struct Expr *
+sql_register_expr_create(struct Parse *parser, struct Table *table,
+			 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 = pTab->def->fields[iCol].type;
-		} else {
-			pExpr->iTable = regBase;
-			pExpr->type = FIELD_TYPE_INTEGER;
-		}
+	struct Expr *expr = sql_op_expr_create(parser, TK_REGISTER, NULL);
+	if (expr == NULL)
+		return NULL;
+	if (column >= 0) {
+		expr->iTable = reg_base + column + 1;
+		expr->type = table->def->fields[column].type;
+	} else {
+		expr->iTable = reg_base;
+		expr->type = FIELD_TYPE_INTEGER;
 	}
-	return pExpr;
+	return expr;
 }
 
 /**
  * Return an Expr object that refers to column of space_def which
  * has cursor cursor.
- * @param db The database connection.
+ * @param parser The parsing context.
  * @param def space definition.
  * @param cursor The open cursor on the table.
  * @param column The column that is wanted.
  * @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 Parse *parser, 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(parser, TK_COLUMN, NULL);
+	if (expr == NULL)
+		return NULL;
+	expr->space_def = def;
+	expr->iTable = cursor;
+	expr->iColumn = column;
+	return expr;
 }
 
 /*
@@ -430,12 +431,14 @@ fkey_scan_children(struct Parse *parser, struct SrcList *src, struct Table *tab,
 	for (uint32_t i = 0; i < fkey->field_count; i++) {
 		uint32_t fieldno = fkey->links[i].parent_field;
 		struct Expr *pexpr =
-			exprTableRegister(parser, tab, reg_data, fieldno);
+			sql_register_expr_create(parser, tab, reg_data,
+						 fieldno);
 		fieldno = fkey->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(parser, TK_ID, field_name);
 		struct Expr *eq = sqlPExpr(parser, TK_EQ, pexpr, chexpr);
-		where = sqlExprAnd(db, where, eq);
+		where = sql_and_expr_create(parser, where, eq);
 	}
 
 	/*
@@ -451,15 +454,16 @@ fkey_scan_children(struct Parse *parser, struct SrcList *src, struct Table *tab,
 		struct Expr *expr = NULL, *pexpr, *chexpr, *eq;
 		for (uint32_t i = 0; i < fkey->field_count; i++) {
 			uint32_t fieldno = fkey->links[i].parent_field;
-			pexpr = exprTableRegister(parser, tab, reg_data,
-						  fieldno);
-			chexpr = exprTableColumn(db, tab->def,
-						 src->a[0].iCursor, fieldno);
+			pexpr = sql_register_expr_create(parser, tab, reg_data,
+							 fieldno);
+			int cursor = src->a[0].iCursor;
+			chexpr = sql_column_cursor_expr_create(parser, tab->def,
+							       cursor, fieldno);
 			eq = sqlPExpr(parser, TK_EQ, pexpr, chexpr);
-			expr = sqlExprAnd(db, expr, eq);
+			expr = sql_and_expr_create(parser, expr, eq);
 		}
 		struct Expr *pNe = sqlPExpr(parser, TK_NOT, expr, 0);
-		where = sqlExprAnd(db, where, pNe);
+		where = sql_and_expr_create(parser, where, pNe);
 	}
 
 	/* Resolve the references in the WHERE clause. */
@@ -788,12 +792,13 @@ fkey_action_trigger(struct Parse *pParse, struct Table *pTab, struct fkey *fkey,
 		 */
 		struct Expr *to_col =
 			sqlPExpr(pParse, TK_DOT,
-				     sqlExprAlloc(db, TK_ID, &t_old, 0),
-				     sqlExprAlloc(db, TK_ID, &t_to_col, 0));
+				 sql_expr_create(pParse, TK_ID, &t_old, false),
+				 sql_expr_create(pParse, TK_ID, &t_to_col,
+						 false));
 		struct Expr *from_col =
-			sqlExprAlloc(db, TK_ID, &t_from_col, 0);
+			sql_expr_create(pParse, TK_ID, &t_from_col, false);
 		struct Expr *eq = sqlPExpr(pParse, TK_EQ, to_col, from_col);
-		where = sqlExprAnd(db, where, eq);
+		where = sql_and_expr_create(pParse, where, eq);
 
 		/*
 		 * For ON UPDATE, construct the next term of the
@@ -812,11 +817,13 @@ fkey_action_trigger(struct Parse *pParse, struct Table *pTab, struct fkey *fkey,
 		 */
 		if (is_update) {
 			struct Expr *old_val = sqlPExpr(pParse, TK_DOT,
-				sqlExprAlloc(db, TK_ID, &t_old, 0),
-				sqlExprAlloc(db, TK_ID, &t_to_col, 0));
+				sql_expr_create(pParse, TK_ID, &t_old, false),
+				sql_expr_create(pParse, TK_ID, &t_to_col,
+						false));
 			struct Expr *new_val = sqlPExpr(pParse, TK_DOT,
-				sqlExprAlloc(db, TK_ID, &t_new, 0),
-				sqlExprAlloc(db, TK_ID, &t_to_col, 0));
+				sql_expr_create(pParse, TK_ID, &t_new, false),
+				sql_expr_create(pParse, TK_ID, &t_to_col,
+						false));
 			struct Expr *old_is_null = sqlPExpr(
 				pParse, TK_ISNULL,
 				sqlExprDup(db, old_val, 0), NULL);
@@ -829,7 +836,8 @@ fkey_action_trigger(struct Parse *pParse, struct Table *pTab, struct fkey *fkey,
 			struct Expr *no_action_needed =
 				sqlPExpr(pParse, TK_OR, old_is_null,
 					     non_null_eq);
-			when = sqlExprAnd(db, when, no_action_needed);
+			when = sql_and_expr_create(pParse, when,
+						   no_action_needed);
 		}
 
 		if (action != FKEY_ACTION_RESTRICT &&
@@ -837,21 +845,22 @@ fkey_action_trigger(struct Parse *pParse, struct Table *pTab, struct fkey *fkey,
 			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));
+					       sql_expr_create(pParse, TK_ID,
+							       &t_new, false),
+					       sql_expr_create(pParse, TK_ID,
+							       &t_to_col,
+							       false));
 			} 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(pParse, TK_NULL,
+							      NULL, false);
 				}
 			} else {
-				new = sqlExprAlloc(db, TK_NULL, NULL, 0);
+				new = sql_expr_create(pParse, TK_NULL, NULL,
+						      false);
 			}
 			list = sql_expr_list_append(db, list, new);
 			sqlExprListSetName(pParse, list, &t_from_col, 0);
@@ -865,8 +874,9 @@ fkey_action_trigger(struct Parse *pParse, struct Table *pTab, struct fkey *fkey,
 		struct Token err;
 		err.z = space_name;
 		err.n = name_len;
-		struct Expr *r = sqlExpr(db, TK_RAISE, "FOREIGN KEY "\
-					     "constraint failed");
+		struct Expr *r =
+			sql_op_expr_create(pParse, TK_RAISE,
+					   "FOREIGN KEY constraint failed");
 		if (r != NULL)
 			r->on_conflict_action = ON_CONFLICT_ACTION_ABORT;
 		select = sqlSelectNew(pParse,
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 1a18d5f89..d7b721695 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -523,12 +523,12 @@ 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, TK_ASTERISK, NULL);
   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, TK_ID, &X, true);
   Expr *pDot = sqlPExpr(pParse, TK_DOT, pLeft, pRight);
   A = sql_expr_list_append(pParse->db,A, pDot);
 }
@@ -868,15 +868,15 @@ 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, TK_ID, &X, true);
+  struct Expr *temp2 = sql_expr_create(pParse, TK_ID, &Y, true);
   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, TK_INTEGER, &X, true);
   A.pExpr->type = FIELD_TYPE_INTEGER;
   A.zStart = X.z;
   A.zEnd = X.z + X.n;
@@ -909,7 +909,7 @@ 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, TK_CAST, NULL, true);
   A.pExpr->type = T.type;
   sqlExprAttachSubtrees(pParse->db, A.pExpr, E.pExpr, 0);
 }
@@ -1099,7 +1099,7 @@ 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, TK_INTEGER, &sqlIntTokens[N], true);
   }else if( Y->nExpr==1 ){
     /* Expressions of the form:
     **
@@ -1421,7 +1421,7 @@ 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);
+  A.pExpr = sql_expr_create(pParse, TK_RAISE, &Z, true);
   if( A.pExpr ) {
     A.pExpr->on_conflict_action = (enum on_conflict_action) T;
   }
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index 029eb4055..22397d6f5 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -479,26 +479,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 Parse *parser, 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->pTab->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(parser, TK_COLUMN, NULL, true);
+	if (expr == NULL)
+		return NULL;
+	struct SrcList_item *pItem = &src_list->a[src_idx];
+	expr->space_def = pItem->pTab->def;
+	expr->iTable = pItem->iCursor;
+	expr->iColumn = column;
+	pItem->colUsed |= ((Bitmask) 1) << (column >= BMS ? BMS - 1 : column);
+	ExprSetProperty(expr, EP_Resolved);
+	return expr;
 }
 
 /*
@@ -999,7 +993,9 @@ 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);
+				Expr *pNew =
+					sql_op_expr_create(pParse, TK_INTEGER,
+							   NULL);
 				if (pNew == 0)
 					return 1;
 				pNew->flags |= EP_IntValue;
@@ -1347,9 +1343,8 @@ 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(pParse, TK_INTEGER,
+						    &sqlIntTokens[1], false);
 		} else {
 			if (sqlResolveExprNames(&sNC, p->pHaving))
 				return WRC_Abort;
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index dfd3a375e..5a1bc0c81 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -165,7 +165,9 @@ sqlSelectNew(Parse * pParse,	/* Parsing context */
 	}
 	if (pEList == 0) {
 		pEList = sql_expr_list_append(pParse->db, NULL,
-					      sqlExpr(db, TK_ASTERISK, 0));
+					      sql_op_expr_create(pParse,
+								 TK_ASTERISK,
+								 NULL));
 	}
 	struct session MAYBE_UNUSED *user_session;
 	user_session = current_session();
@@ -483,7 +485,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;
@@ -493,8 +494,8 @@ addWhereTerm(Parse * pParse,	/* Parsing context */
 	assert(pSrc->a[iLeft].pTab);
 	assert(pSrc->a[iRight].pTab);
 
-	pE1 = sqlCreateColumnExpr(db, pSrc, iLeft, iColLeft);
-	pE2 = sqlCreateColumnExpr(db, pSrc, iRight, iColRight);
+	pE1 = sql_column_expr_create(pParse, pSrc, iLeft, iColLeft);
+	pE2 = sql_column_expr_create(pParse, pSrc, iRight, iColRight);
 
 	pEq = sqlPExpr(pParse, TK_EQ, pE1, pE2);
 	if (pEq && isOuterJoin) {
@@ -503,7 +504,7 @@ addWhereTerm(Parse * pParse,	/* Parsing context */
 		ExprSetVVAProperty(pEq, EP_NoReduce);
 		pEq->iRightJoinTable = (i16) pE2->iTable;
 	}
-	*ppWhere = sqlExprAnd(db, *ppWhere, pEq);
+	*ppWhere = sql_and_expr_create(pParse, *ppWhere, pEq);
 }
 
 /*
@@ -624,8 +625,8 @@ sqlProcessJoin(Parse * pParse, Select * p)
 		if (pRight->pOn) {
 			if (isOuter)
 				setJoinExpr(pRight->pOn, pRight->iCursor);
-			p->pWhere =
-			    sqlExprAnd(pParse->db, p->pWhere, pRight->pOn);
+			p->pWhere = sql_and_expr_create(pParse, p->pWhere,
+							pRight->pOn);
 			pRight->pOn = 0;
 		}
 
@@ -3314,7 +3315,9 @@ multiSelectOrderBy(Parse * pParse,	/* Parsing context */
 					break;
 			}
 			if (j == nOrderBy) {
-				Expr *pNew = sqlExpr(db, TK_INTEGER, 0);
+				Expr *pNew =
+					sql_op_expr_create(pParse, TK_INTEGER,
+							   NULL);
 				if (pNew == 0)
 					return SQL_NOMEM_BKPT;
 				pNew->flags |= EP_IntValue;
@@ -4201,17 +4204,18 @@ 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);
+			pParent->pHaving =
+				sql_and_expr_create(pParse,
+						    sqlExprDup(db,
+							       pSub->pHaving, 0),
+						    pParent->pHaving);
 			assert(pParent->pGroupBy == 0);
 			pParent->pGroupBy =
 			    sql_expr_list_dup(db, pSub->pGroupBy, 0);
 		} else {
 			pParent->pWhere =
-			    sqlExprAnd(db, pWhere, pParent->pWhere);
+				sql_and_expr_create(pParse, pWhere,
+						    pParent->pWhere);
 		}
 		substSelect(pParse, pParent, iParent, pSub->pEList, 0);
 
@@ -4317,7 +4321,8 @@ pushDownWhereTerms(Parse * pParse,	/* Parse context (for malloc() and error repo
 			pNew = sqlExprDup(pParse->db, pWhere, 0);
 			pNew = substExpr(pParse, pNew, iCursor, pSubq->pEList);
 			pSubq->pWhere =
-			    sqlExprAnd(pParse->db, pSubq->pWhere, pNew);
+				sql_and_expr_create(pParse, pSubq->pWhere,
+						    pNew);
 			pSubq = pSubq->pPrior;
 		}
 	}
@@ -4500,7 +4505,8 @@ convertCompoundSelectToSubquery(Walker * pWalker, Select * p)
 	*pNew = *p;
 	p->pSrc = pNewSrc;
 	p->pEList = sql_expr_list_append(pParse->db, NULL,
-					 sqlExpr(db, TK_ASTERISK, 0));
+					 sql_op_expr_create(pParse, TK_ASTERISK,
+							    NULL));
 	p->op = TK_SELECT;
 	p->pWhere = 0;
 	pNew->pGroupBy = 0;
@@ -5002,16 +5008,16 @@ selectExpander(Walker * pWalker, Select * p)
 								continue;
 							}
 						}
-						pRight =
-						    sqlExpr(db, TK_ID,
+						pRight = sql_op_expr_create(
+								pParse, TK_ID,
 								zName);
 						zColname = zName;
 						zToFree = 0;
 						if (longNames
 						    || pTabList->nSrc > 1) {
 							Expr *pLeft;
-							pLeft =
-							    sqlExpr(db,
+							pLeft = sql_op_expr_create(
+									pParse,
 									TK_ID,
 									zTabName);
 							pExpr =
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 99c8adbdd..048c814e5 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -3292,13 +3292,69 @@ 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 parser The parsing context.
+ * @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 Parse *parser, int op, const Token *token, bool dequote);
+
+/*
+ * Allocate a new expression node from a zero-terminated token
+ * that has already been dequoted.
+ * @param parser The parsing context.
+ * @param op Expression opcode.
+ * @param name The object name string.
+ * @retval not NULL expression pointer on success, NULL otherwise.
+ */
+struct Expr *
+sql_op_expr_create(struct Parse *parser, 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 parser The parsing context.
+ * @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 Parse *parser, struct Expr *left_expr,
+		    struct Expr *right_expr);
+
 Expr *sqlExprFunction(Parse *, ExprList *, Token *);
 void sqlExprAssignVarNumber(Parse *, Expr *, u32);
 ExprList *sqlExprListAppendVector(Parse *, ExprList *, IdList *, Expr *);
@@ -4725,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 parser The parsing context.
+ * @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 Parse *parser, 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 a0838a26b..23af1693d 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -1437,7 +1437,8 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo,	/* Complete information about the W
 					continue;
 				testcase(pWC->a[iTerm].wtFlags & TERM_ORINFO);
 				pExpr = sqlExprDup(db, pExpr, 0);
-				pAndExpr = sqlExprAnd(db, pAndExpr, pExpr);
+				pAndExpr = sql_and_expr_create(pParse, pAndExpr,
+							       pExpr);
 			}
 			if (pAndExpr) {
 				pAndExpr =
diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c
index 611a3896d..79cd09d4e 100644
--- a/src/box/sql/whereexpr.c
+++ b/src/box/sql/whereexpr.c
@@ -303,7 +303,7 @@ 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);
+			pPrefix = sql_op_expr_create(pParse, TK_STRING, z);
 			if (pPrefix)
 				pPrefix->u.zToken[cnt] = 0;
 			*ppPrefix = pPrefix;
@@ -1297,9 +1297,9 @@ exprAnalyze(SrcList * pSrc,	/* the FROM clause */
 		int idxNew;
 		WhereTerm *pNewTerm;
 
-		pNewExpr = sqlPExpr(pParse, TK_GT,
-					sqlExprDup(db, pLeft, 0),
-					sqlExprAlloc(db, TK_NULL, 0, 0));
+		pNewExpr = sqlPExpr(pParse, TK_GT, sqlExprDup(db, pLeft, 0),
+				    sql_expr_create(pParse, TK_NULL, NULL,
+						    false));
 
 		idxNew = whereClauseInsert(pWC, pNewExpr,
 					   TERM_VIRTUAL | TERM_DYNAMIC |
@@ -1495,7 +1495,7 @@ sqlWhereTabFuncArgs(Parse * pParse,	/* Parsing context */
 					pTab->def->name, j);
 			return;
 		}
-		pColRef = sqlExprAlloc(pParse->db, TK_COLUMN, 0, 0);
+		pColRef = sql_expr_create(pParse, TK_COLUMN, NULL, false);
 		if (pColRef == 0)
 			return;
 		pColRef->iTable = pItem->iCursor;
-- 
2.20.1