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

  • From: "v.shpilevoy@xxxxxxxxxxxxx" <v.shpilevoy@xxxxxxxxxxxxx>
  • To: tarantool-patches@xxxxxxxxxxxxx
  • Date: Tue, 20 Mar 2018 13:56:22 +0300

Ack.

20 марта 2018 г., в 11:35, Kirill Yukhin <kyukhin@xxxxxxxxxxxxx> написал(а):

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




Other related posts: