[tarantool-patches] Re: [PATCH] sql: get rid off tnum field of struct Table

  • From: "n.pettik" <korablev@xxxxxxxxxxxxx>
  • To: tarantool-patches@xxxxxxxxxxxxx
  • Date: Fri, 20 Jul 2018 17:07:03 +0300


diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 3af9f9a..504701d 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -384,7 +384,7 @@ sqlite3Insert(Parse * pParse,     /* Parser context */
      if (pTab == NULL)
              goto insert_cleanup;

-     space_id = SQLITE_PAGENO_TO_SPACEID(pTab->tnum);
+     space_id = pTab->def->id;

      /* Figure out if we have any triggers and if the table being
       * inserted into is a view
@@ -742,7 +742,7 @@ sqlite3Insert(Parse * pParse,     /* Parser context */
                              if (i == pTab->iAutoIncPKey) {
                                      sqlite3VdbeAddOp2(v,
                                                        OP_NextAutoincValue,
-                                                       pTab->tnum,
+                                                       
SQLITE_PAGENO_FROM_SPACEID_AND_INDEXID(pTab->def->id, 0),

It looks disgusting.. Lets also refactor OP_NextAutoincValue so that
it accepts raw space_id. After that, remove these SQLITE_PAGENO
macroses at all.

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index cabe22b..0c838fa 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -432,9 +432,8 @@ sqlite3Pragma(Parse * pParse, Token * pId,        /* 
First part of [schema.]id field */
                      for (i = sqliteHashFirst(&db->pSchema->tblHash); i;
                           i = sqliteHashNext(i)) {
                              Table *pTab = sqliteHashData(i);
-                             uint32_t space_id =
-                                     SQLITE_PAGENO_TO_SPACEID(pTab->tnum);
-                             struct space *space = space_by_id(space_id);
+                             struct space *space;
+                             space = space_by_id(pTab->def->id);

Squash two lines into one:
struct space *space = space_by_id(pTab->def->id);

@@ -161,18 +142,18 @@ extern int
sqlite3InitDatabase(sqlite3 * db)
{
      int rc;
-     InitData initData;
+     struct init_data init;

      assert(db->pSchema != NULL);

-     memset(&initData, 0, sizeof(InitData));
-     initData.db = db;
+     memset(&init, 0, sizeof(init));
+     init.db = db;

      /* Load schema from Tarantool - into the primary db only. */
-     tarantoolSqlite3LoadSchema(&initData);
+     tarantoolSqlite3LoadSchema(&init);

-     if (initData.rc) {
-             rc = initData.rc;
+     if (init.rc) {
+             rc = init.rc;
              goto error_out;
      }

-/*
+/**
 * A pointer to this structure is used to communicate information
- * from sqlite3Init and OP_ParseSchema into the sqlite3InitCallback.
- */
-typedef struct {
-     sqlite3 *db;            /* The database being initialized */
-     char **pzErrMsg;        /* Error message stored here */
-     int rc;                 /* Result code stored here */
-} InitData;
+ * from sqlite3Init and OP_ParseSchema into the sql_init_callback.
+ */
+struct init_data {
+     /* The database being initialized */
+     sqlite3 *db;
+     /* Error message stored here */
+     char **pzErrMsg;
+     /* Result code stored here */

Comments inside struct definition should look like:
/** … */
And put dot at the end of sentences.


diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h
index f043a60..b34e671 100644
--- a/src/box/sql/tarantoolInt.h
+++ b/src/box/sql/tarantoolInt.h
@@ -44,7 +44,7 @@
  ((pgno) & 1023)

/* Load database schema from Tarantool. */
-void tarantoolSqlite3LoadSchema(InitData * init);
+void tarantoolSqlite3LoadSchema(struct init_data * init);

struct init_data *init — remove extra space.

@@ -2178,8 +2171,8 @@ whereLoopInsert(WhereLoopBuilder * pBuilder, WhereLoop 
* pTemplate)
      }
      rc = whereLoopXfer(db, p, pTemplate);
      Index *pIndex = p->pIndex;
-     if (pIndex && pIndex->tnum == 0)
-             p->pIndex = 0;
+     if (pIndex != NULL && pIndex->pTable->def->opts.is_view)
+             p->pIndex = NULL;
      return rc;
}

These conditions don’t seem to be equivalent: view features ordinary space_id 
AFAIK.


@@ -2347,8 +2340,8 @@ whereRangeVectorLen(Parse * pParse,     /* Parsing 
context */
 * terms only. If it is modified, this value is restored before this
 * function returns.
 *
- * If pProbe->tnum==0, that means pIndex is a fake index used for the
- * INTEGER PRIMARY KEY.
+ * If pProbe->def->space_id==0, that means pIndex is a fake index
+ * used for the INTEGER PRIMARY KEY.

But in fact you use another sign of fake index:
iid == UINT32_MAX

 */
static int
whereLoopAddBtreeIndex(WhereLoopBuilder * pBuilder,   /* The WhereLoop 
factory */
@@ -2391,12 +2384,11 @@ whereLoopAddBtreeIndex(WhereLoopBuilder * pBuilder,   
/* The WhereLoop factory */
              opMask =
                  WO_EQ | WO_IN | WO_GT | WO_GE | WO_LT | WO_LE | WO_ISNULL;
      }
-     struct space *space =
-             space_by_id(SQLITE_PAGENO_TO_SPACEID(pProbe->tnum));
+     struct space *space = space_by_id(pProbe->def->space_id);
      struct index *idx = NULL;
      struct index_stat *stat = NULL;
-     if (space != NULL) {
-             idx = space_index(space, 
SQLITE_PAGENO_TO_INDEXID(pProbe->tnum));
+     if (space != NULL && pProbe->def->iid != UINT32_MAX) {
+             idx = space_index(space, pProbe->def->iid);
              assert(idx != NULL);
              stat = idx->def->opts.stat;
      }

@@ -2577,7 +2569,7 @@ whereLoopAddBtreeIndex(WhereLoopBuilder * pBuilder,     
/* The WhereLoop factory */
                      assert(eOp & (WO_ISNULL | WO_EQ | WO_IN));

                      assert(pNew->nOut == saved_nOut);
-                     if (pTerm->truthProb <= 0 && pProbe->tnum != 0 ) {
+                     if (pTerm->truthProb <= 0 && 
!pProbe->pTable->def->opts.is_view) {

The same as previous remark: these conditions don’t seem to be equivalent



Other related posts:

  1. Home
  2. tarantool-patches
  3. Archive
  4. 07-2018