Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alex Khatskevich <avkhatskevich@tarantool.org>
To: "n.pettik" <korablev@tarantool.org>, tarantool-patches@freelists.org
Subject: [tarantool-patches] Re: [PATCH v2] sql: fix ephemeral table next rowid generation
Date: Wed, 1 Aug 2018 20:23:54 +0300	[thread overview]
Message-ID: <a2eae1df-79ba-3171-0b88-f46378319f63@tarantool.org> (raw)
In-Reply-To: <1368F28D-35EB-4BFA-B672-2BD8BD37963D@tarantool.org>


>> diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
>> index f2d512f21..5a80d48ab 100644
>> --- a/src/box/memtx_space.c
>> +++ b/src/box/memtx_space.c
>> @@ -843,6 +843,8 @@ static void
>> memtx_init_ephemeral_space(struct space *space)
>> {
>> 	memtx_space_add_primary_key(space);
>> +	struct memtx_space * ephem_space = (struct memtx_space *) space;
>> +	ephem_space->next_rowid = 0;
>> }
>>
>> static int
>> diff --git a/src/box/memtx_space.h b/src/box/memtx_space.h
>> index 7dc341079..26181166a 100644
>> --- a/src/box/memtx_space.h
>> +++ b/src/box/memtx_space.h
>> @@ -48,6 +48,11 @@ struct memtx_space {
>> 	 */
>> 	int (*replace)(struct space *, struct tuple *, struct tuple *,
>> 		       enum dup_replace_mode, struct tuple **);
>> +	/**
>> +	 * Next rowid. This number is added to the end of pk to
> In fact, to the end of tuple to be inserted. Now ofc PK of ephemeral
> spaces consists of all fields, but it may change someday, I guess.
>
> Moreover, ephemeral spaces aren’t supposed to be feature exclusively
> of memtx engine. Vinyl ephemeral spaces also have right to exist.
1. Deleted those details.
2. Created vtab entity `ephemeral_next_rowid`.
>> +	 * allow to store non-unique rows in ephemeral tables.
>> +	 */
>> +	uint64_t next_rowid;
>> };
>>
>> /**
>> diff --git a/src/box/sql.c b/src/box/sql.c
>> index 6104a6d0f..fdf0313d3 100644
>> --- a/src/box/sql.c
>> +++ b/src/box/sql.c
>> @@ -47,6 +47,7 @@
>> #include "box.h"
>> #include "txn.h"
>> #include "space.h"
>> +#include "memtx_space.h"
>> #include "space_def.h"
>> #include "index_def.h"
>> #include "tuple.h"
>> @@ -448,6 +449,18 @@ int tarantoolSqlite3EphemeralDrop(BtCursor *pCur)
>> 	return SQLITE_OK;
>> }
>>
>> +/**
>> + * Get next rowid for an ephemeral table.
>> + */
>> +uint64_t
>> +tarantool_ephemeral_next_rowid(BtCursor *bt_cur)
> Firstly, we don’t use ’tarantool_’ prefix anymore. Also we use ’struct’ prefix
> for compound types: struct BtCursor *bt_cur. Moreover, I don’t see comment
> for this function.
fixed.
Added comment to `tarantoolInt.h`.
> Secondly, I see your message within issue discussion:
> "
> Discussed verbally with
> @kyukhin , @Korablev77 , @kostja and decided, that the unique rowid should be encapsulated into
> vtab method of ephemeral space.
> “
>
> But this function doesn’t seem to be method of vtab…
Created a dedicated vtab function.
>> +{
>> +	assert(bt_cur);
>> +	assert(bt_cur->curFlags & BTCF_TEphemCursor);
>> +	struct memtx_space * ephem_space = (struct memtx_space *) bt_cur->space;
>> +	return ephem_space->next_rowid++;
>> +}
>> +
>> static inline int
>> insertOrReplace(struct space *space, const char *tuple, const char *tuple_end,
>> 		enum iproto_type type)
>> @@ -1080,7 +1093,6 @@ key_alloc(BtCursor *cur, size_t key_size)
>> 		}
>> 		cur->key = new_key;
>> 	}
>> -	cur->nKey = key_size;
>> 	return 0;
>> }
>>
>> @@ -1621,37 +1633,6 @@ sql_debug_info(struct info_handler *h)
>> 	info_end(h);
>> }
>>
>>
>> diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h
>> index 94f94cb44..369af77ff 100644
>> --- a/src/box/sql/tarantoolInt.h
>> +++ b/src/box/sql/tarantoolInt.h
>> @@ -116,8 +116,8 @@ 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);
>> +uint64_t
>> +tarantool_ephemeral_next_rowid(BtCursor *bt_cur);
>>
>> /**
>>   * Performs exactly as extract_key + sqlite3VdbeCompareMsgpack,
>> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
>> index 89c35d218..532c98e54 100644
>> --- a/src/box/sql/vdbe.c
>> +++ b/src/box/sql/vdbe.c
>> @@ -3790,27 +3790,19 @@ 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.
> Nitpicking: generally speaking, ephemeral spaces can contain duplicates
> when it comes for NULLs. You may investigate how this request works:
>
> SELECT (SELECT NULL UNION SELECT NULL) EXCEPT SELECT NULL;
>
> :)
>
>>   */
>> 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;
>> +	pOut->u.i = tarantool_ephemeral_next_rowid(bt_cur);
> Should we add some checks on rowid overflow? Ephemeral spaces can be extensively
> used for JOIN’s where number of inserted to ephemeral space tuples is O(n^2) in
> worst case (for two tables). On the other hand, even simple check can slow down execution
> especially when it concerns millions of tuples...
In rowid is increasing 10^9 times per second, it would take 1017 years 
to overflow the counter.
So, no.
>> 	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);
> Use upper-case for all SQL statements pls.
Fixed.


Full diff

commit 3b30cd4ee27ccf9046a4623d6455479bf980cd1a
Author: AKhatskevich <avkhatskevich@tarantool.org>
Date:   Tue May 15 22:00:28 2018 +0300

     sql: fix ephemeral table next rowid generation

     Before this commit, next rowid was retrieved from an ephemeral table
     using `index_max`. That led to wrong behavior because the index is not
     sorted by rowid and `index_max` doesn`t return a tuple with the 
greatest
     rowid.

     New implementation stores next `rowid` value in `struct memtx_space`,
     and increments it after each insert to the ephemeral space.

     Extra refactoring:
      * `nKey` attribute is deleted from BtCursor, because it is not used
     anymore.

     Closes #3297

diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
index f2d512f21..9015c56ca 100644
--- a/src/box/memtx_space.c
+++ b/src/box/memtx_space.c
@@ -603,6 +603,20 @@ memtx_space_ephemeral_delete(struct space *space, 
const char *key)
      return 0;
  }

+/**
+ * Generate unique number.
+ * This rowid may be used to store non-unique values in an
+ * ephemeral space.
+ *
+ * This function isn't involved in any transaction routine.
+ */
+static uint64_t
+memtx_space_ephemeral_next_rowid(struct space *space)
+{
+    struct memtx_space *memtx_space = (struct memtx_space *)space;
+    return memtx_space->next_rowid++;
+}
+
  /* }}} DML */

  /* {{{ DDL */
@@ -843,6 +857,8 @@ static void
  memtx_init_ephemeral_space(struct space *space)
  {
      memtx_space_add_primary_key(space);
+    struct memtx_space * ephem_space = (struct memtx_space *) space;
+    ephem_space->next_rowid = 0;
  }

  static int
@@ -949,6 +965,7 @@ static const struct space_vtab memtx_space_vtab = {
      /* .execute_upsert = */ memtx_space_execute_upsert,
      /* .ephemeral_replace = */ memtx_space_ephemeral_replace,
      /* .ephemeral_delete = */ memtx_space_ephemeral_delete,
+    /* .ephemeral_next_rowid = */ memtx_space_ephemeral_next_rowid,
      /* .ephemeral_cleanup = */ memtx_space_ephemeral_cleanup,
      /* .init_system_space = */ memtx_init_system_space,
      /* .init_ephemeral_space = */ memtx_init_ephemeral_space,
diff --git a/src/box/memtx_space.h b/src/box/memtx_space.h
index 7dc341079..bb1d8b74b 100644
--- a/src/box/memtx_space.h
+++ b/src/box/memtx_space.h
@@ -48,6 +48,11 @@ struct memtx_space {
       */
      int (*replace)(struct space *, struct tuple *, struct tuple *,
                 enum dup_replace_mode, struct tuple **);
+    /**
+     * Next rowid. This number is used to allow to store
+     * non-unique rows in ephemeral tables.
+     */
+    uint64_t next_rowid;
  };

  /**
diff --git a/src/box/space.h b/src/box/space.h
index 8f675dfe1..e4ae29747 100644
--- a/src/box/space.h
+++ b/src/box/space.h
@@ -70,6 +70,10 @@ struct space_vtab {
      int (*ephemeral_replace)(struct space *, const char *, const char *);

      int (*ephemeral_delete)(struct space *, const char *);
+    /**
+     * Generate unique number.
+     */
+    uint64_t (*ephemeral_next_rowid)(struct space *);

      void (*ephemeral_cleanup)(struct space *);

diff --git a/src/box/sql.c b/src/box/sql.c
index 6104a6d0f..c27595afd 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -47,6 +47,7 @@
  #include "box.h"
  #include "txn.h"
  #include "space.h"
+#include "memtx_space.h"
  #include "space_def.h"
  #include "index_def.h"
  #include "tuple.h"
@@ -448,6 +449,15 @@ int tarantoolSqlite3EphemeralDrop(BtCursor *pCur)
      return SQLITE_OK;
  }

+uint64_t
+ephemeral_next_rowid(struct BtCursor *bt_cur)
+{
+    assert(bt_cur);
+    assert(bt_cur->curFlags & BTCF_TEphemCursor);
+    struct space * ephem_space = (struct space *) bt_cur->space;
+    return ephem_space->vtab->ephemeral_next_rowid(ephem_space);
+}
+
  static inline int
  insertOrReplace(struct space *space, const char *tuple, const char 
*tuple_end,
          enum iproto_type type)
@@ -1080,7 +1090,6 @@ key_alloc(BtCursor *cur, size_t key_size)
          }
          cur->key = new_key;
      }
-    cur->nKey = key_size;
      return 0;
  }

@@ -1621,37 +1630,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..ab379f01e 100644
--- a/src/box/sql/cursor.h
+++ b/src/box/sql/cursor.h
@@ -57,7 +57,6 @@ typedef struct BtCursor BtCursor;
   * for ordinary space, or to TEphemCursor for ephemeral space.
   */
  struct BtCursor {
-    i64 nKey;        /* Size of pKey, or last integer key */
      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..38f6a56c1 100644
--- a/src/box/sql/tarantoolInt.h
+++ b/src/box/sql/tarantoolInt.h
@@ -116,8 +116,14 @@ 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);
+/**
+ * Get next rowid for a cursor which points to an ephemeral table.
+ * @param bt_cur Cursor which will point to the new ephemeral space.
+ *
+ * @retval New unique rowid.
+ */
+uint64_t
+ephemeral_next_rowid(struct BtCursor *bt_cur);

  /**
   * Performs exactly as extract_key + sqlite3VdbeCompareMsgpack,
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 89c35d218..6ac12dec0 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -3790,27 +3790,19 @@ 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;
+    pOut->u.i = ephemeral_next_rowid(bt_cur);
      pOut->flags = MEM_Int;
      break;
  }
diff --git a/src/box/sysview_engine.c b/src/box/sysview_engine.c
index 308a3328a..64fa6ee23 100644
--- a/src/box/sysview_engine.c
+++ b/src/box/sysview_engine.c
@@ -119,6 +119,14 @@ sysview_space_ephemeral_delete(struct space *space, 
const char *key)
      return -1;
  }

+static uint64_t
+sysview_space_ephemeral_next_rowid(struct space *space)
+{
+    (void)space;
+    unreachable();
+    return 0;
+}
+
  static void
  sysview_space_ephemeral_cleanup(struct space *space)
  {
@@ -206,6 +214,7 @@ static const struct space_vtab sysview_space_vtab = {
      /* .execute_upsert = */ sysview_space_execute_upsert,
      /* .ephemeral_replace = */ sysview_space_ephemeral_replace,
      /* .ephemeral_delete = */ sysview_space_ephemeral_delete,
+    /* .ephemeral_next_rowid = */ sysview_space_ephemeral_next_rowid,
      /* .ephemeral_cleanup = */ sysview_space_ephemeral_cleanup,
      /* .init_system_space = */ sysview_init_system_space,
      /* .init_ephemeral_space = */ sysview_init_ephemeral_space,
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index b7238ae4c..b13b21dfb 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -2379,6 +2379,14 @@ vinyl_space_ephemeral_delete(struct space *space, 
const char *key)
      return -1;
  }

+static uint64_t
+vinyl_space_ephemeral_next_rowid(struct space *space)
+{
+    (void)space;
+    unreachable();
+    return 0;
+}
+
  static void
  vinyl_space_ephemeral_cleanup(struct space *space)
  {
@@ -4053,6 +4061,7 @@ static const struct space_vtab vinyl_space_vtab = {
      /* .execute_upsert = */ vinyl_space_execute_upsert,
      /* .ephemeral_replace = */ vinyl_space_ephemeral_replace,
      /* .ephemeral_delete = */ vinyl_space_ephemeral_delete,
+    /* .ephemeral_next_rowid = */ vinyl_space_ephemeral_next_rowid,
      /* .ephemeral_cleanup = */ vinyl_space_ephemeral_cleanup,
      /* .init_system_space = */ vinyl_init_system_space,
      /* .init_ephemeral_space = */ vinyl_init_ephemeral_space,
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..d49baf300
--- /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()

      reply	other threads:[~2018-08-01 17:24 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-18 15:47 [tarantool-patches] " AKhatskevich
2018-07-25 15:06 ` [tarantool-patches] " n.pettik
2018-08-01 17:23   ` Alex Khatskevich [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a2eae1df-79ba-3171-0b88-f46378319f63@tarantool.org \
    --to=avkhatskevich@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH v2] sql: fix ephemeral table next rowid generation' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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