From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id D995228FC0 for ; Thu, 15 Mar 2018 14:22:15 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id dCaqBkw5770h for ; Thu, 15 Mar 2018 14:22:15 -0400 (EDT) Received: from smtp55.i.mail.ru (smtp55.i.mail.ru [217.69.128.35]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id E5F3F2904C for ; Thu, 15 Mar 2018 14:22:14 -0400 (EDT) From: Kirill Yukhin Subject: [tarantool-patches] [PATCH 1/2] sql: refactor initialization routines Date: Thu, 15 Mar 2018 21:21:39 +0300 Message-Id: <0bf2e1909c97298d450fc4e0612cf10a1e4403d0.1521137308.git.kyukhin@tarantool.org> In-Reply-To: References: In-Reply-To: References: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: 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