[tarantool-patches] [PATCH 0/2] Refactor SQL initialization, block triggers in transaction

[tarantool-patches] [PATCH 0/2] Refactor SQL initialization, block triggers in transaction

[Kirill Yukhin]

Issue: https://github.com/tarantool/tarantool/issues
Branch: kyukhin/gh-3239-ddl-trigger

Kirill Yukhin (2):
  sql: refactor initialization routines
  sql: block trigger creation inside a transaction

 src/box/alter.cc               |  15 +++
 src/box/alter.h                |   1 +
 src/box/schema.cc              |   2 +-
 src/box/sql.c                  |  16 +---
 src/box/sql/analyze.c          |   7 +-
 src/box/sql/build.c            |  42 +++------
 src/box/sql/main.c             | 203 ++++++++++++-----------------------------
 src/box/sql/parse.c            | 147 ++++++++++++++---------------
 src/box/sql/parse.y            |   2 +
 src/box/sql/pragma.c           |   3 +-
 src/box/sql/prepare.c          |  53 -----------
 src/box/sql/sqlite3.h          |   7 +-
 src/box/sql/sqliteInt.h        |   2 -
 src/box/sql/trigger.c          |   5 +-
 src/box/sql/vdbe.c             |   1 -
 test/sql-tap/trigger1.test.lua |  25 ++++-
 test/sql-tap/trigger9.test.lua |  77 +++++++++-------
 17 files changed, 242 insertions(+), 366 deletions(-)

-- 
2.11.0

[tarantool-patches] [PATCH 1/2] sql: refactor initialization routines

[Kirill Yukhin]

Make SQL load schema during initialization. It was
initialized on first query before the patch. Replace
check that schema is loaded w/ assert arouns the code base.

As far as DDL is prohibited inside the transaction, schema
cannot be changed inside a transaction and hence, no need to
reload it during rollback. No internal changes
(SQLITE_InernChanges) are possible.

Part of #3235
---
 src/box/sql.c           |  16 +---
 src/box/sql/analyze.c   |   7 +-
 src/box/sql/build.c     |  33 ++------
 src/box/sql/main.c      | 204 +++++++++++++++---------------------------------
 src/box/sql/pragma.c    |   3 +-
 src/box/sql/prepare.c   |  53 -------------
 src/box/sql/sqlite3.h   |   7 +-
 src/box/sql/sqliteInt.h |   2 -
 src/box/sql/trigger.c   |   5 +-
 9 files changed, 80 insertions(+), 250 deletions(-)

diff --git a/src/box/sql.c b/src/box/sql.c
index 817926226..224747157 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -71,24 +71,12 @@ static const uint32_t default_sql_flags = SQLITE_ShortColNames
 void
 sql_init()
 {
-	int rc;
 	default_flags |= default_sql_flags;
 
-	rc = sqlite3_open("", &db);
-	if (rc != SQLITE_OK) {
-		panic("failed to initialize SQL subsystem");
-	} else {
-		/* XXX */
-	}
-
 	current_session()->sql_flags |= default_sql_flags;
 
-	rc = sqlite3Init(db);
-	if (rc != SQLITE_OK) {
-		panic("database initialization failed");
-	} else {
-		/* XXX */
-	}
+	if (sql_init_db(&db) != SQLITE_OK)
+		panic("failed to initialize SQL subsystem");
 
 	assert(db != NULL);
 }
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 130e87665..38f48c5a0 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -1176,12 +1176,7 @@ sqlite3Analyze(Parse * pParse, Token * pName)
 	Table *pTab;
 	Vdbe *v;
 
-	/* Read the database schema. If an error occurs, leave an error message
-	 * and code in pParse and return NULL.
-	 */
-	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
-		return;
-	}
+	assert(db->pSchema);
 
 	if (pName == 0) {
 		/* Form 1:  Analyze everything */
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 5d2876d68..a719136cd 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -217,12 +217,7 @@ sqlite3LocateTable(Parse * pParse,	/* context in which to report errors */
 {
 	Table *p;
 
-	/* Read the database schema. If an error occurs, leave an error message
-	 * and code in pParse and return NULL.
-	 */
-	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
-		return 0;
-	}
+	assert(pParse->db->pSchema != NULL);
 
 	p = sqlite3FindTable(pParse->db, zName);
 	if (p == 0) {
@@ -647,13 +642,7 @@ sqlite3StartTable(Parse *pParse, Token *pName, int noErr)
 	if (zName == 0)
 		return;
 
-	/*
-	 * Make sure the new table name does not collide with an
-	 * existing index or table name in the same database.
-	 * Issue an error message if it does.
-	 */
-	if (SQLITE_OK != sqlite3ReadSchema(pParse))
-		goto begin_table_error;
+	assert(db->pSchema != NULL);
 	pTable = sqlite3FindTable(db, zName);
 	if (pTable) {
 		if (!noErr) {
@@ -2401,8 +2390,7 @@ sqlite3DropTable(Parse * pParse, SrcList * pName, int isView, int noErr)
 		sqlite3VdbeCountChanges(v);
 	assert(pParse->nErr == 0);
 	assert(pName->nSrc == 1);
-	if (sqlite3ReadSchema(pParse))
-		goto exit_drop_table;
+	assert(db->pSchema != NULL);
 	if (noErr)
 		db->suppressErr++;
 	assert(isView == 0 || isView == LOCATE_VIEW);
@@ -2878,9 +2866,7 @@ sqlite3CreateIndex(Parse * pParse,	/* All information about this parse */
 			goto exit_create_index;
 		sqlite3VdbeCountChanges(v);
 	}
-	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
-		goto exit_create_index;
-	}
+	assert(db->pSchema != NULL);
 
 	/*
 	 * Find the table that is to be indexed.  Return early if not found.
@@ -3389,9 +3375,7 @@ sqlite3DropIndex(Parse * pParse, SrcList * pName, Token * pName2, int ifExists)
 	if (!pParse->nested)
 		sqlite3VdbeCountChanges(v);
 	assert(pName->nSrc == 1);
-	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
-		goto exit_drop_index;
-	}
+	assert(db->pSchema != NULL);
 
 	assert(pName2->n > 0);
 	zTableName = sqlite3NameFromToken(db, pName2);
@@ -4151,12 +4135,7 @@ sqlite3Reindex(Parse * pParse, Token * pName1, Token * pName2)
 	Index *pIndex;		/* An index associated with pTab */
 	sqlite3 *db = pParse->db;	/* The database connection */
 
-	/* Read the database schema. If an error occurs, leave an error message
-	 * and code in pParse and return NULL.
-	 */
-	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
-		return;
-	}
+	assert(db->pSchema != NULL);
 
 	if (pName1 == 0) {
 		reindexDatabases(pParse, 0);
diff --git a/src/box/sql/main.c b/src/box/sql/main.c
index 540570dc8..5a0f372b3 100644
--- a/src/box/sql/main.c
+++ b/src/box/sql/main.c
@@ -981,6 +981,15 @@ sqlite3RollbackAll(Vdbe * pVdbe, int tripCode)
 	    && db->init.busy == 0) {
 		sqlite3ExpirePreparedStatements(db);
 		sqlite3ResetAllSchemasOfConnection(db);
+
+		db->init.busy = 1;
+		db->pSchema = sqlite3SchemaCreate(db);
+		int rc = sqlite3InitDatabase(db);
+		if (rc != SQLITE_OK)
+			sqlite3SchemaClear(db);
+		db->init.busy = 0;
+		if (rc == SQLITE_OK)
+			sqlite3CommitInternalChanges();
 	}
 
 	/* Any deferred constraint violations have now been resolved. */
@@ -2246,122 +2255,62 @@ sqlite3ParseUri(const char *zDefaultVfs,	/* VFS to use if no "vfs=xxx" query opt
 	return rc;
 }
 
-/*
- * This routine does the work of opening a database on behalf of
- * sqlite3_open() and database filename "zFilename"
- * is UTF-8 encoded.
+/**
+ * This routine does the work of initialization of main
+ * SQL connection instance.
+ *
+ * @param[out] db returned database handle.
+ * @return error status code.
  */
-static int
-openDatabase(const char *zFilename,	/* Database filename UTF-8 encoded */
-	     sqlite3 ** ppDb,		/* OUT: Returned database handle */
-	     unsigned int flags,	/* Operational flags */
-	     const char *zVfs)		/* Name of the VFS to use */
+int
+sql_init_db(sqlite3 **db)
 {
-	sqlite3 *db;		/* Store allocated handle here */
+	sqlite3 *loc_db;
 	int rc;			/* Return code */
 	int isThreadsafe;	/* True for threadsafe connections */
-	char *zOpen = 0;	/* Filename argument to pass to BtreeOpen() */
-	char *zErrMsg = 0;	/* Error message from sqlite3ParseUri() */
 
 #ifdef SQLITE_ENABLE_API_ARMOR
 	if (ppDb == 0)
 		return SQLITE_MISUSE_BKPT;
 #endif
-	*ppDb = 0;
 #ifndef SQLITE_OMIT_AUTOINIT
 	rc = sqlite3_initialize();
 	if (rc)
 		return rc;
 #endif
 
-	/* Only allow sensible combinations of bits in the flags argument.
-	 * Throw an error if any non-sense combination is used.  If we
-	 * do not block illegal combinations here, it could trigger
-	 * assert() statements in deeper layers.  Sensible combinations
-	 * are:
-	 *
-	 *  1:  SQLITE_OPEN_READONLY
-	 *  2:  SQLITE_OPEN_READWRITE
-	 *  6:  SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE
-	 */
-	assert(SQLITE_OPEN_READONLY == 0x01);
-	assert(SQLITE_OPEN_READWRITE == 0x02);
-	assert(SQLITE_OPEN_CREATE == 0x04);
-	testcase((1 << (flags & 7)) == 0x02);	/* READONLY */
-	testcase((1 << (flags & 7)) == 0x04);	/* READWRITE */
-	testcase((1 << (flags & 7)) == 0x40);	/* READWRITE | CREATE */
-	if (((1 << (flags & 7)) & 0x46) == 0) {
-		return SQLITE_MISUSE_BKPT;	/* IMP: R-65497-44594 */
-	}
-
 	if (sqlite3GlobalConfig.bCoreMutex == 0) {
 		isThreadsafe = 0;
-	} else if (flags & SQLITE_OPEN_NOMUTEX) {
-		isThreadsafe = 0;
-	} else if (flags & SQLITE_OPEN_FULLMUTEX) {
-		isThreadsafe = 1;
 	} else {
 		isThreadsafe = sqlite3GlobalConfig.bFullMutex;
 	}
-	if (flags & SQLITE_OPEN_PRIVATECACHE) {
-		flags &= ~SQLITE_OPEN_SHAREDCACHE;
-	} else if (sqlite3GlobalConfig.sharedCacheEnabled) {
-		flags |= SQLITE_OPEN_SHAREDCACHE;
-	}
-	/* Remove harmful bits from the flags parameter
-	 *
-	 * The SQLITE_OPEN_NOMUTEX and SQLITE_OPEN_FULLMUTEX flags were
-	 * dealt with in the previous code block. Besides these, the only
-	 * valid input flags for sqlite3_open_v2() are SQLITE_OPEN_READONLY,
-	 * SQLITE_OPEN_READWRITE, SQLITE_OPEN_CREATE, SQLITE_OPEN_SHAREDCACHE,
-	 * SQLITE_OPEN_PRIVATECACHE, and some reserved bits. Silently mask
-	 * off all other flags.
-	 */
-	flags &= ~(SQLITE_OPEN_DELETEONCLOSE |
-		   SQLITE_OPEN_EXCLUSIVE |
-		   SQLITE_OPEN_MAIN_DB |
-		   SQLITE_OPEN_TEMP_DB |
-		   SQLITE_OPEN_TRANSIENT_DB |
-		   SQLITE_OPEN_NOMUTEX |
-		   SQLITE_OPEN_FULLMUTEX);
-	flags |= SQLITE_OPEN_MEMORY;
 	/* Allocate the sqlite data structure */
-	db = sqlite3MallocZero(sizeof(sqlite3));
+	loc_db = sqlite3MallocZero(sizeof(sqlite3));
 	if (db == 0)
 		goto opendb_out;
 	if (isThreadsafe) {
-		db->mutex = sqlite3MutexAlloc(SQLITE_MUTEX_RECURSIVE);
-		if (db->mutex == 0) {
+		loc_db->mutex = sqlite3MutexAlloc(SQLITE_MUTEX_RECURSIVE);
+		if (loc_db->mutex == 0) {
 			sqlite3_free(db);
 			db = 0;
 			goto opendb_out;
 		}
 	}
 	sqlite3_mutex_enter(db->mutex);
-	db->errMask = 0xff;
-	db->magic = SQLITE_MAGIC_BUSY;
-
-	assert(sizeof(db->aLimit) == sizeof(aHardLimit));
-	memcpy(db->aLimit, aHardLimit, sizeof(db->aLimit));
-	db->aLimit[SQLITE_LIMIT_WORKER_THREADS] = SQLITE_DEFAULT_WORKER_THREADS;
-	db->szMmap = sqlite3GlobalConfig.szMmap;
-	db->nMaxSorterMmap = 0x7FFFFFFF;
-
-	db->openFlags = flags;
-	/* Parse the filename/URI argument. */
-	rc = sqlite3ParseUri(zVfs, zFilename,
-			     &flags, &db->pVfs, &zOpen, &zErrMsg);
-	if (rc != SQLITE_OK) {
-		if (rc == SQLITE_NOMEM)
-			sqlite3OomFault(db);
-		sqlite3ErrorWithMsg(db, rc, zErrMsg ? "%s" : 0, zErrMsg);
-		sqlite3_free(zErrMsg);
-		goto opendb_out;
-	}
+	loc_db->errMask = 0xff;
+	loc_db->magic = SQLITE_MAGIC_BUSY;
 
-	db->pSchema = NULL;
-	db->magic = SQLITE_MAGIC_OPEN;
-	if (db->mallocFailed) {
+	loc_db->pVfs = sqlite3_vfs_find(0);
+
+	assert(sizeof(loc_db->aLimit) == sizeof(aHardLimit));
+	memcpy(loc_db->aLimit, aHardLimit, sizeof(loc_db->aLimit));
+	loc_db->aLimit[SQLITE_LIMIT_WORKER_THREADS] = SQLITE_DEFAULT_WORKER_THREADS;
+	loc_db->szMmap = sqlite3GlobalConfig.szMmap;
+	loc_db->nMaxSorterMmap = 0x7FFFFFFF;
+
+	loc_db->pSchema = NULL;
+	loc_db->magic = SQLITE_MAGIC_OPEN;
+	if (loc_db->mallocFailed) {
 		goto opendb_out;
 	}
 
@@ -2369,9 +2318,9 @@ openDatabase(const char *zFilename,	/* Database filename UTF-8 encoded */
 	 * database schema yet. This is delayed until the first time the database
 	 * is accessed.
 	 */
-	sqlite3Error(db, SQLITE_OK);
-	sqlite3RegisterPerConnectionBuiltinFunctions(db);
-	rc = sqlite3_errcode(db);
+	sqlite3Error(loc_db, SQLITE_OK);
+	sqlite3RegisterPerConnectionBuiltinFunctions(loc_db);
+	rc = sqlite3_errcode(loc_db);
 
 #ifdef SQLITE_ENABLE_FTS5
 	/* Register any built-in FTS5 module before loading the automatic
@@ -2422,27 +2371,42 @@ openDatabase(const char *zFilename,	/* Database filename UTF-8 encoded */
 #endif
 
 	if (rc)
-		sqlite3Error(db, rc);
+		sqlite3Error(loc_db, rc);
 
 	/* Enable the lookaside-malloc subsystem */
-	setupLookaside(db, 0, sqlite3GlobalConfig.szLookaside,
+	setupLookaside(loc_db, 0, sqlite3GlobalConfig.szLookaside,
 		       sqlite3GlobalConfig.nLookaside);
 
- opendb_out:
-	if (db) {
-		assert(db->mutex != 0 || isThreadsafe == 0
+	if (rc == SQLITE_OK) {
+		struct session *user_session = current_session();
+		int commit_internal = !(user_session->sql_flags
+					& SQLITE_InternChanges);
+
+		assert(loc_db->init.busy == 0);
+		loc_db->init.busy = 1;
+		loc_db->pSchema = sqlite3SchemaCreate(loc_db);
+		rc = sqlite3InitDatabase(loc_db);
+		if (rc != SQLITE_OK)
+			sqlite3SchemaClear(loc_db);
+		loc_db->init.busy = 0;
+		if (rc == SQLITE_OK && commit_internal)
+			sqlite3CommitInternalChanges();
+	}
+opendb_out:
+	if (loc_db) {
+		assert(loc_db->mutex != 0 || isThreadsafe == 0
 		       || sqlite3GlobalConfig.bFullMutex == 0);
 		sqlite3_mutex_leave(db->mutex);
 	}
-	rc = sqlite3_errcode(db);
-	assert(db != 0 || rc == SQLITE_NOMEM);
+	rc = sqlite3_errcode(loc_db);
+	assert(loc_db != 0 || rc == SQLITE_NOMEM);
 	if (rc == SQLITE_NOMEM) {
-		sqlite3_close(db);
+		sqlite3_close(loc_db);
 		db = 0;
-	} else if (rc != SQLITE_OK) {
-		db->magic = SQLITE_MAGIC_SICK;
-	}
-	*ppDb = db;
+	} else if (rc != SQLITE_OK)
+		loc_db->magic = SQLITE_MAGIC_SICK;
+
+	*db = loc_db;
 #ifdef SQLITE_ENABLE_SQLLOG
 	if (sqlite3GlobalConfig.xSqllog) {
 		/* Opening a db handle. Fourth parameter is passed 0. */
@@ -2450,46 +2414,8 @@ openDatabase(const char *zFilename,	/* Database filename UTF-8 encoded */
 		sqlite3GlobalConfig.xSqllog(pArg, db, zFilename, 0);
 	}
 #endif
-#if defined(SQLITE_HAS_CODEC)
-	if (rc == SQLITE_OK) {
-		const char *zHexKey = sqlite3_uri_parameter(zOpen, "hexkey");
-		if (zHexKey && zHexKey[0]) {
-			u8 iByte;
-			int i;
-			char zKey[40];
-			for (i = 0, iByte = 0;
-			     i < sizeof(zKey) * 2
-			     && sqlite3Isxdigit(zHexKey[i]); i++) {
-				iByte =
-				    (iByte << 4) + sqlite3HexToInt(zHexKey[i]);
-				if ((i & 1) != 0)
-					zKey[i / 2] = iByte;
-			}
-			sqlite3_key_v2(db, 0, zKey, i / 2);
-		}
-	}
-#endif
-	sqlite3_free(zOpen);
-	return rc & 0xff;
-}
 
-/*
- * Open a new database handle.
- */
-int
-sqlite3_open(const char *zFilename, sqlite3 ** ppDb)
-{
-	return openDatabase(zFilename, ppDb,
-			    SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, 0);
-}
-
-int
-sqlite3_open_v2(const char *filename,	/* Database filename (UTF-8) */
-		sqlite3 ** ppDb,	/* OUT: SQLite db handle */
-		int flags,		/* Flags */
-		const char *zVfs)	/* Name of VFS module to use */
-{
-	return openDatabase(filename, ppDb, (unsigned int)flags, zVfs);
+	return rc;
 }
 
 /*
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index aa135c4ce..4db553bcf 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -292,8 +292,7 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 
 	/* Make sure the database schema is loaded if the pragma requires that */
 	if ((pPragma->mPragFlg & PragFlg_NeedSchema) != 0) {
-		if (sqlite3ReadSchema(pParse))
-			goto pragma_out;
+		assert(db->pSchema != NULL);
 	}
 	/* Register the result column names for pragmas that return results */
 	if ((pPragma->mPragFlg & PragFlg_NoColumns) == 0
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index 85d578eb6..9b8028277 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -207,59 +207,6 @@ sqlite3InitDatabase(sqlite3 * db)
 }
 
 /*
- * Initialize all database files - the main database file
- * Return a success code.
- * After a database is initialized, the pSchema field in database
- * structure will be not NULL.
- */
-int
-sqlite3Init(sqlite3 * db)
-{
-	int rc;
-	struct session *user_session = current_session();
-	int commit_internal = !(user_session->sql_flags & SQLITE_InternChanges);
-
-	assert(sqlite3_mutex_held(db->mutex));
-	assert(db->init.busy == 0);
-	rc = SQLITE_OK;
-	db->init.busy = 1;
-	if (!db->pSchema) {
-		db->pSchema = sqlite3SchemaCreate(db);
-		rc = sqlite3InitDatabase(db);
-		if (rc) {
-			sqlite3SchemaClear(db);
-		}
-	}
-
-	db->init.busy = 0;
-	if (rc == SQLITE_OK && commit_internal) {
-		sqlite3CommitInternalChanges();
-	}
-
-	return rc;
-}
-
-/*
- * This routine is a no-op if the database schema is already initialized.
- * Otherwise, the schema is loaded. An error code is returned.
- */
-int
-sqlite3ReadSchema(Parse * pParse)
-{
-	int rc = SQLITE_OK;
-	sqlite3 *db = pParse->db;
-	assert(sqlite3_mutex_held(db->mutex));
-	if (!db->init.busy) {
-		rc = sqlite3Init(db);
-	}
-	if (rc != SQLITE_OK) {
-		pParse->rc = rc;
-		pParse->nErr++;
-	}
-	return rc;
-}
-
-/*
  * Convert a schema pointer into the 0 index that indicates
  * that schema refers to a single database.
  * This method is inherited from SQLite, which has several dbs.
diff --git a/src/box/sql/sqlite3.h b/src/box/sql/sqlite3.h
index 73e916991..d60d74d0e 100644
--- a/src/box/sql/sqlite3.h
+++ b/src/box/sql/sqlite3.h
@@ -2869,10 +2869,9 @@ sqlite3_progress_handler(sqlite3 *, int,
  *
  * See also: [sqlite3_temp_directory]
 */
-SQLITE_API int
-sqlite3_open(const char *filename,	/* Database filename (UTF-8) */
-	     sqlite3 ** ppDb	/* OUT: SQLite db handle */
-	);
+
+int
+sql_init_db(sqlite3 **db);
 
 SQLITE_API int
 sqlite3_open16(const void *filename,	/* Database filename (UTF-16) */
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index de2b47bfc..cda862d68 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -960,7 +960,6 @@ struct sqlite3 {
 	sqlite3_mutex *mutex;	/* Connection mutex */
 	struct Schema *pSchema; /* Schema of the database */
 	i64 szMmap;		/* Default mmap_size setting */
-	unsigned int openFlags;	/* Flags passed to sqlite3_vfs.xOpen() */
 	int errCode;		/* Most recent error code (SQLITE_*) */
 	int errMask;		/* & result codes with this before returning */
 	int iSysErrno;		/* Errno value from last system error */
@@ -3259,7 +3258,6 @@ const char *sqlite3ErrName(int);
 #endif
 
 const char *sqlite3ErrStr(int);
-int sqlite3ReadSchema(Parse * pParse);
 struct coll *sqlite3FindCollSeq(const char *);
 struct coll *sqlite3LocateCollSeq(Parse * pParse, sqlite3 * db, const char *zName);
 struct coll *sqlite3ExprCollSeq(Parse * pParse, Expr * pExpr);
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index a2827c882..08963cb79 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -507,9 +507,8 @@ sqlite3DropTrigger(Parse * pParse, SrcList * pName, int noErr)
 
 	if (db->mallocFailed)
 		goto drop_trigger_cleanup;
-	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
-		goto drop_trigger_cleanup;
-	}
+	assert(db->pSchema != NULL);
+
 	/* Do not account nested operations: the count of such
 	 * operations depends on Tarantool data dictionary internals,
 	 * such as data layout in system spaces. Activate the counter
-- 
2.11.0

[tarantool-patches] Re: [PATCH 1/2] sql: refactor initialization routines

[v.shpilevoy]

See below 6 comments.

> 15 марта 2018 г., в 21:21, Kirill Yukhin <kyukhin@tarantool.org> написал(а):
> 
> Make SQL load schema during initialization. It was
> initialized on first query before the patch. Replace
> check that schema is loaded w/ assert arouns the code base.
> 
> As far as DDL is prohibited inside the transaction, schema
> cannot be changed inside a transaction and hence, no need to
> reload it during rollback. No internal changes
> (SQLITE_InernChanges) are possible.
> 
> Part of #3235
> ---
> src/box/sql.c           |  16 +---
> src/box/sql/analyze.c   |   7 +-
> src/box/sql/build.c     |  33 ++------
> src/box/sql/main.c      | 204 +++++++++++++++---------------------------------
> src/box/sql/pragma.c    |   3 +-
> src/box/sql/prepare.c   |  53 -------------
> src/box/sql/sqlite3.h   |   7 +-
> src/box/sql/sqliteInt.h |   2 -
> src/box/sql/trigger.c   |   5 +-
> 9 files changed, 80 insertions(+), 250 deletions(-)

> diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
> index 130e87665..38f48c5a0 100644
> --- a/src/box/sql/analyze.c
> +++ b/src/box/sql/analyze.c
> @@ -1176,12 +1176,7 @@ sqlite3Analyze(Parse * pParse, Token * pName)
> 	Table *pTab;
> 	Vdbe *v;
> 
> -	/* Read the database schema. If an error occurs, leave an error message
> -	 * and code in pParse and return NULL.
> -	 */
> -	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
> -		return;
> -	}
> +	assert(db->pSchema);

1. Please, use explicit "!= NULL".

> 
> 	if (pName == 0) {
> 		/* Form 1:  Analyze everything */
> diff --git a/src/box/sql/main.c b/src/box/sql/main.c
> index 540570dc8..5a0f372b3 100644
> --- a/src/box/sql/main.c
> +++ b/src/box/sql/main.c
> @@ -981,6 +981,15 @@ sqlite3RollbackAll(Vdbe * pVdbe, int tripCode)
> 	    && db->init.busy == 0) {
> 		sqlite3ExpirePreparedStatements(db);
> 		sqlite3ResetAllSchemasOfConnection(db);
> +
> +		db->init.busy = 1;

2. Why do you initialise DB in sqlite3RollbackAll? And do you really need to add place this code here, if in a next commit you remove it?

> +		db->pSchema = sqlite3SchemaCreate(db);
> +		int rc = sqlite3InitDatabase(db);
> +		if (rc != SQLITE_OK)
> +			sqlite3SchemaClear(db);
> +		db->init.busy = 0;
> +		if (rc == SQLITE_OK)
> +			sqlite3CommitInternalChanges();
> 	}
> 
> 	/* Any deferred constraint violations have now been resolved. */
> @@ -2246,122 +2255,62 @@ sqlite3ParseUri(const char *zDefaultVfs,	/* VFS to use if no "vfs=xxx" query opt
> 	return rc;
> }
> 
> -/*
> - * This routine does the work of opening a database on behalf of
> - * sqlite3_open() and database filename "zFilename"
> - * is UTF-8 encoded.
> +/**
> + * This routine does the work of initialization of main
> + * SQL connection instance.
> + *
> + * @param[out] db returned database handle.
> + * @return error status code.
>  */
> -static int
> -openDatabase(const char *zFilename,	/* Database filename UTF-8 encoded */
> -	     sqlite3 ** ppDb,		/* OUT: Returned database handle */
> -	     unsigned int flags,	/* Operational flags */
> -	     const char *zVfs)		/* Name of the VFS to use */
> +int
> +sql_init_db(sqlite3 **db)
> {
> -	sqlite3 *db;		/* Store allocated handle here */
> +	sqlite3 *loc_db;

3. If you call a parameter 'db' as 'out_db', and save the local name as 'db', then the diff becomes smaller. You can fix or not fix it at your convenience.

> 
> +	loc_db->pVfs = sqlite3_vfs_find(0);

4. What is sqlite3_vfs_find? I do not see this in the original code.

> +
> +	assert(sizeof(loc_db->aLimit) == sizeof(aHardLimit));
> +	memcpy(loc_db->aLimit, aHardLimit, sizeof(loc_db->aLimit));
> +	loc_db->aLimit[SQLITE_LIMIT_WORKER_THREADS] = SQLITE_DEFAULT_WORKER_THREADS;

5. Please, try to fit code in 80 symbols.

> @@ -2422,27 +2371,42 @@ openDatabase(const char *zFilename,	/* Database filename UTF-8 encoded */
> 
> -	if (db) {
> -		assert(db->mutex != 0 || isThreadsafe == 0
> +	if (rc == SQLITE_OK) {
> +		struct session *user_session = current_session();
> +		int commit_internal = !(user_session->sql_flags
> +					& SQLITE_InternChanges);
> +
> +		assert(loc_db->init.busy == 0);
> +		loc_db->init.busy = 1;
> +		loc_db->pSchema = sqlite3SchemaCreate(loc_db);
> +		rc = sqlite3InitDatabase(loc_db);
> +		if (rc != SQLITE_OK)
> +			sqlite3SchemaClear(loc_db);
> +		loc_db->init.busy = 0;

6. Why do you do it both in rollback and here?
> 
> 

[tarantool-patches] Re: [PATCH 1/2] sql: refactor initialization routines

[Kirill Yukhin]

Hello Vlad,

I've fixed/commented your inputs. Could you pls take a look?

On 16 мар 23:32, v.shpilevoy@tarantool.org wrote:
> See below 6 comments.
> 
> > 15 марта 2018 г., в 21:21, Kirill Yukhin <kyukhin@tarantool.org> написал(а):
> > 
> > Make SQL load schema during initialization. It was
> > initialized on first query before the patch. Replace
> > check that schema is loaded w/ assert arouns the code base.
> > 
> > As far as DDL is prohibited inside the transaction, schema
> > cannot be changed inside a transaction and hence, no need to
> > reload it during rollback. No internal changes
> > (SQLITE_InernChanges) are possible.
> > 
> > Part of #3235
> > ---
> > src/box/sql.c           |  16 +---
> > src/box/sql/analyze.c   |   7 +-
> > src/box/sql/build.c     |  33 ++------
> > src/box/sql/main.c      | 204 +++++++++++++++---------------------------------
> > src/box/sql/pragma.c    |   3 +-
> > src/box/sql/prepare.c   |  53 -------------
> > src/box/sql/sqlite3.h   |   7 +-
> > src/box/sql/sqliteInt.h |   2 -
> > src/box/sql/trigger.c   |   5 +-
> > 9 files changed, 80 insertions(+), 250 deletions(-)
> 
> > diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
> > index 130e87665..38f48c5a0 100644
> > --- a/src/box/sql/analyze.c
> > +++ b/src/box/sql/analyze.c
> > @@ -1176,12 +1176,7 @@ sqlite3Analyze(Parse * pParse, Token * pName)
> > 	Table *pTab;
> > 	Vdbe *v;
> > 
> > -	/* Read the database schema. If an error occurs, leave an error message
> > -	 * and code in pParse and return NULL.
> > -	 */
> > -	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
> > -		return;
> > -	}
> > +	assert(db->pSchema);
> 
> 1. Please, use explicit "!= NULL".
Fixed.

> > diff --git a/src/box/sql/main.c b/src/box/sql/main.c
> > index 540570dc8..5a0f372b3 100644
> > --- a/src/box/sql/main.c
> > +++ b/src/box/sql/main.c
> > @@ -981,6 +981,15 @@ sqlite3RollbackAll(Vdbe * pVdbe, int tripCode)
> > 	    && db->init.busy == 0) {
> > 		sqlite3ExpirePreparedStatements(db);
> > 		sqlite3ResetAllSchemasOfConnection(db);
> > +
> > +		db->init.busy = 1;
> 
> 2. Why do you initialise DB in sqlite3RollbackAll? And do you really need to add place this code here, if in a next commit you remove it?
I hope we'll have bisect up and running. So, all patches are pass testing.
This is actual re-read in memory DD representation in case schema id (aka
cookie) was changed during transaction. This might happen if trigger was
added or deleted. Next patch blocks this explicitly.

> > @@ -2246,122 +2255,62 @@ sqlite3ParseUri(const char *zDefaultVfs,	/* VFS to use if no "vfs=xxx" query opt
> > 	return rc;
> > }
> > 
> > -/*
> > - * This routine does the work of opening a database on behalf of
> > - * sqlite3_open() and database filename "zFilename"
> > - * is UTF-8 encoded.
> > +/**
> > + * This routine does the work of initialization of main
> > + * SQL connection instance.
> > + *
> > + * @param[out] db returned database handle.
> > + * @return error status code.
> >  */
> > -static int
> > -openDatabase(const char *zFilename,	/* Database filename UTF-8 encoded */
> > -	     sqlite3 ** ppDb,		/* OUT: Returned database handle */
> > -	     unsigned int flags,	/* Operational flags */
> > -	     const char *zVfs)		/* Name of the VFS to use */
> > +int
> > +sql_init_db(sqlite3 **db)
> > {
> > -	sqlite3 *db;		/* Store allocated handle here */
> > +	sqlite3 *loc_db;
> 
> 3. If you call a parameter 'db' as 'out_db', and save the local name as 'db', then the diff becomes smaller. You can fix or not fix it at your convenience.
Fixed.

> > 
> > +	loc_db->pVfs = sqlite3_vfs_find(0);
> 
> 4. What is sqlite3_vfs_find? I do not see this in the original code.
This is OS callback, which should be removed in future. It is used for now
to get date/time from operating system. Hypotetically, there can be multiple
different OS connectors. It was called in openDatabase, passing zero as
backend requested.

> > +
> > +	assert(sizeof(loc_db->aLimit) == sizeof(aHardLimit));
> > +	memcpy(loc_db->aLimit, aHardLimit, sizeof(loc_db->aLimit));
> > +	loc_db->aLimit[SQLITE_LIMIT_WORKER_THREADS] = SQLITE_DEFAULT_WORKER_THREADS;
> 5. Please, try to fit code in 80 symbols.
Fixed w/ new name :)

> > @@ -2422,27 +2371,42 @@ openDatabase(const char *zFilename,	/* Database filename UTF-8 encoded */
> > 
> > -	if (db) {
> > -		assert(db->mutex != 0 || isThreadsafe == 0
> > +	if (rc == SQLITE_OK) {
> > +		struct session *user_session = current_session();
> > +		int commit_internal = !(user_session->sql_flags
> > +					& SQLITE_InternChanges);
> > +
> > +		assert(loc_db->init.busy == 0);
> > +		loc_db->init.busy = 1;
> > +		loc_db->pSchema = sqlite3SchemaCreate(loc_db);
> > +		rc = sqlite3InitDatabase(loc_db);
> > +		if (rc != SQLITE_OK)
> > +			sqlite3SchemaClear(loc_db);
> > +		loc_db->init.busy = 0;
> 
> 6. Why do you do it both in rollback and here?
Since this was part of sqlite4Init, which I was erased. Next patch
makes this copy paste single.

--
Kirill

[tarantool-patches] [PATCH 1/2] sql: Refactor initialization routines

[Kirill Yukhin]

Make SQL load schema during initialization. It was
initialized on first query before the patch. Replace
check that schema is loaded w/ assert arouns the code base.

As far as DDL is prohibited inside the transaction, schema
cannot be changed inside a transaction and hence, no need to
reload it during rollback. No internal changes
(SQLITE_InernChanges) are possible.

Part of #3235
---
 src/box/sql.c           |  16 +----
 src/box/sql/analyze.c   |   7 +--
 src/box/sql/build.c     |  33 ++---------
 src/box/sql/main.c      | 154 +++++++++++++-----------------------------------
 src/box/sql/pragma.c    |   3 +-
 src/box/sql/prepare.c   |  53 -----------------
 src/box/sql/sqlite3.h   |   7 +--
 src/box/sql/sqliteInt.h |   2 -
 src/box/sql/trigger.c   |   5 +-
 9 files changed, 55 insertions(+), 225 deletions(-)

diff --git a/src/box/sql.c b/src/box/sql.c
index 817926226..224747157 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -71,24 +71,12 @@ static const uint32_t default_sql_flags = SQLITE_ShortColNames
 void
 sql_init()
 {
-	int rc;
 	default_flags |= default_sql_flags;
 
-	rc = sqlite3_open("", &db);
-	if (rc != SQLITE_OK) {
-		panic("failed to initialize SQL subsystem");
-	} else {
-		/* XXX */
-	}
-
 	current_session()->sql_flags |= default_sql_flags;
 
-	rc = sqlite3Init(db);
-	if (rc != SQLITE_OK) {
-		panic("database initialization failed");
-	} else {
-		/* XXX */
-	}
+	if (sql_init_db(&db) != SQLITE_OK)
+		panic("failed to initialize SQL subsystem");
 
 	assert(db != NULL);
 }
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 130e87665..a77b7c758 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -1176,12 +1176,7 @@ sqlite3Analyze(Parse * pParse, Token * pName)
 	Table *pTab;
 	Vdbe *v;
 
-	/* Read the database schema. If an error occurs, leave an error message
-	 * and code in pParse and return NULL.
-	 */
-	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
-		return;
-	}
+	assert(db->pSchema != NULL);
 
 	if (pName == 0) {
 		/* Form 1:  Analyze everything */
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 5d2876d68..a719136cd 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -217,12 +217,7 @@ sqlite3LocateTable(Parse * pParse,	/* context in which to report errors */
 {
 	Table *p;
 
-	/* Read the database schema. If an error occurs, leave an error message
-	 * and code in pParse and return NULL.
-	 */
-	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
-		return 0;
-	}
+	assert(pParse->db->pSchema != NULL);
 
 	p = sqlite3FindTable(pParse->db, zName);
 	if (p == 0) {
@@ -647,13 +642,7 @@ sqlite3StartTable(Parse *pParse, Token *pName, int noErr)
 	if (zName == 0)
 		return;
 
-	/*
-	 * Make sure the new table name does not collide with an
-	 * existing index or table name in the same database.
-	 * Issue an error message if it does.
-	 */
-	if (SQLITE_OK != sqlite3ReadSchema(pParse))
-		goto begin_table_error;
+	assert(db->pSchema != NULL);
 	pTable = sqlite3FindTable(db, zName);
 	if (pTable) {
 		if (!noErr) {
@@ -2401,8 +2390,7 @@ sqlite3DropTable(Parse * pParse, SrcList * pName, int isView, int noErr)
 		sqlite3VdbeCountChanges(v);
 	assert(pParse->nErr == 0);
 	assert(pName->nSrc == 1);
-	if (sqlite3ReadSchema(pParse))
-		goto exit_drop_table;
+	assert(db->pSchema != NULL);
 	if (noErr)
 		db->suppressErr++;
 	assert(isView == 0 || isView == LOCATE_VIEW);
@@ -2878,9 +2866,7 @@ sqlite3CreateIndex(Parse * pParse,	/* All information about this parse */
 			goto exit_create_index;
 		sqlite3VdbeCountChanges(v);
 	}
-	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
-		goto exit_create_index;
-	}
+	assert(db->pSchema != NULL);
 
 	/*
 	 * Find the table that is to be indexed.  Return early if not found.
@@ -3389,9 +3375,7 @@ sqlite3DropIndex(Parse * pParse, SrcList * pName, Token * pName2, int ifExists)
 	if (!pParse->nested)
 		sqlite3VdbeCountChanges(v);
 	assert(pName->nSrc == 1);
-	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
-		goto exit_drop_index;
-	}
+	assert(db->pSchema != NULL);
 
 	assert(pName2->n > 0);
 	zTableName = sqlite3NameFromToken(db, pName2);
@@ -4151,12 +4135,7 @@ sqlite3Reindex(Parse * pParse, Token * pName1, Token * pName2)
 	Index *pIndex;		/* An index associated with pTab */
 	sqlite3 *db = pParse->db;	/* The database connection */
 
-	/* Read the database schema. If an error occurs, leave an error message
-	 * and code in pParse and return NULL.
-	 */
-	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
-		return;
-	}
+	assert(db->pSchema != NULL);
 
 	if (pName1 == 0) {
 		reindexDatabases(pParse, 0);
diff --git a/src/box/sql/main.c b/src/box/sql/main.c
index 540570dc8..4ae6df6dd 100644
--- a/src/box/sql/main.c
+++ b/src/box/sql/main.c
@@ -981,6 +981,15 @@ sqlite3RollbackAll(Vdbe * pVdbe, int tripCode)
 	    && db->init.busy == 0) {
 		sqlite3ExpirePreparedStatements(db);
 		sqlite3ResetAllSchemasOfConnection(db);
+
+		db->init.busy = 1;
+		db->pSchema = sqlite3SchemaCreate(db);
+		int rc = sqlite3InitDatabase(db);
+		if (rc != SQLITE_OK)
+			sqlite3SchemaClear(db);
+		db->init.busy = 0;
+		if (rc == SQLITE_OK)
+			sqlite3CommitInternalChanges();
 	}
 
 	/* Any deferred constraint violations have now been resolved. */
@@ -2246,85 +2255,35 @@ sqlite3ParseUri(const char *zDefaultVfs,	/* VFS to use if no "vfs=xxx" query opt
 	return rc;
 }
 
-/*
- * This routine does the work of opening a database on behalf of
- * sqlite3_open() and database filename "zFilename"
- * is UTF-8 encoded.
+/**
+ * This routine does the work of initialization of main
+ * SQL connection instance.
+ *
+ * @param[out] out_db returned database handle.
+ * @return error status code.
  */
-static int
-openDatabase(const char *zFilename,	/* Database filename UTF-8 encoded */
-	     sqlite3 ** ppDb,		/* OUT: Returned database handle */
-	     unsigned int flags,	/* Operational flags */
-	     const char *zVfs)		/* Name of the VFS to use */
+int
+sql_init_db(sqlite3 **out_db)
 {
-	sqlite3 *db;		/* Store allocated handle here */
+	sqlite3 *db;
 	int rc;			/* Return code */
 	int isThreadsafe;	/* True for threadsafe connections */
-	char *zOpen = 0;	/* Filename argument to pass to BtreeOpen() */
-	char *zErrMsg = 0;	/* Error message from sqlite3ParseUri() */
 
 #ifdef SQLITE_ENABLE_API_ARMOR
 	if (ppDb == 0)
 		return SQLITE_MISUSE_BKPT;
 #endif
-	*ppDb = 0;
 #ifndef SQLITE_OMIT_AUTOINIT
 	rc = sqlite3_initialize();
 	if (rc)
 		return rc;
 #endif
 
-	/* Only allow sensible combinations of bits in the flags argument.
-	 * Throw an error if any non-sense combination is used.  If we
-	 * do not block illegal combinations here, it could trigger
-	 * assert() statements in deeper layers.  Sensible combinations
-	 * are:
-	 *
-	 *  1:  SQLITE_OPEN_READONLY
-	 *  2:  SQLITE_OPEN_READWRITE
-	 *  6:  SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE
-	 */
-	assert(SQLITE_OPEN_READONLY == 0x01);
-	assert(SQLITE_OPEN_READWRITE == 0x02);
-	assert(SQLITE_OPEN_CREATE == 0x04);
-	testcase((1 << (flags & 7)) == 0x02);	/* READONLY */
-	testcase((1 << (flags & 7)) == 0x04);	/* READWRITE */
-	testcase((1 << (flags & 7)) == 0x40);	/* READWRITE | CREATE */
-	if (((1 << (flags & 7)) & 0x46) == 0) {
-		return SQLITE_MISUSE_BKPT;	/* IMP: R-65497-44594 */
-	}
-
 	if (sqlite3GlobalConfig.bCoreMutex == 0) {
 		isThreadsafe = 0;
-	} else if (flags & SQLITE_OPEN_NOMUTEX) {
-		isThreadsafe = 0;
-	} else if (flags & SQLITE_OPEN_FULLMUTEX) {
-		isThreadsafe = 1;
 	} else {
 		isThreadsafe = sqlite3GlobalConfig.bFullMutex;
 	}
-	if (flags & SQLITE_OPEN_PRIVATECACHE) {
-		flags &= ~SQLITE_OPEN_SHAREDCACHE;
-	} else if (sqlite3GlobalConfig.sharedCacheEnabled) {
-		flags |= SQLITE_OPEN_SHAREDCACHE;
-	}
-	/* Remove harmful bits from the flags parameter
-	 *
-	 * The SQLITE_OPEN_NOMUTEX and SQLITE_OPEN_FULLMUTEX flags were
-	 * dealt with in the previous code block. Besides these, the only
-	 * valid input flags for sqlite3_open_v2() are SQLITE_OPEN_READONLY,
-	 * SQLITE_OPEN_READWRITE, SQLITE_OPEN_CREATE, SQLITE_OPEN_SHAREDCACHE,
-	 * SQLITE_OPEN_PRIVATECACHE, and some reserved bits. Silently mask
-	 * off all other flags.
-	 */
-	flags &= ~(SQLITE_OPEN_DELETEONCLOSE |
-		   SQLITE_OPEN_EXCLUSIVE |
-		   SQLITE_OPEN_MAIN_DB |
-		   SQLITE_OPEN_TEMP_DB |
-		   SQLITE_OPEN_TRANSIENT_DB |
-		   SQLITE_OPEN_NOMUTEX |
-		   SQLITE_OPEN_FULLMUTEX);
-	flags |= SQLITE_OPEN_MEMORY;
 	/* Allocate the sqlite data structure */
 	db = sqlite3MallocZero(sizeof(sqlite3));
 	if (db == 0)
@@ -2341,24 +2300,14 @@ openDatabase(const char *zFilename,	/* Database filename UTF-8 encoded */
 	db->errMask = 0xff;
 	db->magic = SQLITE_MAGIC_BUSY;
 
+	db->pVfs = sqlite3_vfs_find(0);
+
 	assert(sizeof(db->aLimit) == sizeof(aHardLimit));
 	memcpy(db->aLimit, aHardLimit, sizeof(db->aLimit));
 	db->aLimit[SQLITE_LIMIT_WORKER_THREADS] = SQLITE_DEFAULT_WORKER_THREADS;
 	db->szMmap = sqlite3GlobalConfig.szMmap;
 	db->nMaxSorterMmap = 0x7FFFFFFF;
 
-	db->openFlags = flags;
-	/* Parse the filename/URI argument. */
-	rc = sqlite3ParseUri(zVfs, zFilename,
-			     &flags, &db->pVfs, &zOpen, &zErrMsg);
-	if (rc != SQLITE_OK) {
-		if (rc == SQLITE_NOMEM)
-			sqlite3OomFault(db);
-		sqlite3ErrorWithMsg(db, rc, zErrMsg ? "%s" : 0, zErrMsg);
-		sqlite3_free(zErrMsg);
-		goto opendb_out;
-	}
-
 	db->pSchema = NULL;
 	db->magic = SQLITE_MAGIC_OPEN;
 	if (db->mallocFailed) {
@@ -2428,7 +2377,22 @@ openDatabase(const char *zFilename,	/* Database filename UTF-8 encoded */
 	setupLookaside(db, 0, sqlite3GlobalConfig.szLookaside,
 		       sqlite3GlobalConfig.nLookaside);
 
- opendb_out:
+	if (rc == SQLITE_OK) {
+		struct session *user_session = current_session();
+		int commit_internal = !(user_session->sql_flags
+					& SQLITE_InternChanges);
+
+		assert(db->init.busy == 0);
+		db->init.busy = 1;
+		db->pSchema = sqlite3SchemaCreate(db);
+		rc = sqlite3InitDatabase(db);
+		if (rc != SQLITE_OK)
+			sqlite3SchemaClear(db);
+		db->init.busy = 0;
+		if (rc == SQLITE_OK && commit_internal)
+			sqlite3CommitInternalChanges();
+	}
+opendb_out:
 	if (db) {
 		assert(db->mutex != 0 || isThreadsafe == 0
 		       || sqlite3GlobalConfig.bFullMutex == 0);
@@ -2439,10 +2403,10 @@ openDatabase(const char *zFilename,	/* Database filename UTF-8 encoded */
 	if (rc == SQLITE_NOMEM) {
 		sqlite3_close(db);
 		db = 0;
-	} else if (rc != SQLITE_OK) {
+	} else if (rc != SQLITE_OK)
 		db->magic = SQLITE_MAGIC_SICK;
-	}
-	*ppDb = db;
+
+	*out_db = db;
 #ifdef SQLITE_ENABLE_SQLLOG
 	if (sqlite3GlobalConfig.xSqllog) {
 		/* Opening a db handle. Fourth parameter is passed 0. */
@@ -2450,46 +2414,8 @@ openDatabase(const char *zFilename,	/* Database filename UTF-8 encoded */
 		sqlite3GlobalConfig.xSqllog(pArg, db, zFilename, 0);
 	}
 #endif
-#if defined(SQLITE_HAS_CODEC)
-	if (rc == SQLITE_OK) {
-		const char *zHexKey = sqlite3_uri_parameter(zOpen, "hexkey");
-		if (zHexKey && zHexKey[0]) {
-			u8 iByte;
-			int i;
-			char zKey[40];
-			for (i = 0, iByte = 0;
-			     i < sizeof(zKey) * 2
-			     && sqlite3Isxdigit(zHexKey[i]); i++) {
-				iByte =
-				    (iByte << 4) + sqlite3HexToInt(zHexKey[i]);
-				if ((i & 1) != 0)
-					zKey[i / 2] = iByte;
-			}
-			sqlite3_key_v2(db, 0, zKey, i / 2);
-		}
-	}
-#endif
-	sqlite3_free(zOpen);
-	return rc & 0xff;
-}
 
-/*
- * Open a new database handle.
- */
-int
-sqlite3_open(const char *zFilename, sqlite3 ** ppDb)
-{
-	return openDatabase(zFilename, ppDb,
-			    SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, 0);
-}
-
-int
-sqlite3_open_v2(const char *filename,	/* Database filename (UTF-8) */
-		sqlite3 ** ppDb,	/* OUT: SQLite db handle */
-		int flags,		/* Flags */
-		const char *zVfs)	/* Name of VFS module to use */
-{
-	return openDatabase(filename, ppDb, (unsigned int)flags, zVfs);
+	return rc;
 }
 
 /*
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index aa135c4ce..4db553bcf 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -292,8 +292,7 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 
 	/* Make sure the database schema is loaded if the pragma requires that */
 	if ((pPragma->mPragFlg & PragFlg_NeedSchema) != 0) {
-		if (sqlite3ReadSchema(pParse))
-			goto pragma_out;
+		assert(db->pSchema != NULL);
 	}
 	/* Register the result column names for pragmas that return results */
 	if ((pPragma->mPragFlg & PragFlg_NoColumns) == 0
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index 85d578eb6..9b8028277 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -207,59 +207,6 @@ sqlite3InitDatabase(sqlite3 * db)
 }
 
 /*
- * Initialize all database files - the main database file
- * Return a success code.
- * After a database is initialized, the pSchema field in database
- * structure will be not NULL.
- */
-int
-sqlite3Init(sqlite3 * db)
-{
-	int rc;
-	struct session *user_session = current_session();
-	int commit_internal = !(user_session->sql_flags & SQLITE_InternChanges);
-
-	assert(sqlite3_mutex_held(db->mutex));
-	assert(db->init.busy == 0);
-	rc = SQLITE_OK;
-	db->init.busy = 1;
-	if (!db->pSchema) {
-		db->pSchema = sqlite3SchemaCreate(db);
-		rc = sqlite3InitDatabase(db);
-		if (rc) {
-			sqlite3SchemaClear(db);
-		}
-	}
-
-	db->init.busy = 0;
-	if (rc == SQLITE_OK && commit_internal) {
-		sqlite3CommitInternalChanges();
-	}
-
-	return rc;
-}
-
-/*
- * This routine is a no-op if the database schema is already initialized.
- * Otherwise, the schema is loaded. An error code is returned.
- */
-int
-sqlite3ReadSchema(Parse * pParse)
-{
-	int rc = SQLITE_OK;
-	sqlite3 *db = pParse->db;
-	assert(sqlite3_mutex_held(db->mutex));
-	if (!db->init.busy) {
-		rc = sqlite3Init(db);
-	}
-	if (rc != SQLITE_OK) {
-		pParse->rc = rc;
-		pParse->nErr++;
-	}
-	return rc;
-}
-
-/*
  * Convert a schema pointer into the 0 index that indicates
  * that schema refers to a single database.
  * This method is inherited from SQLite, which has several dbs.
diff --git a/src/box/sql/sqlite3.h b/src/box/sql/sqlite3.h
index 73e916991..d60d74d0e 100644
--- a/src/box/sql/sqlite3.h
+++ b/src/box/sql/sqlite3.h
@@ -2869,10 +2869,9 @@ sqlite3_progress_handler(sqlite3 *, int,
  *
  * See also: [sqlite3_temp_directory]
 */
-SQLITE_API int
-sqlite3_open(const char *filename,	/* Database filename (UTF-8) */
-	     sqlite3 ** ppDb	/* OUT: SQLite db handle */
-	);
+
+int
+sql_init_db(sqlite3 **db);
 
 SQLITE_API int
 sqlite3_open16(const void *filename,	/* Database filename (UTF-16) */
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index de2b47bfc..cda862d68 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -960,7 +960,6 @@ struct sqlite3 {
 	sqlite3_mutex *mutex;	/* Connection mutex */
 	struct Schema *pSchema; /* Schema of the database */
 	i64 szMmap;		/* Default mmap_size setting */
-	unsigned int openFlags;	/* Flags passed to sqlite3_vfs.xOpen() */
 	int errCode;		/* Most recent error code (SQLITE_*) */
 	int errMask;		/* & result codes with this before returning */
 	int iSysErrno;		/* Errno value from last system error */
@@ -3259,7 +3258,6 @@ const char *sqlite3ErrName(int);
 #endif
 
 const char *sqlite3ErrStr(int);
-int sqlite3ReadSchema(Parse * pParse);
 struct coll *sqlite3FindCollSeq(const char *);
 struct coll *sqlite3LocateCollSeq(Parse * pParse, sqlite3 * db, const char *zName);
 struct coll *sqlite3ExprCollSeq(Parse * pParse, Expr * pExpr);
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index a2827c882..08963cb79 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -507,9 +507,8 @@ sqlite3DropTrigger(Parse * pParse, SrcList * pName, int noErr)
 
 	if (db->mallocFailed)
 		goto drop_trigger_cleanup;
-	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
-		goto drop_trigger_cleanup;
-	}
+	assert(db->pSchema != NULL);
+
 	/* Do not account nested operations: the count of such
 	 * operations depends on Tarantool data dictionary internals,
 	 * such as data layout in system spaces. Activate the counter
-- 
2.11.0

[tarantool-patches] Re: [PATCH 1/2] sql: Refactor initialization routines

[v.shpilevoy]

Ack.

> 20 марта 2018 г., в 11:35, Kirill Yukhin <kyukhin@tarantool.org> написал(а):
> 
> Make SQL load schema during initialization. It was
> initialized on first query before the patch. Replace
> check that schema is loaded w/ assert arouns the code base.
> 
> As far as DDL is prohibited inside the transaction, schema
> cannot be changed inside a transaction and hence, no need to
> reload it during rollback. No internal changes
> (SQLITE_InernChanges) are possible.
> 
> Part of #3235
> ---
> src/box/sql.c           |  16 +----
> src/box/sql/analyze.c   |   7 +--
> src/box/sql/build.c     |  33 ++---------
> src/box/sql/main.c      | 154 +++++++++++++-----------------------------------
> src/box/sql/pragma.c    |   3 +-
> src/box/sql/prepare.c   |  53 -----------------
> src/box/sql/sqlite3.h   |   7 +--
> src/box/sql/sqliteInt.h |   2 -
> src/box/sql/trigger.c   |   5 +-
> 9 files changed, 55 insertions(+), 225 deletions(-)
> 
> diff --git a/src/box/sql.c b/src/box/sql.c
> index 817926226..224747157 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c
> @@ -71,24 +71,12 @@ static const uint32_t default_sql_flags = SQLITE_ShortColNames
> void
> sql_init()
> {
> -	int rc;
> 	default_flags |= default_sql_flags;
> 
> -	rc = sqlite3_open("", &db);
> -	if (rc != SQLITE_OK) {
> -		panic("failed to initialize SQL subsystem");
> -	} else {
> -		/* XXX */
> -	}
> -
> 	current_session()->sql_flags |= default_sql_flags;
> 
> -	rc = sqlite3Init(db);
> -	if (rc != SQLITE_OK) {
> -		panic("database initialization failed");
> -	} else {
> -		/* XXX */
> -	}
> +	if (sql_init_db(&db) != SQLITE_OK)
> +		panic("failed to initialize SQL subsystem");
> 
> 	assert(db != NULL);
> }
> diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
> index 130e87665..a77b7c758 100644
> --- a/src/box/sql/analyze.c
> +++ b/src/box/sql/analyze.c
> @@ -1176,12 +1176,7 @@ sqlite3Analyze(Parse * pParse, Token * pName)
> 	Table *pTab;
> 	Vdbe *v;
> 
> -	/* Read the database schema. If an error occurs, leave an error message
> -	 * and code in pParse and return NULL.
> -	 */
> -	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
> -		return;
> -	}
> +	assert(db->pSchema != NULL);
> 
> 	if (pName == 0) {
> 		/* Form 1:  Analyze everything */
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index 5d2876d68..a719136cd 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -217,12 +217,7 @@ sqlite3LocateTable(Parse * pParse,	/* context in which to report errors */
> {
> 	Table *p;
> 
> -	/* Read the database schema. If an error occurs, leave an error message
> -	 * and code in pParse and return NULL.
> -	 */
> -	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
> -		return 0;
> -	}
> +	assert(pParse->db->pSchema != NULL);
> 
> 	p = sqlite3FindTable(pParse->db, zName);
> 	if (p == 0) {
> @@ -647,13 +642,7 @@ sqlite3StartTable(Parse *pParse, Token *pName, int noErr)
> 	if (zName == 0)
> 		return;
> 
> -	/*
> -	 * Make sure the new table name does not collide with an
> -	 * existing index or table name in the same database.
> -	 * Issue an error message if it does.
> -	 */
> -	if (SQLITE_OK != sqlite3ReadSchema(pParse))
> -		goto begin_table_error;
> +	assert(db->pSchema != NULL);
> 	pTable = sqlite3FindTable(db, zName);
> 	if (pTable) {
> 		if (!noErr) {
> @@ -2401,8 +2390,7 @@ sqlite3DropTable(Parse * pParse, SrcList * pName, int isView, int noErr)
> 		sqlite3VdbeCountChanges(v);
> 	assert(pParse->nErr == 0);
> 	assert(pName->nSrc == 1);
> -	if (sqlite3ReadSchema(pParse))
> -		goto exit_drop_table;
> +	assert(db->pSchema != NULL);
> 	if (noErr)
> 		db->suppressErr++;
> 	assert(isView == 0 || isView == LOCATE_VIEW);
> @@ -2878,9 +2866,7 @@ sqlite3CreateIndex(Parse * pParse,	/* All information about this parse */
> 			goto exit_create_index;
> 		sqlite3VdbeCountChanges(v);
> 	}
> -	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
> -		goto exit_create_index;
> -	}
> +	assert(db->pSchema != NULL);
> 
> 	/*
> 	 * Find the table that is to be indexed.  Return early if not found.
> @@ -3389,9 +3375,7 @@ sqlite3DropIndex(Parse * pParse, SrcList * pName, Token * pName2, int ifExists)
> 	if (!pParse->nested)
> 		sqlite3VdbeCountChanges(v);
> 	assert(pName->nSrc == 1);
> -	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
> -		goto exit_drop_index;
> -	}
> +	assert(db->pSchema != NULL);
> 
> 	assert(pName2->n > 0);
> 	zTableName = sqlite3NameFromToken(db, pName2);
> @@ -4151,12 +4135,7 @@ sqlite3Reindex(Parse * pParse, Token * pName1, Token * pName2)
> 	Index *pIndex;		/* An index associated with pTab */
> 	sqlite3 *db = pParse->db;	/* The database connection */
> 
> -	/* Read the database schema. If an error occurs, leave an error message
> -	 * and code in pParse and return NULL.
> -	 */
> -	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
> -		return;
> -	}
> +	assert(db->pSchema != NULL);
> 
> 	if (pName1 == 0) {
> 		reindexDatabases(pParse, 0);
> diff --git a/src/box/sql/main.c b/src/box/sql/main.c
> index 540570dc8..4ae6df6dd 100644
> --- a/src/box/sql/main.c
> +++ b/src/box/sql/main.c
> @@ -981,6 +981,15 @@ sqlite3RollbackAll(Vdbe * pVdbe, int tripCode)
> 	    && db->init.busy == 0) {
> 		sqlite3ExpirePreparedStatements(db);
> 		sqlite3ResetAllSchemasOfConnection(db);
> +
> +		db->init.busy = 1;
> +		db->pSchema = sqlite3SchemaCreate(db);
> +		int rc = sqlite3InitDatabase(db);
> +		if (rc != SQLITE_OK)
> +			sqlite3SchemaClear(db);
> +		db->init.busy = 0;
> +		if (rc == SQLITE_OK)
> +			sqlite3CommitInternalChanges();
> 	}
> 
> 	/* Any deferred constraint violations have now been resolved. */
> @@ -2246,85 +2255,35 @@ sqlite3ParseUri(const char *zDefaultVfs,	/* VFS to use if no "vfs=xxx" query opt
> 	return rc;
> }
> 
> -/*
> - * This routine does the work of opening a database on behalf of
> - * sqlite3_open() and database filename "zFilename"
> - * is UTF-8 encoded.
> +/**
> + * This routine does the work of initialization of main
> + * SQL connection instance.
> + *
> + * @param[out] out_db returned database handle.
> + * @return error status code.
>  */
> -static int
> -openDatabase(const char *zFilename,	/* Database filename UTF-8 encoded */
> -	     sqlite3 ** ppDb,		/* OUT: Returned database handle */
> -	     unsigned int flags,	/* Operational flags */
> -	     const char *zVfs)		/* Name of the VFS to use */
> +int
> +sql_init_db(sqlite3 **out_db)
> {
> -	sqlite3 *db;		/* Store allocated handle here */
> +	sqlite3 *db;
> 	int rc;			/* Return code */
> 	int isThreadsafe;	/* True for threadsafe connections */
> -	char *zOpen = 0;	/* Filename argument to pass to BtreeOpen() */
> -	char *zErrMsg = 0;	/* Error message from sqlite3ParseUri() */
> 
> #ifdef SQLITE_ENABLE_API_ARMOR
> 	if (ppDb == 0)
> 		return SQLITE_MISUSE_BKPT;
> #endif
> -	*ppDb = 0;
> #ifndef SQLITE_OMIT_AUTOINIT
> 	rc = sqlite3_initialize();
> 	if (rc)
> 		return rc;
> #endif
> 
> -	/* Only allow sensible combinations of bits in the flags argument.
> -	 * Throw an error if any non-sense combination is used.  If we
> -	 * do not block illegal combinations here, it could trigger
> -	 * assert() statements in deeper layers.  Sensible combinations
> -	 * are:
> -	 *
> -	 *  1:  SQLITE_OPEN_READONLY
> -	 *  2:  SQLITE_OPEN_READWRITE
> -	 *  6:  SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE
> -	 */
> -	assert(SQLITE_OPEN_READONLY == 0x01);
> -	assert(SQLITE_OPEN_READWRITE == 0x02);
> -	assert(SQLITE_OPEN_CREATE == 0x04);
> -	testcase((1 << (flags & 7)) == 0x02);	/* READONLY */
> -	testcase((1 << (flags & 7)) == 0x04);	/* READWRITE */
> -	testcase((1 << (flags & 7)) == 0x40);	/* READWRITE | CREATE */
> -	if (((1 << (flags & 7)) & 0x46) == 0) {
> -		return SQLITE_MISUSE_BKPT;	/* IMP: R-65497-44594 */
> -	}
> -
> 	if (sqlite3GlobalConfig.bCoreMutex == 0) {
> 		isThreadsafe = 0;
> -	} else if (flags & SQLITE_OPEN_NOMUTEX) {
> -		isThreadsafe = 0;
> -	} else if (flags & SQLITE_OPEN_FULLMUTEX) {
> -		isThreadsafe = 1;
> 	} else {
> 		isThreadsafe = sqlite3GlobalConfig.bFullMutex;
> 	}
> -	if (flags & SQLITE_OPEN_PRIVATECACHE) {
> -		flags &= ~SQLITE_OPEN_SHAREDCACHE;
> -	} else if (sqlite3GlobalConfig.sharedCacheEnabled) {
> -		flags |= SQLITE_OPEN_SHAREDCACHE;
> -	}
> -	/* Remove harmful bits from the flags parameter
> -	 *
> -	 * The SQLITE_OPEN_NOMUTEX and SQLITE_OPEN_FULLMUTEX flags were
> -	 * dealt with in the previous code block. Besides these, the only
> -	 * valid input flags for sqlite3_open_v2() are SQLITE_OPEN_READONLY,
> -	 * SQLITE_OPEN_READWRITE, SQLITE_OPEN_CREATE, SQLITE_OPEN_SHAREDCACHE,
> -	 * SQLITE_OPEN_PRIVATECACHE, and some reserved bits. Silently mask
> -	 * off all other flags.
> -	 */
> -	flags &= ~(SQLITE_OPEN_DELETEONCLOSE |
> -		   SQLITE_OPEN_EXCLUSIVE |
> -		   SQLITE_OPEN_MAIN_DB |
> -		   SQLITE_OPEN_TEMP_DB |
> -		   SQLITE_OPEN_TRANSIENT_DB |
> -		   SQLITE_OPEN_NOMUTEX |
> -		   SQLITE_OPEN_FULLMUTEX);
> -	flags |= SQLITE_OPEN_MEMORY;
> 	/* Allocate the sqlite data structure */
> 	db = sqlite3MallocZero(sizeof(sqlite3));
> 	if (db == 0)
> @@ -2341,24 +2300,14 @@ openDatabase(const char *zFilename,	/* Database filename UTF-8 encoded */
> 	db->errMask = 0xff;
> 	db->magic = SQLITE_MAGIC_BUSY;
> 
> +	db->pVfs = sqlite3_vfs_find(0);
> +
> 	assert(sizeof(db->aLimit) == sizeof(aHardLimit));
> 	memcpy(db->aLimit, aHardLimit, sizeof(db->aLimit));
> 	db->aLimit[SQLITE_LIMIT_WORKER_THREADS] = SQLITE_DEFAULT_WORKER_THREADS;
> 	db->szMmap = sqlite3GlobalConfig.szMmap;
> 	db->nMaxSorterMmap = 0x7FFFFFFF;
> 
> -	db->openFlags = flags;
> -	/* Parse the filename/URI argument. */
> -	rc = sqlite3ParseUri(zVfs, zFilename,
> -			     &flags, &db->pVfs, &zOpen, &zErrMsg);
> -	if (rc != SQLITE_OK) {
> -		if (rc == SQLITE_NOMEM)
> -			sqlite3OomFault(db);
> -		sqlite3ErrorWithMsg(db, rc, zErrMsg ? "%s" : 0, zErrMsg);
> -		sqlite3_free(zErrMsg);
> -		goto opendb_out;
> -	}
> -
> 	db->pSchema = NULL;
> 	db->magic = SQLITE_MAGIC_OPEN;
> 	if (db->mallocFailed) {
> @@ -2428,7 +2377,22 @@ openDatabase(const char *zFilename,	/* Database filename UTF-8 encoded */
> 	setupLookaside(db, 0, sqlite3GlobalConfig.szLookaside,
> 		       sqlite3GlobalConfig.nLookaside);
> 
> - opendb_out:
> +	if (rc == SQLITE_OK) {
> +		struct session *user_session = current_session();
> +		int commit_internal = !(user_session->sql_flags
> +					& SQLITE_InternChanges);
> +
> +		assert(db->init.busy == 0);
> +		db->init.busy = 1;
> +		db->pSchema = sqlite3SchemaCreate(db);
> +		rc = sqlite3InitDatabase(db);
> +		if (rc != SQLITE_OK)
> +			sqlite3SchemaClear(db);
> +		db->init.busy = 0;
> +		if (rc == SQLITE_OK && commit_internal)
> +			sqlite3CommitInternalChanges();
> +	}
> +opendb_out:
> 	if (db) {
> 		assert(db->mutex != 0 || isThreadsafe == 0
> 		       || sqlite3GlobalConfig.bFullMutex == 0);
> @@ -2439,10 +2403,10 @@ openDatabase(const char *zFilename,	/* Database filename UTF-8 encoded */
> 	if (rc == SQLITE_NOMEM) {
> 		sqlite3_close(db);
> 		db = 0;
> -	} else if (rc != SQLITE_OK) {
> +	} else if (rc != SQLITE_OK)
> 		db->magic = SQLITE_MAGIC_SICK;
> -	}
> -	*ppDb = db;
> +
> +	*out_db = db;
> #ifdef SQLITE_ENABLE_SQLLOG
> 	if (sqlite3GlobalConfig.xSqllog) {
> 		/* Opening a db handle. Fourth parameter is passed 0. */
> @@ -2450,46 +2414,8 @@ openDatabase(const char *zFilename,	/* Database filename UTF-8 encoded */
> 		sqlite3GlobalConfig.xSqllog(pArg, db, zFilename, 0);
> 	}
> #endif
> -#if defined(SQLITE_HAS_CODEC)
> -	if (rc == SQLITE_OK) {
> -		const char *zHexKey = sqlite3_uri_parameter(zOpen, "hexkey");
> -		if (zHexKey && zHexKey[0]) {
> -			u8 iByte;
> -			int i;
> -			char zKey[40];
> -			for (i = 0, iByte = 0;
> -			     i < sizeof(zKey) * 2
> -			     && sqlite3Isxdigit(zHexKey[i]); i++) {
> -				iByte =
> -				    (iByte << 4) + sqlite3HexToInt(zHexKey[i]);
> -				if ((i & 1) != 0)
> -					zKey[i / 2] = iByte;
> -			}
> -			sqlite3_key_v2(db, 0, zKey, i / 2);
> -		}
> -	}
> -#endif
> -	sqlite3_free(zOpen);
> -	return rc & 0xff;
> -}
> 
> -/*
> - * Open a new database handle.
> - */
> -int
> -sqlite3_open(const char *zFilename, sqlite3 ** ppDb)
> -{
> -	return openDatabase(zFilename, ppDb,
> -			    SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, 0);
> -}
> -
> -int
> -sqlite3_open_v2(const char *filename,	/* Database filename (UTF-8) */
> -		sqlite3 ** ppDb,	/* OUT: SQLite db handle */
> -		int flags,		/* Flags */
> -		const char *zVfs)	/* Name of VFS module to use */
> -{
> -	return openDatabase(filename, ppDb, (unsigned int)flags, zVfs);
> +	return rc;
> }
> 
> /*
> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
> index aa135c4ce..4db553bcf 100644
> --- a/src/box/sql/pragma.c
> +++ b/src/box/sql/pragma.c
> @@ -292,8 +292,7 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
> 
> 	/* Make sure the database schema is loaded if the pragma requires that */
> 	if ((pPragma->mPragFlg & PragFlg_NeedSchema) != 0) {
> -		if (sqlite3ReadSchema(pParse))
> -			goto pragma_out;
> +		assert(db->pSchema != NULL);
> 	}
> 	/* Register the result column names for pragmas that return results */
> 	if ((pPragma->mPragFlg & PragFlg_NoColumns) == 0
> diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
> index 85d578eb6..9b8028277 100644
> --- a/src/box/sql/prepare.c
> +++ b/src/box/sql/prepare.c
> @@ -207,59 +207,6 @@ sqlite3InitDatabase(sqlite3 * db)
> }
> 
> /*
> - * Initialize all database files - the main database file
> - * Return a success code.
> - * After a database is initialized, the pSchema field in database
> - * structure will be not NULL.
> - */
> -int
> -sqlite3Init(sqlite3 * db)
> -{
> -	int rc;
> -	struct session *user_session = current_session();
> -	int commit_internal = !(user_session->sql_flags & SQLITE_InternChanges);
> -
> -	assert(sqlite3_mutex_held(db->mutex));
> -	assert(db->init.busy == 0);
> -	rc = SQLITE_OK;
> -	db->init.busy = 1;
> -	if (!db->pSchema) {
> -		db->pSchema = sqlite3SchemaCreate(db);
> -		rc = sqlite3InitDatabase(db);
> -		if (rc) {
> -			sqlite3SchemaClear(db);
> -		}
> -	}
> -
> -	db->init.busy = 0;
> -	if (rc == SQLITE_OK && commit_internal) {
> -		sqlite3CommitInternalChanges();
> -	}
> -
> -	return rc;
> -}
> -
> -/*
> - * This routine is a no-op if the database schema is already initialized.
> - * Otherwise, the schema is loaded. An error code is returned.
> - */
> -int
> -sqlite3ReadSchema(Parse * pParse)
> -{
> -	int rc = SQLITE_OK;
> -	sqlite3 *db = pParse->db;
> -	assert(sqlite3_mutex_held(db->mutex));
> -	if (!db->init.busy) {
> -		rc = sqlite3Init(db);
> -	}
> -	if (rc != SQLITE_OK) {
> -		pParse->rc = rc;
> -		pParse->nErr++;
> -	}
> -	return rc;
> -}
> -
> -/*
>  * Convert a schema pointer into the 0 index that indicates
>  * that schema refers to a single database.
>  * This method is inherited from SQLite, which has several dbs.
> diff --git a/src/box/sql/sqlite3.h b/src/box/sql/sqlite3.h
> index 73e916991..d60d74d0e 100644
> --- a/src/box/sql/sqlite3.h
> +++ b/src/box/sql/sqlite3.h
> @@ -2869,10 +2869,9 @@ sqlite3_progress_handler(sqlite3 *, int,
>  *
>  * See also: [sqlite3_temp_directory]
> */
> -SQLITE_API int
> -sqlite3_open(const char *filename,	/* Database filename (UTF-8) */
> -	     sqlite3 ** ppDb	/* OUT: SQLite db handle */
> -	);
> +
> +int
> +sql_init_db(sqlite3 **db);
> 
> SQLITE_API int
> sqlite3_open16(const void *filename,	/* Database filename (UTF-16) */
> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
> index de2b47bfc..cda862d68 100644
> --- a/src/box/sql/sqliteInt.h
> +++ b/src/box/sql/sqliteInt.h
> @@ -960,7 +960,6 @@ struct sqlite3 {
> 	sqlite3_mutex *mutex;	/* Connection mutex */
> 	struct Schema *pSchema; /* Schema of the database */
> 	i64 szMmap;		/* Default mmap_size setting */
> -	unsigned int openFlags;	/* Flags passed to sqlite3_vfs.xOpen() */
> 	int errCode;		/* Most recent error code (SQLITE_*) */
> 	int errMask;		/* & result codes with this before returning */
> 	int iSysErrno;		/* Errno value from last system error */
> @@ -3259,7 +3258,6 @@ const char *sqlite3ErrName(int);
> #endif
> 
> const char *sqlite3ErrStr(int);
> -int sqlite3ReadSchema(Parse * pParse);
> struct coll *sqlite3FindCollSeq(const char *);
> struct coll *sqlite3LocateCollSeq(Parse * pParse, sqlite3 * db, const char *zName);
> struct coll *sqlite3ExprCollSeq(Parse * pParse, Expr * pExpr);
> diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
> index a2827c882..08963cb79 100644
> --- a/src/box/sql/trigger.c
> +++ b/src/box/sql/trigger.c
> @@ -507,9 +507,8 @@ sqlite3DropTrigger(Parse * pParse, SrcList * pName, int noErr)
> 
> 	if (db->mallocFailed)
> 		goto drop_trigger_cleanup;
> -	if (SQLITE_OK != sqlite3ReadSchema(pParse)) {
> -		goto drop_trigger_cleanup;
> -	}
> +	assert(db->pSchema != NULL);
> +
> 	/* Do not account nested operations: the count of such
> 	 * operations depends on Tarantool data dictionary internals,
> 	 * such as data layout in system spaces. Activate the counter
> -- 
> 2.11.0
> 
> 

[tarantool-patches] [PATCH 2/2] sql: block trigger creation inside a transaction

[Kirill Yukhin]

Since DDL is prohibited inside a transaction, this patch
prohibits trigger creation and drop inside a transaction.
This was done by adding dummy trigger on replace into
_trigger space. This trigger only check if this not multistatement
transaction. Also, make assert that schema cokie wasn't
changed during transaction rollback if any.

Testsuite updated as well.

Closes #3239
---
 src/box/alter.cc               |  15 +++++
 src/box/alter.h                |   1 +
 src/box/schema.cc              |   2 +-
 src/box/sql/build.c            |   9 ++-
 src/box/sql/main.c             |  17 +----
 src/box/sql/parse.c            | 147 +++++++++++++++++++++--------------------
 src/box/sql/parse.y            |   2 +
 src/box/sql/vdbe.c             |   1 -
 test/sql-tap/trigger1.test.lua |  25 ++++++-
 test/sql-tap/trigger9.test.lua |  77 +++++++++++----------
 10 files changed, 171 insertions(+), 125 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 77b2342e2..9f9a08d1f 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -3057,6 +3057,17 @@ lock_before_dd(struct trigger *trigger, void *event)
 	txn_on_rollback(txn, on_rollback);
 }
 
+/**
+ * A trigger invoked on replace in a space containing
+ * SQL triggers.
+ */
+static void
+on_replace_dd_trigger(struct trigger * /* trigger */, void *event)
+{
+	struct txn *txn = (struct txn *) event;
+	txn_check_singlestatement_xc(txn, "Space _trigger");
+}
+
 struct trigger alter_space_on_replace_space = {
 	RLIST_LINK_INITIALIZER, on_replace_dd_space, NULL, NULL
 };
@@ -3117,4 +3128,8 @@ struct trigger on_stmt_begin_truncate = {
 	RLIST_LINK_INITIALIZER, lock_before_dd, NULL, NULL
 };
 
+struct trigger on_replace_trigger = {
+	RLIST_LINK_INITIALIZER, on_replace_dd_trigger, NULL, NULL
+};
+
 /* vim: set foldmethod=marker */
diff --git a/src/box/alter.h b/src/box/alter.h
index fb5f65a68..8ea29c77b 100644
--- a/src/box/alter.h
+++ b/src/box/alter.h
@@ -44,6 +44,7 @@ extern struct trigger on_replace_cluster;
 extern struct trigger on_replace_sequence;
 extern struct trigger on_replace_sequence_data;
 extern struct trigger on_replace_space_sequence;
+extern struct trigger on_replace_trigger;
 extern struct trigger on_stmt_begin_space;
 extern struct trigger on_stmt_begin_index;
 extern struct trigger on_stmt_begin_truncate;
diff --git a/src/box/schema.cc b/src/box/schema.cc
index 038f8ce2b..8f25161ac 100644
--- a/src/box/schema.cc
+++ b/src/box/schema.cc
@@ -330,7 +330,7 @@ schema_init()
 	/* _trigger - all existing SQL triggers. */
 	key_def_set_part(key_def, 0 /* part no */, 0 /* field no */,
 			 FIELD_TYPE_STRING, ON_CONFLICT_ACTION_ABORT, NULL);
-	sc_space_new(BOX_TRIGGER_ID, "_trigger", key_def, NULL, NULL);
+	sc_space_new(BOX_TRIGGER_ID, "_trigger", key_def, &on_replace_trigger, NULL);
 
 	free(key_def);
 	key_def = key_def_new(2); /* part count */
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index a719136cd..13e5384bd 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -2364,7 +2364,14 @@ sqlite3CodeDropTable(Parse * pParse, Table * pTab, int isView)
 	 */
 
 	sqlite3VdbeAddOp4(v, OP_DropTable, 0, 0, 0, pTab->zName, 0);
-	sqlite3ChangeCookie(pParse);
+
+	/* FIXME: DDL is impossible inside a transaction so far.
+	 * Replace to _space/_index will fail if active
+	 * transaction. So, don't pretend, that we are able to
+	 * anything back. To be fixed when transactions
+	 * DDL are enabled.
+	 */
+	/* sqlite3ChangeCookie(pParse); */
 	sqliteViewResetAll(db);
 }
 
diff --git a/src/box/sql/main.c b/src/box/sql/main.c
index 5a0f372b3..e0aa61f40 100644
--- a/src/box/sql/main.c
+++ b/src/box/sql/main.c
@@ -977,20 +977,9 @@ sqlite3RollbackAll(Vdbe * pVdbe, int tripCode)
 	struct session *user_session = current_session();
 	assert(sqlite3_mutex_held(db->mutex));
 
-	if ((user_session->sql_flags & SQLITE_InternChanges) != 0
-	    && db->init.busy == 0) {
-		sqlite3ExpirePreparedStatements(db);
-		sqlite3ResetAllSchemasOfConnection(db);
-
-		db->init.busy = 1;
-		db->pSchema = sqlite3SchemaCreate(db);
-		int rc = sqlite3InitDatabase(db);
-		if (rc != SQLITE_OK)
-			sqlite3SchemaClear(db);
-		db->init.busy = 0;
-		if (rc == SQLITE_OK)
-			sqlite3CommitInternalChanges();
-	}
+	/* DDL is impossible inside a transaction.  */
+	assert((user_session->sql_flags & SQLITE_InternChanges) == 0
+	       || db->init.busy == 1);
 
 	/* Any deferred constraint violations have now been resolved. */
 	pVdbe->nDeferredCons = 0;
diff --git a/src/box/sql/parse.c b/src/box/sql/parse.c
index 0019b77dc..f938a9a3c 100644
--- a/src/box/sql/parse.c
+++ b/src/box/sql/parse.c
@@ -1512,7 +1512,7 @@ sqlite3SrcListDelete(pParse->db, (yypminor->yy387));
     case 184: /* with */
     case 229: /* wqlist */
 {
-#line 1510 "parse.y"
+#line 1511 "parse.y"
 sqlite3WithDelete(pParse->db, (yypminor->yy151));
 #line 1518 "parse.c"
 }
@@ -1541,14 +1541,14 @@ sqlite3IdListDelete(pParse->db, (yypminor->yy40));
     case 221: /* trigger_cmd_list */
     case 226: /* trigger_cmd */
 {
-#line 1384 "parse.y"
+#line 1385 "parse.y"
 sqlite3DeleteTriggerStep(pParse->db, (yypminor->yy427));
 #line 1547 "parse.c"
 }
       break;
     case 223: /* trigger_event */
 {
-#line 1370 "parse.y"
+#line 1371 "parse.y"
 sqlite3IdListDelete(pParse->db, (yypminor->yy10).b);
 #line 1554 "parse.c"
 }
@@ -3498,126 +3498,127 @@ static void yy_reduce(
   Token all;
   all.z = yymsp[-3].minor.yy0.z;
   all.n = (int)(yymsp[0].minor.yy0.z - yymsp[-3].minor.yy0.z) + yymsp[0].minor.yy0.n;
+  pParse->initiateTTrans = false;
   sqlite3FinishTrigger(pParse, yymsp[-1].minor.yy427, &all);
 }
-#line 3504 "parse.c"
+#line 3505 "parse.c"
         break;
       case 221: /* trigger_decl ::= TRIGGER ifnotexists nm trigger_time trigger_event ON fullname foreach_clause when_clause */
-#line 1358 "parse.y"
+#line 1359 "parse.y"
 {
   sqlite3BeginTrigger(pParse, &yymsp[-6].minor.yy0, yymsp[-5].minor.yy52, yymsp[-4].minor.yy10.a, yymsp[-4].minor.yy10.b, yymsp[-2].minor.yy387, yymsp[0].minor.yy362, yymsp[-7].minor.yy52);
   yymsp[-8].minor.yy0 = yymsp[-6].minor.yy0; /*yymsp[-8].minor.yy0-overwrites-T*/
 }
-#line 3512 "parse.c"
+#line 3513 "parse.c"
         break;
       case 222: /* trigger_time ::= BEFORE */
-#line 1364 "parse.y"
+#line 1365 "parse.y"
 { yymsp[0].minor.yy52 = TK_BEFORE; }
-#line 3517 "parse.c"
+#line 3518 "parse.c"
         break;
       case 223: /* trigger_time ::= AFTER */
-#line 1365 "parse.y"
+#line 1366 "parse.y"
 { yymsp[0].minor.yy52 = TK_AFTER;  }
-#line 3522 "parse.c"
+#line 3523 "parse.c"
         break;
       case 224: /* trigger_time ::= INSTEAD OF */
-#line 1366 "parse.y"
+#line 1367 "parse.y"
 { yymsp[-1].minor.yy52 = TK_INSTEAD;}
-#line 3527 "parse.c"
+#line 3528 "parse.c"
         break;
       case 225: /* trigger_time ::= */
-#line 1367 "parse.y"
+#line 1368 "parse.y"
 { yymsp[1].minor.yy52 = TK_BEFORE; }
-#line 3532 "parse.c"
+#line 3533 "parse.c"
         break;
       case 226: /* trigger_event ::= DELETE|INSERT */
       case 227: /* trigger_event ::= UPDATE */ yytestcase(yyruleno==227);
-#line 1371 "parse.y"
+#line 1372 "parse.y"
 {yymsp[0].minor.yy10.a = yymsp[0].major; /*A-overwrites-X*/ yymsp[0].minor.yy10.b = 0;}
-#line 3538 "parse.c"
+#line 3539 "parse.c"
         break;
       case 228: /* trigger_event ::= UPDATE OF idlist */
-#line 1373 "parse.y"
+#line 1374 "parse.y"
 {yymsp[-2].minor.yy10.a = TK_UPDATE; yymsp[-2].minor.yy10.b = yymsp[0].minor.yy40;}
-#line 3543 "parse.c"
+#line 3544 "parse.c"
         break;
       case 229: /* when_clause ::= */
-#line 1380 "parse.y"
+#line 1381 "parse.y"
 { yymsp[1].minor.yy362 = 0; }
-#line 3548 "parse.c"
+#line 3549 "parse.c"
         break;
       case 230: /* when_clause ::= WHEN expr */
-#line 1381 "parse.y"
+#line 1382 "parse.y"
 { yymsp[-1].minor.yy362 = yymsp[0].minor.yy162.pExpr; }
-#line 3553 "parse.c"
+#line 3554 "parse.c"
         break;
       case 231: /* trigger_cmd_list ::= trigger_cmd_list trigger_cmd SEMI */
-#line 1385 "parse.y"
+#line 1386 "parse.y"
 {
   assert( yymsp[-2].minor.yy427!=0 );
   yymsp[-2].minor.yy427->pLast->pNext = yymsp[-1].minor.yy427;
   yymsp[-2].minor.yy427->pLast = yymsp[-1].minor.yy427;
 }
-#line 3562 "parse.c"
+#line 3563 "parse.c"
         break;
       case 232: /* trigger_cmd_list ::= trigger_cmd SEMI */
-#line 1390 "parse.y"
+#line 1391 "parse.y"
 { 
   assert( yymsp[-1].minor.yy427!=0 );
   yymsp[-1].minor.yy427->pLast = yymsp[-1].minor.yy427;
 }
-#line 3570 "parse.c"
+#line 3571 "parse.c"
         break;
       case 233: /* trnm ::= nm DOT nm */
-#line 1401 "parse.y"
+#line 1402 "parse.y"
 {
   yymsp[-2].minor.yy0 = yymsp[0].minor.yy0;
   sqlite3ErrorMsg(pParse, 
         "qualified table names are not allowed on INSERT, UPDATE, and DELETE "
         "statements within triggers");
 }
-#line 3580 "parse.c"
+#line 3581 "parse.c"
         break;
       case 234: /* tridxby ::= INDEXED BY nm */
-#line 1413 "parse.y"
+#line 1414 "parse.y"
 {
   sqlite3ErrorMsg(pParse,
         "the INDEXED BY clause is not allowed on UPDATE or DELETE statements "
         "within triggers");
 }
-#line 3589 "parse.c"
+#line 3590 "parse.c"
         break;
       case 235: /* tridxby ::= NOT INDEXED */
-#line 1418 "parse.y"
+#line 1419 "parse.y"
 {
   sqlite3ErrorMsg(pParse,
         "the NOT INDEXED clause is not allowed on UPDATE or DELETE statements "
         "within triggers");
 }
-#line 3598 "parse.c"
+#line 3599 "parse.c"
         break;
       case 236: /* trigger_cmd ::= UPDATE orconf trnm tridxby SET setlist where_opt */
-#line 1431 "parse.y"
+#line 1432 "parse.y"
 {yymsp[-6].minor.yy427 = sqlite3TriggerUpdateStep(pParse->db, &yymsp[-4].minor.yy0, yymsp[-1].minor.yy382, yymsp[0].minor.yy362, yymsp[-5].minor.yy52);}
-#line 3603 "parse.c"
+#line 3604 "parse.c"
         break;
       case 237: /* trigger_cmd ::= insert_cmd INTO trnm idlist_opt select */
-#line 1435 "parse.y"
+#line 1436 "parse.y"
 {yymsp[-4].minor.yy427 = sqlite3TriggerInsertStep(pParse->db, &yymsp[-2].minor.yy0, yymsp[-1].minor.yy40, yymsp[0].minor.yy279, yymsp[-4].minor.yy52);/*A-overwrites-R*/}
-#line 3608 "parse.c"
+#line 3609 "parse.c"
         break;
       case 238: /* trigger_cmd ::= DELETE FROM trnm tridxby where_opt */
-#line 1439 "parse.y"
+#line 1440 "parse.y"
 {yymsp[-4].minor.yy427 = sqlite3TriggerDeleteStep(pParse->db, &yymsp[-2].minor.yy0, yymsp[0].minor.yy362);}
-#line 3613 "parse.c"
+#line 3614 "parse.c"
         break;
       case 239: /* trigger_cmd ::= select */
-#line 1443 "parse.y"
+#line 1444 "parse.y"
 {yymsp[0].minor.yy427 = sqlite3TriggerSelectStep(pParse->db, yymsp[0].minor.yy279); /*A-overwrites-X*/}
-#line 3618 "parse.c"
+#line 3619 "parse.c"
         break;
       case 240: /* expr ::= RAISE LP IGNORE RP */
-#line 1446 "parse.y"
+#line 1447 "parse.y"
 {
   spanSet(&yymsp[-3].minor.yy162,&yymsp[-3].minor.yy0,&yymsp[0].minor.yy0);  /*A-overwrites-X*/
   yymsp[-3].minor.yy162.pExpr = sqlite3PExpr(pParse, TK_RAISE, 0, 0); 
@@ -3625,10 +3626,10 @@ static void yy_reduce(
     yymsp[-3].minor.yy162.pExpr->affinity = ON_CONFLICT_ACTION_IGNORE;
   }
 }
-#line 3629 "parse.c"
+#line 3630 "parse.c"
         break;
       case 241: /* expr ::= RAISE LP raisetype COMMA STRING RP */
-#line 1453 "parse.y"
+#line 1454 "parse.y"
 {
   spanSet(&yymsp[-5].minor.yy162,&yymsp[-5].minor.yy0,&yymsp[0].minor.yy0);  /*A-overwrites-X*/
   yymsp[-5].minor.yy162.pExpr = sqlite3ExprAlloc(pParse->db, TK_RAISE, &yymsp[-1].minor.yy0, 1); 
@@ -3636,85 +3637,85 @@ static void yy_reduce(
     yymsp[-5].minor.yy162.pExpr->affinity = (char)yymsp[-3].minor.yy52;
   }
 }
-#line 3640 "parse.c"
+#line 3641 "parse.c"
         break;
       case 242: /* raisetype ::= ROLLBACK */
-#line 1463 "parse.y"
+#line 1464 "parse.y"
 {yymsp[0].minor.yy52 = ON_CONFLICT_ACTION_ROLLBACK;}
-#line 3645 "parse.c"
+#line 3646 "parse.c"
         break;
       case 244: /* raisetype ::= FAIL */
-#line 1465 "parse.y"
+#line 1466 "parse.y"
 {yymsp[0].minor.yy52 = ON_CONFLICT_ACTION_FAIL;}
-#line 3650 "parse.c"
+#line 3651 "parse.c"
         break;
       case 245: /* cmd ::= DROP TRIGGER ifexists fullname */
-#line 1470 "parse.y"
+#line 1471 "parse.y"
 {
   sqlite3DropTrigger(pParse,yymsp[0].minor.yy387,yymsp[-1].minor.yy52);
 }
-#line 3657 "parse.c"
+#line 3658 "parse.c"
         break;
       case 246: /* cmd ::= REINDEX */
-#line 1477 "parse.y"
+#line 1478 "parse.y"
 {sqlite3Reindex(pParse, 0, 0);}
-#line 3662 "parse.c"
+#line 3663 "parse.c"
         break;
       case 247: /* cmd ::= REINDEX nm */
-#line 1478 "parse.y"
+#line 1479 "parse.y"
 {sqlite3Reindex(pParse, &yymsp[0].minor.yy0, 0);}
-#line 3667 "parse.c"
+#line 3668 "parse.c"
         break;
       case 248: /* cmd ::= REINDEX nm ON nm */
-#line 1479 "parse.y"
+#line 1480 "parse.y"
 {sqlite3Reindex(pParse, &yymsp[-2].minor.yy0, &yymsp[0].minor.yy0);}
-#line 3672 "parse.c"
+#line 3673 "parse.c"
         break;
       case 249: /* cmd ::= ANALYZE */
-#line 1484 "parse.y"
+#line 1485 "parse.y"
 {sqlite3Analyze(pParse, 0);}
-#line 3677 "parse.c"
+#line 3678 "parse.c"
         break;
       case 250: /* cmd ::= ANALYZE nm */
-#line 1485 "parse.y"
+#line 1486 "parse.y"
 {sqlite3Analyze(pParse, &yymsp[0].minor.yy0);}
-#line 3682 "parse.c"
+#line 3683 "parse.c"
         break;
       case 251: /* cmd ::= ALTER TABLE fullname RENAME TO nm */
-#line 1490 "parse.y"
+#line 1491 "parse.y"
 {
   sqlite3AlterRenameTable(pParse,yymsp[-3].minor.yy387,&yymsp[0].minor.yy0);
 }
-#line 3689 "parse.c"
+#line 3690 "parse.c"
         break;
       case 252: /* with ::= */
-#line 1513 "parse.y"
+#line 1514 "parse.y"
 {yymsp[1].minor.yy151 = 0;}
-#line 3694 "parse.c"
+#line 3695 "parse.c"
         break;
       case 253: /* with ::= WITH wqlist */
-#line 1515 "parse.y"
+#line 1516 "parse.y"
 { yymsp[-1].minor.yy151 = yymsp[0].minor.yy151; }
-#line 3699 "parse.c"
+#line 3700 "parse.c"
         break;
       case 254: /* with ::= WITH RECURSIVE wqlist */
-#line 1516 "parse.y"
+#line 1517 "parse.y"
 { yymsp[-2].minor.yy151 = yymsp[0].minor.yy151; }
-#line 3704 "parse.c"
+#line 3705 "parse.c"
         break;
       case 255: /* wqlist ::= nm eidlist_opt AS LP select RP */
-#line 1518 "parse.y"
+#line 1519 "parse.y"
 {
   yymsp[-5].minor.yy151 = sqlite3WithAdd(pParse, 0, &yymsp[-5].minor.yy0, yymsp[-4].minor.yy382, yymsp[-1].minor.yy279); /*A-overwrites-X*/
 }
-#line 3711 "parse.c"
+#line 3712 "parse.c"
         break;
       case 256: /* wqlist ::= wqlist COMMA nm eidlist_opt AS LP select RP */
-#line 1521 "parse.y"
+#line 1522 "parse.y"
 {
   yymsp[-7].minor.yy151 = sqlite3WithAdd(pParse, yymsp[-7].minor.yy151, &yymsp[-5].minor.yy0, yymsp[-4].minor.yy382, yymsp[-1].minor.yy279);
 }
-#line 3718 "parse.c"
+#line 3719 "parse.c"
         break;
       default:
       /* (257) input ::= ecmd */ yytestcase(yyruleno==257);
@@ -3825,7 +3826,7 @@ static void yy_syntax_error(
   } else {
     sqlite3ErrorMsg(pParse, "near \"%T\": syntax error", &TOKEN);
   }
-#line 3829 "parse.c"
+#line 3830 "parse.c"
 /************ End %syntax_error code ******************************************/
   sqlite3ParserARG_STORE; /* Suppress warning about unused %extra_argument variable */
 }
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 914fc53b8..11db58d94 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -1350,12 +1350,14 @@ cmd ::= createkw trigger_decl(A) BEGIN trigger_cmd_list(S) END(Z). {
   Token all;
   all.z = A.z;
   all.n = (int)(Z.z - A.z) + Z.n;
+  pParse->initiateTTrans = false;
   sqlite3FinishTrigger(pParse, S, &all);
 }
 
 trigger_decl(A) ::= TRIGGER ifnotexists(NOERR) nm(B)
                     trigger_time(C) trigger_event(D)
                     ON fullname(E) foreach_clause when_clause(G). {
+  pParse->initiateTTrans = false;
   sqlite3BeginTrigger(pParse, &B, C, D.a, D.b, E, G, NOERR);
   A = B; /*A-overwrites-T*/
 }
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index a194a6e72..96609b5ca 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2920,7 +2920,6 @@ case OP_Savepoint: {
 				}
 				if (isSchemaChange) {
 					sqlite3ExpirePreparedStatements(db);
-					sqlite3ResetAllSchemasOfConnection(db);
 					user_session->sql_flags |= SQLITE_InternChanges;
 				}
 			}
diff --git a/test/sql-tap/trigger1.test.lua b/test/sql-tap/trigger1.test.lua
index 93dd3aaac..40daba4d8 100755
--- a/test/sql-tap/trigger1.test.lua
+++ b/test/sql-tap/trigger1.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(31)
+test:plan(33)
 
 --!./tcltestrunner.lua
 -- The author disclaims copyright to this source code.  In place of
@@ -848,6 +848,29 @@ test:do_catchsql_test(
         -- </trigger1-16.7>
     })
 
+test:do_catchsql_test(
+    "trigger1-16.8",
+    [[
+        BEGIN;
+          CREATE TRIGGER tr168 INSERT ON tA BEGIN
+            INSERT INTO t16 values(1);
+          END;
+   ]], {
+        1, [[Space _trigger does not support multi-statement transactions]]
+})
+
+test:execsql [[
+    ROLLBACK;
+]]
+
+test:do_catchsql_test(
+    "trigger1-16.9",
+    [[
+        BEGIN;
+          DROP TRIGGER t16err3;
+   ]], {
+        1, [[Space _trigger does not support multi-statement transactions]]
+})
 -- MUST_WORK_TEST
 -- #-------------------------------------------------------------------------
 -- # Test that bug [34cd55d68e0e6e7c] has been fixed.
diff --git a/test/sql-tap/trigger9.test.lua b/test/sql-tap/trigger9.test.lua
index af8181811..840d184bf 100755
--- a/test/sql-tap/trigger9.test.lua
+++ b/test/sql-tap/trigger9.test.lua
@@ -66,10 +66,10 @@ test:do_execsql_test(
 test:do_execsql_test(
     "trigger9-1.2.1",
     [[
+        CREATE TRIGGER trig1 BEFORE DELETE ON t1 BEGIN
+          INSERT INTO t2 VALUES(old.x);
+        END;
         BEGIN;
-          CREATE TRIGGER trig1 BEFORE DELETE ON t1 BEGIN
-            INSERT INTO t2 VALUES(old.x);
-          END;
           DELETE FROM t1;
           SELECT * FROM t2;
     ]], {
@@ -101,10 +101,11 @@ test:do_execsql_test(
 test:do_execsql_test(
     "trigger9-1.3.1",
     [[
+        DROP TRIGGER IF EXISTS trig1;
+        CREATE TRIGGER trig1 BEFORE DELETE ON t1 BEGIN
+          INSERT INTO t2 VALUES(old.x);
+        END;
         BEGIN;
-          CREATE TRIGGER trig1 BEFORE DELETE ON t1 BEGIN
-            INSERT INTO t2 VALUES(old.x);
-          END;
           DELETE FROM t1;
           SELECT * FROM t2;
     ]], {
@@ -136,10 +137,11 @@ test:do_execsql_test(
 test:do_execsql_test(
     "trigger9-1.4.1",
     [[
+        DROP TRIGGER IF EXISTS trig1;
+        CREATE TRIGGER trig1 BEFORE DELETE ON t1 WHEN old.x='1' BEGIN
+          INSERT INTO t2 VALUES(old.x);
+        END;
         BEGIN;
-          CREATE TRIGGER trig1 BEFORE DELETE ON t1 WHEN old.x='1' BEGIN
-            INSERT INTO t2 VALUES(old.x);
-          END;
           DELETE FROM t1;
           SELECT * FROM t2;
     ]], {
@@ -171,10 +173,11 @@ test:do_execsql_test(
 test:do_execsql_test(
     "trigger9-1.5.1",
     [[
+        DROP TRIGGER IF EXISTS trig1;
+        CREATE TRIGGER trig1 BEFORE UPDATE ON t1 BEGIN
+          INSERT INTO t2 VALUES(old.x);
+        END;
         BEGIN;
-          CREATE TRIGGER trig1 BEFORE UPDATE ON t1 BEGIN
-            INSERT INTO t2 VALUES(old.x);
-          END;
           UPDATE t1 SET y = '';
           SELECT * FROM t2;
     ]], {
@@ -206,10 +209,11 @@ test:do_execsql_test(
 test:do_execsql_test(
     "trigger9-1.6.1",
     [[
+        DROP TRIGGER IF EXISTS trig1;
+        CREATE TRIGGER trig1 BEFORE UPDATE ON t1 BEGIN
+          INSERT INTO t2 VALUES(old.x);
+        END;
         BEGIN;
-          CREATE TRIGGER trig1 BEFORE UPDATE ON t1 BEGIN
-            INSERT INTO t2 VALUES(old.x);
-          END;
           UPDATE t1 SET y = '';
           SELECT * FROM t2;
     ]], {
@@ -241,10 +245,11 @@ test:do_execsql_test(
 test:do_execsql_test(
     "trigger9-1.7.1",
     [[
+        DROP TRIGGER IF EXISTS trig1;
+        CREATE TRIGGER trig1 BEFORE UPDATE ON t1 WHEN old.x>='2' BEGIN
+          INSERT INTO t2 VALUES(old.x);
+        END;
         BEGIN;
-          CREATE TRIGGER trig1 BEFORE UPDATE ON t1 WHEN old.x>='2' BEGIN
-            INSERT INTO t2 VALUES(old.x);
-          END;
           UPDATE t1 SET y = '';
           SELECT * FROM t2;
     ]], {
@@ -290,10 +295,11 @@ test:do_execsql_test(
     "trigger9-3.2",
     [[
         CREATE VIEW v1 AS SELECT * FROM t3;
+        DROP TRIGGER IF EXISTS trig1;
+        CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
+          INSERT INTO t2 VALUES(old.a);
+        END;
         BEGIN;
-          CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
-            INSERT INTO t2 VALUES(old.a);
-          END;
           UPDATE v1 SET b = 'hello';
           SELECT * FROM t2;
         ROLLBACK;
@@ -314,10 +320,11 @@ test:do_test(
         --
         return test:execsql([[
             CREATE VIEW v1 AS SELECT a, b AS c FROM t3 WHERE c > 'one';
+            DROP TRIGGER IF EXISTS trig1;
+            CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
+              INSERT INTO t2 VALUES(old.a);
+            END;
             BEGIN;
-              CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
-                INSERT INTO t2 VALUES(old.a);
-              END;
               UPDATE v1 SET c = 'hello';
               SELECT * FROM t2;
             ROLLBACK;
@@ -333,11 +340,12 @@ test:do_execsql_test(
     "trigger9-3.4",
     [[
         CREATE VIEW v1 AS SELECT DISTINCT a, b FROM t3;
+        DROP TRIGGER IF EXISTS trig1;
+        CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
+          INSERT INTO t2 VALUES(old.a);
+        END;
         BEGIN;
           INSERT INTO t3 VALUES(4, 3, 'three');
-          CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
-            INSERT INTO t2 VALUES(old.a);
-          END;
           UPDATE v1 SET b = 'hello';
           SELECT * FROM t2;
         ROLLBACK;
@@ -352,11 +360,12 @@ test:do_execsql_test(
     "trigger9-3.5",
     [[
         CREATE VIEW v1 AS SELECT a, b FROM t3 EXCEPT SELECT 1, 'one';
+        DROP TRIGGER IF EXISTS trig1;
+        CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
+          INSERT INTO t2 VALUES(old.a);
+        END;
         BEGIN;
           INSERT INTO t3 VALUES(5, 1, 'uno');
-          CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
-            INSERT INTO t2 VALUES(old.a);
-          END;
           UPDATE v1 SET b = 'hello';
           SELECT * FROM t2;
         ROLLBACK;
@@ -372,11 +381,12 @@ test:do_execsql_test(
     [[
         CREATE VIEW v1 AS
           SELECT sum(a) AS a, max(b) AS b FROM t3 GROUP BY t3.a HAVING b>'two';
+        DROP TRIGGER IF EXISTS trig1;
+        CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
+          INSERT INTO t2 VALUES(old.a);
+        END;
         BEGIN;
           INSERT INTO t3 VALUES(6, 1, 'zero');
-          CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
-            INSERT INTO t2 VALUES(old.a);
-          END;
           UPDATE v1 SET b = 'hello';
           SELECT * FROM t2;
         ROLLBACK;
@@ -436,5 +446,4 @@ test:do_execsql_test(
     })
 
 
-
 test:finish_test()
-- 
2.11.0

[tarantool-patches] Re: [PATCH 2/2] sql: block trigger creation inside a transaction

[v.shpilevoy]

LGTM.

> 15 марта 2018 г., в 21:21, Kirill Yukhin <kyukhin@tarantool.org> написал(а):
> 
> Since DDL is prohibited inside a transaction, this patch
> prohibits trigger creation and drop inside a transaction.
> This was done by adding dummy trigger on replace into
> _trigger space. This trigger only check if this not multistatement
> transaction. Also, make assert that schema cokie wasn't
> changed during transaction rollback if any.
> 
> Testsuite updated as well.
> 
> Closes #3239
> ---
> src/box/alter.cc               |  15 +++++
> src/box/alter.h                |   1 +
> src/box/schema.cc              |   2 +-
> src/box/sql/build.c            |   9 ++-
> src/box/sql/main.c             |  17 +----
> src/box/sql/parse.c            | 147 +++++++++++++++++++++--------------------
> src/box/sql/parse.y            |   2 +
> src/box/sql/vdbe.c             |   1 -
> test/sql-tap/trigger1.test.lua |  25 ++++++-
> test/sql-tap/trigger9.test.lua |  77 +++++++++++----------
> 10 files changed, 171 insertions(+), 125 deletions(-)
> 
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index 77b2342e2..9f9a08d1f 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -3057,6 +3057,17 @@ lock_before_dd(struct trigger *trigger, void *event)
> 	txn_on_rollback(txn, on_rollback);
> }
> 
> +/**
> + * A trigger invoked on replace in a space containing
> + * SQL triggers.
> + */
> +static void
> +on_replace_dd_trigger(struct trigger * /* trigger */, void *event)
> +{
> +	struct txn *txn = (struct txn *) event;
> +	txn_check_singlestatement_xc(txn, "Space _trigger");
> +}
> +
> struct trigger alter_space_on_replace_space = {
> 	RLIST_LINK_INITIALIZER, on_replace_dd_space, NULL, NULL
> };
> @@ -3117,4 +3128,8 @@ struct trigger on_stmt_begin_truncate = {
> 	RLIST_LINK_INITIALIZER, lock_before_dd, NULL, NULL
> };
> 
> +struct trigger on_replace_trigger = {
> +	RLIST_LINK_INITIALIZER, on_replace_dd_trigger, NULL, NULL
> +};
> +
> /* vim: set foldmethod=marker */
> diff --git a/src/box/alter.h b/src/box/alter.h
> index fb5f65a68..8ea29c77b 100644
> --- a/src/box/alter.h
> +++ b/src/box/alter.h
> @@ -44,6 +44,7 @@ extern struct trigger on_replace_cluster;
> extern struct trigger on_replace_sequence;
> extern struct trigger on_replace_sequence_data;
> extern struct trigger on_replace_space_sequence;
> +extern struct trigger on_replace_trigger;
> extern struct trigger on_stmt_begin_space;
> extern struct trigger on_stmt_begin_index;
> extern struct trigger on_stmt_begin_truncate;
> diff --git a/src/box/schema.cc b/src/box/schema.cc
> index 038f8ce2b..8f25161ac 100644
> --- a/src/box/schema.cc
> +++ b/src/box/schema.cc
> @@ -330,7 +330,7 @@ schema_init()
> 	/* _trigger - all existing SQL triggers. */
> 	key_def_set_part(key_def, 0 /* part no */, 0 /* field no */,
> 			 FIELD_TYPE_STRING, ON_CONFLICT_ACTION_ABORT, NULL);
> -	sc_space_new(BOX_TRIGGER_ID, "_trigger", key_def, NULL, NULL);
> +	sc_space_new(BOX_TRIGGER_ID, "_trigger", key_def, &on_replace_trigger, NULL);
> 
> 	free(key_def);
> 	key_def = key_def_new(2); /* part count */
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index a719136cd..13e5384bd 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -2364,7 +2364,14 @@ sqlite3CodeDropTable(Parse * pParse, Table * pTab, int isView)
> 	 */
> 
> 	sqlite3VdbeAddOp4(v, OP_DropTable, 0, 0, 0, pTab->zName, 0);
> -	sqlite3ChangeCookie(pParse);
> +
> +	/* FIXME: DDL is impossible inside a transaction so far.
> +	 * Replace to _space/_index will fail if active
> +	 * transaction. So, don't pretend, that we are able to
> +	 * anything back. To be fixed when transactions
> +	 * DDL are enabled.
> +	 */
> +	/* sqlite3ChangeCookie(pParse); */
> 	sqliteViewResetAll(db);
> }
> 
> diff --git a/src/box/sql/main.c b/src/box/sql/main.c
> index 5a0f372b3..e0aa61f40 100644
> --- a/src/box/sql/main.c
> +++ b/src/box/sql/main.c
> @@ -977,20 +977,9 @@ sqlite3RollbackAll(Vdbe * pVdbe, int tripCode)
> 	struct session *user_session = current_session();
> 	assert(sqlite3_mutex_held(db->mutex));
> 
> -	if ((user_session->sql_flags & SQLITE_InternChanges) != 0
> -	    && db->init.busy == 0) {
> -		sqlite3ExpirePreparedStatements(db);
> -		sqlite3ResetAllSchemasOfConnection(db);
> -
> -		db->init.busy = 1;
> -		db->pSchema = sqlite3SchemaCreate(db);
> -		int rc = sqlite3InitDatabase(db);
> -		if (rc != SQLITE_OK)
> -			sqlite3SchemaClear(db);
> -		db->init.busy = 0;
> -		if (rc == SQLITE_OK)
> -			sqlite3CommitInternalChanges();
> -	}
> +	/* DDL is impossible inside a transaction.  */
> +	assert((user_session->sql_flags & SQLITE_InternChanges) == 0
> +	       || db->init.busy == 1);
> 
> 	/* Any deferred constraint violations have now been resolved. */
> 	pVdbe->nDeferredCons = 0;
> diff --git a/src/box/sql/parse.c b/src/box/sql/parse.c
> index 0019b77dc..f938a9a3c 100644
> --- a/src/box/sql/parse.c
> +++ b/src/box/sql/parse.c
> @@ -1512,7 +1512,7 @@ sqlite3SrcListDelete(pParse->db, (yypminor->yy387));
>     case 184: /* with */
>     case 229: /* wqlist */
> {
> -#line 1510 "parse.y"
> +#line 1511 "parse.y"
> sqlite3WithDelete(pParse->db, (yypminor->yy151));
> #line 1518 "parse.c"
> }
> @@ -1541,14 +1541,14 @@ sqlite3IdListDelete(pParse->db, (yypminor->yy40));
>     case 221: /* trigger_cmd_list */
>     case 226: /* trigger_cmd */
> {
> -#line 1384 "parse.y"
> +#line 1385 "parse.y"
> sqlite3DeleteTriggerStep(pParse->db, (yypminor->yy427));
> #line 1547 "parse.c"
> }
>       break;
>     case 223: /* trigger_event */
> {
> -#line 1370 "parse.y"
> +#line 1371 "parse.y"
> sqlite3IdListDelete(pParse->db, (yypminor->yy10).b);
> #line 1554 "parse.c"
> }
> @@ -3498,126 +3498,127 @@ static void yy_reduce(
>   Token all;
>   all.z = yymsp[-3].minor.yy0.z;
>   all.n = (int)(yymsp[0].minor.yy0.z - yymsp[-3].minor.yy0.z) + yymsp[0].minor.yy0.n;
> +  pParse->initiateTTrans = false;
>   sqlite3FinishTrigger(pParse, yymsp[-1].minor.yy427, &all);
> }
> -#line 3504 "parse.c"
> +#line 3505 "parse.c"
>         break;
>       case 221: /* trigger_decl ::= TRIGGER ifnotexists nm trigger_time trigger_event ON fullname foreach_clause when_clause */
> -#line 1358 "parse.y"
> +#line 1359 "parse.y"
> {
>   sqlite3BeginTrigger(pParse, &yymsp[-6].minor.yy0, yymsp[-5].minor.yy52, yymsp[-4].minor.yy10.a, yymsp[-4].minor.yy10.b, yymsp[-2].minor.yy387, yymsp[0].minor.yy362, yymsp[-7].minor.yy52);
>   yymsp[-8].minor.yy0 = yymsp[-6].minor.yy0; /*yymsp[-8].minor.yy0-overwrites-T*/
> }
> -#line 3512 "parse.c"
> +#line 3513 "parse.c"
>         break;
>       case 222: /* trigger_time ::= BEFORE */
> -#line 1364 "parse.y"
> +#line 1365 "parse.y"
> { yymsp[0].minor.yy52 = TK_BEFORE; }
> -#line 3517 "parse.c"
> +#line 3518 "parse.c"
>         break;
>       case 223: /* trigger_time ::= AFTER */
> -#line 1365 "parse.y"
> +#line 1366 "parse.y"
> { yymsp[0].minor.yy52 = TK_AFTER;  }
> -#line 3522 "parse.c"
> +#line 3523 "parse.c"
>         break;
>       case 224: /* trigger_time ::= INSTEAD OF */
> -#line 1366 "parse.y"
> +#line 1367 "parse.y"
> { yymsp[-1].minor.yy52 = TK_INSTEAD;}
> -#line 3527 "parse.c"
> +#line 3528 "parse.c"
>         break;
>       case 225: /* trigger_time ::= */
> -#line 1367 "parse.y"
> +#line 1368 "parse.y"
> { yymsp[1].minor.yy52 = TK_BEFORE; }
> -#line 3532 "parse.c"
> +#line 3533 "parse.c"
>         break;
>       case 226: /* trigger_event ::= DELETE|INSERT */
>       case 227: /* trigger_event ::= UPDATE */ yytestcase(yyruleno==227);
> -#line 1371 "parse.y"
> +#line 1372 "parse.y"
> {yymsp[0].minor.yy10.a = yymsp[0].major; /*A-overwrites-X*/ yymsp[0].minor.yy10.b = 0;}
> -#line 3538 "parse.c"
> +#line 3539 "parse.c"
>         break;
>       case 228: /* trigger_event ::= UPDATE OF idlist */
> -#line 1373 "parse.y"
> +#line 1374 "parse.y"
> {yymsp[-2].minor.yy10.a = TK_UPDATE; yymsp[-2].minor.yy10.b = yymsp[0].minor.yy40;}
> -#line 3543 "parse.c"
> +#line 3544 "parse.c"
>         break;
>       case 229: /* when_clause ::= */
> -#line 1380 "parse.y"
> +#line 1381 "parse.y"
> { yymsp[1].minor.yy362 = 0; }
> -#line 3548 "parse.c"
> +#line 3549 "parse.c"
>         break;
>       case 230: /* when_clause ::= WHEN expr */
> -#line 1381 "parse.y"
> +#line 1382 "parse.y"
> { yymsp[-1].minor.yy362 = yymsp[0].minor.yy162.pExpr; }
> -#line 3553 "parse.c"
> +#line 3554 "parse.c"
>         break;
>       case 231: /* trigger_cmd_list ::= trigger_cmd_list trigger_cmd SEMI */
> -#line 1385 "parse.y"
> +#line 1386 "parse.y"
> {
>   assert( yymsp[-2].minor.yy427!=0 );
>   yymsp[-2].minor.yy427->pLast->pNext = yymsp[-1].minor.yy427;
>   yymsp[-2].minor.yy427->pLast = yymsp[-1].minor.yy427;
> }
> -#line 3562 "parse.c"
> +#line 3563 "parse.c"
>         break;
>       case 232: /* trigger_cmd_list ::= trigger_cmd SEMI */
> -#line 1390 "parse.y"
> +#line 1391 "parse.y"
> { 
>   assert( yymsp[-1].minor.yy427!=0 );
>   yymsp[-1].minor.yy427->pLast = yymsp[-1].minor.yy427;
> }
> -#line 3570 "parse.c"
> +#line 3571 "parse.c"
>         break;
>       case 233: /* trnm ::= nm DOT nm */
> -#line 1401 "parse.y"
> +#line 1402 "parse.y"
> {
>   yymsp[-2].minor.yy0 = yymsp[0].minor.yy0;
>   sqlite3ErrorMsg(pParse, 
>         "qualified table names are not allowed on INSERT, UPDATE, and DELETE "
>         "statements within triggers");
> }
> -#line 3580 "parse.c"
> +#line 3581 "parse.c"
>         break;
>       case 234: /* tridxby ::= INDEXED BY nm */
> -#line 1413 "parse.y"
> +#line 1414 "parse.y"
> {
>   sqlite3ErrorMsg(pParse,
>         "the INDEXED BY clause is not allowed on UPDATE or DELETE statements "
>         "within triggers");
> }
> -#line 3589 "parse.c"
> +#line 3590 "parse.c"
>         break;
>       case 235: /* tridxby ::= NOT INDEXED */
> -#line 1418 "parse.y"
> +#line 1419 "parse.y"
> {
>   sqlite3ErrorMsg(pParse,
>         "the NOT INDEXED clause is not allowed on UPDATE or DELETE statements "
>         "within triggers");
> }
> -#line 3598 "parse.c"
> +#line 3599 "parse.c"
>         break;
>       case 236: /* trigger_cmd ::= UPDATE orconf trnm tridxby SET setlist where_opt */
> -#line 1431 "parse.y"
> +#line 1432 "parse.y"
> {yymsp[-6].minor.yy427 = sqlite3TriggerUpdateStep(pParse->db, &yymsp[-4].minor.yy0, yymsp[-1].minor.yy382, yymsp[0].minor.yy362, yymsp[-5].minor.yy52);}
> -#line 3603 "parse.c"
> +#line 3604 "parse.c"
>         break;
>       case 237: /* trigger_cmd ::= insert_cmd INTO trnm idlist_opt select */
> -#line 1435 "parse.y"
> +#line 1436 "parse.y"
> {yymsp[-4].minor.yy427 = sqlite3TriggerInsertStep(pParse->db, &yymsp[-2].minor.yy0, yymsp[-1].minor.yy40, yymsp[0].minor.yy279, yymsp[-4].minor.yy52);/*A-overwrites-R*/}
> -#line 3608 "parse.c"
> +#line 3609 "parse.c"
>         break;
>       case 238: /* trigger_cmd ::= DELETE FROM trnm tridxby where_opt */
> -#line 1439 "parse.y"
> +#line 1440 "parse.y"
> {yymsp[-4].minor.yy427 = sqlite3TriggerDeleteStep(pParse->db, &yymsp[-2].minor.yy0, yymsp[0].minor.yy362);}
> -#line 3613 "parse.c"
> +#line 3614 "parse.c"
>         break;
>       case 239: /* trigger_cmd ::= select */
> -#line 1443 "parse.y"
> +#line 1444 "parse.y"
> {yymsp[0].minor.yy427 = sqlite3TriggerSelectStep(pParse->db, yymsp[0].minor.yy279); /*A-overwrites-X*/}
> -#line 3618 "parse.c"
> +#line 3619 "parse.c"
>         break;
>       case 240: /* expr ::= RAISE LP IGNORE RP */
> -#line 1446 "parse.y"
> +#line 1447 "parse.y"
> {
>   spanSet(&yymsp[-3].minor.yy162,&yymsp[-3].minor.yy0,&yymsp[0].minor.yy0);  /*A-overwrites-X*/
>   yymsp[-3].minor.yy162.pExpr = sqlite3PExpr(pParse, TK_RAISE, 0, 0); 
> @@ -3625,10 +3626,10 @@ static void yy_reduce(
>     yymsp[-3].minor.yy162.pExpr->affinity = ON_CONFLICT_ACTION_IGNORE;
>   }
> }
> -#line 3629 "parse.c"
> +#line 3630 "parse.c"
>         break;
>       case 241: /* expr ::= RAISE LP raisetype COMMA STRING RP */
> -#line 1453 "parse.y"
> +#line 1454 "parse.y"
> {
>   spanSet(&yymsp[-5].minor.yy162,&yymsp[-5].minor.yy0,&yymsp[0].minor.yy0);  /*A-overwrites-X*/
>   yymsp[-5].minor.yy162.pExpr = sqlite3ExprAlloc(pParse->db, TK_RAISE, &yymsp[-1].minor.yy0, 1); 
> @@ -3636,85 +3637,85 @@ static void yy_reduce(
>     yymsp[-5].minor.yy162.pExpr->affinity = (char)yymsp[-3].minor.yy52;
>   }
> }
> -#line 3640 "parse.c"
> +#line 3641 "parse.c"
>         break;
>       case 242: /* raisetype ::= ROLLBACK */
> -#line 1463 "parse.y"
> +#line 1464 "parse.y"
> {yymsp[0].minor.yy52 = ON_CONFLICT_ACTION_ROLLBACK;}
> -#line 3645 "parse.c"
> +#line 3646 "parse.c"
>         break;
>       case 244: /* raisetype ::= FAIL */
> -#line 1465 "parse.y"
> +#line 1466 "parse.y"
> {yymsp[0].minor.yy52 = ON_CONFLICT_ACTION_FAIL;}
> -#line 3650 "parse.c"
> +#line 3651 "parse.c"
>         break;
>       case 245: /* cmd ::= DROP TRIGGER ifexists fullname */
> -#line 1470 "parse.y"
> +#line 1471 "parse.y"
> {
>   sqlite3DropTrigger(pParse,yymsp[0].minor.yy387,yymsp[-1].minor.yy52);
> }
> -#line 3657 "parse.c"
> +#line 3658 "parse.c"
>         break;
>       case 246: /* cmd ::= REINDEX */
> -#line 1477 "parse.y"
> +#line 1478 "parse.y"
> {sqlite3Reindex(pParse, 0, 0);}
> -#line 3662 "parse.c"
> +#line 3663 "parse.c"
>         break;
>       case 247: /* cmd ::= REINDEX nm */
> -#line 1478 "parse.y"
> +#line 1479 "parse.y"
> {sqlite3Reindex(pParse, &yymsp[0].minor.yy0, 0);}
> -#line 3667 "parse.c"
> +#line 3668 "parse.c"
>         break;
>       case 248: /* cmd ::= REINDEX nm ON nm */
> -#line 1479 "parse.y"
> +#line 1480 "parse.y"
> {sqlite3Reindex(pParse, &yymsp[-2].minor.yy0, &yymsp[0].minor.yy0);}
> -#line 3672 "parse.c"
> +#line 3673 "parse.c"
>         break;
>       case 249: /* cmd ::= ANALYZE */
> -#line 1484 "parse.y"
> +#line 1485 "parse.y"
> {sqlite3Analyze(pParse, 0);}
> -#line 3677 "parse.c"
> +#line 3678 "parse.c"
>         break;
>       case 250: /* cmd ::= ANALYZE nm */
> -#line 1485 "parse.y"
> +#line 1486 "parse.y"
> {sqlite3Analyze(pParse, &yymsp[0].minor.yy0);}
> -#line 3682 "parse.c"
> +#line 3683 "parse.c"
>         break;
>       case 251: /* cmd ::= ALTER TABLE fullname RENAME TO nm */
> -#line 1490 "parse.y"
> +#line 1491 "parse.y"
> {
>   sqlite3AlterRenameTable(pParse,yymsp[-3].minor.yy387,&yymsp[0].minor.yy0);
> }
> -#line 3689 "parse.c"
> +#line 3690 "parse.c"
>         break;
>       case 252: /* with ::= */
> -#line 1513 "parse.y"
> +#line 1514 "parse.y"
> {yymsp[1].minor.yy151 = 0;}
> -#line 3694 "parse.c"
> +#line 3695 "parse.c"
>         break;
>       case 253: /* with ::= WITH wqlist */
> -#line 1515 "parse.y"
> +#line 1516 "parse.y"
> { yymsp[-1].minor.yy151 = yymsp[0].minor.yy151; }
> -#line 3699 "parse.c"
> +#line 3700 "parse.c"
>         break;
>       case 254: /* with ::= WITH RECURSIVE wqlist */
> -#line 1516 "parse.y"
> +#line 1517 "parse.y"
> { yymsp[-2].minor.yy151 = yymsp[0].minor.yy151; }
> -#line 3704 "parse.c"
> +#line 3705 "parse.c"
>         break;
>       case 255: /* wqlist ::= nm eidlist_opt AS LP select RP */
> -#line 1518 "parse.y"
> +#line 1519 "parse.y"
> {
>   yymsp[-5].minor.yy151 = sqlite3WithAdd(pParse, 0, &yymsp[-5].minor.yy0, yymsp[-4].minor.yy382, yymsp[-1].minor.yy279); /*A-overwrites-X*/
> }
> -#line 3711 "parse.c"
> +#line 3712 "parse.c"
>         break;
>       case 256: /* wqlist ::= wqlist COMMA nm eidlist_opt AS LP select RP */
> -#line 1521 "parse.y"
> +#line 1522 "parse.y"
> {
>   yymsp[-7].minor.yy151 = sqlite3WithAdd(pParse, yymsp[-7].minor.yy151, &yymsp[-5].minor.yy0, yymsp[-4].minor.yy382, yymsp[-1].minor.yy279);
> }
> -#line 3718 "parse.c"
> +#line 3719 "parse.c"
>         break;
>       default:
>       /* (257) input ::= ecmd */ yytestcase(yyruleno==257);
> @@ -3825,7 +3826,7 @@ static void yy_syntax_error(
>   } else {
>     sqlite3ErrorMsg(pParse, "near \"%T\": syntax error", &TOKEN);
>   }
> -#line 3829 "parse.c"
> +#line 3830 "parse.c"
> /************ End %syntax_error code ******************************************/
>   sqlite3ParserARG_STORE; /* Suppress warning about unused %extra_argument variable */
> }
> diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
> index 914fc53b8..11db58d94 100644
> --- a/src/box/sql/parse.y
> +++ b/src/box/sql/parse.y
> @@ -1350,12 +1350,14 @@ cmd ::= createkw trigger_decl(A) BEGIN trigger_cmd_list(S) END(Z). {
>   Token all;
>   all.z = A.z;
>   all.n = (int)(Z.z - A.z) + Z.n;
> +  pParse->initiateTTrans = false;
>   sqlite3FinishTrigger(pParse, S, &all);
> }
> 
> trigger_decl(A) ::= TRIGGER ifnotexists(NOERR) nm(B)
>                     trigger_time(C) trigger_event(D)
>                     ON fullname(E) foreach_clause when_clause(G). {
> +  pParse->initiateTTrans = false;
>   sqlite3BeginTrigger(pParse, &B, C, D.a, D.b, E, G, NOERR);
>   A = B; /*A-overwrites-T*/
> }
> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> index a194a6e72..96609b5ca 100644
> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -2920,7 +2920,6 @@ case OP_Savepoint: {
> 				}
> 				if (isSchemaChange) {
> 					sqlite3ExpirePreparedStatements(db);
> -					sqlite3ResetAllSchemasOfConnection(db);
> 					user_session->sql_flags |= SQLITE_InternChanges;
> 				}
> 			}
> diff --git a/test/sql-tap/trigger1.test.lua b/test/sql-tap/trigger1.test.lua
> index 93dd3aaac..40daba4d8 100755
> --- a/test/sql-tap/trigger1.test.lua
> +++ b/test/sql-tap/trigger1.test.lua
> @@ -1,6 +1,6 @@
> #!/usr/bin/env tarantool
> test = require("sqltester")
> -test:plan(31)
> +test:plan(33)
> 
> --!./tcltestrunner.lua
> -- The author disclaims copyright to this source code.  In place of
> @@ -848,6 +848,29 @@ test:do_catchsql_test(
>         -- </trigger1-16.7>
>     })
> 
> +test:do_catchsql_test(
> +    "trigger1-16.8",
> +    [[
> +        BEGIN;
> +          CREATE TRIGGER tr168 INSERT ON tA BEGIN
> +            INSERT INTO t16 values(1);
> +          END;
> +   ]], {
> +        1, [[Space _trigger does not support multi-statement transactions]]
> +})
> +
> +test:execsql [[
> +    ROLLBACK;
> +]]
> +
> +test:do_catchsql_test(
> +    "trigger1-16.9",
> +    [[
> +        BEGIN;
> +          DROP TRIGGER t16err3;
> +   ]], {
> +        1, [[Space _trigger does not support multi-statement transactions]]
> +})
> -- MUST_WORK_TEST
> -- #-------------------------------------------------------------------------
> -- # Test that bug [34cd55d68e0e6e7c] has been fixed.
> diff --git a/test/sql-tap/trigger9.test.lua b/test/sql-tap/trigger9.test.lua
> index af8181811..840d184bf 100755
> --- a/test/sql-tap/trigger9.test.lua
> +++ b/test/sql-tap/trigger9.test.lua
> @@ -66,10 +66,10 @@ test:do_execsql_test(
> test:do_execsql_test(
>     "trigger9-1.2.1",
>     [[
> +        CREATE TRIGGER trig1 BEFORE DELETE ON t1 BEGIN
> +          INSERT INTO t2 VALUES(old.x);
> +        END;
>         BEGIN;
> -          CREATE TRIGGER trig1 BEFORE DELETE ON t1 BEGIN
> -            INSERT INTO t2 VALUES(old.x);
> -          END;
>           DELETE FROM t1;
>           SELECT * FROM t2;
>     ]], {
> @@ -101,10 +101,11 @@ test:do_execsql_test(
> test:do_execsql_test(
>     "trigger9-1.3.1",
>     [[
> +        DROP TRIGGER IF EXISTS trig1;
> +        CREATE TRIGGER trig1 BEFORE DELETE ON t1 BEGIN
> +          INSERT INTO t2 VALUES(old.x);
> +        END;
>         BEGIN;
> -          CREATE TRIGGER trig1 BEFORE DELETE ON t1 BEGIN
> -            INSERT INTO t2 VALUES(old.x);
> -          END;
>           DELETE FROM t1;
>           SELECT * FROM t2;
>     ]], {
> @@ -136,10 +137,11 @@ test:do_execsql_test(
> test:do_execsql_test(
>     "trigger9-1.4.1",
>     [[
> +        DROP TRIGGER IF EXISTS trig1;
> +        CREATE TRIGGER trig1 BEFORE DELETE ON t1 WHEN old.x='1' BEGIN
> +          INSERT INTO t2 VALUES(old.x);
> +        END;
>         BEGIN;
> -          CREATE TRIGGER trig1 BEFORE DELETE ON t1 WHEN old.x='1' BEGIN
> -            INSERT INTO t2 VALUES(old.x);
> -          END;
>           DELETE FROM t1;
>           SELECT * FROM t2;
>     ]], {
> @@ -171,10 +173,11 @@ test:do_execsql_test(
> test:do_execsql_test(
>     "trigger9-1.5.1",
>     [[
> +        DROP TRIGGER IF EXISTS trig1;
> +        CREATE TRIGGER trig1 BEFORE UPDATE ON t1 BEGIN
> +          INSERT INTO t2 VALUES(old.x);
> +        END;
>         BEGIN;
> -          CREATE TRIGGER trig1 BEFORE UPDATE ON t1 BEGIN
> -            INSERT INTO t2 VALUES(old.x);
> -          END;
>           UPDATE t1 SET y = '';
>           SELECT * FROM t2;
>     ]], {
> @@ -206,10 +209,11 @@ test:do_execsql_test(
> test:do_execsql_test(
>     "trigger9-1.6.1",
>     [[
> +        DROP TRIGGER IF EXISTS trig1;
> +        CREATE TRIGGER trig1 BEFORE UPDATE ON t1 BEGIN
> +          INSERT INTO t2 VALUES(old.x);
> +        END;
>         BEGIN;
> -          CREATE TRIGGER trig1 BEFORE UPDATE ON t1 BEGIN
> -            INSERT INTO t2 VALUES(old.x);
> -          END;
>           UPDATE t1 SET y = '';
>           SELECT * FROM t2;
>     ]], {
> @@ -241,10 +245,11 @@ test:do_execsql_test(
> test:do_execsql_test(
>     "trigger9-1.7.1",
>     [[
> +        DROP TRIGGER IF EXISTS trig1;
> +        CREATE TRIGGER trig1 BEFORE UPDATE ON t1 WHEN old.x>='2' BEGIN
> +          INSERT INTO t2 VALUES(old.x);
> +        END;
>         BEGIN;
> -          CREATE TRIGGER trig1 BEFORE UPDATE ON t1 WHEN old.x>='2' BEGIN
> -            INSERT INTO t2 VALUES(old.x);
> -          END;
>           UPDATE t1 SET y = '';
>           SELECT * FROM t2;
>     ]], {
> @@ -290,10 +295,11 @@ test:do_execsql_test(
>     "trigger9-3.2",
>     [[
>         CREATE VIEW v1 AS SELECT * FROM t3;
> +        DROP TRIGGER IF EXISTS trig1;
> +        CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
> +          INSERT INTO t2 VALUES(old.a);
> +        END;
>         BEGIN;
> -          CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
> -            INSERT INTO t2 VALUES(old.a);
> -          END;
>           UPDATE v1 SET b = 'hello';
>           SELECT * FROM t2;
>         ROLLBACK;
> @@ -314,10 +320,11 @@ test:do_test(
>         --
>         return test:execsql([[
>             CREATE VIEW v1 AS SELECT a, b AS c FROM t3 WHERE c > 'one';
> +            DROP TRIGGER IF EXISTS trig1;
> +            CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
> +              INSERT INTO t2 VALUES(old.a);
> +            END;
>             BEGIN;
> -              CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
> -                INSERT INTO t2 VALUES(old.a);
> -              END;
>               UPDATE v1 SET c = 'hello';
>               SELECT * FROM t2;
>             ROLLBACK;
> @@ -333,11 +340,12 @@ test:do_execsql_test(
>     "trigger9-3.4",
>     [[
>         CREATE VIEW v1 AS SELECT DISTINCT a, b FROM t3;
> +        DROP TRIGGER IF EXISTS trig1;
> +        CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
> +          INSERT INTO t2 VALUES(old.a);
> +        END;
>         BEGIN;
>           INSERT INTO t3 VALUES(4, 3, 'three');
> -          CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
> -            INSERT INTO t2 VALUES(old.a);
> -          END;
>           UPDATE v1 SET b = 'hello';
>           SELECT * FROM t2;
>         ROLLBACK;
> @@ -352,11 +360,12 @@ test:do_execsql_test(
>     "trigger9-3.5",
>     [[
>         CREATE VIEW v1 AS SELECT a, b FROM t3 EXCEPT SELECT 1, 'one';
> +        DROP TRIGGER IF EXISTS trig1;
> +        CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
> +          INSERT INTO t2 VALUES(old.a);
> +        END;
>         BEGIN;
>           INSERT INTO t3 VALUES(5, 1, 'uno');
> -          CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
> -            INSERT INTO t2 VALUES(old.a);
> -          END;
>           UPDATE v1 SET b = 'hello';
>           SELECT * FROM t2;
>         ROLLBACK;
> @@ -372,11 +381,12 @@ test:do_execsql_test(
>     [[
>         CREATE VIEW v1 AS
>           SELECT sum(a) AS a, max(b) AS b FROM t3 GROUP BY t3.a HAVING b>'two';
> +        DROP TRIGGER IF EXISTS trig1;
> +        CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
> +          INSERT INTO t2 VALUES(old.a);
> +        END;
>         BEGIN;
>           INSERT INTO t3 VALUES(6, 1, 'zero');
> -          CREATE TRIGGER trig1 INSTEAD OF UPDATE ON v1 BEGIN
> -            INSERT INTO t2 VALUES(old.a);
> -          END;
>           UPDATE v1 SET b = 'hello';
>           SELECT * FROM t2;
>         ROLLBACK;
> @@ -436,5 +446,4 @@ test:do_execsql_test(
>     })
> 
> 
> -
> test:finish_test()
> -- 
> 2.11.0
> 
>