[tarantool-patches] [PATCH] Fix ephemeral table next rowid generation
AKhatskevich
avkhatskevich at tarantool.org
Tue May 15 22:49:43 MSK 2018
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
More information about the Tarantool-patches
mailing list