Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH] Fix ephemeral table next rowid generation
@ 2018-05-15 19:49 AKhatskevich
  2018-05-31 23:53 ` [tarantool-patches] " n.pettik
  0 siblings, 1 reply; 5+ messages in thread
From: AKhatskevich @ 2018-05-15 19:49 UTC (permalink / raw)
  To: tarantool-patches

branch: kh/gh-3297-fix_ephem_id-2
notes: Kiril asked me to store rowid in BtCursor (implemented in this patch)
       however I prefer to create a single static variable for the whole
       tarantool (rowid just need to be unique in a space) as implemented
       here: kh/gh-3297-fix_ephem_id-1

------ commit message ------------

Next `rowid` value is stored in BtCursor, and is incremented after each
insert to the ephemeral space.

Closes #3297
---
 src/box/sql.c                                 | 33 +--------------------------
 src/box/sql/cursor.h                          |  2 +-
 src/box/sql/insert.c                          | 16 +++----------
 src/box/sql/select.c                          | 10 ++++----
 src/box/sql/tarantoolInt.h                    |  2 --
 src/box/sql/vdbe.c                            | 25 ++++++++------------
 test/sql-tap/gh-3297_ephemeral_rowid.test.lua | 30 ++++++++++++++++++++++++
 7 files changed, 49 insertions(+), 69 deletions(-)
 create mode 100755 test/sql-tap/gh-3297_ephemeral_rowid.test.lua

diff --git a/src/box/sql.c b/src/box/sql.c
index 6104a6d0f..cd7ac8a50 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -423,6 +423,7 @@ int tarantoolSqlite3EphemeralCreate(BtCursor *pCur, uint32_t field_count,
 	}
 	pCur->space = ephemer_new_space;
 	pCur->index = *ephemer_new_space->index;
+	pCur->ephemeral_rowid = 0;
 
 	int unused;
 	return tarantoolSqlite3First(pCur, &unused);
@@ -1080,7 +1081,6 @@ key_alloc(BtCursor *cur, size_t key_size)
 		}
 		cur->key = new_key;
 	}
-	cur->nKey = key_size;
 	return 0;
 }
 
@@ -1621,37 +1621,6 @@ sql_debug_info(struct info_handler *h)
 	info_end(h);
 }
 
-/*
- * Extract maximum integer value from ephemeral space.
- * If index is empty - return 0 in max_id and success status.
- *
- * @param pCur Cursor pointing to ephemeral space.
- * @param fieldno Number of field from fetching tuple.
- * @param[out] max_id Fetched max value.
- *
- * @retval 0 on success, -1 otherwise.
- */
-int tarantoolSqlite3EphemeralGetMaxId(BtCursor *pCur, uint32_t fieldno,
-				       uint64_t *max_id)
-{
-	struct space *ephem_space = pCur->space;
-	assert(ephem_space);
-	struct index *primary_index = *ephem_space->index;
-
-	struct tuple *tuple;
-	if (index_max(primary_index, NULL, 0, &tuple) != 0) {
-		return SQL_TARANTOOL_ERROR;
-	}
-	if (tuple == NULL) {
-		*max_id = 0;
-		return SQLITE_OK;
-	}
-	if (tuple_field_u64(tuple, fieldno, max_id) == -1)
-		return SQL_TARANTOOL_ERROR;
-
-	return SQLITE_OK;
-}
-
 int
 tarantoolSqlNextSeqId(uint64_t *max_id)
 {
diff --git a/src/box/sql/cursor.h b/src/box/sql/cursor.h
index c55941784..6958a9f23 100644
--- a/src/box/sql/cursor.h
+++ b/src/box/sql/cursor.h
@@ -57,7 +57,7 @@ typedef struct BtCursor BtCursor;
  * for ordinary space, or to TEphemCursor for ephemeral space.
  */
 struct BtCursor {
-	i64 nKey;		/* Size of pKey, or last integer key */
+	u64 ephemeral_rowid;	/* Next unique id for Ephemeral space */
 	u8 curFlags;		/* zero or more BTCF_* flags defined below */
 	u8 eState;		/* One of the CURSOR_XXX constants (see below) */
 	u8 hints;		/* As configured by CursorSetHints() */
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 0b9e75a93..bddfdc50c 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -557,15 +557,12 @@ sqlite3Insert(Parse * pParse,	/* Parser context */
 			 *      M: ...
 			 */
 			int regRec;	/* Register to hold packed record */
-			int regTempId;	/* Register to hold temp table ID */
 			int regCopy;    /* Register to keep copy of registers from select */
 			int addrL;	/* Label "L" */
-			int64_t initial_pk = 0;
 
 			srcTab = pParse->nTab++;
 			regRec = sqlite3GetTempReg(pParse);
-			regCopy = sqlite3GetTempRange(pParse, nColumn);
-			regTempId = sqlite3GetTempReg(pParse);
+			regCopy = sqlite3GetTempRange(pParse, nColumn + 1);
 			struct key_def *def = key_def_new(nColumn + 1);
 			if (def == NULL) {
 				sqlite3OomFault(db);
@@ -573,16 +570,10 @@ sqlite3Insert(Parse * pParse,	/* Parser context */
 			}
 			sqlite3VdbeAddOp4(v, OP_OpenTEphemeral, srcTab, nColumn+1,
 					  0, (char*)def, P4_KEYDEF);
-			/* Create counter for rowid */
-			sqlite3VdbeAddOp4Dup8(v, OP_Int64,
-					      0 /* unused */,
-					      regTempId,
-					      0 /* unused */,
-					      (const unsigned char*) &initial_pk,
-					      P4_INT64);
 			addrL = sqlite3VdbeAddOp1(v, OP_Yield, dest.iSDParm);
 			VdbeCoverage(v);
-			sqlite3VdbeAddOp2(v, OP_AddImm, regTempId, 1);
+			sqlite3VdbeAddOp2(v, OP_NextIdEphemeral, srcTab,
+					  regCopy + nColumn);
 			sqlite3VdbeAddOp3(v, OP_Copy, regFromSelect, regCopy, nColumn-1);
 			sqlite3VdbeAddOp3(v, OP_MakeRecord, regCopy,
 					  nColumn + 1, regRec);
@@ -593,7 +584,6 @@ sqlite3Insert(Parse * pParse,	/* Parser context */
 			sqlite3VdbeGoto(v, addrL);
 			sqlite3VdbeJumpHere(v, addrL);
 			sqlite3ReleaseTempReg(pParse, regRec);
-			sqlite3ReleaseTempReg(pParse, regTempId);
 			sqlite3ReleaseTempRange(pParse, regCopy, nColumn);
 		}
 	} else {
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 1f27efdb5..5cd289ccc 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -1047,8 +1047,8 @@ selectInnerLoop(Parse * pParse,		/* The parser context */
 				int regRec = sqlite3GetTempReg(pParse);
 				/* Last column is required for ID. */
 				int regCopy = sqlite3GetTempRange(pParse, nResultCol + 1);
-				sqlite3VdbeAddOp3(v, OP_NextIdEphemeral, iParm,
-						  nResultCol, regCopy + nResultCol);
+				sqlite3VdbeAddOp2(v, OP_NextIdEphemeral, iParm,
+						  regCopy + nResultCol);
 				/* Positioning ID column to be last in inserted tuple.
 				 * NextId -> regCopy + n + 1
 				 * Copy [regResult, regResult + n] -> [regCopy, regCopy + n]
@@ -1418,7 +1418,7 @@ generateSortTail(Parse * pParse,	/* Parsing context */
 	case SRT_Table:
 	case SRT_EphemTab: {
 			int regCopy = sqlite3GetTempRange(pParse,  nColumn);
-			sqlite3VdbeAddOp3(v, OP_NextIdEphemeral, iParm, nColumn, regTupleid);
+			sqlite3VdbeAddOp2(v, OP_NextIdEphemeral, iParm, regTupleid);
 			sqlite3VdbeAddOp3(v, OP_Copy, regRow, regCopy, nSortData - 1);
 			sqlite3VdbeAddOp3(v, OP_MakeRecord, regCopy, nColumn + 1, regRow);
 			sqlite3VdbeAddOp2(v, OP_IdxInsert, iParm, regRow);
@@ -2898,8 +2898,8 @@ generateOutputSubroutine(struct Parse *parse, struct Select *p,
 	case SRT_EphemTab:{
 			int regRec = sqlite3GetTempReg(parse);
 			int regCopy = sqlite3GetTempRange(parse, in->nSdst + 1);
-			sqlite3VdbeAddOp3(v, OP_NextIdEphemeral, dest->iSDParm,
-					  in->nSdst, regCopy + in->nSdst);
+			sqlite3VdbeAddOp2(v, OP_NextIdEphemeral, dest->iSDParm,
+					  regCopy + in->nSdst);
 			sqlite3VdbeAddOp3(v, OP_Copy, in->iSdst, regCopy,
 					  in->nSdst - 1);
 			sqlite3VdbeAddOp3(v, OP_MakeRecord, regCopy,
diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h
index 94f94cb44..7da1f423b 100644
--- a/src/box/sql/tarantoolInt.h
+++ b/src/box/sql/tarantoolInt.h
@@ -116,8 +116,6 @@ int tarantoolSqlite3EphemeralDelete(BtCursor * pCur);
 int tarantoolSqlite3EphemeralCount(BtCursor * pCur, i64 * pnEntry);
 int tarantoolSqlite3EphemeralDrop(BtCursor * pCur);
 int tarantoolSqlite3EphemeralClearTable(BtCursor * pCur);
-int tarantoolSqlite3EphemeralGetMaxId(BtCursor * pCur, uint32_t fieldno,
-				       uint64_t * max_id);
 
 /**
  * Performs exactly as extract_key + sqlite3VdbeCompareMsgpack,
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 89c35d218..743d6e8c5 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -3790,27 +3790,20 @@ case OP_NextSequenceId: {
 	break;
 }
 
-/* Opcode: NextIdEphemeral P1 P2 P3 * *
- * Synopsis: r[P3]=get_max(space_index[P1]{Column[P2]})
+/* Opcode: NextIdEphemeral P1 P2 * * *
+ * Synopsis: r[P2]=get_next_rowid(space_index[P1]})
  *
- * This opcode works in the same way as OP_NextId does, except it is
- * only applied for ephemeral tables. The difference is in the fact that
- * all ephemeral tables don't have space_id (to be more precise it equals to zero).
+ * This opcode stores next `rowid` for the ephemeral space to P2 register.
+ * `rowid` is necessary, because inserted to ephemeral space tuples may be not
+ * unique, while Tarantool`s spaces can contain only unique tuples.
  */
 case OP_NextIdEphemeral: {
 	VdbeCursor *pC;
-	int p2;
 	pC = p->apCsr[pOp->p1];
-	p2 = pOp->p2;
-	pOut = &aMem[pOp->p3];
-
-	assert(pC->uc.pCursor->curFlags & BTCF_TEphemCursor);
-
-	rc = tarantoolSqlite3EphemeralGetMaxId(pC->uc.pCursor, p2,
-					       (uint64_t *) &pOut->u.i);
-	if (rc) goto abort_due_to_error;
-
-	pOut->u.i += 1;
+	pOut = &aMem[pOp->p2];
+	struct BtCursor * bt_cur = pC->uc.pCursor;
+	assert(bt_cur->curFlags & BTCF_TEphemCursor);
+	pOut->u.i = bt_cur->ephemeral_rowid++;
 	pOut->flags = MEM_Int;
 	break;
 }
diff --git a/test/sql-tap/gh-3297_ephemeral_rowid.test.lua b/test/sql-tap/gh-3297_ephemeral_rowid.test.lua
new file mode 100755
index 000000000..ed596e7ad
--- /dev/null
+++ b/test/sql-tap/gh-3297_ephemeral_rowid.test.lua
@@ -0,0 +1,30 @@
+#!/usr/bin/env tarantool
+test = require("sqltester")
+test:plan(1)
+
+-- Check that OP_NextIdEphemeral generates unique ids.
+
+test:execsql [[
+    CREATE TABLE t1(a primary key);
+    CREATE TABLE t2(a primary key, b);
+    insert into t1 values(12);
+    insert into t2 values(1, 5);
+    insert into t2 values(2, 2);
+    insert into t2 values(3, 2);
+    insert into t2 values(4, 2);
+]]
+
+test:do_execsql_test(
+    "gh-3297-1",
+    [[
+        select * from ( select a from t1 limit 1), (select b from t2 limit 10);
+    ]],
+    {
+        12, 2,
+        12, 2,
+        12, 2,
+        12, 5
+    }
+)
+
+test:finish_test()
-- 
2.14.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-06-19 14:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-15 19:49 [tarantool-patches] [PATCH] Fix ephemeral table next rowid generation AKhatskevich
2018-05-31 23:53 ` [tarantool-patches] " n.pettik
     [not found]   ` <95f6ed87-03fb-c0ad-76e1-3776a0714e8b@tarantool.org>
2018-06-01 11:50     ` n.pettik
2018-06-01 12:16       ` Alex Khatskevich
2018-06-19 14:55         ` n.pettik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox