From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 709992360C for ; Tue, 15 May 2018 15:50:03 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id fp34bZlnlYts for ; Tue, 15 May 2018 15:50:03 -0400 (EDT) Received: from smtp15.mail.ru (smtp15.mail.ru [94.100.176.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 9DFBF234B3 for ; Tue, 15 May 2018 15:50:02 -0400 (EDT) Received: by smtp15.mail.ru with esmtpa (envelope-from ) id 1fIfxQ-0006m6-1r for tarantool-patches@freelists.org; Tue, 15 May 2018 22:50:00 +0300 From: AKhatskevich Subject: [tarantool-patches] [PATCH] Fix ephemeral table next rowid generation Date: Tue, 15 May 2018 22:49:43 +0300 Message-Id: <20180515194943.5137-1-avkhatskevich@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org 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