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

[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.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