[tarantool-patches] [PATCH v1 0/3] box: local sql_flags for parser and vdbe

[tarantool-patches] [PATCH v1 0/3] box: local sql_flags for parser and vdbe

[Kirill Shcherbatov]

The sql_flags is a parser parameter that describe how to parse the
SQL request, but now this information is taken from the global
user session object.
When we need to run the parser with some other parameters, it is
necessary to change global session object, which may lead to
unpredictable consequences in general case.
Introduced a new parser and vdbe field sql_flags is responsible
for SQL parsing results.

Also fixed bug #4219 ban sql functions coinciding with builtins.

Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-3691-sql-flags-in-parser
Issue: https://github.com/tarantool/tarantool/issues/3961

Kirill Shcherbatov (3):
  sql: get rid of SQL_NullCallback flag
  sql: ban sql functions coinciding with builtins
  box: local sql_flags for parser and vdbe

 src/box/lua/lua_sql.c         |   8 +-
 src/box/sql.c                 |   2 +-
 src/box/sql.h                 |   3 +-
 src/box/sql/callback.c        | 174 +++++++++++++++-------------------
 src/box/sql/delete.c          |  12 +--
 src/box/sql/expr.c            |  25 +++--
 src/box/sql/fk_constraint.c   |   7 +-
 src/box/sql/func.c            |  33 +++----
 src/box/sql/insert.c          |  18 ++--
 src/box/sql/legacy.c          |   6 +-
 src/box/sql/main.c            |  33 ++++---
 src/box/sql/prepare.c         |   5 +-
 src/box/sql/resolve.c         |   7 +-
 src/box/sql/select.c          |  24 ++---
 src/box/sql/sqlInt.h          |  32 ++++++-
 src/box/sql/tokenize.c        |   7 +-
 src/box/sql/trigger.c         |  13 +--
 src/box/sql/update.c          |  15 ++-
 src/box/sql/vdbe.c            |  72 +++++++-------
 src/box/sql/vdbeInt.h         |   2 +
 src/box/sql/vdbemem.c         |   2 +-
 src/box/sql/where.c           |  13 ++-
 test/sql-tap/lua_sql.test.lua |  11 ++-
 23 files changed, 254 insertions(+), 270 deletions(-)

-- 
2.21.0

[tarantool-patches] [PATCH v1 1/3] sql: get rid of SQL_NullCallback flag

[Kirill Shcherbatov]

The SQL_NullCallback flag is never set now so it redundant.
Let's get red if it because we are going to pass
user_session->sql_flags to parser and use it instead of
session instance.

Needed for #3691
---
 src/box/sql/legacy.c | 6 +-----
 src/box/sql/sqlInt.h | 2 --
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/src/box/sql/legacy.c b/src/box/sql/legacy.c
index ee680efe2..a57677e89 100644
--- a/src/box/sql/legacy.c
+++ b/src/box/sql/legacy.c
@@ -63,7 +63,6 @@ sql_exec(sql * db,	/* The database on which the SQL executes */
 	sql_stmt *pStmt = 0;	/* The current SQL statement */
 	char **azCols = 0;	/* Names of result columns */
 	int callbackIsInit;	/* True if callback data is initialized */
-	struct session *user_session = current_session();
 
 	if (!sqlSafetyCheckOk(db))
 		return SQL_MISUSE;
@@ -94,10 +93,7 @@ sql_exec(sql * db,	/* The database on which the SQL executes */
 			int i;
 			rc = sql_step(pStmt);
 			/* Invoke the callback function if required */
-			if (xCallback && (SQL_ROW == rc ||
-					  (SQL_DONE == rc && !callbackIsInit
-					   && user_session->
-					   sql_flags & SQL_NullCallback))) {
+			if (xCallback != NULL && rc == SQL_ROW) {
 				if (!callbackIsInit) {
 					azCols =
 					    sqlDbMallocZero(db,
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 997a46535..d2be6701f 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -1498,8 +1498,6 @@ struct sql {
 #define SQL_CountRows      0x00000080	/* Count rows changed by INSERT, */
 					  /*   DELETE, or UPDATE and return */
 					  /*   the count using a callback. */
-#define SQL_NullCallback   0x00000100	/* Invoke the callback once if the */
-					  /*   result set is empty */
 #define SQL_SqlTrace       0x00000200	/* Debug print SQL as it executes */
 #define SQL_SelectTrace    0x00000800       /* Debug info about select statement */
 #define SQL_WhereTrace     0x00008000       /* Debug info about optimizer's work */
-- 
2.21.0

[tarantool-patches] Re: [PATCH v1 1/3] sql: get rid of SQL_NullCallback flag

[n.pettik]

LGTM

[tarantool-patches] [PATCH v1 2/3] sql: ban sql functions coinciding with builtins

[Kirill Shcherbatov]

Previously, it was possible to create a function with name
that is already reserved for some predefined builtin.
It is a bug. This patch bans sql_create_function for such cases.

Also removed FUNC_PERFECT_MATCH user session flag.

Closes #4219
Needed for #3691
---
 src/box/lua/lua_sql.c         |   8 +-
 src/box/sql/callback.c        | 174 +++++++++++++++-------------------
 src/box/sql/expr.c            |  18 ++--
 src/box/sql/func.c            |  33 +++----
 src/box/sql/main.c            |  33 ++++---
 src/box/sql/resolve.c         |   7 +-
 src/box/sql/sqlInt.h          |  25 ++++-
 src/box/sql/vdbemem.c         |   2 +-
 test/sql-tap/lua_sql.test.lua |  11 ++-
 9 files changed, 157 insertions(+), 154 deletions(-)

diff --git a/src/box/lua/lua_sql.c b/src/box/lua/lua_sql.c
index 36b75ff08..561353520 100644
--- a/src/box/lua/lua_sql.c
+++ b/src/box/lua/lua_sql.c
@@ -195,7 +195,11 @@ lbox_sql_create_function(struct lua_State *L)
 					   is_deterministic ? SQL_DETERMINISTIC : 0,
 					   func_info, lua_sql_call, NULL, NULL,
 					   lua_sql_destroy);
-	if (rc != 0)
-		return luaL_error(L, sqlErrStr(rc));
+	if (rc != 0) {
+		const char *err = rc == SQL_TARANTOOL_ERROR ?
+				  box_error_message(box_error_last()) :
+				  sqlErrStr(rc);
+		return luaL_error(L, err);
+	}
 	return 0;
 }
diff --git a/src/box/sql/callback.c b/src/box/sql/callback.c
index 49197532e..b4247324d 100644
--- a/src/box/sql/callback.c
+++ b/src/box/sql/callback.c
@@ -131,6 +131,19 @@ functionSearch(int h,		/* Hash of the name */
 	return 0;
 }
 
+/**
+ * Cache function is used to organise sqlBuiltinFunctions table.
+ * @param func_name The name of builtin function.
+ * @param func_name_len The length of the @a name.
+ * @retval Hash value is calculated for given name.
+ */
+static int
+sql_builtin_func_name_hash(const char *func_name, uint32_t func_name_len)
+{
+	return (sqlUpperToLower[(u8) func_name[0]] +
+		func_name_len) % SQL_FUNC_HASH_SZ;
+}
+
 /*
  * Insert a new FuncDef into a FuncDefHash hash table.
  */
@@ -144,9 +157,7 @@ sqlInsertBuiltinFuncs(FuncDef * aDef,	/* List of global functions to be inserted
 		FuncDef *pOther;
 		const char *zName = aDef[i].zName;
 		int nName = sqlStrlen30(zName);
-		int h =
-		    (sqlUpperToLower[(u8) zName[0]] +
-		     nName) % SQL_FUNC_HASH_SZ;
+		int h = sql_builtin_func_name_hash(zName, nName);
 		pOther = functionSearch(h, zName);
 		if (pOther) {
 			assert(pOther != &aDef[i] && pOther->pNext != &aDef[i]);
@@ -160,110 +171,79 @@ sqlInsertBuiltinFuncs(FuncDef * aDef,	/* List of global functions to be inserted
 	}
 }
 
-/*
- * Locate a user function given a name, a number of arguments and a flag
- * indicating whether the function prefers UTF-16 over UTF-8.  Return a
- * pointer to the FuncDef structure that defines that function, or return
- * NULL if the function does not exist.
- *
- * If the createFlag argument is true, then a new (blank) FuncDef
- * structure is created and liked into the "db" structure if a
- * no matching function previously existed.
- *
- * If nArg is -2, then the first valid function found is returned.  A
- * function is valid if xSFunc is non-zero.  The nArg==(-2)
- * case is used to see if zName is a valid function name for some number
- * of arguments.  If nArg is -2, then createFlag must be 0.
- *
- * If createFlag is false, then a function with the required name and
- * number of arguments may be returned even if the eTextRep flag does not
- * match that requested.
- */
-FuncDef *
-sqlFindFunction(sql * db,	/* An open database */
-		    const char *zName,	/* Name of the function.  zero-terminated */
-		    int nArg,	/* Number of arguments.  -1 means any number */
-		    u8 createFlag	/* Create new entry if true and does not otherwise exist */
-    )
+struct FuncDef *
+sql_find_function(struct sql *db, const char *func_name, int arg_count,
+		  bool is_builtin, bool is_create)
 {
-	FuncDef *p;		/* Iterator variable */
-	FuncDef *pBest = 0;	/* Best match found so far */
-	int bestScore = 0;	/* Score of best match */
-	int h;			/* Hash value */
-	int nName;		/* Length of the name */
-	struct session *user_session = current_session();
-
-	assert(nArg >= (-2));
-	assert(nArg >= (-1) || createFlag == 0);
-	nName = sqlStrlen30(zName);
-
-	/* First search for a match amongst the application-defined functions.
+	assert(arg_count >= -2);
+	assert(arg_count >= -1 || !is_create);
+	assert(!is_create || !is_builtin);
+	uint32_t func_name_len = strlen(func_name);
+	/*
+	 * The 'score' of the best match is calculated
+	 * with matchQuality estimator.
+	 */
+	int func_score = 0;
+	/*
+	 * The pointer of a function having the highest 'score'
+	 * for given name and arg_count parameters.
 	 */
-	p = (FuncDef *) sqlHashFind(&db->aFunc, zName);
-	while (p) {
-		int score = matchQuality(p, nArg);
-		if (score > bestScore) {
-			pBest = p;
-			bestScore = score;
+	struct FuncDef *func = NULL;
+	if (is_builtin)
+		goto lookup_for_builtin;
+	/* Search amongst the user-defined functions. */
+	for (struct FuncDef *p = sqlHashFind(&db->aFunc, func_name);
+	     p != NULL; p = p->pNext) {
+		int score = matchQuality(p, arg_count);
+		if (score > func_score) {
+			func = p;
+			func_score = score;
 		}
-		p = p->pNext;
 	}
-
-	/* If no match is found, search the built-in functions.
-	 *
-	 * If the SQL_PreferBuiltin flag is set, then search the built-in
-	 * functions even if a prior app-defined function was found.  And give
-	 * priority to built-in functions.
-	 *
-	 * Except, if createFlag is true, that means that we are trying to
-	 * install a new function.  Whatever FuncDef structure is returned it will
-	 * have fields overwritten with new information appropriate for the
-	 * new function.  But the FuncDefs for built-in functions are read-only.
-	 * So we must not search for built-ins when creating a new function.
-	 */
-	if (!createFlag &&
-	    (pBest == 0
-	     || (user_session->sql_flags & SQL_PreferBuiltin) != 0)) {
-		bestScore = 0;
-		h = (sqlUpperToLower[(u8) zName[0]] +
-		     nName) % SQL_FUNC_HASH_SZ;
-		p = functionSearch(h, zName);
-		while (p) {
-			int score = matchQuality(p, nArg);
-			if (score > bestScore) {
-				pBest = p;
-				bestScore = score;
+	if (!is_create && func == NULL) {
+lookup_for_builtin:
+		func_score = 0;
+		int h = sql_builtin_func_name_hash(func_name, func_name_len);
+		for (struct FuncDef *p = functionSearch(h, func_name);
+		     p != NULL; p = p->pNext) {
+			int score = matchQuality(p, arg_count);
+			if (score > func_score) {
+				func = p;
+				func_score = score;
 			}
-			p = p->pNext;
 		}
 	}
-
-	/* If the createFlag parameter is true and the search did not reveal an
-	 * exact match for the name, number of arguments and encoding, then add a
-	 * new entry to the hash table and return it.
+	/*
+	 * If the is_create parameter is true and the search did
+	 * not reveal an exact match for the name, number of
+	 * arguments, then add a new entry to the hash table and
+	 * return it.
 	 */
-	if (createFlag && bestScore < FUNC_PERFECT_MATCH &&
-	    (pBest =
-	     sqlDbMallocZero(db, sizeof(*pBest) + nName + 1)) != 0) {
-		FuncDef *pOther;
-		pBest->zName = (const char *)&pBest[1];
-		pBest->nArg = (u16) nArg;
-		pBest->funcFlags = 0;
-		memcpy((char *)&pBest[1], zName, nName + 1);
-		pOther =
-		    (FuncDef *) sqlHashInsert(&db->aFunc, pBest->zName,
-						  pBest);
-		if (pOther == pBest) {
-			sqlDbFree(db, pBest);
+	if (is_create && func_score < FUNC_PERFECT_MATCH) {
+		uint32_t func_sz = sizeof(func[0]) + func_name_len + 1;
+		func = sqlDbMallocZero(db, func_sz);
+		if (func == NULL) {
+			diag_set(OutOfMemory, func_sz, "sqlDbMallocZero",
+				 "func");
+			return NULL;
+		}
+		func->zName = (const char *) &func[1];
+		func->nArg = (u16) arg_count;
+		func->funcFlags = 0;
+		memcpy((char *)&func[1], func_name, func_name_len + 1);
+		struct FuncDef *old_func =
+		    (struct FuncDef *) sqlHashInsert(&db->aFunc, func->zName,
+						     func);
+		if (old_func == func) {
+			sqlDbFree(db, func);
 			sqlOomFault(db);
-			return 0;
+			diag_set(OutOfMemory, func_sz, "sqlHashInsert",
+				 "func");
+			return NULL;
 		} else {
-			pBest->pNext = pOther;
+			func->pNext = old_func;
 		}
 	}
-
-	if (pBest && (pBest->xSFunc || createFlag)) {
-		return pBest;
-	}
-	return 0;
+	return func != NULL &&
+	       (func->xSFunc != NULL || is_create || is_builtin) ? func : NULL;
 }
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index ba7eea59d..f7507938d 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -285,9 +285,9 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id,
 		if (op == TK_FUNCTION) {
 			uint32_t arg_count = p->x.pList == NULL ? 0 :
 					     p->x.pList->nExpr;
-			struct FuncDef *func = sqlFindFunction(parse->db,
-							       p->u.zToken,
-							       arg_count, 0);
+			struct FuncDef *func =
+				sql_find_function(parse->db, p->u.zToken,
+						  arg_count, false, false);
 			if (func == NULL)
 				break;
 			if (func->is_coll_derived) {
@@ -3991,14 +3991,14 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 			nFarg = pFarg ? pFarg->nExpr : 0;
 			assert(!ExprHasProperty(pExpr, EP_IntValue));
 			zId = pExpr->u.zToken;
-			pDef = sqlFindFunction(db, zId, nFarg, 0);
+			pDef = sql_find_function(db, zId, nFarg, false, false);
 #ifdef SQL_ENABLE_UNKNOWN_SQL_FUNCTION
 			if (pDef == 0 && pParse->explain) {
-				pDef =
-				    sqlFindFunction(db, "unknown", nFarg, 0);
+				pDef = sql_find_function(db, "unknown", nFarg,
+							 false, false);
 			}
 #endif
-			if (pDef == 0 || pDef->xFinalize != 0) {
+			if (pDef == NULL || pDef->xFinalize != NULL) {
 				diag_set(ClientError, ER_NO_SUCH_FUNCTION,
 					 zId);
 				pParse->is_aborted = true;
@@ -5461,12 +5461,12 @@ analyzeAggregate(Walker * pWalker, Expr * pExpr)
 						pItem->iMem = ++pParse->nMem;
 						assert(!ExprHasProperty
 						       (pExpr, EP_IntValue));
-						pItem->pFunc = sqlFindFunction(
+						pItem->pFunc = sql_find_function(
 							pParse->db,
 							pExpr->u.zToken,
 							pExpr->x.pList ?
 							pExpr->x.pList->nExpr : 0,
-							0);
+							false, false);
 						if (pExpr->flags & EP_Distinct) {
 							pItem->iDistinct =
 								pParse->nTab++;
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index bb7405e68..cbcf26125 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -1811,7 +1811,7 @@ sql_overload_function(sql * db, const char *zName,
 {
 	int rc = SQL_OK;
 
-	if (sqlFindFunction(db, zName, nArg, 0) == 0) {
+	if (sql_find_function(db, zName, nArg, false, false) == NULL) {
 		rc = sqlCreateFunc(db, zName, type, nArg, 0, 0,
 				       sqlInvalidFunction, 0, 0, 0);
 	}
@@ -1834,19 +1834,6 @@ sqlRegisterPerConnectionBuiltinFunctions(sql * db)
 	}
 }
 
-/*
- * Set the LIKEOPT flag on the 2-argument function with the given name.
- */
-static void
-setLikeOptFlag(sql * db, const char *zName, u8 flagVal)
-{
-	FuncDef *pDef;
-	pDef = sqlFindFunction(db, zName, 2, 0);
-	if (ALWAYS(pDef)) {
-		pDef->funcFlags |= flagVal;
-	}
-}
-
 /**
  * Register the built-in LIKE function.
  */
@@ -1859,13 +1846,14 @@ sqlRegisterLikeFunctions(sql *db, int is_case_insensitive)
 	 * supplied pattern and FALSE otherwise.
 	 */
 	int *is_like_ci = SQL_INT_TO_PTR(is_case_insensitive);
-	sqlCreateFunc(db, "LIKE", FIELD_TYPE_BOOLEAN, 2, 0,
-			  is_like_ci, likeFunc, 0, 0, 0);
-	sqlCreateFunc(db, "LIKE", FIELD_TYPE_BOOLEAN, 3, 0,
-			  is_like_ci, likeFunc, 0, 0, 0);
-	setLikeOptFlag(db, "LIKE",
-		       !(is_case_insensitive) ? (SQL_FUNC_LIKE |
-		       SQL_FUNC_CASE) : SQL_FUNC_LIKE);
+	struct FuncDef *like2 = sql_find_function(db, "LIKE", 2, true, false);
+	like2->funcFlags = is_case_insensitive ? SQL_FUNC_LIKE :
+			   SQL_FUNC_LIKE | SQL_FUNC_CASE;
+	like2->pUserData = is_like_ci;
+	assert(like2 != NULL);
+	struct FuncDef *like3 = sql_find_function(db, "LIKE", 2, true, false);
+	assert(like3 != NULL);
+	like3->pUserData = is_like_ci;
 }
 
 int
@@ -1875,7 +1863,8 @@ sql_is_like_func(struct sql *db, struct Expr *expr, int *is_like_ci)
 	    expr->x.pList->nExpr != 2)
 		return 0;
 	assert(!ExprHasProperty(expr, EP_xIsSelect));
-	struct FuncDef *func = sqlFindFunction(db, expr->u.zToken, 2, 0);
+	struct FuncDef *func =
+		sql_find_function(db, expr->u.zToken, 2, false, false);
 	assert(func != NULL);
 	if ((func->funcFlags & SQL_FUNC_LIKE) == 0)
 		return 0;
diff --git a/src/box/sql/main.c b/src/box/sql/main.c
index fe1135a71..d52ec1c49 100644
--- a/src/box/sql/main.c
+++ b/src/box/sql/main.c
@@ -428,29 +428,28 @@ sqlCreateFunc(sql * db,
 	assert(SQL_FUNC_CONSTANT == SQL_DETERMINISTIC);
 	extraFlags = flags & SQL_DETERMINISTIC;
 
-
+	/* Ensure there is no builtin function with this name. */
+	p = sql_find_function(db, zFunctionName, -2, true, false);
+	if (p != NULL) {
+		diag_set(ClientError, ER_SQL, "cannot create a function with "
+			 "a signature that coincides builtin function");
+		return SQL_TARANTOOL_ERROR;
+	}
 	/* Check if an existing function is being overridden or deleted. If so,
 	 * and there are active VMs, then return SQL_BUSY. If a function
 	 * is being overridden/deleted but there are no active VMs, allow the
 	 * operation to continue but invalidate all precompiled statements.
 	 */
-	p = sqlFindFunction(db, zFunctionName, nArg, 0);
-	if (p && p->nArg == nArg) {
-		if (db->nVdbeActive) {
-			sqlErrorWithMsg(db, SQL_BUSY,
-					    "unable to delete/modify user-function due to active statements");
-			assert(!db->mallocFailed);
-			return SQL_BUSY;
-		} else {
-			sqlExpirePreparedStatements(db);
-		}
-	}
-
-	p = sqlFindFunction(db, zFunctionName, nArg, 1);
-	assert(p || db->mallocFailed);
-	if (!p) {
-		return SQL_NOMEM;
+	p = sql_find_function(db, zFunctionName, nArg, false, false);
+	if (p != NULL && p->nArg == nArg && db->nVdbeActive > 0) {
+		sqlErrorWithMsg(db, SQL_BUSY, "unable to delete/modify "
+				"user-function due to active statements");
+		return SQL_BUSY;
 	}
+	/* Create a new function with given signature. */
+	p = sql_find_function(db, zFunctionName, nArg, false, true);
+	if (p == NULL)
+		return SQL_TARANTOOL_ERROR;
 
 	/* If an older version of the function with a configured destructor is
 	 * being replaced invoke the destructor function here.
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index 504096e6d..7cf79692c 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -602,10 +602,11 @@ resolveExprStep(Walker * pWalker, Expr * pExpr)
 			assert(!ExprHasProperty(pExpr, EP_xIsSelect));
 			zId = pExpr->u.zToken;
 			nId = sqlStrlen30(zId);
-			pDef = sqlFindFunction(pParse->db, zId, n, 0);
+			pDef = sql_find_function(pParse->db, zId, n,
+						 false, false);
 			if (pDef == 0) {
-				pDef =
-				    sqlFindFunction(pParse->db, zId, -2,0);
+				pDef = sql_find_function(pParse->db, zId, -2,
+							 false, false);
 				if (pDef == 0) {
 					no_such_func = 1;
 				} else {
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index d2be6701f..9384ec9e8 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -1506,7 +1506,6 @@ struct sql {
 #define SQL_ReverseOrder   0x00020000	/* Reverse unordered SELECTs */
 #define SQL_RecTriggers    0x00040000	/* Enable recursive triggers */
 #define SQL_AutoIndex      0x00100000	/* Enable automatic indexes */
-#define SQL_PreferBuiltin  0x00200000	/* Preference to built-in funcs */
 #define SQL_EnableTrigger  0x01000000	/* True to enable triggers */
 #define SQL_DeferFKs       0x02000000	/* Defer all FK constraints */
 #define SQL_VdbeEQP        0x08000000	/* Debug EXPLAIN QUERY PLAN */
@@ -3927,7 +3926,29 @@ void sqlSelectSetName(Select *, const char *);
 #define sqlSelectSetName(A,B)
 #endif
 void sqlInsertBuiltinFuncs(FuncDef *, int);
-FuncDef *sqlFindFunction(sql *, const char *, int, u8);
+
+/**
+ * Find a user function by given name and number of arguments.
+ * @param db The database handle.
+ * @param func_name The function name 0-terminated string.
+ * @param arg_count The count of function's arguments.
+ *                  May be set -1 for any number.
+ * @param is_builtin If this flag is set true, perform lookup
+ *                   for builtin function.
+ * @param is_create If this flag is set true, then a new (blank)
+ *                  FuncDef structure is created and liked into
+ *                  the "db" structure if a no matching function
+ *                  previously existed.
+ * @retval Returns FuncDef pointer when object was found and NULL
+ *         otherwise (if is_create is not set true).
+ *         When is_create is set true returns NULL only in case of
+ *         memory allocation error.
+ *         Sets diag message in case of error.
+ */
+struct FuncDef *
+sql_find_function(struct sql *db, const char *func_name, int arg_count,
+		  bool is_builtin, bool is_create);
+
 void sqlRegisterBuiltinFunctions(void);
 void sqlRegisterDateTimeFunctions(void);
 void sqlRegisterPerConnectionBuiltinFunctions(sql *);
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index f73ea0a71..537a72f10 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -1245,7 +1245,7 @@ valueFromFunction(sql * db,	/* The database connection */
 	pList = p->x.pList;
 	if (pList)
 		nVal = pList->nExpr;
-	pFunc = sqlFindFunction(db, p->u.zToken, nVal, 0);
+	pFunc = sql_find_function(db, p->u.zToken, nVal, false, false);
 	assert(pFunc);
 	if ((pFunc->funcFlags & (SQL_FUNC_CONSTANT | SQL_FUNC_SLOCHNG)) ==
 	    0 || (pFunc->funcFlags & SQL_FUNC_NEEDCOLL)
diff --git a/test/sql-tap/lua_sql.test.lua b/test/sql-tap/lua_sql.test.lua
index b0913e7f4..fd7cd732a 100755
--- a/test/sql-tap/lua_sql.test.lua
+++ b/test/sql-tap/lua_sql.test.lua
@@ -1,7 +1,7 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
 NULL = require('msgpack').NULL
-test:plan(24)
+test:plan(25)
 
 local function func1(a)
     return a
@@ -18,6 +18,15 @@ test:do_test(
     end,
     {2})
 
+test:do_test(
+    "lua_sql-1.1",
+    function ()
+        local ret, errmsg = pcall(box.internal.sql_create_function,
+                                  "upper", "TEXT", func1)
+        return {ret, errmsg}
+    end,
+    {0, "SQL error: cannot create a function with a signature that coincides builtin function"})
+
 -- new function should replace prewious one
 test:do_test(
     "lua_sql-1.1",
-- 
2.21.0

[tarantool-patches] Re: [PATCH v1 2/3] sql: ban sql functions coinciding with builtins

[n.pettik]

> On 15 May 2019, at 20:34, Kirill Shcherbatov <kshcherbatov@tarantool.org> wrote:
> 
> Previously, it was possible to create a function with name
> that is already reserved for some predefined builtin.
> It is a bug. This patch bans sql_create_function for such cases.
> 
> Also removed FUNC_PERFECT_MATCH user session flag.
> 
> Closes #4219
> Needed for #3691

Could you please split this patch into two: first one would
provide only code style fixes and the second one - functional
changes (i.e. exactly what commit subject says)?

[tarantool-patches] Re: [PATCH v1 2/3] sql: ban sql functions coinciding with builtins

[Kirill Shcherbatov]

The code of the original sqlFindFunction function does not match
our codestyle and is difficult to maintain and extend. This patch
does not make any functional changes in Tarantool, only optimizes
the sqlFindFunction function,

Needed #3691
---
 src/box/lua/lua_sql.c  |   8 +-
 src/box/sql/callback.c | 179 ++++++++++++++++++++---------------------
 src/box/sql/expr.c     |  16 ++--
 src/box/sql/func.c     |   6 +-
 src/box/sql/main.c     |  10 +--
 src/box/sql/resolve.c  |   6 +-
 src/box/sql/sqlInt.h   |  22 ++++-
 src/box/sql/vdbemem.c  |   2 +-
 8 files changed, 135 insertions(+), 114 deletions(-)

diff --git a/src/box/lua/lua_sql.c b/src/box/lua/lua_sql.c
index 36b75ff08..561353520 100644
--- a/src/box/lua/lua_sql.c
+++ b/src/box/lua/lua_sql.c
@@ -195,7 +195,11 @@ lbox_sql_create_function(struct lua_State *L)
 					   is_deterministic ? SQL_DETERMINISTIC : 0,
 					   func_info, lua_sql_call, NULL, NULL,
 					   lua_sql_destroy);
-	if (rc != 0)
-		return luaL_error(L, sqlErrStr(rc));
+	if (rc != 0) {
+		const char *err = rc == SQL_TARANTOOL_ERROR ?
+				  box_error_message(box_error_last()) :
+				  sqlErrStr(rc);
+		return luaL_error(L, err);
+	}
 	return 0;
 }
diff --git a/src/box/sql/callback.c b/src/box/sql/callback.c
index 49197532e..54d59bbd3 100644
--- a/src/box/sql/callback.c
+++ b/src/box/sql/callback.c
@@ -131,6 +131,19 @@ functionSearch(int h,		/* Hash of the name */
 	return 0;
 }
 
+/**
+ * Cache function is used to organise sqlBuiltinFunctions table.
+ * @param func_name The name of builtin function.
+ * @param func_name_len The length of the @a name.
+ * @retval Hash value is calculated for given name.
+ */
+static int
+sql_builtin_func_name_hash(const char *func_name, uint32_t func_name_len)
+{
+	return (sqlUpperToLower[(u8) func_name[0]] +
+		func_name_len) % SQL_FUNC_HASH_SZ;
+}
+
 /*
  * Insert a new FuncDef into a FuncDefHash hash table.
  */
@@ -144,9 +157,7 @@ sqlInsertBuiltinFuncs(FuncDef * aDef,	/* List of global functions to be inserted
 		FuncDef *pOther;
 		const char *zName = aDef[i].zName;
 		int nName = sqlStrlen30(zName);
-		int h =
-		    (sqlUpperToLower[(u8) zName[0]] +
-		     nName) % SQL_FUNC_HASH_SZ;
+		int h = sql_builtin_func_name_hash(zName, nName);
 		pOther = functionSearch(h, zName);
 		if (pOther) {
 			assert(pOther != &aDef[i] && pOther->pNext != &aDef[i]);
@@ -160,110 +171,98 @@ sqlInsertBuiltinFuncs(FuncDef * aDef,	/* List of global functions to be inserted
 	}
 }
 
-/*
- * Locate a user function given a name, a number of arguments and a flag
- * indicating whether the function prefers UTF-16 over UTF-8.  Return a
- * pointer to the FuncDef structure that defines that function, or return
- * NULL if the function does not exist.
- *
- * If the createFlag argument is true, then a new (blank) FuncDef
- * structure is created and liked into the "db" structure if a
- * no matching function previously existed.
- *
- * If nArg is -2, then the first valid function found is returned.  A
- * function is valid if xSFunc is non-zero.  The nArg==(-2)
- * case is used to see if zName is a valid function name for some number
- * of arguments.  If nArg is -2, then createFlag must be 0.
- *
- * If createFlag is false, then a function with the required name and
- * number of arguments may be returned even if the eTextRep flag does not
- * match that requested.
- */
-FuncDef *
-sqlFindFunction(sql * db,	/* An open database */
-		    const char *zName,	/* Name of the function.  zero-terminated */
-		    int nArg,	/* Number of arguments.  -1 means any number */
-		    u8 createFlag	/* Create new entry if true and does not otherwise exist */
-    )
+struct FuncDef *
+sql_find_function(struct sql *db, const char *func_name, int arg_count,
+		  bool is_create)
 {
-	FuncDef *p;		/* Iterator variable */
-	FuncDef *pBest = 0;	/* Best match found so far */
-	int bestScore = 0;	/* Score of best match */
-	int h;			/* Hash value */
-	int nName;		/* Length of the name */
-	struct session *user_session = current_session();
+	assert(arg_count >= -2);
+	assert(arg_count >= -1 || !is_create);
+	uint32_t func_name_len = strlen(func_name);
+	/*
+	 * The 'score' of the best match is calculated with
+	 * matchQuality estimator.
+	 */
+	int func_score = 0;
+	/* Best match found so far. */
+	struct FuncDef *func = NULL;
 
-	assert(nArg >= (-2));
-	assert(nArg >= (-1) || createFlag == 0);
-	nName = sqlStrlen30(zName);
+	struct session *user_session = current_session();
+	bool is_builtin = (user_session->sql_flags & SQL_PreferBuiltin) != 0;
+	if (is_builtin)
+		goto lookup_for_builtin;
 
-	/* First search for a match amongst the application-defined functions.
+	/*
+	 * First search for a match amongst the
+	 * application-defined functions.
 	 */
-	p = (FuncDef *) sqlHashFind(&db->aFunc, zName);
-	while (p) {
-		int score = matchQuality(p, nArg);
-		if (score > bestScore) {
-			pBest = p;
-			bestScore = score;
+	for (struct FuncDef *p = sqlHashFind(&db->aFunc, func_name);
+	      p != NULL; p = p->pNext) {
+		int score = matchQuality(p, arg_count);
+		if (score > func_score) {
+			func = p;
+			func_score = score;
 		}
-		p = p->pNext;
 	}
 
-	/* If no match is found, search the built-in functions.
+	/**
+	 * If no match is found, search the built-in functions.
 	 *
-	 * If the SQL_PreferBuiltin flag is set, then search the built-in
-	 * functions even if a prior app-defined function was found.  And give
-	 * priority to built-in functions.
+	 * If the SQL_PreferBuiltin flag is set, then search the
+	 * built-in functions even if a prior app-defined function
+	 * was found.  And give priority to built-in functions.
 	 *
-	 * Except, if createFlag is true, that means that we are trying to
-	 * install a new function.  Whatever FuncDef structure is returned it will
-	 * have fields overwritten with new information appropriate for the
-	 * new function.  But the FuncDefs for built-in functions are read-only.
-	 * So we must not search for built-ins when creating a new function.
+	 * Except, if is_create is true, that means that we are
+	 * trying to install a new function. Whatever FuncDef
+	 * structure is returned it will have fields overwritten
+	 * with new information appropriate for the new function.
+	 * But the FuncDefs for built-in functions are read-only.
+	 * So we must not search for built-ins when creating a
+	 * new function.
 	 */
-	if (!createFlag &&
-	    (pBest == 0
-	     || (user_session->sql_flags & SQL_PreferBuiltin) != 0)) {
-		bestScore = 0;
-		h = (sqlUpperToLower[(u8) zName[0]] +
-		     nName) % SQL_FUNC_HASH_SZ;
-		p = functionSearch(h, zName);
-		while (p) {
-			int score = matchQuality(p, nArg);
-			if (score > bestScore) {
-				pBest = p;
-				bestScore = score;
+	if (!is_create && (func == NULL || is_builtin)) {
+lookup_for_builtin:
+		func_score = 0;
+		int h = sql_builtin_func_name_hash(func_name, func_name_len);
+		for (struct FuncDef *p = functionSearch(h, func_name);
+		     p != NULL; p = p->pNext) {
+			int score = matchQuality(p, arg_count);
+			if (score > func_score) {
+				func = p;
+				func_score = score;
 			}
-			p = p->pNext;
 		}
 	}
 
-	/* If the createFlag parameter is true and the search did not reveal an
-	 * exact match for the name, number of arguments and encoding, then add a
-	 * new entry to the hash table and return it.
+	/*
+	 * If the is_create parameter is true and the search did
+	 * not reveal an exact match for the name, number of
+	 * arguments and encoding, then add a new entry to the
+	 * hash table and return it.
 	 */
-	if (createFlag && bestScore < FUNC_PERFECT_MATCH &&
-	    (pBest =
-	     sqlDbMallocZero(db, sizeof(*pBest) + nName + 1)) != 0) {
-		FuncDef *pOther;
-		pBest->zName = (const char *)&pBest[1];
-		pBest->nArg = (u16) nArg;
-		pBest->funcFlags = 0;
-		memcpy((char *)&pBest[1], zName, nName + 1);
-		pOther =
-		    (FuncDef *) sqlHashInsert(&db->aFunc, pBest->zName,
-						  pBest);
-		if (pOther == pBest) {
-			sqlDbFree(db, pBest);
+	if (is_create && func_score < FUNC_PERFECT_MATCH) {
+		uint32_t func_sz = sizeof(func[0]) + func_name_len + 1;
+		func = sqlDbMallocZero(db, func_sz);
+		if (func == NULL) {
+			diag_set(OutOfMemory, func_sz, "sqlDbMallocZero",
+				 "func");
+			return NULL;
+		}
+		func->zName = (const char *) &func[1];
+		func->nArg = (u16) arg_count;
+		func->funcFlags = 0;
+		memcpy((char *)&func[1], func_name, func_name_len + 1);
+		struct FuncDef *old_func =
+			(struct FuncDef *) sqlHashInsert(&db->aFunc, func->zName,
+							 func);
+		if (old_func == func) {
+			sqlDbFree(db, func);
 			sqlOomFault(db);
-			return 0;
+			diag_set(OutOfMemory, func_sz, "sqlHashInsert",
+				 "func");
 		} else {
-			pBest->pNext = pOther;
+			func->pNext = old_func;
 		}
 	}
-
-	if (pBest && (pBest->xSFunc || createFlag)) {
-		return pBest;
-	}
-	return 0;
+	return func != NULL &&
+	       (func->xSFunc != NULL || is_create || is_builtin) ? func : NULL;
 }
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index ba7eea59d..12b54ff65 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -285,9 +285,9 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id,
 		if (op == TK_FUNCTION) {
 			uint32_t arg_count = p->x.pList == NULL ? 0 :
 					     p->x.pList->nExpr;
-			struct FuncDef *func = sqlFindFunction(parse->db,
-							       p->u.zToken,
-							       arg_count, 0);
+			struct FuncDef *func =
+				sql_find_function(parse->db, p->u.zToken,
+						  arg_count, false);
 			if (func == NULL)
 				break;
 			if (func->is_coll_derived) {
@@ -3991,11 +3991,11 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 			nFarg = pFarg ? pFarg->nExpr : 0;
 			assert(!ExprHasProperty(pExpr, EP_IntValue));
 			zId = pExpr->u.zToken;
-			pDef = sqlFindFunction(db, zId, nFarg, 0);
+			pDef = sql_find_function(db, zId, nFarg, false);
 #ifdef SQL_ENABLE_UNKNOWN_SQL_FUNCTION
 			if (pDef == 0 && pParse->explain) {
-				pDef =
-				    sqlFindFunction(db, "unknown", nFarg, 0);
+				pDef = sql_find_function(db, "unknown", nFarg,
+							 false);
 			}
 #endif
 			if (pDef == 0 || pDef->xFinalize != 0) {
@@ -5461,12 +5461,12 @@ analyzeAggregate(Walker * pWalker, Expr * pExpr)
 						pItem->iMem = ++pParse->nMem;
 						assert(!ExprHasProperty
 						       (pExpr, EP_IntValue));
-						pItem->pFunc = sqlFindFunction(
+						pItem->pFunc = sql_find_function(
 							pParse->db,
 							pExpr->u.zToken,
 							pExpr->x.pList ?
 							pExpr->x.pList->nExpr : 0,
-							0);
+							false);
 						if (pExpr->flags & EP_Distinct) {
 							pItem->iDistinct =
 								pParse->nTab++;
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index bb7405e68..c110b205c 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -1811,7 +1811,7 @@ sql_overload_function(sql * db, const char *zName,
 {
 	int rc = SQL_OK;
 
-	if (sqlFindFunction(db, zName, nArg, 0) == 0) {
+	if (sql_find_function(db, zName, nArg, false) == 0) {
 		rc = sqlCreateFunc(db, zName, type, nArg, 0, 0,
 				       sqlInvalidFunction, 0, 0, 0);
 	}
@@ -1841,7 +1841,7 @@ static void
 setLikeOptFlag(sql * db, const char *zName, u8 flagVal)
 {
 	FuncDef *pDef;
-	pDef = sqlFindFunction(db, zName, 2, 0);
+	pDef = sql_find_function(db, zName, 2, false);
 	if (ALWAYS(pDef)) {
 		pDef->funcFlags |= flagVal;
 	}
@@ -1875,7 +1875,7 @@ sql_is_like_func(struct sql *db, struct Expr *expr, int *is_like_ci)
 	    expr->x.pList->nExpr != 2)
 		return 0;
 	assert(!ExprHasProperty(expr, EP_xIsSelect));
-	struct FuncDef *func = sqlFindFunction(db, expr->u.zToken, 2, 0);
+	struct FuncDef *func = sql_find_function(db, expr->u.zToken, 2, false);
 	assert(func != NULL);
 	if ((func->funcFlags & SQL_FUNC_LIKE) == 0)
 		return 0;
diff --git a/src/box/sql/main.c b/src/box/sql/main.c
index fe1135a71..0e8f3e559 100644
--- a/src/box/sql/main.c
+++ b/src/box/sql/main.c
@@ -434,7 +434,7 @@ sqlCreateFunc(sql * db,
 	 * is being overridden/deleted but there are no active VMs, allow the
 	 * operation to continue but invalidate all precompiled statements.
 	 */
-	p = sqlFindFunction(db, zFunctionName, nArg, 0);
+	p = sql_find_function(db, zFunctionName, nArg, false);
 	if (p && p->nArg == nArg) {
 		if (db->nVdbeActive) {
 			sqlErrorWithMsg(db, SQL_BUSY,
@@ -446,11 +446,9 @@ sqlCreateFunc(sql * db,
 		}
 	}
 
-	p = sqlFindFunction(db, zFunctionName, nArg, 1);
-	assert(p || db->mallocFailed);
-	if (!p) {
-		return SQL_NOMEM;
-	}
+	p = sql_find_function(db, zFunctionName, nArg, true);
+	if (p == NULL)
+		return SQL_TARANTOOL_ERROR;
 
 	/* If an older version of the function with a configured destructor is
 	 * being replaced invoke the destructor function here.
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index 504096e6d..7e2eed7bb 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -602,10 +602,10 @@ resolveExprStep(Walker * pWalker, Expr * pExpr)
 			assert(!ExprHasProperty(pExpr, EP_xIsSelect));
 			zId = pExpr->u.zToken;
 			nId = sqlStrlen30(zId);
-			pDef = sqlFindFunction(pParse->db, zId, n, 0);
+			pDef = sql_find_function(pParse->db, zId, n, false);
 			if (pDef == 0) {
-				pDef =
-				    sqlFindFunction(pParse->db, zId, -2,0);
+				pDef = sql_find_function(pParse->db, zId, -2,
+							 false);
 				if (pDef == 0) {
 					no_such_func = 1;
 				} else {
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index d2be6701f..61406cdc9 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -3927,7 +3927,27 @@ void sqlSelectSetName(Select *, const char *);
 #define sqlSelectSetName(A,B)
 #endif
 void sqlInsertBuiltinFuncs(FuncDef *, int);
-FuncDef *sqlFindFunction(sql *, const char *, int, u8);
+
+/**
+ * Find a user function by given name and number of arguments.
+ * @param db The database handle.
+ * @param func_name The function name 0-terminated string.
+ * @param arg_count The count of function's arguments.
+ *                  May be set -1 for any number.
+ * @param is_create If this flag is set true, then a new (blank)
+ *                  FuncDef structure is created and liked into
+ *                  the "db" structure if a no matching function
+ *                  previously existed.
+ * @retval Returns FuncDef pointer when object was found and NULL
+ *         otherwise (if is_create is not set true).
+ *         When is_create is set true returns NULL only in case of
+ *         memory allocation error.
+ *         Sets diag message in case of error.
+ */
+struct FuncDef *
+sql_find_function(struct sql *db, const char *func_name, int arg_count,
+		  bool is_create);
+
 void sqlRegisterBuiltinFunctions(void);
 void sqlRegisterDateTimeFunctions(void);
 void sqlRegisterPerConnectionBuiltinFunctions(sql *);
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index f73ea0a71..c56eb73e6 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -1245,7 +1245,7 @@ valueFromFunction(sql * db,	/* The database connection */
 	pList = p->x.pList;
 	if (pList)
 		nVal = pList->nExpr;
-	pFunc = sqlFindFunction(db, p->u.zToken, nVal, 0);
+	pFunc = sql_find_function(db, p->u.zToken, nVal, false);
 	assert(pFunc);
 	if ((pFunc->funcFlags & (SQL_FUNC_CONSTANT | SQL_FUNC_SLOCHNG)) ==
 	    0 || (pFunc->funcFlags & SQL_FUNC_NEEDCOLL)
-- 
2.21.0

[tarantool-patches] Re: [PATCH v1 2/3] sql: ban sql functions coinciding with builtins

[n.pettik]

> On 17 May 2019, at 11:22, Kirill Shcherbatov <kshcherbatov@tarantool.org> wrote:
> 
> The code of the original sqlFindFunction function does not match
> our codestyle and is difficult to maintain and extend. This patch
> does not make any functional changes in Tarantool, only optimizes
> the sqlFindFunction function,
> 
> Needed #3691
> ---
> diff --git a/src/box/lua/lua_sql.c b/src/box/lua/lua_sql.c
> index 36b75ff08..561353520 100644
> --- a/src/box/lua/lua_sql.c
> +++ b/src/box/lua/lua_sql.c
> @@ -195,7 +195,11 @@ lbox_sql_create_function(struct lua_State *L)
> 					   is_deterministic ? SQL_DETERMINISTIC : 0,
> 					   func_info, lua_sql_call, NULL, NULL,
> 					   lua_sql_destroy);
> -	if (rc != 0)
> -		return luaL_error(L, sqlErrStr(rc));
> +	if (rc != 0) {
> +		const char *err = rc == SQL_TARANTOOL_ERROR ?
> +				  box_error_message(box_error_last()) :
> +				  sqlErrStr(rc);
> +		return luaL_error(L, err);
> +	}

TBO, this looks more like functional change requiring separate test.

> 	return 0;
> }
> diff --git a/src/box/sql/callback.c b/src/box/sql/callback.c
> index 49197532e..54d59bbd3 100644
> --- a/src/box/sql/callback.c
> +++ b/src/box/sql/callback.c
> @@ -131,6 +131,19 @@ functionSearch(int h,		/* Hash of the name */
> 	return 0;
> }
> 
> +/**
> + * Cache function is used to organise sqlBuiltinFunctions table.
> + * @param func_name The name of builtin function.
> + * @param func_name_len The length of the @a name.
> + * @retval Hash value is calculated for given name.
> + */
> +static int

Why not inline?

> +sql_builtin_func_name_hash(const char *func_name, uint32_t func_name_len)
> +{
> +	return (sqlUpperToLower[(u8) func_name[0]] +
> +		func_name_len) % SQL_FUNC_HASH_SZ;
> +}
> +
> @@ -160,110 +171,98 @@ sqlInsertBuiltinFuncs(FuncDef * aDef,	/* List of global functions to be inserted
> 
> +struct FuncDef *
> +sql_find_function(struct sql *db, const char *func_name, int arg_count,
> +		  bool is_create)
> {
> -	FuncDef *p;		/* Iterator variable */
> -	FuncDef *pBest = 0;	/* Best match found so far */
> -	int bestScore = 0;	/* Score of best match */
> -	int h;			/* Hash value */
> -	int nName;		/* Length of the name */
> -	struct session *user_session = current_session();
> +	assert(arg_count >= -2);
> +	assert(arg_count >= -1 || !is_create);
> +	uint32_t func_name_len = strlen(func_name);
> +	/*
> +	 * The 'score' of the best match is calculated with
> +	 * matchQuality estimator.
> +	 */
> +	int func_score = 0;
> +	/* Best match found so far. */
> +	struct FuncDef *func = NULL;
> 
> -	assert(nArg >= (-2));
> -	assert(nArg >= (-1) || createFlag == 0);
> -	nName = sqlStrlen30(zName);
> +	struct session *user_session = current_session();
> +	bool is_builtin = (user_session->sql_flags & SQL_PreferBuiltin) != 0;
> +	if (is_builtin)
> +		goto lookup_for_builtin;

Could you please explain why do we need separate lookups for
built-in functions and user-defined one? Patch states that now
we share one name-space for both function types. Let’s utilise
the same algorithm for searching functions in hash.
The only problem I see is that there are several built-in functions
with the same name, but different number of arguments. And
depending on number of arguments we one or another function
is used. For example, when max() takes more than one arg, it
becomes non-aggregate function returning maximum from its
arguments. However, ANSI doesn’t declare such behaviour.
As an alternative, several DBs use greatest() and least()
names. So, I suggest to remove ambiguous and make
name hash contains only unique values.   

What is more, lets separate lookup from adding function to hash:
introduce sql_function_by_name() and sql_function_hash_add()

These steps would make work with functions way much more
cleaner and simpler.

[tarantool-patches] Re: [PATCH v1 2/3] sql: ban sql functions coinciding with builtins

[Kirill Shcherbatov]

Previously, it was possible to create a function with name
that is already reserved for some predefined builtin.
It is a bug. This patch bans sql_create_function for such cases.

Also removed SQL_PreferBuiltin user session flag.

Closes #4219
Needed for #3691
---
 src/box/sql/callback.c        | 12 ++----------
 src/box/sql/expr.c            |  8 ++++----
 src/box/sql/func.c            | 33 +++++++++++----------------------
 src/box/sql/main.c            | 12 +++++++++---
 src/box/sql/resolve.c         |  5 +++--
 src/box/sql/sqlInt.h          |  5 +++--
 src/box/sql/vdbemem.c         |  2 +-
 test/sql-tap/lua_sql.test.lua | 11 ++++++++++-
 8 files changed, 43 insertions(+), 45 deletions(-)

diff --git a/src/box/sql/callback.c b/src/box/sql/callback.c
index 54d59bbd3..b5163c208 100644
--- a/src/box/sql/callback.c
+++ b/src/box/sql/callback.c
@@ -37,7 +37,6 @@
 
 #include "box/coll_id_cache.h"
 #include "sqlInt.h"
-#include "box/session.h"
 
 struct coll *
 sql_get_coll_seq(Parse *parser, const char *name, uint32_t *coll_id)
@@ -173,7 +172,7 @@ sqlInsertBuiltinFuncs(FuncDef * aDef,	/* List of global functions to be inserted
 
 struct FuncDef *
 sql_find_function(struct sql *db, const char *func_name, int arg_count,
-		  bool is_create)
+		  bool is_builtin, bool is_create)
 {
 	assert(arg_count >= -2);
 	assert(arg_count >= -1 || !is_create);
@@ -185,9 +184,6 @@ sql_find_function(struct sql *db, const char *func_name, int arg_count,
 	int func_score = 0;
 	/* Best match found so far. */
 	struct FuncDef *func = NULL;
-
-	struct session *user_session = current_session();
-	bool is_builtin = (user_session->sql_flags & SQL_PreferBuiltin) != 0;
 	if (is_builtin)
 		goto lookup_for_builtin;
 
@@ -207,10 +203,6 @@ sql_find_function(struct sql *db, const char *func_name, int arg_count,
 	/**
 	 * If no match is found, search the built-in functions.
 	 *
-	 * If the SQL_PreferBuiltin flag is set, then search the
-	 * built-in functions even if a prior app-defined function
-	 * was found.  And give priority to built-in functions.
-	 *
 	 * Except, if is_create is true, that means that we are
 	 * trying to install a new function. Whatever FuncDef
 	 * structure is returned it will have fields overwritten
@@ -219,7 +211,7 @@ sql_find_function(struct sql *db, const char *func_name, int arg_count,
 	 * So we must not search for built-ins when creating a
 	 * new function.
 	 */
-	if (!is_create && (func == NULL || is_builtin)) {
+	if (!is_create && func == NULL) {
 lookup_for_builtin:
 		func_score = 0;
 		int h = sql_builtin_func_name_hash(func_name, func_name_len);
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 12b54ff65..7133ec3ac 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -287,7 +287,7 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id,
 					     p->x.pList->nExpr;
 			struct FuncDef *func =
 				sql_find_function(parse->db, p->u.zToken,
-						  arg_count, false);
+						  arg_count, false, false);
 			if (func == NULL)
 				break;
 			if (func->is_coll_derived) {
@@ -3991,11 +3991,11 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 			nFarg = pFarg ? pFarg->nExpr : 0;
 			assert(!ExprHasProperty(pExpr, EP_IntValue));
 			zId = pExpr->u.zToken;
-			pDef = sql_find_function(db, zId, nFarg, false);
+			pDef = sql_find_function(db, zId, nFarg, false, false);
 #ifdef SQL_ENABLE_UNKNOWN_SQL_FUNCTION
 			if (pDef == 0 && pParse->explain) {
 				pDef = sql_find_function(db, "unknown", nFarg,
-							 false);
+							 false, false);
 			}
 #endif
 			if (pDef == 0 || pDef->xFinalize != 0) {
@@ -5466,7 +5466,7 @@ analyzeAggregate(Walker * pWalker, Expr * pExpr)
 							pExpr->u.zToken,
 							pExpr->x.pList ?
 							pExpr->x.pList->nExpr : 0,
-							false);
+							false, false);
 						if (pExpr->flags & EP_Distinct) {
 							pItem->iDistinct =
 								pParse->nTab++;
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index c110b205c..afc25aab5 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -1811,7 +1811,7 @@ sql_overload_function(sql * db, const char *zName,
 {
 	int rc = SQL_OK;
 
-	if (sql_find_function(db, zName, nArg, false) == 0) {
+	if (sql_find_function(db, zName, nArg, false, false) == 0) {
 		rc = sqlCreateFunc(db, zName, type, nArg, 0, 0,
 				       sqlInvalidFunction, 0, 0, 0);
 	}
@@ -1834,19 +1834,6 @@ sqlRegisterPerConnectionBuiltinFunctions(sql * db)
 	}
 }
 
-/*
- * Set the LIKEOPT flag on the 2-argument function with the given name.
- */
-static void
-setLikeOptFlag(sql * db, const char *zName, u8 flagVal)
-{
-	FuncDef *pDef;
-	pDef = sql_find_function(db, zName, 2, false);
-	if (ALWAYS(pDef)) {
-		pDef->funcFlags |= flagVal;
-	}
-}
-
 /**
  * Register the built-in LIKE function.
  */
@@ -1859,13 +1846,14 @@ sqlRegisterLikeFunctions(sql *db, int is_case_insensitive)
 	 * supplied pattern and FALSE otherwise.
 	 */
 	int *is_like_ci = SQL_INT_TO_PTR(is_case_insensitive);
-	sqlCreateFunc(db, "LIKE", FIELD_TYPE_BOOLEAN, 2, 0,
-			  is_like_ci, likeFunc, 0, 0, 0);
-	sqlCreateFunc(db, "LIKE", FIELD_TYPE_BOOLEAN, 3, 0,
-			  is_like_ci, likeFunc, 0, 0, 0);
-	setLikeOptFlag(db, "LIKE",
-		       !(is_case_insensitive) ? (SQL_FUNC_LIKE |
-		       SQL_FUNC_CASE) : SQL_FUNC_LIKE);
+	struct FuncDef *like2 = sql_find_function(db, "LIKE", 2, true, false);
+	like2->funcFlags = is_case_insensitive ? SQL_FUNC_LIKE :
+			   SQL_FUNC_LIKE | SQL_FUNC_CASE;
+	like2->pUserData = is_like_ci;
+	assert(like2 != NULL);
+	struct FuncDef *like3 = sql_find_function(db, "LIKE", 2, true, false);
+	assert(like3 != NULL);
+	like3->pUserData = is_like_ci;
 }
 
 int
@@ -1875,7 +1863,8 @@ sql_is_like_func(struct sql *db, struct Expr *expr, int *is_like_ci)
 	    expr->x.pList->nExpr != 2)
 		return 0;
 	assert(!ExprHasProperty(expr, EP_xIsSelect));
-	struct FuncDef *func = sql_find_function(db, expr->u.zToken, 2, false);
+	struct FuncDef *func =
+		sql_find_function(db, expr->u.zToken, 2, false, false);
 	assert(func != NULL);
 	if ((func->funcFlags & SQL_FUNC_LIKE) == 0)
 		return 0;
diff --git a/src/box/sql/main.c b/src/box/sql/main.c
index 0e8f3e559..cd2d00642 100644
--- a/src/box/sql/main.c
+++ b/src/box/sql/main.c
@@ -428,13 +428,19 @@ sqlCreateFunc(sql * db,
 	assert(SQL_FUNC_CONSTANT == SQL_DETERMINISTIC);
 	extraFlags = flags & SQL_DETERMINISTIC;
 
-
+	/* Ensure there is no builtin function with this name. */
+	p = sql_find_function(db, zFunctionName, -2, true, false);
+	if (p != NULL) {
+		diag_set(ClientError, ER_SQL, "cannot create a function with "
+			 "a signature that coincides builtin function");
+		return SQL_TARANTOOL_ERROR;
+	}
 	/* Check if an existing function is being overridden or deleted. If so,
 	 * and there are active VMs, then return SQL_BUSY. If a function
 	 * is being overridden/deleted but there are no active VMs, allow the
 	 * operation to continue but invalidate all precompiled statements.
 	 */
-	p = sql_find_function(db, zFunctionName, nArg, false);
+	p = sql_find_function(db, zFunctionName, nArg, false, false);
 	if (p && p->nArg == nArg) {
 		if (db->nVdbeActive) {
 			sqlErrorWithMsg(db, SQL_BUSY,
@@ -446,7 +452,7 @@ sqlCreateFunc(sql * db,
 		}
 	}
 
-	p = sql_find_function(db, zFunctionName, nArg, true);
+	p = sql_find_function(db, zFunctionName, nArg, false, true);
 	if (p == NULL)
 		return SQL_TARANTOOL_ERROR;
 
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index 7e2eed7bb..7cf79692c 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -602,10 +602,11 @@ resolveExprStep(Walker * pWalker, Expr * pExpr)
 			assert(!ExprHasProperty(pExpr, EP_xIsSelect));
 			zId = pExpr->u.zToken;
 			nId = sqlStrlen30(zId);
-			pDef = sql_find_function(pParse->db, zId, n, false);
+			pDef = sql_find_function(pParse->db, zId, n,
+						 false, false);
 			if (pDef == 0) {
 				pDef = sql_find_function(pParse->db, zId, -2,
-							 false);
+							 false, false);
 				if (pDef == 0) {
 					no_such_func = 1;
 				} else {
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 61406cdc9..8cdad4749 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -1506,7 +1506,6 @@ struct sql {
 #define SQL_ReverseOrder   0x00020000	/* Reverse unordered SELECTs */
 #define SQL_RecTriggers    0x00040000	/* Enable recursive triggers */
 #define SQL_AutoIndex      0x00100000	/* Enable automatic indexes */
-#define SQL_PreferBuiltin  0x00200000	/* Preference to built-in funcs */
 #define SQL_EnableTrigger  0x01000000	/* True to enable triggers */
 #define SQL_DeferFKs       0x02000000	/* Defer all FK constraints */
 #define SQL_VdbeEQP        0x08000000	/* Debug EXPLAIN QUERY PLAN */
@@ -3934,6 +3933,8 @@ void sqlInsertBuiltinFuncs(FuncDef *, int);
  * @param func_name The function name 0-terminated string.
  * @param arg_count The count of function's arguments.
  *                  May be set -1 for any number.
+ * @param is_builtin If this flag is set true, perform lookup
+ *                   for the built-in function.
  * @param is_create If this flag is set true, then a new (blank)
  *                  FuncDef structure is created and liked into
  *                  the "db" structure if a no matching function
@@ -3946,7 +3947,7 @@ void sqlInsertBuiltinFuncs(FuncDef *, int);
  */
 struct FuncDef *
 sql_find_function(struct sql *db, const char *func_name, int arg_count,
-		  bool is_create);
+		  bool is_builtin, bool is_create);
 
 void sqlRegisterBuiltinFunctions(void);
 void sqlRegisterDateTimeFunctions(void);
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index c56eb73e6..537a72f10 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -1245,7 +1245,7 @@ valueFromFunction(sql * db,	/* The database connection */
 	pList = p->x.pList;
 	if (pList)
 		nVal = pList->nExpr;
-	pFunc = sql_find_function(db, p->u.zToken, nVal, false);
+	pFunc = sql_find_function(db, p->u.zToken, nVal, false, false);
 	assert(pFunc);
 	if ((pFunc->funcFlags & (SQL_FUNC_CONSTANT | SQL_FUNC_SLOCHNG)) ==
 	    0 || (pFunc->funcFlags & SQL_FUNC_NEEDCOLL)
diff --git a/test/sql-tap/lua_sql.test.lua b/test/sql-tap/lua_sql.test.lua
index b0913e7f4..fd7cd732a 100755
--- a/test/sql-tap/lua_sql.test.lua
+++ b/test/sql-tap/lua_sql.test.lua
@@ -1,7 +1,7 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
 NULL = require('msgpack').NULL
-test:plan(24)
+test:plan(25)
 
 local function func1(a)
     return a
@@ -18,6 +18,15 @@ test:do_test(
     end,
     {2})
 
+test:do_test(
+    "lua_sql-1.1",
+    function ()
+        local ret, errmsg = pcall(box.internal.sql_create_function,
+                                  "upper", "TEXT", func1)
+        return {ret, errmsg}
+    end,
+    {0, "SQL error: cannot create a function with a signature that coincides builtin function"})
+
 -- new function should replace prewious one
 test:do_test(
     "lua_sql-1.1",
-- 
2.21.0

[tarantool-patches] [PATCH v1 3/3] box: local sql_flags for parser and vdbe

[Kirill Shcherbatov]

The sql_flags is a parser parameter that describe how to parse the
SQL request, but now this information is taken from the global
user session object.
When we need to run the parser with some other parameters, it is
necessary to change global session object, which may lead to
unpredictable consequences in general case.
Introduced a new parser and vdbe field sql_flags is responsible
for SQL parsing results.

Needed for #3691
---
 src/box/sql.c               |  2 +-
 src/box/sql.h               |  3 +-
 src/box/sql/delete.c        | 12 +++----
 src/box/sql/expr.c          |  7 ++--
 src/box/sql/fk_constraint.c |  7 ++--
 src/box/sql/insert.c        | 18 +++++-----
 src/box/sql/prepare.c       |  5 +--
 src/box/sql/select.c        | 24 +++++--------
 src/box/sql/sqlInt.h        |  5 ++-
 src/box/sql/tokenize.c      |  7 ++--
 src/box/sql/trigger.c       | 13 +++----
 src/box/sql/update.c        | 15 ++++----
 src/box/sql/vdbe.c          | 72 ++++++++++++++++++-------------------
 src/box/sql/vdbeInt.h       |  2 ++
 src/box/sql/where.c         | 13 ++++---
 15 files changed, 96 insertions(+), 109 deletions(-)

diff --git a/src/box/sql.c b/src/box/sql.c
index fbfa59992..385676055 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -1354,7 +1354,7 @@ sql_checks_resolve_space_def_reference(ExprList *expr_list,
 				       struct space_def *def)
 {
 	Parse parser;
-	sql_parser_create(&parser, sql_get());
+	sql_parser_create(&parser, sql_get(), default_flags);
 	parser.parse_only = true;
 
 	sql_resolve_self_reference(&parser, def, NC_IsCheck, NULL, expr_list);
diff --git a/src/box/sql.h b/src/box/sql.h
index 262a48bcb..15ef74b19 100644
--- a/src/box/sql.h
+++ b/src/box/sql.h
@@ -327,9 +327,10 @@ sql_checks_update_space_def_reference(struct ExprList *expr_list,
  * which is also cleared in the destroy function.
  * @param parser object to initialize.
  * @param db sql object.
+ * @param sql_flags flags to control parser behaviour.
  */
 void
-sql_parser_create(struct Parse *parser, struct sql *db);
+sql_parser_create(struct Parse *parser, struct sql *db, uint32_t sql_flags);
 
 /**
  * Release the parser object resources.
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index a95b07155..bde70a4d3 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -30,7 +30,6 @@
  */
 
 #include "box/box.h"
-#include "box/session.h"
 #include "box/schema.h"
 #include "sqlInt.h"
 #include "tarantoolInt.h"
@@ -151,7 +150,8 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 	struct space *space = sql_lookup_space(parse, tab_list->a);
 	if (space == NULL)
 		goto delete_from_cleanup;
-	trigger_list = sql_triggers_exist(space->def, TK_DELETE, NULL, NULL);
+	trigger_list = sql_triggers_exist(parse, space->def, TK_DELETE,
+					  NULL, NULL);
 	bool is_complex = trigger_list != NULL || fk_constraint_is_required(space, NULL);
 	bool is_view = space->def->opts.is_view;
 
@@ -195,8 +195,8 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 	 * if we are counting rows.
 	 */
 	int reg_count = -1;
-	struct session *user_session = current_session();
-	if (user_session->sql_flags & SQL_CountRows) {
+	uint32_t sql_flags = parse->sql_flags;
+	if ((sql_flags & SQL_CountRows) != 0) {
 		reg_count = ++parse->nMem;
 		sqlVdbeAddOp2(v, OP_Integer, 0, reg_count);
 	}
@@ -287,7 +287,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 		/* Keep track of the number of rows to be
 		 * deleted.
 		 */
-		if (user_session->sql_flags & SQL_CountRows)
+		if ((sql_flags & SQL_CountRows) != 0)
 			sqlVdbeAddOp2(v, OP_AddImm, reg_count, 1);
 
 		/* Extract the primary key for the current row */
@@ -417,7 +417,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 	}
 
 	/* Return the number of rows that were deleted. */
-	if ((user_session->sql_flags & SQL_CountRows) != 0 &&
+	if ((sql_flags & SQL_CountRows) != 0 &&
 	    parse->triggered_space != NULL) {
 		sqlVdbeAddOp2(v, OP_ResultRow, reg_count, 1);
 		sqlVdbeSetNumCols(v, 1);
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index f7507938d..81bcc7839 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -3456,8 +3456,7 @@ sqlExprCachePush(Parse * pParse)
 	struct session MAYBE_UNUSED *user_session;
 	pParse->iCacheLevel++;
 #ifdef SQL_DEBUG
-	user_session = current_session();
-	if (user_session->sql_flags & SQL_VdbeAddopTrace) {
+	if ((pParse->sql_flags & SQL_VdbeAddopTrace) != 0) {
 		printf("PUSH to %d\n", pParse->iCacheLevel);
 	}
 #endif
@@ -3477,7 +3476,7 @@ sqlExprCachePop(Parse * pParse)
 	assert(pParse->iCacheLevel >= 1);
 	pParse->iCacheLevel--;
 #ifdef SQL_DEBUG
-	if (user_session->sql_flags & SQL_VdbeAddopTrace) {
+	if ((pParse->sql_flags & SQL_VdbeAddopTrace) != 0) {
 		printf("POP  to %d\n", pParse->iCacheLevel);
 	}
 #endif
@@ -3566,7 +3565,7 @@ sqlExprCacheClear(Parse * pParse)
 	user_session = current_session();
 
 #if SQL_DEBUG
-	if (user_session->sql_flags & SQL_VdbeAddopTrace) {
+	if ((pParse->sql_flags & SQL_VdbeAddopTrace) != 0) {
 		printf("CLEAR\n");
 	}
 #endif
diff --git a/src/box/sql/fk_constraint.c b/src/box/sql/fk_constraint.c
index 8151c66e5..3bcc0fd24 100644
--- a/src/box/sql/fk_constraint.c
+++ b/src/box/sql/fk_constraint.c
@@ -37,7 +37,6 @@
 #include "sqlInt.h"
 #include "box/fk_constraint.h"
 #include "box/schema.h"
-#include "box/session.h"
 
 /*
  * Deferred and Immediate FKs
@@ -274,9 +273,8 @@ fk_constraint_lookup_parent(struct Parse *parse_context, struct space *parent,
 		sqlReleaseTempReg(parse_context, rec_reg);
 		sqlReleaseTempRange(parse_context, temp_regs, field_count);
 	}
-	struct session *session = current_session();
 	if (!fk_def->is_deferred &&
-	    (session->sql_flags & SQL_DeferFKs) == 0 &&
+	    (parse_context->sql_flags & SQL_DeferFKs) == 0 &&
 	    parse_context->pToplevel == NULL && !parse_context->isMultiWrite) {
 		/*
 		 * If this is an INSERT statement that will insert
@@ -536,7 +534,6 @@ fk_constraint_emit_check(struct Parse *parser, struct space *space, int reg_old,
 {
 	bool is_update = changed_cols != NULL;
 	struct sql *db = parser->db;
-	struct session *user_session = current_session();
 
 	/*
 	 * Exactly one of reg_old and reg_new should be non-zero.
@@ -600,7 +597,7 @@ fk_constraint_emit_check(struct Parse *parser, struct space *space, int reg_old,
 					       changed_cols))
 			continue;
 		if (!fk_def->is_deferred &&
-		    (user_session->sql_flags & SQL_DeferFKs) == 0 &&
+		    (parser->sql_flags & SQL_DeferFKs) == 0 &&
 		    parser->pToplevel == NULL && !parser->isMultiWrite) {
 			assert(reg_old == 0 && reg_new != 0);
 			/*
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index c2aac553f..010695067 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -36,7 +36,6 @@
 #include "sqlInt.h"
 #include "tarantoolInt.h"
 #include "vdbeInt.h"
-#include "box/session.h"
 #include "box/schema.h"
 #include "bit/bit.h"
 #include "box/box.h"
@@ -262,7 +261,6 @@ sqlInsert(Parse * pParse,	/* Parser context */
 	u8 useTempTable = 0;	/* Store SELECT results in intermediate table */
 	u8 bIdListInOrder;	/* True if IDLIST is in table order */
 	ExprList *pList = 0;	/* List of VALUES() to be inserted  */
-	struct session *user_session = current_session();
 
 	/* Register allocations */
 	int regFromSelect = 0;	/* Base register for data coming from SELECT */
@@ -308,7 +306,8 @@ sqlInsert(Parse * pParse,	/* Parser context */
 	 * inserted into is a view
 	 */
 	struct space_def *space_def = space->def;
-	trigger = sql_triggers_exist(space_def, TK_INSERT, NULL, &tmask);
+	trigger = sql_triggers_exist(pParse, space_def, TK_INSERT, NULL,
+				     &tmask);
 
 	bool is_view = space_def->opts.is_view;
 	assert((trigger != NULL && tmask != 0) ||
@@ -529,7 +528,7 @@ sqlInsert(Parse * pParse,	/* Parser context */
 
 	/* Initialize the count of rows to be inserted
 	 */
-	if (user_session->sql_flags & SQL_CountRows) {
+	if ((pParse->sql_flags & SQL_CountRows) != 0) {
 		regRowCount = ++pParse->nMem;
 		sqlVdbeAddOp2(v, OP_Integer, 0, regRowCount);
 	}
@@ -761,7 +760,7 @@ sqlInsert(Parse * pParse,	/* Parser context */
 
 	/* Update the count of rows that are inserted
 	 */
-	if ((user_session->sql_flags & SQL_CountRows) != 0) {
+	if ((pParse->sql_flags & SQL_CountRows) != 0) {
 		sqlVdbeAddOp2(v, OP_AddImm, regRowCount, 1);
 	}
 
@@ -790,7 +789,7 @@ sqlInsert(Parse * pParse,	/* Parser context */
  insert_end:
 
 	/* Return the number of rows inserted. */
-	if ((user_session->sql_flags & SQL_CountRows) != 0 &&
+	if ((pParse->sql_flags & SQL_CountRows) != 0 &&
 	    pParse->triggered_space == NULL) {
 		sqlVdbeAddOp2(v, OP_ResultRow, regRowCount, 1);
 		sqlVdbeSetNumCols(v, 1);
@@ -1039,8 +1038,8 @@ process_index:  ;
 					     part_count);
 			sql_set_multi_write(parse_context, true);
 			struct sql_trigger *trigger =
-				sql_triggers_exist(space->def, TK_DELETE, NULL,
-						   NULL);
+				sql_triggers_exist(parse_context, space->def,
+						   TK_DELETE, NULL, NULL);
 			sql_generate_row_delete(parse_context, space, trigger,
 						cursor, idx_key_reg, part_count,
 						true,
@@ -1130,7 +1129,6 @@ xferOptimization(Parse * pParse,	/* Parser context */
 	int addr1;		/* Loop addresses */
 	int emptyDestTest = 0;	/* Address of test for empty pDest */
 	int regData, regTupleid;	/* Registers holding data and tupleid */
-	struct session *user_session = current_session();
 	bool is_err_action_default = false;
 
 	if (pSelect == NULL)
@@ -1260,7 +1258,7 @@ xferOptimization(Parse * pParse,	/* Parser context */
 	 */
 	if (!rlist_empty(&dest->child_fk_constraint))
 		return 0;
-	if ((user_session->sql_flags & SQL_CountRows) != 0) {
+	if ((pParse->sql_flags & SQL_CountRows) != 0) {
 		return 0;	/* xfer opt does not play well with PRAGMA count_changes */
 	}
 
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index 3df6b5c36..ad4c71cb6 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -54,7 +54,7 @@ sqlPrepare(sql * db,	/* Database handle. */
 {
 	int rc = SQL_OK;	/* Result code */
 	Parse sParse;		/* Parsing context */
-	sql_parser_create(&sParse, db);
+	sql_parser_create(&sParse, db, current_session()->sql_flags);
 	sParse.pReprepare = pReprepare;
 	assert(ppStmt && *ppStmt == 0);
 	/* assert( !db->mallocFailed ); // not true with SQL_USE_ALLOCA */
@@ -281,10 +281,11 @@ sql_prepare_v2(sql * db,	/* Database handle. */
 }
 
 void
-sql_parser_create(struct Parse *parser, sql *db)
+sql_parser_create(struct Parse *parser, struct sql *db, uint32_t sql_flags)
 {
 	memset(parser, 0, sizeof(struct Parse));
 	parser->db = db;
+	parser->sql_flags = sql_flags;
 	rlist_create(&parser->record_list);
 	region_create(&parser->region, &cord()->slabc);
 }
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index d3472a922..d7376ae11 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -40,7 +40,6 @@
 #include "box/box.h"
 #include "box/coll_id_cache.h"
 #include "box/schema.h"
-#include "box/session.h"
 
 /*
  * Trace output macros
@@ -169,8 +168,6 @@ sqlSelectNew(Parse * pParse,	/* Parsing context */
 			pParse->is_aborted = true;
 		pEList = sql_expr_list_append(db, NULL, expr);
 	}
-	struct session MAYBE_UNUSED *user_session;
-	user_session = current_session();
 	pNew->pEList = pEList;
 	pNew->op = TK_SELECT;
 	pNew->selFlags = selFlags;
@@ -178,7 +175,7 @@ sqlSelectNew(Parse * pParse,	/* Parsing context */
 	pNew->iOffset = 0;
 #ifdef SQL_DEBUG
 	pNew->zSelName[0] = 0;
-	if (user_session->sql_flags & SQL_SelectTrace)
+	if ((pParse->sql_flags & SQL_SelectTrace) != 0)
 		sqlSelectTrace = 0xfff;
 	else
 		sqlSelectTrace = 0;
@@ -1733,7 +1730,6 @@ generateColumnNames(Parse * pParse,	/* Parser context */
 	int i, j;
 	sql *db = pParse->db;
 	int fullNames, shortNames;
-	struct session *user_session = current_session();
 	/* If this is an EXPLAIN, skip this step */
 	if (pParse->explain) {
 		return;
@@ -1751,8 +1747,8 @@ generateColumnNames(Parse * pParse,	/* Parser context */
 	}
 	assert(pTabList != 0);
 	pParse->colNamesSet = 1;
-	fullNames = (user_session->sql_flags & SQL_FullColNames) != 0;
-	shortNames = (user_session->sql_flags & SQL_ShortColNames) != 0;
+	fullNames = (pParse->sql_flags & SQL_FullColNames) != 0;
+	shortNames = (pParse->sql_flags & SQL_ShortColNames) != 0;
 	sqlVdbeSetNumCols(v, pEList->nExpr);
 	uint32_t var_count = 0;
 	for (i = 0; i < pEList->nExpr; i++) {
@@ -1994,18 +1990,16 @@ struct space *
 sqlResultSetOfSelect(Parse * pParse, Select * pSelect)
 {
 	sql *db = pParse->db;
-	uint32_t savedFlags;
-	struct session *user_session = current_session();
 
-	savedFlags = user_session->sql_flags;
-	user_session->sql_flags |= ~SQL_FullColNames;
-	user_session->sql_flags &= SQL_ShortColNames;
+	uint32_t saved_flags = pParse->sql_flags;
+	pParse->sql_flags |= ~SQL_FullColNames;
+	pParse->sql_flags &= SQL_ShortColNames;
 	sqlSelectPrep(pParse, pSelect, 0);
 	if (pParse->is_aborted)
 		return NULL;
 	while (pSelect->pPrior)
 		pSelect = pSelect->pPrior;
-	user_session->sql_flags = savedFlags;
+	pParse->sql_flags = saved_flags;
 	struct space *space = sql_ephemeral_space_new(pParse, NULL);
 	if (space == NULL)
 		return NULL;
@@ -2030,6 +2024,7 @@ allocVdbe(Parse * pParse)
 	Vdbe *v = pParse->pVdbe = sqlVdbeCreate(pParse);
 	if (v == NULL)
 		return NULL;
+	v->sql_flags = pParse->sql_flags;
 	sqlVdbeAddOp2(v, OP_Init, 0, 1);
 	if (pParse->pToplevel == 0
 	    && OptimizationEnabled(pParse->db, SQL_FactorOutConst)
@@ -4782,7 +4777,6 @@ selectExpander(Walker * pWalker, Select * p)
 	sql *db = pParse->db;
 	Expr *pE, *pRight, *pExpr;
 	u16 selFlags = p->selFlags;
-	struct session *user_session = current_session();
 
 	p->selFlags |= SF_Expanded;
 	if (db->mallocFailed) {
@@ -4917,7 +4911,7 @@ selectExpander(Walker * pWalker, Select * p)
 		 */
 		struct ExprList_item *a = pEList->a;
 		ExprList *pNew = 0;
-		uint32_t flags = user_session->sql_flags;
+		uint32_t flags = pParse->sql_flags;
 		int longNames = (flags & SQL_FullColNames) != 0
 		    && (flags & SQL_ShortColNames) == 0;
 
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 9384ec9e8..1b3f9afd1 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -2693,6 +2693,8 @@ struct Parse {
 	bool parse_only;
 	/** Type of parsed_ast member. */
 	enum ast_type parsed_ast_type;
+	/** SQL parsing flags. */
+	uint32_t sql_flags;
 	/**
 	 * Members of this union are valid only
 	 * if parse_only is set to true.
@@ -4023,6 +4025,7 @@ vdbe_code_drop_trigger(struct Parse *parser, const char *trigger_name,
  * table, and, if that operation is an UPDATE, if at least one
  * of the columns in changes_list is being modified.
  *
+ * @param parser Parser context.
  * @param space_def The definition of the space that contains
  *        the triggers.
  * @param op operation one of TK_DELETE, TK_INSERT, TK_UPDATE.
@@ -4030,7 +4033,7 @@ vdbe_code_drop_trigger(struct Parse *parser, const char *trigger_name,
  * @param[out] pMask Mask of TRIGGER_BEFORE|TRIGGER_AFTER
  */
 struct sql_trigger *
-sql_triggers_exist(struct space_def *space_def, int op,
+sql_triggers_exist(struct Parse *parser, struct space_def *space_def, int op,
 		   struct ExprList *changes_list, int *mask_ptr);
 
 /**
diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
index 8cc35323c..11ef7b97e 100644
--- a/src/box/sql/tokenize.c
+++ b/src/box/sql/tokenize.c
@@ -40,6 +40,7 @@
 #include <unicode/utf8.h>
 #include <unicode/uchar.h>
 
+#include "box/session.h"
 #include "say.h"
 #include "sqlInt.h"
 #include "tarantoolInt.h"
@@ -544,7 +545,7 @@ sql_expr_compile(sql *db, const char *expr, int expr_len)
 	int len = strlen(outer) + expr_len;
 
 	struct Parse parser;
-	sql_parser_create(&parser, db);
+	sql_parser_create(&parser, db, default_flags);
 	parser.parse_only = true;
 
 	struct Expr *expression = NULL;
@@ -569,7 +570,7 @@ struct Select *
 sql_view_compile(struct sql *db, const char *view_stmt)
 {
 	struct Parse parser;
-	sql_parser_create(&parser, db);
+	sql_parser_create(&parser, db, default_flags);
 	parser.parse_only = true;
 
 	struct Select *select = NULL;
@@ -590,7 +591,7 @@ struct sql_trigger *
 sql_trigger_compile(struct sql *db, const char *sql)
 {
 	struct Parse parser;
-	sql_parser_create(&parser, db);
+	sql_parser_create(&parser, db, default_flags);
 	parser.parse_only = true;
 	struct sql_trigger *trigger = NULL;
 	if (sqlRunParser(&parser, sql) == 0 &&
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index 14e4198a8..97f69696c 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -37,7 +37,6 @@
 #include "sqlInt.h"
 #include "tarantoolInt.h"
 #include "vdbeInt.h"
-#include "box/session.h"
 
 /* See comment in sqlInt.h */
 int sqlSubProgramsRemaining;
@@ -533,13 +532,12 @@ checkColumnOverlap(IdList * pIdList, ExprList * pEList)
 }
 
 struct sql_trigger *
-sql_triggers_exist(struct space_def *space_def, int op,
+sql_triggers_exist(struct Parse *parser, struct space_def *space_def, int op,
 		   struct ExprList *changes_list, int *mask_ptr)
 {
 	int mask = 0;
 	struct sql_trigger *trigger_list = NULL;
-	struct session *user_session = current_session();
-	if ((user_session->sql_flags & SQL_EnableTrigger) != 0)
+	if ((parser->sql_flags & SQL_EnableTrigger) != 0)
 		trigger_list = space_trigger_list(space_def->id);
 	for (struct sql_trigger *p = trigger_list; p != NULL; p = p->next) {
 		if (p->op == op && checkColumnOverlap(p->pColumns,
@@ -753,7 +751,7 @@ sql_row_trigger_program(struct Parse *parser, struct sql_trigger *trigger,
 	pSubParse = sqlStackAllocZero(db, sizeof(Parse));
 	if (!pSubParse)
 		return 0;
-	sql_parser_create(pSubParse, db);
+	sql_parser_create(pSubParse, db, parser->sql_flags);
 	memset(&sNC, 0, sizeof(sNC));
 	sNC.pParse = pSubParse;
 	pSubParse->triggered_space = space;
@@ -893,9 +891,8 @@ vdbe_code_row_trigger_direct(struct Parse *parser, struct sql_trigger *trigger,
 	if (pPrg == NULL)
 		return;
 
-	struct session *user_session = current_session();
-	bool is_recursive = (trigger->zName && !(user_session->sql_flags &
-						 SQL_RecTriggers));
+	bool is_recursive =
+		trigger->zName && (parser->sql_flags & SQL_RecTriggers) == 0;
 
 	sqlVdbeAddOp4(v, OP_Program, reg, ignore_jump,
 			  ++parser->nMem, (const char *)pPrg->pProgram,
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index 35c001939..40b103243 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -34,7 +34,6 @@
  * to handle UPDATE statements.
  */
 #include "sqlInt.h"
-#include "box/session.h"
 #include "tarantoolInt.h"
 #include "box/tuple_format.h"
 #include "box/schema.h"
@@ -92,7 +91,6 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 	int hasFK;		/* True if foreign key processing is required */
 	int labelBreak;		/* Jump here to break out of UPDATE loop */
 	int labelContinue;	/* Jump here to continue next step of UPDATE loop */
-	struct session *user_session = current_session();
 
 	bool is_view;		/* True when updating a view (INSTEAD OF trigger) */
 	/* List of triggers on pTab, if required. */
@@ -127,7 +125,8 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 	/* Figure out if we have any triggers and if the table being
 	 * updated is a view.
 	 */
-	trigger = sql_triggers_exist(space->def, TK_UPDATE, pChanges, &tmask);
+	trigger = sql_triggers_exist(pParse, space->def, TK_UPDATE, pChanges,
+				     &tmask);
 	is_view = space->def->opts.is_view;
 	assert(trigger != NULL || tmask == 0);
 
@@ -298,8 +297,8 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 
 	/* Initialize the count of updated rows
 	 */
-	if ((user_session->sql_flags & SQL_CountRows)
-	    && pParse->triggered_space == NULL) {
+	if ((pParse->sql_flags & SQL_CountRows) != 0 &&
+	    pParse->triggered_space == NULL) {
 		regRowCount = ++pParse->nMem;
 		sqlVdbeAddOp2(v, OP_Integer, 0, regRowCount);
 	}
@@ -500,8 +499,8 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 
 	/* Increment the row counter
 	 */
-	if ((user_session->sql_flags & SQL_CountRows)
-	    && pParse->triggered_space == NULL) {
+	if ((pParse->sql_flags & SQL_CountRows) != 0 &&
+	     pParse->triggered_space == NULL) {
 		sqlVdbeAddOp2(v, OP_AddImm, regRowCount, 1);
 	}
 
@@ -522,7 +521,7 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 	sqlVdbeResolveLabel(v, labelBreak);
 
 	/* Return the number of rows that were changed. */
-	if (user_session->sql_flags & SQL_CountRows &&
+	if ((pParse->sql_flags & SQL_CountRows) != 0 &&
 	    pParse->triggered_space == NULL) {
 		sqlVdbeAddOp2(v, OP_ResultRow, regRowCount, 1);
 		sqlVdbeSetNumCols(v, 1);
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index a34395cdf..b268f20ea 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -43,7 +43,6 @@
 #include "box/error.h"
 #include "box/fk_constraint.h"
 #include "box/txn.h"
-#include "box/session.h"
 #include "sqlInt.h"
 #include "vdbeInt.h"
 #include "tarantoolInt.h"
@@ -524,10 +523,10 @@ registerTrace(int iReg, Mem *p) {
 #endif
 
 #ifdef SQL_DEBUG
-#  define REGISTER_TRACE(R,M)						\
-	if(user_session->sql_flags&SQL_VdbeTrace) registerTrace(R,M);
+#  define REGISTER_TRACE(P,R,M)						\
+	if(P->sql_flags&SQL_VdbeTrace) registerTrace(R,M);
 #else
-#  define REGISTER_TRACE(R,M)
+#  define REGISTER_TRACE(P,R,M)
 #endif
 
 
@@ -643,7 +642,6 @@ int sqlVdbeExec(Vdbe *p)
 #ifdef VDBE_PROFILE
 	u64 start;                 /* CPU clock count at start of opcode */
 #endif
-	struct session *user_session = current_session();
 	/*** INSERT STACK UNION HERE ***/
 
 	assert(p->magic==VDBE_MAGIC_RUN);  /* sql_step() verifies this */
@@ -669,20 +667,18 @@ int sqlVdbeExec(Vdbe *p)
 #endif
 #ifdef SQL_DEBUG
 	sqlBeginBenignMalloc();
-	if (p->pc==0
-	    && (user_session->sql_flags&
-		(SQL_VdbeListing|SQL_VdbeEQP|SQL_VdbeTrace))!=0
-		) {
+	if (p->pc == 0 &&
+	    (p->sql_flags &(SQL_VdbeListing|SQL_VdbeEQP|SQL_VdbeTrace)) != 0) {
 		int i;
 		int once = 1;
 		sqlVdbePrintSql(p);
-		if (user_session->sql_flags & SQL_VdbeListing) {
+		if (p->sql_flags & SQL_VdbeListing) {
 			printf("VDBE Program Listing:\n");
 			for(i=0; i<p->nOp; i++) {
 				sqlVdbePrintOp(stdout, i, &aOp[i]);
 			}
 		}
-		if (user_session->sql_flags & SQL_VdbeEQP) {
+		if (p->sql_flags & SQL_VdbeEQP) {
 			for(i=0; i<p->nOp; i++) {
 				if (aOp[i].opcode==OP_Explain) {
 					if (once) printf("VDBE Query Plan:\n");
@@ -691,7 +687,7 @@ int sqlVdbeExec(Vdbe *p)
 				}
 			}
 		}
-		if (user_session->sql_flags & SQL_VdbeTrace)  printf("VDBE Trace:\n");
+		if (p->sql_flags & SQL_VdbeTrace)  printf("VDBE Trace:\n");
 	}
 	sqlEndBenignMalloc();
 #endif
@@ -713,7 +709,7 @@ int sqlVdbeExec(Vdbe *p)
 		/* Only allow tracing if SQL_DEBUG is defined.
 		 */
 #ifdef SQL_DEBUG
-		if (user_session->sql_flags & SQL_VdbeTrace) {
+		if (p->sql_flags & SQL_VdbeTrace) {
 			sqlVdbePrintOp(stdout, (int)(pOp - aOp), pOp);
 		}
 #endif
@@ -728,21 +724,21 @@ int sqlVdbeExec(Vdbe *p)
 				assert(pOp->p1<=(p->nMem+1 - p->nCursor));
 				assert(memIsValid(&aMem[pOp->p1]));
 				assert(sqlVdbeCheckMemInvariants(&aMem[pOp->p1]));
-				REGISTER_TRACE(pOp->p1, &aMem[pOp->p1]);
+				REGISTER_TRACE(p, pOp->p1, &aMem[pOp->p1]);
 			}
 			if ((opProperty & OPFLG_IN2)!=0) {
 				assert(pOp->p2>0);
 				assert(pOp->p2<=(p->nMem+1 - p->nCursor));
 				assert(memIsValid(&aMem[pOp->p2]));
 				assert(sqlVdbeCheckMemInvariants(&aMem[pOp->p2]));
-				REGISTER_TRACE(pOp->p2, &aMem[pOp->p2]);
+				REGISTER_TRACE(p, pOp->p2, &aMem[pOp->p2]);
 			}
 			if ((opProperty & OPFLG_IN3)!=0) {
 				assert(pOp->p3>0);
 				assert(pOp->p3<=(p->nMem+1 - p->nCursor));
 				assert(memIsValid(&aMem[pOp->p3]));
 				assert(sqlVdbeCheckMemInvariants(&aMem[pOp->p3]));
-				REGISTER_TRACE(pOp->p3, &aMem[pOp->p3]);
+				REGISTER_TRACE(p, pOp->p3, &aMem[pOp->p3]);
 			}
 			if ((opProperty & OPFLG_OUT2)!=0) {
 				assert(pOp->p2>0);
@@ -858,7 +854,7 @@ case OP_Gosub: {            /* jump */
 	memAboutToChange(p, pIn1);
 	pIn1->flags = MEM_Int;
 	pIn1->u.i = (int)(pOp-aOp);
-	REGISTER_TRACE(pOp->p1, pIn1);
+	REGISTER_TRACE(p, pOp->p1, pIn1);
 
 	/* Most jump operations do a goto to this spot in order to update
 	 * the pOp pointer.
@@ -945,7 +941,7 @@ case OP_Yield: {            /* in1, jump */
 	pIn1->flags = MEM_Int;
 	pcDest = (int)pIn1->u.i;
 	pIn1->u.i = (int)(pOp - aOp);
-	REGISTER_TRACE(pOp->p1, pIn1);
+	REGISTER_TRACE(p, pOp->p1, pIn1);
 	pOp = &aOp[pcDest];
 	break;
 }
@@ -1312,7 +1308,7 @@ case OP_Move: {
 		}
 #endif
 		Deephemeralize(pOut);
-		REGISTER_TRACE(p2++, pOut);
+		REGISTER_TRACE(p, p2++, pOut);
 		pIn1++;
 		pOut++;
 	}while( --n);
@@ -1340,7 +1336,7 @@ case OP_Copy: {
 #ifdef SQL_DEBUG
 		pOut->pScopyFrom = 0;
 #endif
-		REGISTER_TRACE(pOp->p2+pOp->p3-n, pOut);
+		REGISTER_TRACE(p, pOp->p2+pOp->p3-n, pOut);
 		if ((n--)==0) break;
 		pOut++;
 		pIn1++;
@@ -1422,7 +1418,7 @@ case OP_ResultRow: {
 	 * transaction. It needs to be rolled back.
 	 */
 	if (SQL_OK!=(rc = sqlVdbeCheckFk(p, 0))) {
-		assert(user_session->sql_flags&SQL_CountRows);
+		assert(p->sql_flags&SQL_CountRows);
 		goto abort_due_to_error;
 	}
 
@@ -1441,7 +1437,7 @@ case OP_ResultRow: {
 	 * The statement transaction is never a top-level transaction.  Hence
 	 * the RELEASE call below can never fail.
 	 */
-	assert(p->iStatement==0 || user_session->sql_flags&SQL_CountRows);
+	assert(p->iStatement==0 || p->sql_flags&SQL_CountRows);
 	rc = sqlVdbeCloseStatement(p, SAVEPOINT_RELEASE);
 	assert(rc==SQL_OK);
 
@@ -1459,7 +1455,7 @@ case OP_ResultRow: {
 		assert((pMem[i].flags & MEM_Ephem)==0
 		       || (pMem[i].flags & (MEM_Str|MEM_Blob))==0);
 		sqlVdbeMemNulTerminate(&pMem[i]);
-		REGISTER_TRACE(pOp->p1+i, &pMem[i]);
+		REGISTER_TRACE(p, pOp->p1+i, &pMem[i]);
 	}
 	if (db->mallocFailed) goto no_mem;
 
@@ -1799,7 +1795,7 @@ case OP_Function: {
 #ifdef SQL_DEBUG
 	for(i=0; i<pCtx->argc; i++) {
 		assert(memIsValid(pCtx->argv[i]));
-		REGISTER_TRACE(pOp->p2+i, pCtx->argv[i]);
+		REGISTER_TRACE(p, pOp->p2+i, pCtx->argv[i]);
 	}
 #endif
 	MemSetTypeFlag(pCtx->pOut, MEM_Null);
@@ -1821,7 +1817,7 @@ case OP_Function: {
 		if (sqlVdbeMemTooBig(pCtx->pOut)) goto too_big;
 	}
 
-	REGISTER_TRACE(pOp->p3, pCtx->pOut);
+	REGISTER_TRACE(p, pOp->p3, pCtx->pOut);
 	UPDATE_MAX_BLOBSIZE(pCtx->pOut);
 	break;
 }
@@ -2133,7 +2129,7 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
 				iCompare = 1;    /* Operands are not equal */
 				memAboutToChange(p, pOut);
 				MemSetTypeFlag(pOut, MEM_Null);
-				REGISTER_TRACE(pOp->p2, pOut);
+				REGISTER_TRACE(p, pOp->p2, pOut);
 			} else {
 				VdbeBranchTaken(2,3);
 				if (pOp->p5 & SQL_JUMPIFNULL) {
@@ -2249,7 +2245,7 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
 		}
 		memAboutToChange(p, pOut);
 		mem_set_bool(pOut, res2);
-		REGISTER_TRACE(pOp->p2, pOut);
+		REGISTER_TRACE(p, pOp->p2, pOut);
 	} else {
 		VdbeBranchTaken(res!=0, (pOp->p5 & SQL_NULLEQ)?2:3);
 		if (res2) {
@@ -2351,8 +2347,8 @@ case OP_Compare: {
 		idx = aPermute ? aPermute[i] : i;
 		assert(memIsValid(&aMem[p1+idx]));
 		assert(memIsValid(&aMem[p2+idx]));
-		REGISTER_TRACE(p1+idx, &aMem[p1+idx]);
-		REGISTER_TRACE(p2+idx, &aMem[p2+idx]);
+		REGISTER_TRACE(p, p1+idx, &aMem[p1+idx]);
+		REGISTER_TRACE(p, p2+idx, &aMem[p2+idx]);
 		assert(i < (int)def->part_count);
 		struct coll *coll = def->parts[i].coll;
 		bool is_rev = def->parts[i].sort_order == SORT_ORDER_DESC;
@@ -2783,7 +2779,7 @@ case OP_Column: {
 	if (zData!=pC->aRow) sqlVdbeMemRelease(&sMem);
 			op_column_out:
 	UPDATE_MAX_BLOBSIZE(pDest);
-	REGISTER_TRACE(pOp->p3, pDest);
+	REGISTER_TRACE(p, pOp->p3, pDest);
 	break;
 
 			op_column_error:
@@ -2924,7 +2920,7 @@ case OP_MakeRecord: {
 		goto no_mem;
 	assert(sqlVdbeCheckMemInvariants(pOut));
 	assert(pOp->p3>0 && pOp->p3<=(p->nMem+1 - p->nCursor));
-	REGISTER_TRACE(pOp->p3, pOut);
+	REGISTER_TRACE(p, pOp->p3, pOut);
 	UPDATE_MAX_BLOBSIZE(pOut);
 	break;
 }
@@ -3740,7 +3736,7 @@ case OP_Found: {        /* jump, in3 */
 		for(ii=0; ii<r.nField; ii++) {
 			assert(memIsValid(&r.aMem[ii]));
 			assert((r.aMem[ii].flags & MEM_Zero)==0 || r.aMem[ii].n==0);
-			if (ii) REGISTER_TRACE(pOp->p3+ii, &r.aMem[ii]);
+			if (ii) REGISTER_TRACE(p, pOp->p3+ii, &r.aMem[ii]);
 		}
 #endif
 		pIdxKey = &r;
@@ -4092,7 +4088,7 @@ case OP_RowData: {
 	rc = sqlCursorPayload(pCrsr, 0, n, pOut->z);
 	if (rc) goto abort_due_to_error;
 	UPDATE_MAX_BLOBSIZE(pOut);
-	REGISTER_TRACE(pOp->p2, pOut);
+	REGISTER_TRACE(p, pOp->p2, pOut);
 	break;
 }
 
@@ -5028,7 +5024,7 @@ case OP_Param: {           /* out2 */
  * statement counter is incremented (immediate foreign key constraints).
  */
 case OP_FkCounter: {
-	if ((user_session->sql_flags & SQL_DeferFKs || pOp->p1 != 0) &&
+	if ((p->sql_flags & SQL_DeferFKs || pOp->p1 != 0) &&
 	    !p->auto_commit) {
 		struct txn *txn = in_txn();
 		assert(txn != NULL && txn->psql_txn != NULL);
@@ -5052,7 +5048,7 @@ case OP_FkCounter: {
  * (immediate foreign key constraint violations).
  */
 case OP_FkIfZero: {         /* jump */
-	if ((user_session->sql_flags & SQL_DeferFKs || pOp->p1) &&
+	if ((p->sql_flags & SQL_DeferFKs || pOp->p1) &&
 	    !p->auto_commit) {
 		struct txn *txn = in_txn();
 		assert(txn != NULL && txn->psql_txn != NULL);
@@ -5236,7 +5232,7 @@ case OP_AggStep: {
 #ifdef SQL_DEBUG
 	for(i=0; i<pCtx->argc; i++) {
 		assert(memIsValid(pCtx->argv[i]));
-		REGISTER_TRACE(pOp->p2+i, pCtx->argv[i]);
+		REGISTER_TRACE(p, pOp->p2+i, pCtx->argv[i]);
 	}
 #endif
 
@@ -5367,7 +5363,7 @@ case OP_Init: {          /* jump */
 		}
 	}
 #ifdef SQL_DEBUG
-	if ((user_session->sql_flags & SQL_SqlTrace)!=0
+	if ((p->sql_flags & SQL_SqlTrace)!=0
 	    && (zTrace = (pOp->p4.z ? pOp->p4.z : p->zSql))!=0
 		) {
 		sqlDebugPrintf("SQL-trace: %s\n", zTrace);
@@ -5444,7 +5440,7 @@ default: {          /* This is really OP_Noop and OP_Explain */
 		assert(pOp>=&aOp[-1] && pOp<&aOp[p->nOp-1]);
 
 #ifdef SQL_DEBUG
-		if (user_session->sql_flags & SQL_VdbeTrace) {
+		if (p->sql_flags & SQL_VdbeTrace) {
 			u8 opProperty = sqlOpcodeProperty[pOrigOp->opcode];
 			if (rc!=0) printf("rc=%d\n",rc);
 			if (opProperty & (OPFLG_OUT2)) {
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index a3100e513..86fd92da1 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -446,6 +446,8 @@ struct Vdbe {
 	u32 expmask;		/* Binding to these vars invalidates VM */
 	SubProgram *pProgram;	/* Linked list of all sub-programs used by VM */
 	AuxData *pAuxData;	/* Linked list of auxdata allocations */
+	/** Parser flags with which this object was built. */
+	uint32_t sql_flags;
 	/* Anonymous savepoint for aborts only */
 	Savepoint *anonymous_savepoint;
 #ifdef SQL_ENABLE_STMT_SCANSTATUS
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index 19ee2d03a..edc2074f9 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -43,7 +43,6 @@
 #include "vdbeInt.h"
 #include "whereInt.h"
 #include "box/coll_id_cache.h"
-#include "box/session.h"
 #include "box/schema.h"
 
 /* Forward declaration of methods */
@@ -2832,9 +2831,10 @@ tnt_error:
 	/* Automatic indexes */
 	LogEst rSize = pTab->nRowLogEst;
 	LogEst rLogSize = estLog(rSize);
-	struct session *user_session = current_session();
-	if (!pBuilder->pOrSet	/* Not part of an OR optimization */
-	    && (pWInfo->wctrlFlags & WHERE_OR_SUBCLAUSE) == 0 && (user_session->sql_flags & SQL_AutoIndex) != 0 && pSrc->pIBIndex == 0	/* Has no INDEXED BY clause */
+	if (!pBuilder->pOrSet && /* Not pqart of an OR optimization */
+	    (pWInfo->wctrlFlags & WHERE_OR_SUBCLAUSE) == 0 &&
+	    (pWInfo->pParse->sql_flags & SQL_AutoIndex) != 0 &&
+	    pSrc->pIBIndex == 0	/* Has no INDEXED BY clause */
 	    && !pSrc->fg.notIndexed	/* Has no NOT INDEXED clause */
 	    && HasRowid(pTab)	/* Not WITHOUT ROWID table. (FIXME: Why not?) */
 	    &&!pSrc->fg.isCorrelated	/* Not a correlated subquery */
@@ -4241,10 +4241,9 @@ sqlWhereBegin(Parse * pParse,	/* The parser context */
 	sql *db;		/* Database connection */
 	int rc;			/* Return code */
 	u8 bFordelete = 0;	/* OPFLAG_FORDELETE or zero, as appropriate */
-	struct session *user_session = current_session();
 
 #ifdef SQL_DEBUG
-	if (user_session->sql_flags & SQL_WhereTrace)
+	if ((pParse->sql_flags & SQL_WhereTrace) != 0)
 		sqlWhereTrace = 0xfff;
 	else
 		sqlWhereTrace = 0;
@@ -4452,7 +4451,7 @@ sqlWhereBegin(Parse * pParse,	/* The parser context */
 		}
 	}
 	if (pWInfo->pOrderBy == 0 &&
-	    (user_session->sql_flags & SQL_ReverseOrder) != 0) {
+	    (pParse->sql_flags & SQL_ReverseOrder) != 0) {
 		pWInfo->revMask = ALLBITS;
 	}
 	if (pParse->is_aborted || NEVER(db->mallocFailed)) {
-- 
2.21.0

[tarantool-patches] Re: [PATCH v1 3/3] box: local sql_flags for parser and vdbe

[Kirill Shcherbatov]

The sql_flags is a parser parameter that describe how to parse the
SQL request, but now this information is taken from the global
user session object.
When we need to run the parser with some other parameters, it is
necessary to change global session object, which may lead to
unpredictable consequences in general case.
Introduced a new parser and vdbe field sql_flags is responsible
for SQL parsing results.

Needed for #3691
---
 src/box/sql.c               |  2 +-
 src/box/sql.h               |  3 +-
 src/box/sql/delete.c        | 12 +++---
 src/box/sql/expr.c          |  7 ++--
 src/box/sql/fk_constraint.c |  7 +---
 src/box/sql/insert.c        | 18 ++++-----
 src/box/sql/prepare.c       |  5 ++-
 src/box/sql/select.c        | 24 +++++------
 src/box/sql/sqlInt.h        |  5 ++-
 src/box/sql/tokenize.c      |  7 ++--
 src/box/sql/trigger.c       | 13 +++---
 src/box/sql/update.c        | 15 ++++---
 src/box/sql/vdbe.c          | 79 +++++++++++++++++--------------------
 src/box/sql/vdbeInt.h       |  2 +
 src/box/sql/where.c         | 13 +++---
 15 files changed, 99 insertions(+), 113 deletions(-)

diff --git a/src/box/sql.c b/src/box/sql.c
index fbfa59992..385676055 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -1354,7 +1354,7 @@ sql_checks_resolve_space_def_reference(ExprList *expr_list,
 				       struct space_def *def)
 {
 	Parse parser;
-	sql_parser_create(&parser, sql_get());
+	sql_parser_create(&parser, sql_get(), default_flags);
 	parser.parse_only = true;
 
 	sql_resolve_self_reference(&parser, def, NC_IsCheck, NULL, expr_list);
diff --git a/src/box/sql.h b/src/box/sql.h
index 262a48bcb..15ef74b19 100644
--- a/src/box/sql.h
+++ b/src/box/sql.h
@@ -327,9 +327,10 @@ sql_checks_update_space_def_reference(struct ExprList *expr_list,
  * which is also cleared in the destroy function.
  * @param parser object to initialize.
  * @param db sql object.
+ * @param sql_flags flags to control parser behaviour.
  */
 void
-sql_parser_create(struct Parse *parser, struct sql *db);
+sql_parser_create(struct Parse *parser, struct sql *db, uint32_t sql_flags);
 
 /**
  * Release the parser object resources.
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index a95b07155..bde70a4d3 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -30,7 +30,6 @@
  */
 
 #include "box/box.h"
-#include "box/session.h"
 #include "box/schema.h"
 #include "sqlInt.h"
 #include "tarantoolInt.h"
@@ -151,7 +150,8 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 	struct space *space = sql_lookup_space(parse, tab_list->a);
 	if (space == NULL)
 		goto delete_from_cleanup;
-	trigger_list = sql_triggers_exist(space->def, TK_DELETE, NULL, NULL);
+	trigger_list = sql_triggers_exist(parse, space->def, TK_DELETE,
+					  NULL, NULL);
 	bool is_complex = trigger_list != NULL || fk_constraint_is_required(space, NULL);
 	bool is_view = space->def->opts.is_view;
 
@@ -195,8 +195,8 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 	 * if we are counting rows.
 	 */
 	int reg_count = -1;
-	struct session *user_session = current_session();
-	if (user_session->sql_flags & SQL_CountRows) {
+	uint32_t sql_flags = parse->sql_flags;
+	if ((sql_flags & SQL_CountRows) != 0) {
 		reg_count = ++parse->nMem;
 		sqlVdbeAddOp2(v, OP_Integer, 0, reg_count);
 	}
@@ -287,7 +287,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 		/* Keep track of the number of rows to be
 		 * deleted.
 		 */
-		if (user_session->sql_flags & SQL_CountRows)
+		if ((sql_flags & SQL_CountRows) != 0)
 			sqlVdbeAddOp2(v, OP_AddImm, reg_count, 1);
 
 		/* Extract the primary key for the current row */
@@ -417,7 +417,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 	}
 
 	/* Return the number of rows that were deleted. */
-	if ((user_session->sql_flags & SQL_CountRows) != 0 &&
+	if ((sql_flags & SQL_CountRows) != 0 &&
 	    parse->triggered_space != NULL) {
 		sqlVdbeAddOp2(v, OP_ResultRow, reg_count, 1);
 		sqlVdbeSetNumCols(v, 1);
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index f7507938d..81bcc7839 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -3456,8 +3456,7 @@ sqlExprCachePush(Parse * pParse)
 	struct session MAYBE_UNUSED *user_session;
 	pParse->iCacheLevel++;
 #ifdef SQL_DEBUG
-	user_session = current_session();
-	if (user_session->sql_flags & SQL_VdbeAddopTrace) {
+	if ((pParse->sql_flags & SQL_VdbeAddopTrace) != 0) {
 		printf("PUSH to %d\n", pParse->iCacheLevel);
 	}
 #endif
@@ -3477,7 +3476,7 @@ sqlExprCachePop(Parse * pParse)
 	assert(pParse->iCacheLevel >= 1);
 	pParse->iCacheLevel--;
 #ifdef SQL_DEBUG
-	if (user_session->sql_flags & SQL_VdbeAddopTrace) {
+	if ((pParse->sql_flags & SQL_VdbeAddopTrace) != 0) {
 		printf("POP  to %d\n", pParse->iCacheLevel);
 	}
 #endif
@@ -3566,7 +3565,7 @@ sqlExprCacheClear(Parse * pParse)
 	user_session = current_session();
 
 #if SQL_DEBUG
-	if (user_session->sql_flags & SQL_VdbeAddopTrace) {
+	if ((pParse->sql_flags & SQL_VdbeAddopTrace) != 0) {
 		printf("CLEAR\n");
 	}
 #endif
diff --git a/src/box/sql/fk_constraint.c b/src/box/sql/fk_constraint.c
index 8151c66e5..3bcc0fd24 100644
--- a/src/box/sql/fk_constraint.c
+++ b/src/box/sql/fk_constraint.c
@@ -37,7 +37,6 @@
 #include "sqlInt.h"
 #include "box/fk_constraint.h"
 #include "box/schema.h"
-#include "box/session.h"
 
 /*
  * Deferred and Immediate FKs
@@ -274,9 +273,8 @@ fk_constraint_lookup_parent(struct Parse *parse_context, struct space *parent,
 		sqlReleaseTempReg(parse_context, rec_reg);
 		sqlReleaseTempRange(parse_context, temp_regs, field_count);
 	}
-	struct session *session = current_session();
 	if (!fk_def->is_deferred &&
-	    (session->sql_flags & SQL_DeferFKs) == 0 &&
+	    (parse_context->sql_flags & SQL_DeferFKs) == 0 &&
 	    parse_context->pToplevel == NULL && !parse_context->isMultiWrite) {
 		/*
 		 * If this is an INSERT statement that will insert
@@ -536,7 +534,6 @@ fk_constraint_emit_check(struct Parse *parser, struct space *space, int reg_old,
 {
 	bool is_update = changed_cols != NULL;
 	struct sql *db = parser->db;
-	struct session *user_session = current_session();
 
 	/*
 	 * Exactly one of reg_old and reg_new should be non-zero.
@@ -600,7 +597,7 @@ fk_constraint_emit_check(struct Parse *parser, struct space *space, int reg_old,
 					       changed_cols))
 			continue;
 		if (!fk_def->is_deferred &&
-		    (user_session->sql_flags & SQL_DeferFKs) == 0 &&
+		    (parser->sql_flags & SQL_DeferFKs) == 0 &&
 		    parser->pToplevel == NULL && !parser->isMultiWrite) {
 			assert(reg_old == 0 && reg_new != 0);
 			/*
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index c2aac553f..010695067 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -36,7 +36,6 @@
 #include "sqlInt.h"
 #include "tarantoolInt.h"
 #include "vdbeInt.h"
-#include "box/session.h"
 #include "box/schema.h"
 #include "bit/bit.h"
 #include "box/box.h"
@@ -262,7 +261,6 @@ sqlInsert(Parse * pParse,	/* Parser context */
 	u8 useTempTable = 0;	/* Store SELECT results in intermediate table */
 	u8 bIdListInOrder;	/* True if IDLIST is in table order */
 	ExprList *pList = 0;	/* List of VALUES() to be inserted  */
-	struct session *user_session = current_session();
 
 	/* Register allocations */
 	int regFromSelect = 0;	/* Base register for data coming from SELECT */
@@ -308,7 +306,8 @@ sqlInsert(Parse * pParse,	/* Parser context */
 	 * inserted into is a view
 	 */
 	struct space_def *space_def = space->def;
-	trigger = sql_triggers_exist(space_def, TK_INSERT, NULL, &tmask);
+	trigger = sql_triggers_exist(pParse, space_def, TK_INSERT, NULL,
+				     &tmask);
 
 	bool is_view = space_def->opts.is_view;
 	assert((trigger != NULL && tmask != 0) ||
@@ -529,7 +528,7 @@ sqlInsert(Parse * pParse,	/* Parser context */
 
 	/* Initialize the count of rows to be inserted
 	 */
-	if (user_session->sql_flags & SQL_CountRows) {
+	if ((pParse->sql_flags & SQL_CountRows) != 0) {
 		regRowCount = ++pParse->nMem;
 		sqlVdbeAddOp2(v, OP_Integer, 0, regRowCount);
 	}
@@ -761,7 +760,7 @@ sqlInsert(Parse * pParse,	/* Parser context */
 
 	/* Update the count of rows that are inserted
 	 */
-	if ((user_session->sql_flags & SQL_CountRows) != 0) {
+	if ((pParse->sql_flags & SQL_CountRows) != 0) {
 		sqlVdbeAddOp2(v, OP_AddImm, regRowCount, 1);
 	}
 
@@ -790,7 +789,7 @@ sqlInsert(Parse * pParse,	/* Parser context */
  insert_end:
 
 	/* Return the number of rows inserted. */
-	if ((user_session->sql_flags & SQL_CountRows) != 0 &&
+	if ((pParse->sql_flags & SQL_CountRows) != 0 &&
 	    pParse->triggered_space == NULL) {
 		sqlVdbeAddOp2(v, OP_ResultRow, regRowCount, 1);
 		sqlVdbeSetNumCols(v, 1);
@@ -1039,8 +1038,8 @@ process_index:  ;
 					     part_count);
 			sql_set_multi_write(parse_context, true);
 			struct sql_trigger *trigger =
-				sql_triggers_exist(space->def, TK_DELETE, NULL,
-						   NULL);
+				sql_triggers_exist(parse_context, space->def,
+						   TK_DELETE, NULL, NULL);
 			sql_generate_row_delete(parse_context, space, trigger,
 						cursor, idx_key_reg, part_count,
 						true,
@@ -1130,7 +1129,6 @@ xferOptimization(Parse * pParse,	/* Parser context */
 	int addr1;		/* Loop addresses */
 	int emptyDestTest = 0;	/* Address of test for empty pDest */
 	int regData, regTupleid;	/* Registers holding data and tupleid */
-	struct session *user_session = current_session();
 	bool is_err_action_default = false;
 
 	if (pSelect == NULL)
@@ -1260,7 +1258,7 @@ xferOptimization(Parse * pParse,	/* Parser context */
 	 */
 	if (!rlist_empty(&dest->child_fk_constraint))
 		return 0;
-	if ((user_session->sql_flags & SQL_CountRows) != 0) {
+	if ((pParse->sql_flags & SQL_CountRows) != 0) {
 		return 0;	/* xfer opt does not play well with PRAGMA count_changes */
 	}
 
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index 3df6b5c36..ad4c71cb6 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -54,7 +54,7 @@ sqlPrepare(sql * db,	/* Database handle. */
 {
 	int rc = SQL_OK;	/* Result code */
 	Parse sParse;		/* Parsing context */
-	sql_parser_create(&sParse, db);
+	sql_parser_create(&sParse, db, current_session()->sql_flags);
 	sParse.pReprepare = pReprepare;
 	assert(ppStmt && *ppStmt == 0);
 	/* assert( !db->mallocFailed ); // not true with SQL_USE_ALLOCA */
@@ -281,10 +281,11 @@ sql_prepare_v2(sql * db,	/* Database handle. */
 }
 
 void
-sql_parser_create(struct Parse *parser, sql *db)
+sql_parser_create(struct Parse *parser, struct sql *db, uint32_t sql_flags)
 {
 	memset(parser, 0, sizeof(struct Parse));
 	parser->db = db;
+	parser->sql_flags = sql_flags;
 	rlist_create(&parser->record_list);
 	region_create(&parser->region, &cord()->slabc);
 }
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index d3472a922..d7376ae11 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -40,7 +40,6 @@
 #include "box/box.h"
 #include "box/coll_id_cache.h"
 #include "box/schema.h"
-#include "box/session.h"
 
 /*
  * Trace output macros
@@ -169,8 +168,6 @@ sqlSelectNew(Parse * pParse,	/* Parsing context */
 			pParse->is_aborted = true;
 		pEList = sql_expr_list_append(db, NULL, expr);
 	}
-	struct session MAYBE_UNUSED *user_session;
-	user_session = current_session();
 	pNew->pEList = pEList;
 	pNew->op = TK_SELECT;
 	pNew->selFlags = selFlags;
@@ -178,7 +175,7 @@ sqlSelectNew(Parse * pParse,	/* Parsing context */
 	pNew->iOffset = 0;
 #ifdef SQL_DEBUG
 	pNew->zSelName[0] = 0;
-	if (user_session->sql_flags & SQL_SelectTrace)
+	if ((pParse->sql_flags & SQL_SelectTrace) != 0)
 		sqlSelectTrace = 0xfff;
 	else
 		sqlSelectTrace = 0;
@@ -1733,7 +1730,6 @@ generateColumnNames(Parse * pParse,	/* Parser context */
 	int i, j;
 	sql *db = pParse->db;
 	int fullNames, shortNames;
-	struct session *user_session = current_session();
 	/* If this is an EXPLAIN, skip this step */
 	if (pParse->explain) {
 		return;
@@ -1751,8 +1747,8 @@ generateColumnNames(Parse * pParse,	/* Parser context */
 	}
 	assert(pTabList != 0);
 	pParse->colNamesSet = 1;
-	fullNames = (user_session->sql_flags & SQL_FullColNames) != 0;
-	shortNames = (user_session->sql_flags & SQL_ShortColNames) != 0;
+	fullNames = (pParse->sql_flags & SQL_FullColNames) != 0;
+	shortNames = (pParse->sql_flags & SQL_ShortColNames) != 0;
 	sqlVdbeSetNumCols(v, pEList->nExpr);
 	uint32_t var_count = 0;
 	for (i = 0; i < pEList->nExpr; i++) {
@@ -1994,18 +1990,16 @@ struct space *
 sqlResultSetOfSelect(Parse * pParse, Select * pSelect)
 {
 	sql *db = pParse->db;
-	uint32_t savedFlags;
-	struct session *user_session = current_session();
 
-	savedFlags = user_session->sql_flags;
-	user_session->sql_flags |= ~SQL_FullColNames;
-	user_session->sql_flags &= SQL_ShortColNames;
+	uint32_t saved_flags = pParse->sql_flags;
+	pParse->sql_flags |= ~SQL_FullColNames;
+	pParse->sql_flags &= SQL_ShortColNames;
 	sqlSelectPrep(pParse, pSelect, 0);
 	if (pParse->is_aborted)
 		return NULL;
 	while (pSelect->pPrior)
 		pSelect = pSelect->pPrior;
-	user_session->sql_flags = savedFlags;
+	pParse->sql_flags = saved_flags;
 	struct space *space = sql_ephemeral_space_new(pParse, NULL);
 	if (space == NULL)
 		return NULL;
@@ -2030,6 +2024,7 @@ allocVdbe(Parse * pParse)
 	Vdbe *v = pParse->pVdbe = sqlVdbeCreate(pParse);
 	if (v == NULL)
 		return NULL;
+	v->sql_flags = pParse->sql_flags;
 	sqlVdbeAddOp2(v, OP_Init, 0, 1);
 	if (pParse->pToplevel == 0
 	    && OptimizationEnabled(pParse->db, SQL_FactorOutConst)
@@ -4782,7 +4777,6 @@ selectExpander(Walker * pWalker, Select * p)
 	sql *db = pParse->db;
 	Expr *pE, *pRight, *pExpr;
 	u16 selFlags = p->selFlags;
-	struct session *user_session = current_session();
 
 	p->selFlags |= SF_Expanded;
 	if (db->mallocFailed) {
@@ -4917,7 +4911,7 @@ selectExpander(Walker * pWalker, Select * p)
 		 */
 		struct ExprList_item *a = pEList->a;
 		ExprList *pNew = 0;
-		uint32_t flags = user_session->sql_flags;
+		uint32_t flags = pParse->sql_flags;
 		int longNames = (flags & SQL_FullColNames) != 0
 		    && (flags & SQL_ShortColNames) == 0;
 
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 9384ec9e8..1b3f9afd1 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -2693,6 +2693,8 @@ struct Parse {
 	bool parse_only;
 	/** Type of parsed_ast member. */
 	enum ast_type parsed_ast_type;
+	/** SQL parsing flags. */
+	uint32_t sql_flags;
 	/**
 	 * Members of this union are valid only
 	 * if parse_only is set to true.
@@ -4023,6 +4025,7 @@ vdbe_code_drop_trigger(struct Parse *parser, const char *trigger_name,
  * table, and, if that operation is an UPDATE, if at least one
  * of the columns in changes_list is being modified.
  *
+ * @param parser Parser context.
  * @param space_def The definition of the space that contains
  *        the triggers.
  * @param op operation one of TK_DELETE, TK_INSERT, TK_UPDATE.
@@ -4030,7 +4033,7 @@ vdbe_code_drop_trigger(struct Parse *parser, const char *trigger_name,
  * @param[out] pMask Mask of TRIGGER_BEFORE|TRIGGER_AFTER
  */
 struct sql_trigger *
-sql_triggers_exist(struct space_def *space_def, int op,
+sql_triggers_exist(struct Parse *parser, struct space_def *space_def, int op,
 		   struct ExprList *changes_list, int *mask_ptr);
 
 /**
diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
index 8cc35323c..11ef7b97e 100644
--- a/src/box/sql/tokenize.c
+++ b/src/box/sql/tokenize.c
@@ -40,6 +40,7 @@
 #include <unicode/utf8.h>
 #include <unicode/uchar.h>
 
+#include "box/session.h"
 #include "say.h"
 #include "sqlInt.h"
 #include "tarantoolInt.h"
@@ -544,7 +545,7 @@ sql_expr_compile(sql *db, const char *expr, int expr_len)
 	int len = strlen(outer) + expr_len;
 
 	struct Parse parser;
-	sql_parser_create(&parser, db);
+	sql_parser_create(&parser, db, default_flags);
 	parser.parse_only = true;
 
 	struct Expr *expression = NULL;
@@ -569,7 +570,7 @@ struct Select *
 sql_view_compile(struct sql *db, const char *view_stmt)
 {
 	struct Parse parser;
-	sql_parser_create(&parser, db);
+	sql_parser_create(&parser, db, default_flags);
 	parser.parse_only = true;
 
 	struct Select *select = NULL;
@@ -590,7 +591,7 @@ struct sql_trigger *
 sql_trigger_compile(struct sql *db, const char *sql)
 {
 	struct Parse parser;
-	sql_parser_create(&parser, db);
+	sql_parser_create(&parser, db, default_flags);
 	parser.parse_only = true;
 	struct sql_trigger *trigger = NULL;
 	if (sqlRunParser(&parser, sql) == 0 &&
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index 14e4198a8..97f69696c 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -37,7 +37,6 @@
 #include "sqlInt.h"
 #include "tarantoolInt.h"
 #include "vdbeInt.h"
-#include "box/session.h"
 
 /* See comment in sqlInt.h */
 int sqlSubProgramsRemaining;
@@ -533,13 +532,12 @@ checkColumnOverlap(IdList * pIdList, ExprList * pEList)
 }
 
 struct sql_trigger *
-sql_triggers_exist(struct space_def *space_def, int op,
+sql_triggers_exist(struct Parse *parser, struct space_def *space_def, int op,
 		   struct ExprList *changes_list, int *mask_ptr)
 {
 	int mask = 0;
 	struct sql_trigger *trigger_list = NULL;
-	struct session *user_session = current_session();
-	if ((user_session->sql_flags & SQL_EnableTrigger) != 0)
+	if ((parser->sql_flags & SQL_EnableTrigger) != 0)
 		trigger_list = space_trigger_list(space_def->id);
 	for (struct sql_trigger *p = trigger_list; p != NULL; p = p->next) {
 		if (p->op == op && checkColumnOverlap(p->pColumns,
@@ -753,7 +751,7 @@ sql_row_trigger_program(struct Parse *parser, struct sql_trigger *trigger,
 	pSubParse = sqlStackAllocZero(db, sizeof(Parse));
 	if (!pSubParse)
 		return 0;
-	sql_parser_create(pSubParse, db);
+	sql_parser_create(pSubParse, db, parser->sql_flags);
 	memset(&sNC, 0, sizeof(sNC));
 	sNC.pParse = pSubParse;
 	pSubParse->triggered_space = space;
@@ -893,9 +891,8 @@ vdbe_code_row_trigger_direct(struct Parse *parser, struct sql_trigger *trigger,
 	if (pPrg == NULL)
 		return;
 
-	struct session *user_session = current_session();
-	bool is_recursive = (trigger->zName && !(user_session->sql_flags &
-						 SQL_RecTriggers));
+	bool is_recursive =
+		trigger->zName && (parser->sql_flags & SQL_RecTriggers) == 0;
 
 	sqlVdbeAddOp4(v, OP_Program, reg, ignore_jump,
 			  ++parser->nMem, (const char *)pPrg->pProgram,
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index 35c001939..40b103243 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -34,7 +34,6 @@
  * to handle UPDATE statements.
  */
 #include "sqlInt.h"
-#include "box/session.h"
 #include "tarantoolInt.h"
 #include "box/tuple_format.h"
 #include "box/schema.h"
@@ -92,7 +91,6 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 	int hasFK;		/* True if foreign key processing is required */
 	int labelBreak;		/* Jump here to break out of UPDATE loop */
 	int labelContinue;	/* Jump here to continue next step of UPDATE loop */
-	struct session *user_session = current_session();
 
 	bool is_view;		/* True when updating a view (INSTEAD OF trigger) */
 	/* List of triggers on pTab, if required. */
@@ -127,7 +125,8 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 	/* Figure out if we have any triggers and if the table being
 	 * updated is a view.
 	 */
-	trigger = sql_triggers_exist(space->def, TK_UPDATE, pChanges, &tmask);
+	trigger = sql_triggers_exist(pParse, space->def, TK_UPDATE, pChanges,
+				     &tmask);
 	is_view = space->def->opts.is_view;
 	assert(trigger != NULL || tmask == 0);
 
@@ -298,8 +297,8 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 
 	/* Initialize the count of updated rows
 	 */
-	if ((user_session->sql_flags & SQL_CountRows)
-	    && pParse->triggered_space == NULL) {
+	if ((pParse->sql_flags & SQL_CountRows) != 0 &&
+	    pParse->triggered_space == NULL) {
 		regRowCount = ++pParse->nMem;
 		sqlVdbeAddOp2(v, OP_Integer, 0, regRowCount);
 	}
@@ -500,8 +499,8 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 
 	/* Increment the row counter
 	 */
-	if ((user_session->sql_flags & SQL_CountRows)
-	    && pParse->triggered_space == NULL) {
+	if ((pParse->sql_flags & SQL_CountRows) != 0 &&
+	     pParse->triggered_space == NULL) {
 		sqlVdbeAddOp2(v, OP_AddImm, regRowCount, 1);
 	}
 
@@ -522,7 +521,7 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 	sqlVdbeResolveLabel(v, labelBreak);
 
 	/* Return the number of rows that were changed. */
-	if (user_session->sql_flags & SQL_CountRows &&
+	if ((pParse->sql_flags & SQL_CountRows) != 0 &&
 	    pParse->triggered_space == NULL) {
 		sqlVdbeAddOp2(v, OP_ResultRow, regRowCount, 1);
 		sqlVdbeSetNumCols(v, 1);
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index a34395cdf..11f5685f3 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -43,7 +43,6 @@
 #include "box/error.h"
 #include "box/fk_constraint.h"
 #include "box/txn.h"
-#include "box/session.h"
 #include "sqlInt.h"
 #include "vdbeInt.h"
 #include "tarantoolInt.h"
@@ -524,10 +523,10 @@ registerTrace(int iReg, Mem *p) {
 #endif
 
 #ifdef SQL_DEBUG
-#  define REGISTER_TRACE(R,M)						\
-	if(user_session->sql_flags&SQL_VdbeTrace) registerTrace(R,M);
+#  define REGISTER_TRACE(P,R,M)					\
+	if(P->sql_flags&SQL_VdbeTrace) registerTrace(R,M);
 #else
-#  define REGISTER_TRACE(R,M)
+#  define REGISTER_TRACE(P,R,M)
 #endif
 
 
@@ -643,7 +642,6 @@ int sqlVdbeExec(Vdbe *p)
 #ifdef VDBE_PROFILE
 	u64 start;                 /* CPU clock count at start of opcode */
 #endif
-	struct session *user_session = current_session();
 	/*** INSERT STACK UNION HERE ***/
 
 	assert(p->magic==VDBE_MAGIC_RUN);  /* sql_step() verifies this */
@@ -669,20 +667,18 @@ int sqlVdbeExec(Vdbe *p)
 #endif
 #ifdef SQL_DEBUG
 	sqlBeginBenignMalloc();
-	if (p->pc==0
-	    && (user_session->sql_flags&
-		(SQL_VdbeListing|SQL_VdbeEQP|SQL_VdbeTrace))!=0
-		) {
+	if (p->pc == 0 &&
+	    (p->sql_flags & (SQL_VdbeListing|SQL_VdbeEQP|SQL_VdbeTrace)) != 0) {
 		int i;
 		int once = 1;
 		sqlVdbePrintSql(p);
-		if (user_session->sql_flags & SQL_VdbeListing) {
+		if ((p->sql_flags & SQL_VdbeListing) != 0) {
 			printf("VDBE Program Listing:\n");
 			for(i=0; i<p->nOp; i++) {
 				sqlVdbePrintOp(stdout, i, &aOp[i]);
 			}
 		}
-		if (user_session->sql_flags & SQL_VdbeEQP) {
+		if ((p->sql_flags & SQL_VdbeEQP) != 0) {
 			for(i=0; i<p->nOp; i++) {
 				if (aOp[i].opcode==OP_Explain) {
 					if (once) printf("VDBE Query Plan:\n");
@@ -691,7 +687,8 @@ int sqlVdbeExec(Vdbe *p)
 				}
 			}
 		}
-		if (user_session->sql_flags & SQL_VdbeTrace)  printf("VDBE Trace:\n");
+		if ((p->sql_flags & SQL_VdbeTrace) != 0)
+			printf("VDBE Trace:\n");
 	}
 	sqlEndBenignMalloc();
 #endif
@@ -713,9 +710,8 @@ int sqlVdbeExec(Vdbe *p)
 		/* Only allow tracing if SQL_DEBUG is defined.
 		 */
 #ifdef SQL_DEBUG
-		if (user_session->sql_flags & SQL_VdbeTrace) {
+		if ((p->sql_flags & SQL_VdbeTrace) != 0)
 			sqlVdbePrintOp(stdout, (int)(pOp - aOp), pOp);
-		}
 #endif
 
 
@@ -728,21 +724,21 @@ int sqlVdbeExec(Vdbe *p)
 				assert(pOp->p1<=(p->nMem+1 - p->nCursor));
 				assert(memIsValid(&aMem[pOp->p1]));
 				assert(sqlVdbeCheckMemInvariants(&aMem[pOp->p1]));
-				REGISTER_TRACE(pOp->p1, &aMem[pOp->p1]);
+				REGISTER_TRACE(p, pOp->p1, &aMem[pOp->p1]);
 			}
 			if ((opProperty & OPFLG_IN2)!=0) {
 				assert(pOp->p2>0);
 				assert(pOp->p2<=(p->nMem+1 - p->nCursor));
 				assert(memIsValid(&aMem[pOp->p2]));
 				assert(sqlVdbeCheckMemInvariants(&aMem[pOp->p2]));
-				REGISTER_TRACE(pOp->p2, &aMem[pOp->p2]);
+				REGISTER_TRACE(p, pOp->p2, &aMem[pOp->p2]);
 			}
 			if ((opProperty & OPFLG_IN3)!=0) {
 				assert(pOp->p3>0);
 				assert(pOp->p3<=(p->nMem+1 - p->nCursor));
 				assert(memIsValid(&aMem[pOp->p3]));
 				assert(sqlVdbeCheckMemInvariants(&aMem[pOp->p3]));
-				REGISTER_TRACE(pOp->p3, &aMem[pOp->p3]);
+				REGISTER_TRACE(p, pOp->p3, &aMem[pOp->p3]);
 			}
 			if ((opProperty & OPFLG_OUT2)!=0) {
 				assert(pOp->p2>0);
@@ -858,7 +854,7 @@ case OP_Gosub: {            /* jump */
 	memAboutToChange(p, pIn1);
 	pIn1->flags = MEM_Int;
 	pIn1->u.i = (int)(pOp-aOp);
-	REGISTER_TRACE(pOp->p1, pIn1);
+	REGISTER_TRACE(p, pOp->p1, pIn1);
 
 	/* Most jump operations do a goto to this spot in order to update
 	 * the pOp pointer.
@@ -945,7 +941,7 @@ case OP_Yield: {            /* in1, jump */
 	pIn1->flags = MEM_Int;
 	pcDest = (int)pIn1->u.i;
 	pIn1->u.i = (int)(pOp - aOp);
-	REGISTER_TRACE(pOp->p1, pIn1);
+	REGISTER_TRACE(p, pOp->p1, pIn1);
 	pOp = &aOp[pcDest];
 	break;
 }
@@ -1312,7 +1308,7 @@ case OP_Move: {
 		}
 #endif
 		Deephemeralize(pOut);
-		REGISTER_TRACE(p2++, pOut);
+		REGISTER_TRACE(p, p2++, pOut);
 		pIn1++;
 		pOut++;
 	}while( --n);
@@ -1340,7 +1336,7 @@ case OP_Copy: {
 #ifdef SQL_DEBUG
 		pOut->pScopyFrom = 0;
 #endif
-		REGISTER_TRACE(pOp->p2+pOp->p3-n, pOut);
+		REGISTER_TRACE(p, pOp->p2+pOp->p3-n, pOut);
 		if ((n--)==0) break;
 		pOut++;
 		pIn1++;
@@ -1422,7 +1418,7 @@ case OP_ResultRow: {
 	 * transaction. It needs to be rolled back.
 	 */
 	if (SQL_OK!=(rc = sqlVdbeCheckFk(p, 0))) {
-		assert(user_session->sql_flags&SQL_CountRows);
+		assert((p->sql_flags & SQL_CountRows) != 0);
 		goto abort_due_to_error;
 	}
 
@@ -1441,7 +1437,7 @@ case OP_ResultRow: {
 	 * The statement transaction is never a top-level transaction.  Hence
 	 * the RELEASE call below can never fail.
 	 */
-	assert(p->iStatement==0 || user_session->sql_flags&SQL_CountRows);
+	assert(p->iStatement == 0 || (p->sql_flags & SQL_CountRows) != 0);
 	rc = sqlVdbeCloseStatement(p, SAVEPOINT_RELEASE);
 	assert(rc==SQL_OK);
 
@@ -1459,7 +1455,7 @@ case OP_ResultRow: {
 		assert((pMem[i].flags & MEM_Ephem)==0
 		       || (pMem[i].flags & (MEM_Str|MEM_Blob))==0);
 		sqlVdbeMemNulTerminate(&pMem[i]);
-		REGISTER_TRACE(pOp->p1+i, &pMem[i]);
+		REGISTER_TRACE(p, pOp->p1+i, &pMem[i]);
 	}
 	if (db->mallocFailed) goto no_mem;
 
@@ -1799,7 +1795,7 @@ case OP_Function: {
 #ifdef SQL_DEBUG
 	for(i=0; i<pCtx->argc; i++) {
 		assert(memIsValid(pCtx->argv[i]));
-		REGISTER_TRACE(pOp->p2+i, pCtx->argv[i]);
+		REGISTER_TRACE(p, pOp->p2+i, pCtx->argv[i]);
 	}
 #endif
 	MemSetTypeFlag(pCtx->pOut, MEM_Null);
@@ -1821,7 +1817,7 @@ case OP_Function: {
 		if (sqlVdbeMemTooBig(pCtx->pOut)) goto too_big;
 	}
 
-	REGISTER_TRACE(pOp->p3, pCtx->pOut);
+	REGISTER_TRACE(p, pOp->p3, pCtx->pOut);
 	UPDATE_MAX_BLOBSIZE(pCtx->pOut);
 	break;
 }
@@ -2133,7 +2129,7 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
 				iCompare = 1;    /* Operands are not equal */
 				memAboutToChange(p, pOut);
 				MemSetTypeFlag(pOut, MEM_Null);
-				REGISTER_TRACE(pOp->p2, pOut);
+				REGISTER_TRACE(p, pOp->p2, pOut);
 			} else {
 				VdbeBranchTaken(2,3);
 				if (pOp->p5 & SQL_JUMPIFNULL) {
@@ -2249,7 +2245,7 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
 		}
 		memAboutToChange(p, pOut);
 		mem_set_bool(pOut, res2);
-		REGISTER_TRACE(pOp->p2, pOut);
+		REGISTER_TRACE(p, pOp->p2, pOut);
 	} else {
 		VdbeBranchTaken(res!=0, (pOp->p5 & SQL_NULLEQ)?2:3);
 		if (res2) {
@@ -2351,8 +2347,8 @@ case OP_Compare: {
 		idx = aPermute ? aPermute[i] : i;
 		assert(memIsValid(&aMem[p1+idx]));
 		assert(memIsValid(&aMem[p2+idx]));
-		REGISTER_TRACE(p1+idx, &aMem[p1+idx]);
-		REGISTER_TRACE(p2+idx, &aMem[p2+idx]);
+		REGISTER_TRACE(p, p1+idx, &aMem[p1+idx]);
+		REGISTER_TRACE(p, p2+idx, &aMem[p2+idx]);
 		assert(i < (int)def->part_count);
 		struct coll *coll = def->parts[i].coll;
 		bool is_rev = def->parts[i].sort_order == SORT_ORDER_DESC;
@@ -2783,7 +2779,7 @@ case OP_Column: {
 	if (zData!=pC->aRow) sqlVdbeMemRelease(&sMem);
 			op_column_out:
 	UPDATE_MAX_BLOBSIZE(pDest);
-	REGISTER_TRACE(pOp->p3, pDest);
+	REGISTER_TRACE(p, pOp->p3, pDest);
 	break;
 
 			op_column_error:
@@ -2924,7 +2920,7 @@ case OP_MakeRecord: {
 		goto no_mem;
 	assert(sqlVdbeCheckMemInvariants(pOut));
 	assert(pOp->p3>0 && pOp->p3<=(p->nMem+1 - p->nCursor));
-	REGISTER_TRACE(pOp->p3, pOut);
+	REGISTER_TRACE(p, pOp->p3, pOut);
 	UPDATE_MAX_BLOBSIZE(pOut);
 	break;
 }
@@ -3740,7 +3736,8 @@ case OP_Found: {        /* jump, in3 */
 		for(ii=0; ii<r.nField; ii++) {
 			assert(memIsValid(&r.aMem[ii]));
 			assert((r.aMem[ii].flags & MEM_Zero)==0 || r.aMem[ii].n==0);
-			if (ii) REGISTER_TRACE(pOp->p3+ii, &r.aMem[ii]);
+			if (ii != 0)
+				REGISTER_TRACE(p, pOp->p3+ii, &r.aMem[ii]);
 		}
 #endif
 		pIdxKey = &r;
@@ -4092,7 +4089,7 @@ case OP_RowData: {
 	rc = sqlCursorPayload(pCrsr, 0, n, pOut->z);
 	if (rc) goto abort_due_to_error;
 	UPDATE_MAX_BLOBSIZE(pOut);
-	REGISTER_TRACE(pOp->p2, pOut);
+	REGISTER_TRACE(p, pOp->p2, pOut);
 	break;
 }
 
@@ -5028,7 +5025,7 @@ case OP_Param: {           /* out2 */
  * statement counter is incremented (immediate foreign key constraints).
  */
 case OP_FkCounter: {
-	if ((user_session->sql_flags & SQL_DeferFKs || pOp->p1 != 0) &&
+	if (((p->sql_flags & SQL_DeferFKs) != 0 || pOp->p1 != 0) &&
 	    !p->auto_commit) {
 		struct txn *txn = in_txn();
 		assert(txn != NULL && txn->psql_txn != NULL);
@@ -5052,7 +5049,7 @@ case OP_FkCounter: {
  * (immediate foreign key constraint violations).
  */
 case OP_FkIfZero: {         /* jump */
-	if ((user_session->sql_flags & SQL_DeferFKs || pOp->p1) &&
+	if (((p->sql_flags & SQL_DeferFKs) != 0 || pOp->p1 != 0) &&
 	    !p->auto_commit) {
 		struct txn *txn = in_txn();
 		assert(txn != NULL && txn->psql_txn != NULL);
@@ -5236,7 +5233,7 @@ case OP_AggStep: {
 #ifdef SQL_DEBUG
 	for(i=0; i<pCtx->argc; i++) {
 		assert(memIsValid(pCtx->argv[i]));
-		REGISTER_TRACE(pOp->p2+i, pCtx->argv[i]);
+		REGISTER_TRACE(p, pOp->p2+i, pCtx->argv[i]);
 	}
 #endif
 
@@ -5367,11 +5364,9 @@ case OP_Init: {          /* jump */
 		}
 	}
 #ifdef SQL_DEBUG
-	if ((user_session->sql_flags & SQL_SqlTrace)!=0
-	    && (zTrace = (pOp->p4.z ? pOp->p4.z : p->zSql))!=0
-		) {
+	if ((p->sql_flags & SQL_SqlTrace) != 0 &&
+	    (zTrace = (pOp->p4.z ? pOp->p4.z : p->zSql)) != 0)
 		sqlDebugPrintf("SQL-trace: %s\n", zTrace);
-	}
 #endif /* SQL_DEBUG */
 #endif /* SQL_OMIT_TRACE */
 	assert(pOp->p2>0);
@@ -5444,7 +5439,7 @@ default: {          /* This is really OP_Noop and OP_Explain */
 		assert(pOp>=&aOp[-1] && pOp<&aOp[p->nOp-1]);
 
 #ifdef SQL_DEBUG
-		if (user_session->sql_flags & SQL_VdbeTrace) {
+		if ((p->sql_flags & SQL_VdbeTrace) != 0) {
 			u8 opProperty = sqlOpcodeProperty[pOrigOp->opcode];
 			if (rc!=0) printf("rc=%d\n",rc);
 			if (opProperty & (OPFLG_OUT2)) {
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index a3100e513..86fd92da1 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -446,6 +446,8 @@ struct Vdbe {
 	u32 expmask;		/* Binding to these vars invalidates VM */
 	SubProgram *pProgram;	/* Linked list of all sub-programs used by VM */
 	AuxData *pAuxData;	/* Linked list of auxdata allocations */
+	/** Parser flags with which this object was built. */
+	uint32_t sql_flags;
 	/* Anonymous savepoint for aborts only */
 	Savepoint *anonymous_savepoint;
 #ifdef SQL_ENABLE_STMT_SCANSTATUS
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index 19ee2d03a..edc2074f9 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -43,7 +43,6 @@
 #include "vdbeInt.h"
 #include "whereInt.h"
 #include "box/coll_id_cache.h"
-#include "box/session.h"
 #include "box/schema.h"
 
 /* Forward declaration of methods */
@@ -2832,9 +2831,10 @@ tnt_error:
 	/* Automatic indexes */
 	LogEst rSize = pTab->nRowLogEst;
 	LogEst rLogSize = estLog(rSize);
-	struct session *user_session = current_session();
-	if (!pBuilder->pOrSet	/* Not part of an OR optimization */
-	    && (pWInfo->wctrlFlags & WHERE_OR_SUBCLAUSE) == 0 && (user_session->sql_flags & SQL_AutoIndex) != 0 && pSrc->pIBIndex == 0	/* Has no INDEXED BY clause */
+	if (!pBuilder->pOrSet && /* Not pqart of an OR optimization */
+	    (pWInfo->wctrlFlags & WHERE_OR_SUBCLAUSE) == 0 &&
+	    (pWInfo->pParse->sql_flags & SQL_AutoIndex) != 0 &&
+	    pSrc->pIBIndex == 0	/* Has no INDEXED BY clause */
 	    && !pSrc->fg.notIndexed	/* Has no NOT INDEXED clause */
 	    && HasRowid(pTab)	/* Not WITHOUT ROWID table. (FIXME: Why not?) */
 	    &&!pSrc->fg.isCorrelated	/* Not a correlated subquery */
@@ -4241,10 +4241,9 @@ sqlWhereBegin(Parse * pParse,	/* The parser context */
 	sql *db;		/* Database connection */
 	int rc;			/* Return code */
 	u8 bFordelete = 0;	/* OPFLAG_FORDELETE or zero, as appropriate */
-	struct session *user_session = current_session();
 
 #ifdef SQL_DEBUG
-	if (user_session->sql_flags & SQL_WhereTrace)
+	if ((pParse->sql_flags & SQL_WhereTrace) != 0)
 		sqlWhereTrace = 0xfff;
 	else
 		sqlWhereTrace = 0;
@@ -4452,7 +4451,7 @@ sqlWhereBegin(Parse * pParse,	/* The parser context */
 		}
 	}
 	if (pWInfo->pOrderBy == 0 &&
-	    (user_session->sql_flags & SQL_ReverseOrder) != 0) {
+	    (pParse->sql_flags & SQL_ReverseOrder) != 0) {
 		pWInfo->revMask = ALLBITS;
 	}
 	if (pParse->is_aborted || NEVER(db->mallocFailed)) {
-- 
2.21.0

[tarantool-patches] Re: [PATCH v1 3/3] box: local sql_flags for parser and vdbe

[n.pettik]

[-- Attachment #1: Type: text/plain, Size: 2367 bytes --]



> On 15 May 2019, at 21:54, Kirill Shcherbatov <kshcherbatov@tarantool.org> wrote:
> The sql_flags is a parser parameter that describe

-> describes

Nit: not only how to parse, but it determines general behaviour:
like whether foreign keys are handled as deferred or not etc.

> how to parse the
> SQL request, but now this information is taken from the global
> user session object.
> When we need to run the parser with some other parameters, it is
> necessary to change global session object, which may lead to
> unpredictable consequences in general case.
> Introduced a new parser and vdbe field sql_flags is responsible

-> which is responsible

> diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
> index a95b07155..bde70a4d3 100644
> --- a/src/box/sql/delete.c
> +++ b/src/box/sql/delete.c
> @@ -30,7 +30,6 @@
>  */
> 
> #include "box/box.h"
> -#include "box/session.h"
> #include "box/schema.h"
> #include "sqlInt.h"
> #include "tarantoolInt.h"
> @@ -151,7 +150,8 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
> 	struct space *space = sql_lookup_space(parse, tab_list->a);
> 	if (space == NULL)
> 		goto delete_from_cleanup;
> -	trigger_list = sql_triggers_exist(space->def, TK_DELETE, NULL, NULL);
> +	trigger_list = sql_triggers_exist(parse, space->def, TK_DELETE,
> +					  NULL, NULL);

Why not pass only flags from parsing context?

> @@ -5444,7 +5439,7 @@ default: {          /* This is really OP_Noop and OP_Explain */
> 		assert(pOp>=&aOp[-1] && pOp<&aOp[p->nOp-1]);
> 
> #ifdef SQL_DEBUG
> -		if (user_session->sql_flags & SQL_VdbeTrace) {
> +		if ((p->sql_flags & SQL_VdbeTrace) != 0) {
> 			u8 opProperty = sqlOpcodeProperty[pOrigOp->opcode];
> 			if (rc!=0) printf("rc=%d\n",rc);
> 			if (opProperty & (OPFLG_OUT2)) {
> diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
> index a3100e513..86fd92da1 100644
> --- a/src/box/sql/vdbeInt.h
> +++ b/src/box/sql/vdbeInt.h
> @@ -446,6 +446,8 @@ struct Vdbe {
> 	u32 expmask;		/* Binding to these vars invalidates VM */
> 	SubProgram *pProgram;	/* Linked list of all sub-programs used by VM */
> 	AuxData *pAuxData;	/* Linked list of auxdata allocations */
> +	/** Parser flags with which this object was built. */

Strictly speaking, they these flags are not only parsing flags.

The rest is OK.

[-- Attachment #2: Type: text/html, Size: 49991 bytes --]

[tarantool-patches] Re: [PATCH v1 3/3] box: local sql_flags for parser and vdbe

[Kirill Shcherbatov]

Hi! Thank you for review.

    box: local sql_flags for parser and vdbe
    
    The sql_flags is a parser parameter that describes how to parse
    the SQL request, determines general behaviour: like whether
    foreign keys are handled as deferred or not etc. But now this
    information is taken from the global user session object.
    
    When we need to run the parser with some other parameters, it is
    necessary to change global session object, which may lead to
    unpredictable consequences in general case.
    Introduced a new parser and vdbe field sql_flags which is
    responsible for SQL parsing results.
    
    Needed for #3691


> Why not pass only flags from parsing context?
Done.

> Strictly speaking, they flags are not flags.
'options

> The rest is OK.