[tarantool-patches] [PATCH v1 1/1] sql: refactor vdbe_emit_open_cursor calls

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

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.
---
https://github.com/tarantool/tarantool/compare/kshch/kshch/vdbe_emit_open_cursor-refactor
over branch
https://github.com/tarantool/tarantool/issues/2847

 src/box/sql/analyze.c |  3 +-
 src/box/sql/build.c   |  4 ++-
 src/box/sql/expr.c    |  5 ++-
 src/box/sql/fkey.c    |  3 +-
 src/box/sql/insert.c  | 95 ++++++++++++++++++++++++++++++---------------------
 src/box/sql/select.c  |  4 +--
 src/box/sql/where.c   | 10 ++++--
 7 files changed, 75 insertions(+), 49 deletions(-)

diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 336d146..ff3e735 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]));
 	}
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 1c00842..82447dc 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1108,6 +1108,7 @@ vdbe_emit_open_cursor(struct Parse *parse_context, int cursor, int index_id,
 		      struct space *space)
 {
 	assert(space != NULL);
+	assert(index_id == SQLITE_PAGENO_TO_INDEXID(index_id));
 	struct Vdbe *vdbe = parse_context->pVdbe;
 	int space_ptr_reg = ++parse_context->nMem;
 	sqlite3VdbeAddOp4(vdbe, OP_LoadPtr, 0, space_ptr_reg, 0, (void*)space,
@@ -2448,7 +2449,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/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..77567e7 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -179,41 +179,50 @@ 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.
+/**
+ * Return true 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.
+ *
+ * @param parser Parse context.
+ * @param table Table AST object.
+ * @retval true or false
  */
-static int
-readsTable(Parse * p, Table * pTab)
+static bool
+vdbe_has_table_read(struct Parse *parser, 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 *pOp = sqlite3VdbeGetOp(v, i);
+		assert(pOp != 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 (pOp->opcode == OP_OpenRead || pOp->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 == space_by_id(table->def->id))
+				return true;
+
+			int idx_id = pOp->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 +535,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) {
@@ -1934,11 +1947,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/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/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