[tarantool-patches] [PATCH v2 3/4] sql: refactor vdbe_emit_open_cursor calls

  • From: Kirill Shcherbatov <kshcherbatov@xxxxxxxxxxxxx>
  • To: tarantool-patches@xxxxxxxxxxxxx
  • Date: Tue, 10 Jul 2018 20:08:10 +0300

Made vdbe_emit_open_cursor calls consistent:
now it uses index id everywhere.
This required to change a way to detect that
VDBE has openned Read cursor to specified table
in vdbe_has_table_read to write result of insert
in temp table if required.
---
 src/box/sql/analyze.c   |   6 +--
 src/box/sql/build.c     |   3 +-
 src/box/sql/delete.c    |   8 +---
 src/box/sql/expr.c      |   5 ++-
 src/box/sql/fkey.c      |   3 +-
 src/box/sql/insert.c    | 100 +++++++++++++++++++++++++++++-------------------
 src/box/sql/pragma.c    |   7 +++-
 src/box/sql/select.c    |   4 +-
 src/box/sql/sqliteInt.h |   6 +--
 src/box/sql/vdbe.c      |   5 +--
 src/box/sql/where.c     |  10 ++++-
 11 files changed, 92 insertions(+), 65 deletions(-)

diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 336d146..067a86e 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -159,8 +159,7 @@ vdbe_emit_stat_space_open(struct Parse *parse, int 
stat_cursor,
        /* Open the sql_stat tables for writing. */
        for (uint i = 0; i < lengthof(space_names); ++i) {
                uint32_t id = space_ids[i];
-               int tnum = SQLITE_PAGENO_FROM_SPACEID_AND_INDEXID(id, 0);
-               vdbe_emit_open_cursor(parse, stat_cursor + i, tnum,
+               vdbe_emit_open_cursor(parse, stat_cursor + i, 0,
                                      space_by_id(id));
                VdbeComment((v, space_names[i]));
        }
@@ -893,10 +892,11 @@ analyzeOneTable(Parse * pParse,   /* Parser context */
                /* Open a read-only cursor on the index being analyzed. */
                struct space *space =
                        space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum));
+               int idx_id = SQLITE_PAGENO_TO_INDEXID(pIdx->tnum);
                assert(space != NULL);
                sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0,
                                  (void*)space, P4_SPACEPTR);
-               sqlite3VdbeAddOp3(v, OP_OpenRead, iIdxCur, pIdx->tnum,
+               sqlite3VdbeAddOp3(v, OP_OpenRead, iIdxCur, idx_id,
                                  space_ptr_reg);
                sql_vdbe_set_p4_key_def(pParse, pIdx);
                VdbeComment((v, "%s", pIdx->zName));
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 1c00842..5f7a35a 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -2448,7 +2448,8 @@ sqlite3RefillIndex(Parse * pParse, Index * pIndex, int 
memRootPage)
                sqlite3VdbeAddOp2(v, OP_Clear, SQLITE_PAGENO_TO_SPACEID(tnum),
                                  0);
        struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(tnum));
-       vdbe_emit_open_cursor(pParse, iIdx, tnum, space);
+       vdbe_emit_open_cursor(pParse, iIdx, SQLITE_PAGENO_TO_INDEXID(tnum),
+                             space);
        sqlite3VdbeChangeP5(v, memRootPage >= 0 ? OPFLAG_P2ISREG : 0);
 
        addr1 = sqlite3VdbeAddOp2(v, OP_SorterSort, iSorter, 0);
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 66dc0fc..d1c5935 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -340,18 +340,14 @@ sql_table_delete_from(struct Parse *parse, struct SrcList 
*tab_list,
                        sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0,
                                          (void *)space, P4_SPACEPTR);
 
-                       int tnum =
-                               SQLITE_PAGENO_FROM_SPACEID_AND_INDEXID(space_id,
-                                                                      0);
-                       sqlite3VdbeAddOp3(v, OP_OpenWrite, tab_cursor,
-                                         tnum, space_ptr_reg);
+                       sqlite3VdbeAddOp3(v, OP_OpenWrite, tab_cursor, 0,
+                                         space_ptr_reg);
                        struct key_def *def = key_def_dup(pk_def);
                        if (def == NULL) {
                                sqlite3OomFault(parse->db);
                                goto delete_from_cleanup;
                        }
                        sqlite3VdbeAppendP4(v, def, P4_KEYDEF);
-
                        VdbeComment((v, "%s", space->index[0]->def->name));
 
                        if (one_pass == ONEPASS_MULTI)
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 3183e3d..b1650cf 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2470,8 +2470,11 @@ sqlite3FindInIndex(Parse * pParse,       /* Parsing 
context */
                                                          P4_DYNAMIC);
                                        struct space *space =
                                                
space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum));
+                                       uint32_t idx_id =
+                                               SQLITE_PAGENO_TO_INDEXID(pIdx->
+                                                                        tnum);
                                        vdbe_emit_open_cursor(pParse, iTab,
-                                                             pIdx->tnum, 
space);
+                                                             idx_id, space);
                                        VdbeComment((v, "%s", pIdx->zName));
                                        assert(IN_INDEX_INDEX_DESC ==
                                               IN_INDEX_INDEX_ASC + 1);
diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c
index 6c75c47..face9cb 100644
--- a/src/box/sql/fkey.c
+++ b/src/box/sql/fkey.c
@@ -441,7 +441,8 @@ fkLookupParent(Parse * pParse,      /* Parse context */
                        int regRec = sqlite3GetTempReg(pParse);
                        struct space *space =
                                
space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum));
-                       vdbe_emit_open_cursor(pParse, iCur, pIdx->tnum, space);
+                       uint32_t idx_id = SQLITE_PAGENO_TO_INDEXID(pIdx->tnum);
+                       vdbe_emit_open_cursor(pParse, iCur, idx_id, space);
                        for (i = 0; i < nCol; i++) {
                                sqlite3VdbeAddOp2(v, OP_Copy,
                                                  aiCol[i] + 1 + regData,
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 58e159c..62b85d5 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -179,41 +179,51 @@ sqlite3TableAffinity(Vdbe * v, Table * pTab, int iReg)
        }
 }
 
-/*
- * Return non-zero if the table pTab in database or any of its indices
- * have been opened at any point in the VDBE program. This is used to see if
- * a statement of the form  "INSERT INTO <pTab> SELECT ..." can
- * run for the results of the SELECT.
+/**
+ * This routine is used to see if a statement of the form
+ * "INSERT INTO <table> SELECT ..." can run for the results of the
+ * SELECT.
+ *
+ * @param parser Parse context.
+ * @param table Table AST object.
+ * @retval  true if the table table in database or any of its
+ *          indices have been opened at any point in the VDBE
+ *          program.
+ * @retval  false else.
  */
-static int
-readsTable(Parse * p, Table * pTab)
+static bool
+vdbe_has_table_read(struct Parse *parser, const struct Table *table)
 {
-       Vdbe *v = sqlite3GetVdbe(p);
-       int i;
-       int iEnd = sqlite3VdbeCurrentAddr(v);
-
-       for (i = 1; i < iEnd; i++) {
-               VdbeOp *pOp = sqlite3VdbeGetOp(v, i);
-               assert(pOp != 0);
-               /* Currently, there is no difference between
-                * Read and Write cursors.
+       struct Vdbe *v = sqlite3GetVdbe(parser);
+       int last_instr = sqlite3VdbeCurrentAddr(v);
+       for (int i = 1; i < last_instr; i++) {
+               struct VdbeOp *op = sqlite3VdbeGetOp(v, i);
+               assert(op != NULL);
+               /*
+                * Currently, there is no difference between Read
+                * and Write cursors.
                 */
-               if (pOp->opcode == OP_OpenRead ||
-                   pOp->opcode == OP_OpenWrite) {
-                       Index *pIndex;
-                       int tnum = pOp->p2;
-                       if (tnum == pTab->tnum) {
-                               return 1;
-                       }
-                       for (pIndex = pTab->pIndex; pIndex;
-                            pIndex = pIndex->pNext) {
-                               if (tnum == pIndex->tnum) {
-                                       return 1;
-                               }
+               if (op->opcode == OP_OpenRead || op->opcode == OP_OpenWrite) {
+                       assert(i > 1);
+                       struct VdbeOp *space_var_op =
+                               sqlite3VdbeGetOp(v, i - 1);
+                       assert(space_var_op != NULL);
+                       assert(space_var_op->opcode == OP_LoadPtr);
+                       struct space *space = space_var_op->p4.space;
+
+                       if (space->def->id == table->def->id)
+                               return true;
+
+                       int idx_id = op->p2;
+                       for (struct Index *pIndex = table->pIndex;
+                               pIndex != NULL; pIndex = pIndex->pNext) {
+                               if (idx_id ==
+                                   SQLITE_PAGENO_TO_INDEXID(pIndex->tnum))
+                                       return true;
                        }
                }
        }
-       return 0;
+       return false;
 }
 
 
@@ -526,16 +536,20 @@ sqlite3Insert(Parse * pParse,     /* Parser context */
                assert(pSelect->pEList);
                nColumn = pSelect->pEList->nExpr;
 
-               /* Set useTempTable to TRUE if the result of the SELECT 
statement
-                * should be written into a temporary table (template 4).  Set 
to
-                * FALSE if each output row of the SELECT can be written 
directly into
-                * the destination table (template 3).
+               /*
+                * Set useTempTable to TRUE if the result of the
+                * SELECT statement should be written into a
+                * temporary table (template 4). Set to FALSE if
+                * each output row of the SELECT can be written
+                * directly into the destination table
+                * (template 3).
                 *
-                * A temp table must be used if the table being updated is also 
one
-                * of the tables being read by the SELECT statement.  Also use a
-                * temp table in the case of row triggers.
+                * A temp table must be used if the table being
+                * updated is also one of the tables being read by
+                * the SELECT statement. Also use a temp table in
+                * the case of row triggers.
                 */
-               if (trigger != NULL || readsTable(pParse, pTab))
+               if (trigger != NULL || vdbe_has_table_read(pParse, pTab))
                        useTempTable = 1;
 
                if (useTempTable) {
@@ -1617,7 +1631,9 @@ sqlite3OpenTableAndIndices(Parse * pParse,        /* 
Parsing context */
                                p5 = 0;
                        }
                        if (aToOpen == 0 || aToOpen[i + 1]) {
-                               sqlite3VdbeAddOp3(v, op, iIdxCur, pIdx->tnum,
+                               int idx_id =
+                                       SQLITE_PAGENO_TO_INDEXID(pIdx->tnum);
+                               sqlite3VdbeAddOp3(v, op, iIdxCur, idx_id,
                                                  space_ptr_reg);
                                sql_vdbe_set_p4_key_def(pParse, pIdx);
                                sqlite3VdbeChangeP5(v, p5);
@@ -1934,11 +1950,15 @@ xferOptimization(Parse * pParse,        /* Parser 
context */
                assert(pSrcIdx);
                struct space *src_space =
                        space_by_id(SQLITE_PAGENO_TO_SPACEID(pSrcIdx->tnum));
-               vdbe_emit_open_cursor(pParse, iSrc, pSrcIdx->tnum, src_space);
+               vdbe_emit_open_cursor(pParse, iSrc,
+                                     SQLITE_PAGENO_TO_INDEXID(pSrcIdx->tnum),
+                                     src_space);
                VdbeComment((v, "%s", pSrcIdx->zName));
                struct space *dest_space =
                        space_by_id(SQLITE_PAGENO_TO_SPACEID(pDestIdx->tnum));
-               vdbe_emit_open_cursor(pParse, iDest, pDestIdx->tnum, 
dest_space);
+               vdbe_emit_open_cursor(pParse, iDest,
+                                     SQLITE_PAGENO_TO_INDEXID(pDestIdx->tnum),
+                                     dest_space);
                VdbeComment((v, "%s", pDestIdx->zName));
                addr1 = sqlite3VdbeAddOp2(v, OP_Rewind, iSrc, 0);
                VdbeCoverage(v);
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 31581b1..ca119e0 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -685,11 +685,14 @@ sqlite3Pragma(Parse * pParse, Token * pId,        /* 
First part of [schema.]id field */
                                                                         
pParent,
                                                                         
OP_OpenRead);
                                                } else {
+                                                       int idx_id =
+                                                               
SQLITE_PAGENO_TO_INDEXID(
+                                                                       pIdx->
+                                                                       tnum);
                                                        sqlite3VdbeAddOp3(v,
                                                                          
OP_OpenRead,
                                                                          i,
-                                                                         pIdx->
-                                                                         tnum,
+                                                                         
idx_id,
                                                                          0);
                                                        
sql_vdbe_set_p4_key_def(pParse,
                                                                                
pIdx);
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 52b3fdd..ceb7e34 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -6279,9 +6279,7 @@ sqlite3Select(Parse * pParse,             /* The parser 
context */
                                 * Open the cursor, execute the OP_Count,
                                 * close the cursor.
                                 */
-                               vdbe_emit_open_cursor(pParse, cursor,
-                                                     space->def->id << 10,
-                                                     space);
+                               vdbe_emit_open_cursor(pParse, cursor, 0, space);
                                sqlite3VdbeAddOp2(v, OP_Count, cursor,
                                                  sAggInfo.aFunc[0].iMem);
                                sqlite3VdbeAddOp1(v, OP_Close, cursor);
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 380c9c6..4a1eef0 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -3583,9 +3583,9 @@ void sqlite3EndTable(Parse *, Token *, Token *, Select *);
  *
  * @param parse_context Parse context.
  * @param cursor Number of cursor to be created.
- * @param index_id Encoded index id (encoding is void actually, so
- *        pas it as is). In future will be replaced with pointer
- *        to struct index.
+ * @param index_id index id. In future will be replaced with
+ *        pointer to struct index.
+ * @param space Pointer to space object.
  * @retval address of last opcode.
  */
 int
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 7a4d376..a0eb41b 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -3185,9 +3185,8 @@ case OP_ReopenIdx: {
        pIn3 = &aMem[pOp->p3];
        assert(pIn3->flags & MEM_Ptr);
        if (pCur && pCur->uc.pCursor->space == (struct space *) pIn3->u.p &&
-           pCur->uc.pCursor->index->def->iid == SQLITE_PAGENO_TO_INDEXID(p2)) {
+           pCur->uc.pCursor->index->def->iid == (uint32_t)p2)
                goto open_cursor_set_hints;
-       }
        /* If the cursor is not currently open or is open on a different
         * index, then fall through into OP_OpenRead to force a reopen
         */
@@ -3213,7 +3212,7 @@ case OP_OpenWrite:
        assert(pIn3->flags & MEM_Ptr);
        struct space *space = ((struct space *) pIn3->u.p);
        assert(space != NULL);
-       struct index *index = space_index(space, SQLITE_PAGENO_TO_INDEXID(p2));
+       struct index *index = space_index(space, p2);
        assert(index != NULL);
        /*
         * Since Tarantool iterator provides the full tuple,
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index 85143ed..6c07fec 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -4759,10 +4759,16 @@ sqlite3WhereBegin(Parse * pParse,       /* The parser 
context */
                        assert(iIndexCur >= 0);
                        if (op) {
                                if (pIx != NULL) {
+                                       uint32_t space_id =
+                                               SQLITE_PAGENO_TO_SPACEID(pIx->
+                                                                        tnum);
                                        struct space *space =
-                                               
space_by_id(SQLITE_PAGENO_TO_SPACEID(pIx->tnum));
+                                               space_by_id(space_id);
+                                       uint32_t idx_id =
+                                               SQLITE_PAGENO_TO_INDEXID(pIx->
+                                                                        tnum);
                                        vdbe_emit_open_cursor(pParse, iIndexCur,
-                                                             pIx->tnum, space);
+                                                             idx_id, space);
                                } else {
                                        vdbe_emit_open_cursor(pParse, iIndexCur,
                                                              idx_def->iid,
-- 
2.7.4


Other related posts: