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 D6D522A328 for ; Tue, 20 Mar 2018 06:57:00 -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 T_5eLJdJOnCp for ; Tue, 20 Mar 2018 06:57:00 -0400 (EDT) Received: from smtp2.mail.ru (smtp2.mail.ru [94.100.179.91]) (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 6C0E72A393 for ; Tue, 20 Mar 2018 06:57:00 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 11.2 \(3445.5.20\)) Subject: [tarantool-patches] Re: [PATCH 1/2] sql: Refactor initialization routines From: "v.shpilevoy@tarantool.org" In-Reply-To: <20180320083532.19266-1-kyukhin@tarantool.org> Date: Tue, 20 Mar 2018 13:56:22 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <853FB846-529F-4897-876D-D595D9FE0C14@tarantool.org> References: <20180320082848.pf5ebs6rlkrxfxj7@tarantool.org> <20180320083532.19266-1-kyukhin@tarantool.org> 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 Ack. > 20 =D0=BC=D0=B0=D1=80=D1=82=D0=B0 2018 =D0=B3., =D0=B2 11:35, Kirill = Yukhin =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(= =D0=B0): >=20 > 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. >=20 > 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. >=20 > 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(-) >=20 > 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 =3D = SQLITE_ShortColNames > void > sql_init() > { > - int rc; > default_flags |=3D default_sql_flags; >=20 > - rc =3D sqlite3_open("", &db); > - if (rc !=3D SQLITE_OK) { > - panic("failed to initialize SQL subsystem"); > - } else { > - /* XXX */ > - } > - > current_session()->sql_flags |=3D default_sql_flags; >=20 > - rc =3D sqlite3Init(db); > - if (rc !=3D SQLITE_OK) { > - panic("database initialization failed"); > - } else { > - /* XXX */ > - } > + if (sql_init_db(&db) !=3D SQLITE_OK) > + panic("failed to initialize SQL subsystem"); >=20 > assert(db !=3D 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; >=20 > - /* Read the database schema. If an error occurs, leave an error = message > - * and code in pParse and return NULL. > - */ > - if (SQLITE_OK !=3D sqlite3ReadSchema(pParse)) { > - return; > - } > + assert(db->pSchema !=3D NULL); >=20 > if (pName =3D=3D 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; >=20 > - /* Read the database schema. If an error occurs, leave an error = message > - * and code in pParse and return NULL. > - */ > - if (SQLITE_OK !=3D sqlite3ReadSchema(pParse)) { > - return 0; > - } > + assert(pParse->db->pSchema !=3D NULL); >=20 > p =3D sqlite3FindTable(pParse->db, zName); > if (p =3D=3D 0) { > @@ -647,13 +642,7 @@ sqlite3StartTable(Parse *pParse, Token *pName, = int noErr) > if (zName =3D=3D 0) > return; >=20 > - /* > - * 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 !=3D sqlite3ReadSchema(pParse)) > - goto begin_table_error; > + assert(db->pSchema !=3D NULL); > pTable =3D 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 =3D=3D 0); > assert(pName->nSrc =3D=3D 1); > - if (sqlite3ReadSchema(pParse)) > - goto exit_drop_table; > + assert(db->pSchema !=3D NULL); > if (noErr) > db->suppressErr++; > assert(isView =3D=3D 0 || isView =3D=3D LOCATE_VIEW); > @@ -2878,9 +2866,7 @@ sqlite3CreateIndex(Parse * pParse, /* All = information about this parse */ > goto exit_create_index; > sqlite3VdbeCountChanges(v); > } > - if (SQLITE_OK !=3D sqlite3ReadSchema(pParse)) { > - goto exit_create_index; > - } > + assert(db->pSchema !=3D NULL); >=20 > /* > * 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 =3D=3D 1); > - if (SQLITE_OK !=3D sqlite3ReadSchema(pParse)) { > - goto exit_drop_index; > - } > + assert(db->pSchema !=3D NULL); >=20 > assert(pName2->n > 0); > zTableName =3D sqlite3NameFromToken(db, pName2); > @@ -4151,12 +4135,7 @@ sqlite3Reindex(Parse * pParse, Token * pName1, = Token * pName2) > Index *pIndex; /* An index associated with pTab */ > sqlite3 *db =3D pParse->db; /* The database connection */ >=20 > - /* Read the database schema. If an error occurs, leave an error = message > - * and code in pParse and return NULL. > - */ > - if (SQLITE_OK !=3D sqlite3ReadSchema(pParse)) { > - return; > - } > + assert(db->pSchema !=3D NULL); >=20 > if (pName1 =3D=3D 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 =3D=3D 0) { > sqlite3ExpirePreparedStatements(db); > sqlite3ResetAllSchemasOfConnection(db); > + > + db->init.busy =3D 1; > + db->pSchema =3D sqlite3SchemaCreate(db); > + int rc =3D sqlite3InitDatabase(db); > + if (rc !=3D SQLITE_OK) > + sqlite3SchemaClear(db); > + db->init.busy =3D 0; > + if (rc =3D=3D SQLITE_OK) > + sqlite3CommitInternalChanges(); > } >=20 > /* Any deferred constraint violations have now been resolved. */ > @@ -2246,85 +2255,35 @@ sqlite3ParseUri(const char *zDefaultVfs, = /* VFS to use if no "vfs=3Dxxx" query opt > return rc; > } >=20 > -/* > - * 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 =3D 0; /* Filename argument to pass to = BtreeOpen() */ > - char *zErrMsg =3D 0; /* Error message from sqlite3ParseUri() = */ >=20 > #ifdef SQLITE_ENABLE_API_ARMOR > if (ppDb =3D=3D 0) > return SQLITE_MISUSE_BKPT; > #endif > - *ppDb =3D 0; > #ifndef SQLITE_OMIT_AUTOINIT > rc =3D sqlite3_initialize(); > if (rc) > return rc; > #endif >=20 > - /* 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 =3D=3D 0x01); > - assert(SQLITE_OPEN_READWRITE =3D=3D 0x02); > - assert(SQLITE_OPEN_CREATE =3D=3D 0x04); > - testcase((1 << (flags & 7)) =3D=3D 0x02); /* READONLY */ > - testcase((1 << (flags & 7)) =3D=3D 0x04); /* READWRITE */ > - testcase((1 << (flags & 7)) =3D=3D 0x40); /* READWRITE | = CREATE */ > - if (((1 << (flags & 7)) & 0x46) =3D=3D 0) { > - return SQLITE_MISUSE_BKPT; /* IMP: R-65497-44594 */ > - } > - > if (sqlite3GlobalConfig.bCoreMutex =3D=3D 0) { > isThreadsafe =3D 0; > - } else if (flags & SQLITE_OPEN_NOMUTEX) { > - isThreadsafe =3D 0; > - } else if (flags & SQLITE_OPEN_FULLMUTEX) { > - isThreadsafe =3D 1; > } else { > isThreadsafe =3D sqlite3GlobalConfig.bFullMutex; > } > - if (flags & SQLITE_OPEN_PRIVATECACHE) { > - flags &=3D ~SQLITE_OPEN_SHAREDCACHE; > - } else if (sqlite3GlobalConfig.sharedCacheEnabled) { > - flags |=3D 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 &=3D ~(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 |=3D SQLITE_OPEN_MEMORY; > /* Allocate the sqlite data structure */ > db =3D sqlite3MallocZero(sizeof(sqlite3)); > if (db =3D=3D 0) > @@ -2341,24 +2300,14 @@ openDatabase(const char *zFilename, /* = Database filename UTF-8 encoded */ > db->errMask =3D 0xff; > db->magic =3D SQLITE_MAGIC_BUSY; >=20 > + db->pVfs =3D sqlite3_vfs_find(0); > + > assert(sizeof(db->aLimit) =3D=3D sizeof(aHardLimit)); > memcpy(db->aLimit, aHardLimit, sizeof(db->aLimit)); > db->aLimit[SQLITE_LIMIT_WORKER_THREADS] =3D = SQLITE_DEFAULT_WORKER_THREADS; > db->szMmap =3D sqlite3GlobalConfig.szMmap; > db->nMaxSorterMmap =3D 0x7FFFFFFF; >=20 > - db->openFlags =3D flags; > - /* Parse the filename/URI argument. */ > - rc =3D sqlite3ParseUri(zVfs, zFilename, > - &flags, &db->pVfs, &zOpen, &zErrMsg); > - if (rc !=3D SQLITE_OK) { > - if (rc =3D=3D SQLITE_NOMEM) > - sqlite3OomFault(db); > - sqlite3ErrorWithMsg(db, rc, zErrMsg ? "%s" : 0, = zErrMsg); > - sqlite3_free(zErrMsg); > - goto opendb_out; > - } > - > db->pSchema =3D NULL; > db->magic =3D 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); >=20 > - opendb_out: > + if (rc =3D=3D SQLITE_OK) { > + struct session *user_session =3D current_session(); > + int commit_internal =3D !(user_session->sql_flags > + & SQLITE_InternChanges); > + > + assert(db->init.busy =3D=3D 0); > + db->init.busy =3D 1; > + db->pSchema =3D sqlite3SchemaCreate(db); > + rc =3D sqlite3InitDatabase(db); > + if (rc !=3D SQLITE_OK) > + sqlite3SchemaClear(db); > + db->init.busy =3D 0; > + if (rc =3D=3D SQLITE_OK && commit_internal) > + sqlite3CommitInternalChanges(); > + } > +opendb_out: > if (db) { > assert(db->mutex !=3D 0 || isThreadsafe =3D=3D 0 > || sqlite3GlobalConfig.bFullMutex =3D=3D 0); > @@ -2439,10 +2403,10 @@ openDatabase(const char *zFilename, /* = Database filename UTF-8 encoded */ > if (rc =3D=3D SQLITE_NOMEM) { > sqlite3_close(db); > db =3D 0; > - } else if (rc !=3D SQLITE_OK) { > + } else if (rc !=3D SQLITE_OK) > db->magic =3D SQLITE_MAGIC_SICK; > - } > - *ppDb =3D db; > + > + *out_db =3D 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 =3D=3D SQLITE_OK) { > - const char *zHexKey =3D sqlite3_uri_parameter(zOpen, = "hexkey"); > - if (zHexKey && zHexKey[0]) { > - u8 iByte; > - int i; > - char zKey[40]; > - for (i =3D 0, iByte =3D 0; > - i < sizeof(zKey) * 2 > - && sqlite3Isxdigit(zHexKey[i]); i++) { > - iByte =3D > - (iByte << 4) + = sqlite3HexToInt(zHexKey[i]); > - if ((i & 1) !=3D 0) > - zKey[i / 2] =3D iByte; > - } > - sqlite3_key_v2(db, 0, zKey, i / 2); > - } > - } > -#endif > - sqlite3_free(zOpen); > - return rc & 0xff; > -} >=20 > -/* > - * 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; > } >=20 > /* > 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 */ >=20 > /* Make sure the database schema is loaded if the pragma = requires that */ > if ((pPragma->mPragFlg & PragFlg_NeedSchema) !=3D 0) { > - if (sqlite3ReadSchema(pParse)) > - goto pragma_out; > + assert(db->pSchema !=3D NULL); > } > /* Register the result column names for pragmas that return = results */ > if ((pPragma->mPragFlg & PragFlg_NoColumns) =3D=3D 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) > } >=20 > /* > - * 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 =3D current_session(); > - int commit_internal =3D !(user_session->sql_flags & = SQLITE_InternChanges); > - > - assert(sqlite3_mutex_held(db->mutex)); > - assert(db->init.busy =3D=3D 0); > - rc =3D SQLITE_OK; > - db->init.busy =3D 1; > - if (!db->pSchema) { > - db->pSchema =3D sqlite3SchemaCreate(db); > - rc =3D sqlite3InitDatabase(db); > - if (rc) { > - sqlite3SchemaClear(db); > - } > - } > - > - db->init.busy =3D 0; > - if (rc =3D=3D 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 =3D SQLITE_OK; > - sqlite3 *db =3D pParse->db; > - assert(sqlite3_mutex_held(db->mutex)); > - if (!db->init.busy) { > - rc =3D sqlite3Init(db); > - } > - if (rc !=3D SQLITE_OK) { > - pParse->rc =3D 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); >=20 > 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 >=20 > 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) >=20 > if (db->mallocFailed) > goto drop_trigger_cleanup; > - if (SQLITE_OK !=3D sqlite3ReadSchema(pParse)) { > - goto drop_trigger_cleanup; > - } > + assert(db->pSchema !=3D 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 > --=20 > 2.11.0 >=20 >=20