[tarantool-patches] Re: [PATCH 1/2] sql: Refactor initialization routines
v.shpilevoy at tarantool.org
v.shpilevoy at tarantool.org
Tue Mar 20 13:56:22 MSK 2018
Ack.
> 20 марта 2018 г., в 11:35, Kirill Yukhin <kyukhin at 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
>
>
More information about the Tarantool-patches
mailing list