[tarantool-patches] Re: [PATCH] sql: xfer optimization issue

  • From: Nikita Tatunov <hollow653@xxxxxxxxx>
  • To: korablev@xxxxxxxxxxxxx
  • Date: Wed, 18 Jul 2018 23:18:04 +0300

Hello, Nikita! Here's newer version of the patch.
Diff can be found at the end.

ср, 18 июл. 2018 г. в 18:13, n.pettik <korablev@xxxxxxxxxxxxx>:

Please, add to commit message results of benchmark to indicate
that this optimisation really matters.


Added.


On 17 Jul 2018, at 00:27, Nikita Tatunov <hollow653@xxxxxxxxx> wrote:

diff --git a/src/box/sql.c b/src/box/sql.c
index fdce224..398b2a6 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -1636,10 +1636,12 @@ sql_debug_info(struct info_handler *h)
      extern int sql_search_count;
      extern int sql_sort_count;
      extern int sql_found_count;
+     extern int sql_xferOpt_count;

Don’t use camel notation. Lets call it simply ’sql_xfer_count’.


Fixed.


      info_begin(h);
      info_append_int(h, "sql_search_count", sql_search_count);
      info_append_int(h, "sql_sort_count", sql_sort_count);
      info_append_int(h, "sql_found_count", sql_found_count);
+     info_append_int(h, "sql_xferOpt_count", sql_xferOpt_count);
      info_end(h);
 }

diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 2c9188e..9a99bab 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -1635,7 +1635,7 @@ sqlite3OpenTableAndIndices(Parse * pParse,      /*
Parsing context */
  * purposes only - to make sure the transfer optimization really
  * is happening when it is supposed to.
  */
-int sqlite3_xferopt_count;
+int sql_xferOpt_count = 0;
 #endif                               /* SQLITE_TEST */

 #ifndef SQLITE_OMIT_XFER_OPT
@@ -1658,6 +1658,8 @@ xferCompatibleIndex(Index * pDest, Index * pSrc)
      assert(pDest->pTable != pSrc->pTable);
      uint32_t nDestCol = index_column_count(pDest);
      uint32_t nSrcCol = index_column_count(pSrc);
+     if ((pDest->idxType != pSrc->idxType))
+             return 0;
      if (nDestCol != nSrcCol) {
              return 0;       /* Different number of columns */
      }
@@ -1725,9 +1727,9 @@ xferOptimization(Parse * pParse,        /* Parser
context */
      int emptyDestTest = 0;  /* Address of test for empty pDest */
      int emptySrcTest = 0;   /* Address of test for empty pSrc */
      Vdbe *v;                /* The VDBE we are building */
-     int destHasUniqueIdx = 0;       /* True if pDest has a UNIQUE
index */
      int regData, regTupleid;        /* Registers holding data and
tupleid */
      struct session *user_session = current_session();
+     bool is_err_action_default = false;

Again: why do you need this flag? Default action is just synonym for ABORT,
so why should we care about it?


It's all about conflict action priorities as I said before.
Consider the following example:
```
CREATE TABLE t1(a INTEGER PRIMARY KEY ON CONFLICT REPLACE, b);
CREATE TABLE t2(a INTEGER PRIMARY KEY ON CONFLICT REPLACE, b);
INSERT INTO t1 VALUES (1, 1), (3, 3), (5, 5);
INSERT INTO t2 VALUES (2, 2), (3, 4);
BEGIN;
INSERT INTO t2 VALUES (4, 4);
INSERT INTO t2 SELECT * FROM t1;
INSERT INTO t2 VALUES (10, 10);
COMMIT;
```
As we understand *_REPLACE should work in this case but
onError == *_ABORT (as it was converted from the default one).
It leads to a situation where an error will occur if xferOptimization is
used.

+     struct space *src_space =
+             space_by_id(SQLITE_PAGENO_TO_SPACEID(pSrc->tnum));
+     struct space *dest_space =
+             space_by_id(SQLITE_PAGENO_TO_SPACEID(pDest->tnum));

You don’t need to again proceed lookup: space is found few lines above.
Moreover, I see those lookups are executed inside ‘for' loop. Lets move
them outside it.


Fixed it.



+     struct index *src_idx = space_index(src_space, 0);
+     struct index *dest_idx = space_index(dest_space, 0);
+
+     /* Xfer optimization is unable to correctly insert data
+      * in case there's a conflict action other than *_ABORT.
+      * This is the reason we want to only run it if the
+      * destination table is initially empty.
+      * That block generates code to make that determination.
+      */

Multi-line comment should be formatted as following:

/*
 * Comment starts here.
 * …
 */


Fixed.



+
+     if (!(onError == ON_CONFLICT_ACTION_ABORT &&
+         is_err_action_default == false)) {
              addr1 = sqlite3VdbeAddOp2(v, OP_Rewind, iDest, 0);
              VdbeCoverage(v);
              emptyDestTest = sqlite3VdbeAddOp0(v, OP_Goto);
              sqlite3VdbeJumpHere(v, addr1);
+#ifdef SQLITE_TEST
+             if (dest_idx->vtab->count(dest_idx, ITER_ALL, NULL, 0) ==
0)
+                     sql_xferOpt_count++;

Actually, I don’t like this approach.
Look, query may be compiled and saved into cache (even thought it is still
not implemented yet). So it might be executed later and it might be not
empty.
Moreover, we are going to avoid doing space lookups and use only def.
With only def you can’t execute count.

Personally, I wanted you to defer incrementing sql_xfer_count till
VDBE execution. For instance, you may add special flag and pass it
to OP_RowData indicating that xFer is currently processing.

+#endif
+     vdbe_emit_open_cursor(pParse, iSrc, 0, src_space);
+     VdbeComment((v, "%s", src_idx->def->name));
+     vdbe_emit_open_cursor(pParse, iDest, 0, dest_space);

I see few lines above:

sqlite3OpenTable(pParse, iDest, pDest, OP_OpenWrite);

So, basically you don’t need to open it again.


Fixed it.



+     VdbeComment((v, "%s", dest_idx->def->name));
+     addr1 = sqlite3VdbeAddOp2(v, OP_Rewind, iSrc, 0);
+     VdbeCoverage(v);
+     sqlite3VdbeAddOp2(v, OP_RowData, iSrc, regData);
+     sqlite3VdbeAddOp2(v, OP_IdxInsert, iDest, regData);
+     sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
+     sqlite3VdbeAddOp2(v, OP_Next, iSrc, addr1 + 1);
+     VdbeCoverage(v);
+     sqlite3VdbeJumpHere(v, addr1);
+     sqlite3VdbeAddOp2(v, OP_Close, iSrc, 0);
+     sqlite3VdbeAddOp2(v, OP_Close, iDest, 0);
+
      if (emptySrcTest)
              sqlite3VdbeJumpHere(v, emptySrcTest);
      sqlite3ReleaseTempReg(pParse, regTupleid);
diff --git a/test/sql-tap/gh-3307-xfer-optimization-issue.test.lua
b/test/sql-tap/gh-3307-xfer-optimization-issue.test.lua
new file mode 100755
index 0000000..e75fabc
--- /dev/null
+test:do_catchsql_test(
+     "xfer-optimization-1.15",
+     [[
+             DROP TABLE t1;
+             DROP TABLE t2;
+             CREATE TABLE t1(a INTEGER PRIMARY KEY, b UNIQUE);
+             CREATE TABLE t2(a INTEGER PRIMARY KEY, b UNIQUE);
+             INSERT INTO t1 VALUES (2, 2), (3, 3), (5, 5);
+             INSERT INTO t2 VALUES (1, 1), (4, 4);
+             INSERT OR ROLLBACK INTO t2 SELECT * FROM t1;

INSERT OT ROLLBACK outside transaction works the same as ABORT and DEFAULT.
So, surround it with transaction and check that it really rollbacks.


There are basically almost the same tests surrounded by transactions (1.30
- 1.35).


+     ]], {
+             -- <xfer-optimization-1.15>
+             0
+             -- <xfer-optimization-1.15>
+     })
+
+test:do_execsql_test(
+     "xfer-optimization-1.16",
+     [[
+             SELECT * FROM t2;
+     ]], {
+             -- <xfer-optimization-1.16>
+             1, 1, 2, 2, 3, 3, 4, 4, 5, 5
+             -- <xfer-optimization-1.16>
+     })
+
+-- The following tests are supposed to test if xfer-optimization is
actually
+-- used in the given cases (if the conflict actually occurs):
+--   1.0) insert w/o explicit confl. action & w/o index replace action
+--   1.1) insert w/o explicit confl. action & w/ index replace action &
empty dest_table
+--   1.2) insert w/o explicit confl. action & w/ index replace action &
non-empty dest_table
+--   2) insert with abort
+--   3.0) insert with rollback (into empty table)
+--   3.1) insert with rollback (into non-empty table)
+--   4) insert with replace
+--   5) insert with fail
+--   6) insert with ignore
+
+
+-- 1.0) insert w/o explicit confl. action & w/o index replace action

+-------------------------------------------------------------------------------------------
+
+bfr = box.sql.debug().sql_xferOpt_count
+
+test:do_catchsql_test(
+     "xfer-optimization-1.17",
+     [[
+             DROP TABLE t1;
+             DROP TABLE t2;
+             CREATE TABLE t1(a INTEGER PRIMARY KEY, b);
+             CREATE TABLE t2(a INTEGER PRIMARY KEY, b);
+             INSERT INTO t1 VALUES (1, 1), (3, 3), (5, 5);
+             INSERT INTO t2 VALUES (2, 2), (3, 4);
+             BEGIN;
+                     INSERT INTO t2 VALUES (4, 4);
+                     INSERT INTO t2 SELECT * FROM t1;
+     ]], {
+             -- <xfer-optimization-1.17>
+             1, "Duplicate key exists in unique index
'sqlite_autoindex_T2_1' in space 'T2'"
+             -- <xfer-optimization-1.17>
+     })
+
+test:do_execsql_test(
+     "xfer-optimization-1.18",
+     [[
+                     INSERT INTO t2 VALUES (10, 10);
+             COMMIT;
+             SELECT * FROM t2;
+     ]], {
+             -- <xfer-optimization-1.18>
+             2, 2, 3, 4, 4, 4, 10, 10
+             -- <xfer-optimization-1.18>
+     })
+
+aftr = box.sql.debug().sql_xferOpt_count
+
+test:do_test(
+     "xfer-optimization-1.19",
+     function()
+             if (aftr - bfr == 1) then
+                     return {1}
+             end
+             if (aftr == bfr) then
+                     return {0}
+             end
+             return {2}

Why do you repeat this snippet each time? You can declare it as named
function once and use it everywhere.

+     end, {
+             -- <xfer-optimization-1.19>
+             0
+             -- <xfer-optimization-1.19>
+     })
+
+-- 1.1) insert w/o explicit confl. action & w/ index replace action &
empty dest_table

Even in tests lets not exceed 80 chars (here and in other places).


diff --git a/src/box/sql.c b/src/box/sql.c
index fdce224..656ba17 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -1636,10 +1636,12 @@ sql_debug_info(struct info_handler *h)
  extern int sql_search_count;
  extern int sql_sort_count;
  extern int sql_found_count;
+ extern int sql_xfer_count;
  info_begin(h);
  info_append_int(h, "sql_search_count", sql_search_count);
  info_append_int(h, "sql_sort_count", sql_sort_count);
  info_append_int(h, "sql_found_count", sql_found_count);
+ info_append_int(h, "sql_xfer_count", sql_xfer_count);
  info_end(h);
 }

diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 2c9188e..c2df1c2 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -1628,16 +1628,6 @@ sqlite3OpenTableAndIndices(Parse * pParse, /*
Parsing context */
  return i;
 }

-#ifdef SQLITE_TEST
-/*
- * The following global variable is incremented whenever the
- * transfer optimization is used.  This is used for testing
- * purposes only - to make sure the transfer optimization really
- * is happening when it is supposed to.
- */
-int sqlite3_xferopt_count;
-#endif /* SQLITE_TEST */
-
 #ifndef SQLITE_OMIT_XFER_OPT
 /*
  * Check to see if index pSrc is compatible as a source of data
@@ -1658,6 +1648,8 @@ xferCompatibleIndex(Index * pDest, Index * pSrc)
  assert(pDest->pTable != pSrc->pTable);
  uint32_t nDestCol = index_column_count(pDest);
  uint32_t nSrcCol = index_column_count(pSrc);
+ if ((pDest->idxType != pSrc->idxType))
+ return 0;
  if (nDestCol != nSrcCol) {
  return 0; /* Different number of columns */
  }
@@ -1724,10 +1716,9 @@ xferOptimization(Parse * pParse, /* Parser context */
  int addr1; /* Loop addresses */
  int emptyDestTest = 0; /* Address of test for empty pDest */
  int emptySrcTest = 0; /* Address of test for empty pSrc */
- Vdbe *v; /* The VDBE we are building */
- int destHasUniqueIdx = 0; /* True if pDest has a UNIQUE index */
  int regData, regTupleid; /* Registers holding data and tupleid */
  struct session *user_session = current_session();
+ bool is_err_action_default = false;

  if (pSelect == NULL)
  return 0; /* Must be of the form  INSERT INTO ... SELECT ... */
@@ -1744,8 +1735,10 @@ xferOptimization(Parse * pParse, /* Parser context */
  if (onError == ON_CONFLICT_ACTION_DEFAULT) {
  if (pDest->iPKey >= 0)
  onError = pDest->keyConf;
- if (onError == ON_CONFLICT_ACTION_DEFAULT)
+ if (onError == ON_CONFLICT_ACTION_DEFAULT) {
  onError = ON_CONFLICT_ACTION_ABORT;
+ is_err_action_default = true;
+ }
  }
  assert(pSelect->pSrc); /* allocated even if there is no FROM clause */
  if (pSelect->pSrc->nSrc != 1) {
@@ -1807,6 +1800,10 @@ xferOptimization(Parse * pParse, /* Parser context */
  /* Both tables must have the same INTEGER PRIMARY KEY. */
  if (pDest->iPKey != pSrc->iPKey)
  return 0;
+ uint32_t src_space_id = SQLITE_PAGENO_TO_SPACEID(pSrc->tnum);
+ struct space *src_space = space_by_id(src_space_id);
+ uint32_t dest_space_id = SQLITE_PAGENO_TO_SPACEID(pDest->tnum);
+ struct space *dest_space = space_by_id(dest_space_id);
  for (i = 0; i < (int)pDest->def->field_count; i++) {
  enum affinity_type dest_affinity =
  pDest->def->fields[i].affinity;
@@ -1826,14 +1823,6 @@ xferOptimization(Parse * pParse, /* Parser context */
  }
  /* Default values for second and subsequent columns need to match. */
  if (i > 0) {
- uint32_t src_space_id =
- SQLITE_PAGENO_TO_SPACEID(pSrc->tnum);
- struct space *src_space =
- space_cache_find(src_space_id);
- uint32_t dest_space_id =
- SQLITE_PAGENO_TO_SPACEID(pDest->tnum);
- struct space *dest_space =
- space_cache_find(dest_space_id);
  assert(src_space != NULL && dest_space != NULL);
  char *src_expr_str =
  src_space->def->fields[i].default_value;
@@ -1848,9 +1837,6 @@ xferOptimization(Parse * pParse, /* Parser context */
  }
  }
  for (pDestIdx = pDest->pIndex; pDestIdx; pDestIdx = pDestIdx->pNext) {
- if (index_is_unique(pDestIdx)) {
- destHasUniqueIdx = 1;
- }
  for (pSrcIdx = pSrc->pIndex; pSrcIdx; pSrcIdx = pSrcIdx->pNext) {
  if (xferCompatibleIndex(pDestIdx, pSrcIdx))
  break;
@@ -1860,10 +1846,8 @@ xferOptimization(Parse * pParse, /* Parser context */
  }
  }
  /* Get server checks. */
- ExprList *pCheck_src = space_checks_expr_list(
- SQLITE_PAGENO_TO_SPACEID(pSrc->tnum));
- ExprList *pCheck_dest = space_checks_expr_list(
- SQLITE_PAGENO_TO_SPACEID(pDest->tnum));
+ ExprList *pCheck_src = space_checks_expr_list(src_space_id);
+ ExprList *pCheck_dest = space_checks_expr_list(dest_space_id);
  if (pCheck_dest != NULL &&
      sqlite3ExprListCompare(pCheck_src, pCheck_dest, -1) != 0) {
  /* Tables have different CHECK constraints.  Ticket #2252 */
@@ -1888,72 +1872,51 @@ xferOptimization(Parse * pParse, /* Parser context
*/
  * least a possibility, though it might only work if the destination
  * table (tab1) is initially empty.
  */
-#ifdef SQLITE_TEST
- sqlite3_xferopt_count++;
-#endif
- v = sqlite3GetVdbe(pParse);
+
+ /* The Vdbe we're building*/
+ Vdbe *v = sqlite3GetVdbe(pParse);
  iSrc = pParse->nTab++;
  iDest = pParse->nTab++;
  regData = sqlite3GetTempReg(pParse);
  regTupleid = sqlite3GetTempReg(pParse);
- sqlite3OpenTable(pParse, iDest, pDest, OP_OpenWrite);
- assert(destHasUniqueIdx);
- if ((pDest->iPKey < 0 && pDest->pIndex != 0) /* (1) */
-     ||destHasUniqueIdx /* (2) */
-     || (onError != ON_CONFLICT_ACTION_ABORT
- && onError != ON_CONFLICT_ACTION_ROLLBACK) /* (3) */
-     ) {
- /* In some circumstances, we are able to run the xfer optimization
- * only if the destination table is initially empty.
- * This block generates code to make
- * that determination.
- *
- * Conditions under which the destination must be empty:
- *
- * (1) There is no INTEGER PRIMARY KEY but there are indices.
- *
- * (2) The destination has a unique index.  (The xfer optimization
- *     is unable to test uniqueness.)
- *
- * (3) onError is something other than ON_CONFLICT_ACTION_ABORT and
_ROLLBACK.
- */
+
+ vdbe_emit_open_cursor(pParse, iDest, 0, dest_space);
+ VdbeComment((v, "%s", pDest->def->name));
+
+ /*
+ * Xfer optimization is unable to correctly insert data
+ * in case there's a conflict action other than *_ABORT.
+ * This is the reason we want to only run it if the
+ * destination table is initially empty.
+ * That block generates code to make that determination.
+ */
+
+ if (!(onError == ON_CONFLICT_ACTION_ABORT &&
+     is_err_action_default == false)) {
  addr1 = sqlite3VdbeAddOp2(v, OP_Rewind, iDest, 0);
  VdbeCoverage(v);
  emptyDestTest = sqlite3VdbeAddOp0(v, OP_Goto);
  sqlite3VdbeJumpHere(v, addr1);
  }

- for (pDestIdx = pDest->pIndex; pDestIdx; pDestIdx = pDestIdx->pNext) {
- for (pSrcIdx = pSrc->pIndex; ALWAYS(pSrcIdx);
-      pSrcIdx = pSrcIdx->pNext) {
- if (xferCompatibleIndex(pDestIdx, pSrcIdx))
- break;
- }
- assert(pSrcIdx);
- struct space *src_space =
- space_by_id(SQLITE_PAGENO_TO_SPACEID(pSrcIdx->tnum));
- 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,
-       SQLITE_PAGENO_TO_INDEXID(pDestIdx->tnum),
-       dest_space);
- VdbeComment((v, "%s", pDestIdx->zName));
- addr1 = sqlite3VdbeAddOp2(v, OP_Rewind, iSrc, 0);
- VdbeCoverage(v);
- sqlite3VdbeAddOp2(v, OP_RowData, iSrc, regData);
- sqlite3VdbeAddOp2(v, OP_IdxInsert, iDest, regData);
- if (pDestIdx->idxType == SQLITE_IDXTYPE_PRIMARYKEY)
- sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
- sqlite3VdbeAddOp2(v, OP_Next, iSrc, addr1 + 1);
- VdbeCoverage(v);
- sqlite3VdbeJumpHere(v, addr1);
- sqlite3VdbeAddOp2(v, OP_Close, iSrc, 0);
- sqlite3VdbeAddOp2(v, OP_Close, iDest, 0);
- }
+ vdbe_emit_open_cursor(pParse, iSrc, 0, src_space);
+ VdbeComment((v, "%s", pSrc->def->name));
+ addr1 = sqlite3VdbeAddOp2(v, OP_Rewind, iSrc, 0);
+ VdbeCoverage(v);
+ sqlite3VdbeAddOp2(v, OP_RowData, iSrc, regData);
+
+#ifdef SQLITE_TEST
+ sqlite3VdbeChangeP5(v, OPFLAG_XFER_OPT);
+#endif
+
+ sqlite3VdbeAddOp2(v, OP_IdxInsert, iDest, regData);
+ sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
+ sqlite3VdbeAddOp2(v, OP_Next, iSrc, addr1 + 1);
+ VdbeCoverage(v);
+ sqlite3VdbeJumpHere(v, addr1);
+ sqlite3VdbeAddOp2(v, OP_Close, iSrc, 0);
+ sqlite3VdbeAddOp2(v, OP_Close, iDest, 0);
+
  if (emptySrcTest)
  sqlite3VdbeJumpHere(v, emptySrcTest);
  sqlite3ReleaseTempReg(pParse, regTupleid);
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 18bf949..4b84695 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -3008,6 +3008,10 @@ struct Parse {
 #define OPFLAG_NOOP_IF_NULL  0x02 /* OP_FCopy: if source register is NULL
  * then do nothing
  */
+/* OP_RowData: xferOptimization started processing */
+#ifdef SQLITE_TEST
+#define OPFLAG_XFER_OPT      0x01
+#endif

 /*
  * Each trigger present in the database schema is stored as an
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index f50e389..5f9bc13 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -77,6 +77,16 @@
 int sql_search_count = 0;
 #endif

+#ifdef SQLITE_TEST
+/*
+ * The following global variable is incremented whenever the
+ * transfer optimization is used.  This is used for testing
+ * purposes only - to make sure the transfer optimization really
+ * is happening when it is supposed to.
+ */
+int sql_xfer_count = 0;
+#endif
+
 /*
  * When this global variable is positive, it gets decremented once before
  * each instruction in the VDBE.  When it reaches zero, the
u1.isInterrupted
@@ -3976,7 +3986,7 @@ case OP_SorterData: {
  break;
 }

-/* Opcode: RowData P1 P2 * * *
+/* Opcode: RowData P1 P2 * * P5
  * Synopsis: r[P2]=data
  *
  * Write into register P2 the complete row content for the row at
@@ -3984,6 +3994,8 @@ case OP_SorterData: {
  * There is no interpretation of the data.
  * It is just copied onto the P2 register exactly as
  * it is found in the database file.
+ * P5 can be used in debug mode to check if xferOptimization has
+ * actually started processing.
  *
  * If cursor P1 is an index, then the content is the key of the row.
  * If cursor P2 is a table, then the content extracted is the data.
@@ -3996,6 +4008,13 @@ case OP_RowData: {
  BtCursor *pCrsr;
  u32 n;

+#ifdef SQLITE_TEST
+ if (pOp->p5 == 1) {
+ pOp->p5 = 0;
+ sql_xfer_count++;
+ }
+#endif
+
  pOut = &aMem[pOp->p2];
  memAboutToChange(p, pOut);

diff --git a/test/sql-tap/gh-3307-xfer-optimization-issue.test.lua
b/test/sql-tap/gh-3307-xfer-optimization-issue.test.lua
new file mode 100755
index 0000000..34f603f
--- /dev/null
+++ b/test/sql-tap/gh-3307-xfer-optimization-issue.test.lua
@@ -0,0 +1,601 @@
+#!/usr/bin/env tarantool
+test = require("sqltester")
+test:plan(44)
+
+local bfr, aftr
+
+test:do_catchsql_test(
+ "xfer-optimization-1.1",
+ [[
+ CREATE TABLE t1(a INTEGER PRIMARY KEY, b INTEGER UNIQUE);
+ INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 3);
+ CREATE TABLE t2(a INTEGER PRIMARY KEY, b INTEGER UNIQUE);
+ INSERT INTO t2 SELECT * FROM t1;
+ ]], {
+ -- <xfer-optimization-1.1>
+ 0
+ -- <xfer-optimization-1.1>
+ })
+
+test:do_execsql_test(
+ "xfer-optimization-1.2",
+ [[
+ SELECT * FROM t2;
+ ]], {
+ -- <xfer-optimization-1.2>
+ 1, 1, 2, 2, 3, 3
+ -- <xfer-optimization-1.2>
+ })
+
+test:do_catchsql_test(
+ "xfer-optimization-1.3",
+ [[
+ DROP TABLE t1;
+ DROP TABLE t2;
+ CREATE TABLE t1(id INTEGER PRIMARY KEY, b INTEGER);
+ CREATE TABLE t2(id INTEGER PRIMARY KEY, b INTEGER);
+ CREATE INDEX i1 ON t1(b);
+ CREATE INDEX i2 ON t2(b);
+ INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 3);
+ INSERT INTO t2 SELECT * FROM t1;
+ ]], {
+ -- <xfer-optimization-1.3>
+ 0
+ -- <xfer-optimization-1.3>
+ })
+
+test:do_execsql_test(
+ "xfer-optimization-1.4",
+ [[
+ SELECT * FROM t2;
+ ]], {
+ -- <xfer-optimization-1.4>
+ 1, 1, 2, 2, 3, 3
+ -- <xfer-optimization-1.4>
+ })
+
+test:do_catchsql_test(
+ "xfer-optimization-1.5",
+ [[
+ DROP TABLE t1;
+ DROP TABLE t2;
+ CREATE TABLE t1(a INTEGER PRIMARY KEY, b INTEGER, c INTEGER);
+ INSERT INTO t1 VALUES (1, 1, 2), (2, 2, 3), (3, 3, 4);
+ CREATE TABLE t2(a INTEGER PRIMARY KEY, b INTEGER);
+ INSERT INTO t2 SELECT * FROM t1;
+ ]], {
+ -- <xfer-optimization-1.5>
+ 1, "table T2 has 2 columns but 3 values were supplied"
+ -- <xfer-optimization-1.5>
+ })
+
+test:do_execsql_test(
+ "xfer-optimization-1.6",
+ [[
+ SELECT * FROM t2;
+ ]], {
+ -- <xfer-optimization-1.6>
+
+ -- <xfer-optimization-1.6>
+ })
+
+test:do_catchsql_test(
+ "xfer-optimization-1.7",
+ [[
+ DROP TABLE t1;
+ DROP TABLE t2;
+ CREATE TABLE t1(a INTEGER PRIMARY KEY, b INTEGER);
+ INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 3);
+ CREATE TABLE t2(a INTEGER PRIMARY KEY, b INTEGER);
+ INSERT INTO t2 SELECT * FROM t1;
+ ]], {
+ -- <xfer-optimization-1.7>
+ 0
+ -- <xfer-optimization-1.7>
+ })
+
+test:do_execsql_test(
+ "xfer-optimization-1.8",
+ [[
+ SELECT * FROM t2;
+ ]], {
+ -- <xfer-optimization-1.6>
+ 1, 1, 2, 2, 3, 3
+ -- <xfer-optimization-1.6>
+ })
+
+test:do_catchsql_test(
+ "xfer-optimization-1.9",
+ [[
+ DROP TABLE t1;
+ DROP TABLE t2;
+ CREATE TABLE t1(a INTEGER PRIMARY KEY, b INTEGER);
+ INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 2);
+ CREATE TABLE t2(b INTEGER, a INTEGER PRIMARY KEY);
+ INSERT INTO t2 SELECT * FROM t1;
+ ]], {
+ -- <xfer-optimization-1.9>
+ 1, "Duplicate key exists in unique index 'sqlite_autoindex_T2_1' in space
'T2'"
+ -- <xfer-optimization-1.9>
+ })
+
+test:do_execsql_test(
+ "xfer-optimization-1.10",
+ [[
+ SELECT * FROM t2;
+ ]], {
+ -- <xfer-optimization-1.10>
+
+ -- <xfer-optimization-1.10>
+ })
+
+test:do_catchsql_test(
+ "xfer-optimization-1.11",
+ [[
+ DROP TABLE t1;
+ DROP TABLE t2;
+ CREATE TABLE t1(a INTEGER PRIMARY KEY, b INTEGER);
+ INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 2);
+ CREATE TABLE t2(b INTEGER PRIMARY KEY, a INTEGER);
+ INSERT INTO t2 SELECT * FROM t1;
+ ]], {
+ -- <xfer-optimization-1.11>
+ 0
+ -- <xfer-optimization-1.11>
+ })
+
+test:do_execsql_test(
+ "xfer-optimization-1.12",
+ [[
+ SELECT * FROM t2;
+ ]], {
+ -- <xfer-optimization-1.12>
+ 1, 1, 2, 2, 3, 2
+ -- <xfer-optimization-1.12>
+ })
+
+test:do_catchsql_test(
+ "xfer-optimization-1.13",
+ [[
+ DROP TABLE t1;
+ DROP TABLE t2;
+ CREATE TABLE t1(a INTEGER PRIMARY KEY, b);
+ CREATE TABLE t2(a INTEGER PRIMARY KEY, b);
+ INSERT INTO t1 VALUES (3, 3), (4, 4), (5, 5);
+ INSERT INTO t2 VALUES (1, 1), (2, 2);
+ INSERT INTO t2 SELECT * FROM t1;
+ ]], {
+ -- <xfer-optimization-1.13>
+ 0
+ -- <xfer-optimization-1.13>
+ })
+
+test:do_execsql_test(
+ "xfer-optimization-1.14",
+ [[
+ SELECT * FROM t2;
+ ]], {
+ -- <xfer-optimization-1.14>
+ 1, 1, 2, 2, 3, 3, 4, 4, 5, 5
+ -- <xfer-optimization-1.14>
+ })
+
+test:do_catchsql_test(
+ "xfer-optimization-1.15",
+ [[
+ DROP TABLE t1;
+ DROP TABLE t2;
+ CREATE TABLE t1(a INTEGER PRIMARY KEY, b UNIQUE);
+ CREATE TABLE t2(a INTEGER PRIMARY KEY, b UNIQUE);
+ INSERT INTO t1 VALUES (2, 2), (3, 3), (5, 5);
+ INSERT INTO t2 VALUES (1, 1), (4, 4);
+ INSERT OR ROLLBACK INTO t2 SELECT * FROM t1;
+ ]], {
+ -- <xfer-optimization-1.15>
+ 0
+ -- <xfer-optimization-1.15>
+ })
+
+test:do_execsql_test(
+ "xfer-optimization-1.16",
+ [[
+ SELECT * FROM t2;
+ ]], {
+ -- <xfer-optimization-1.16>
+ 1, 1, 2, 2, 3, 3, 4, 4, 5, 5
+ -- <xfer-optimization-1.16>
+ })
+
+-- The following tests are supposed to test if xfer-optimization is
actually
+-- used in the given cases (if the conflict actually occurs):
+-- 1.0) insert w/o explicit confl. action & w/o index replace action
+-- 1.1) insert w/o explicit confl. action & w/ index replace action &
+-- empty dest_table
+-- 1.2) insert w/o explicit confl. action & w/ index replace action &
+-- non-empty dest_table
+-- 2) insert with abort
+-- 3.0) insert with rollback (into empty table)
+-- 3.1) insert with rollback (into non-empty table)
+-- 4) insert with replace
+-- 5) insert with fail
+-- 6) insert with ignore
+
+local function do_xfer_test(test_number, return_code)
+ test_name = string.format("xfer-optimization-1.%d", test_number)
+ test:do_test(
+ test_name,
+ function()
+ if (aftr - bfr == 1) then
+ return {1}
+ end
+ if (aftr == bfr) then
+ return {0}
+ end
+ end, {
+ -- <test_name>
+ return_code
+ -- <test_name>
+ })
+end
+
+-- 1.0) insert w/o explicit confl. action & w/o index replace action
+------------------------------------------------------------------------------
+
+bfr = box.sql.debug().sql_xfer_count
+
+test:do_catchsql_test(
+ "xfer-optimization-1.17",
+ [[
+ DROP TABLE t1;
+ DROP TABLE t2;
+ CREATE TABLE t1(a INTEGER PRIMARY KEY, b);
+ CREATE TABLE t2(a INTEGER PRIMARY KEY, b);
+ INSERT INTO t1 VALUES (1, 1), (3, 3), (5, 5);
+ INSERT INTO t2 VALUES (2, 2), (3, 4);
+ BEGIN;
+ INSERT INTO t2 VALUES (4, 4);
+ INSERT INTO t2 SELECT * FROM t1;
+ ]], {
+ -- <xfer-optimization-1.17>
+ 1, "Duplicate key exists in unique index 'sqlite_autoindex_T2_1' in space
'T2'"
+ -- <xfer-optimization-1.17>
+ })
+
+test:do_execsql_test(
+ "xfer-optimization-1.18",
+ [[
+ INSERT INTO t2 VALUES (10, 10);
+ COMMIT;
+ SELECT * FROM t2;
+ ]], {
+ -- <xfer-optimization-1.18>
+ 2, 2, 3, 4, 4, 4, 10, 10
+ -- <xfer-optimization-1.18>
+ })
+
+aftr = box.sql.debug().sql_xfer_count
+
+do_xfer_test(19, 0)
+
+-- 1.1) insert w/o explicit confl. action & w/
+--      index replace action & empty dest_table
+------------------------------------------------------------------------------
+
+bfr = box.sql.debug().sql_xfer_count
+
+test:do_catchsql_test(
+ "xfer-optimization-1.20",
+ [[
+ DROP TABLE t1;
+ DROP TABLE t2;
+ CREATE TABLE t1(a INTEGER PRIMARY KEY ON CONFLICT REPLACE, b);
+ CREATE TABLE t2(a INTEGER PRIMARY KEY ON CONFLICT REPLACE, b);
+ CREATE TABLE t3(id INT PRIMARY KEY);
+ INSERT INTO t1 VALUES (1, 1), (3, 3), (5, 5);
+ BEGIN;
+ INSERT INTO t3 VALUES (1);
+ INSERT INTO t2 SELECT * FROM t1;
+ ]], {
+ -- <xfer-optimization-1.20>
+ 0
+ -- <xfer-optimization-1.20>
+ })
+
+test:do_execsql_test(
+ "xfer-optimization-1.21",
+ [[
+ INSERT INTO t2 VALUES (10, 10);
+ COMMIT;
+ SELECT * FROM t2;
+ ]], {
+ -- <xfer-optimization-1.21>
+ 1, 1, 3, 3, 5, 5, 10, 10
+ -- <xfer-optimization-1.21>
+ })
+
+aftr = box.sql.debug().sql_xfer_count
+
+test:do_execsql_test(
+ "xfer-optimization-1.22",
+ [[
+ SELECT * FROM t3;
+ ]], {
+ -- <xfer-optimization-1.22>
+ 1
+ -- <xfer-optimization-1.22>
+ })
+
+do_xfer_test(23, 1)
+
+-- 1.2) insert w/o explicit confl. action & w/
+-- index replace action & non-empty dest_table
+------------------------------------------------------------------------------
+
+bfr = box.sql.debug().sql_xfer_count
+
+test:do_catchsql_test(
+ "xfer-optimization-1.24",
+ [[
+ DROP TABLE t1;
+ DROP TABLE t2;
+ DROP TABLE t3;
+ CREATE TABLE t1(a INTEGER PRIMARY KEY ON CONFLICT REPLACE, b);
+ CREATE TABLE t2(a INTEGER PRIMARY KEY ON CONFLICT REPLACE, b);
+ INSERT INTO t1 VALUES (1, 1), (3, 3), (5, 5);
+ INSERT INTO t2 VALUES (2, 2), (3, 4);
+ BEGIN;
+ INSERT INTO t2 VALUES (4, 4);
+ INSERT INTO t2 SELECT * FROM t1;
+ ]], {
+ -- <xfer-optimization-1.24>
+ 0
+ -- <xfer-optimization-1.24>
+ })
+
+test:do_execsql_test(
+ "xfer-optimization-1.25",
+ [[
+ INSERT INTO t2 VALUES (10, 10);
+ COMMIT;
+ SELECT * FROM t2;
+ ]], {
+ -- <xfer-optimization-1.25>
+ 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 10, 10
+ -- <xfer-optimization-1.25>
+ })
+
+aftr = box.sql.debug().sql_xfer_count
+
+do_xfer_test(26, 0)
+
+-- 2) insert with abort
+------------------------------------------------------------------------------
+
+bfr = box.sql.debug().sql_xfer_count
+
+test:do_catchsql_test(
+ "xfer-optimization-1.27",
+ [[
+ DROP TABLE t1;
+ DROP TABLE t2;
+ CREATE TABLE t1(a INTEGER PRIMARY KEY, b);
+ CREATE TABLE t2(a INTEGER PRIMARY KEY, b);
+ INSERT INTO t1 VALUES (1, 1), (3, 3), (5, 5);
+ INSERT INTO t2 VALUES (2, 2), (3, 4);
+ BEGIN;
+ INSERT INTO t2 VALUES (4, 4);
+ INSERT OR ABORT INTO t2 SELECT * FROM t1;
+ ]], {
+ -- <xfer-optimization-1.27>
+ 1, "Duplicate key exists in unique index 'sqlite_autoindex_T2_1' in space
'T2'"
+ -- <xfer-optimization-1.27>
+ })
+
+test:do_execsql_test(
+ "xfer-optimization-1.28",
+ [[
+ INSERT INTO t2 VALUES (10, 10);
+ COMMIT;
+ SELECT * FROM t2;
+ ]], {
+ -- <xfer-optimization-1.28>
+ 2, 2, 3, 4, 4, 4, 10, 10
+ -- <xfer-optimization-1.28>
+ })
+
+aftr = box.sql.debug().sql_xfer_count
+
+do_xfer_test(29, 1)
+
+-- 3.0) insert with rollback (into empty table)
+------------------------------------------------------------------------------
+
+bfr = box.sql.debug().sql_xfer_count
+
+test:do_catchsql_test(
+ "xfer-optimization-1.30",
+ [[
+ DROP TABLE t1;
+ DROP TABLE t2;
+ CREATE TABLE t1(a INTEGER PRIMARY KEY, b);
+ CREATE TABLE t2(a INTEGER PRIMARY KEY, b);
+ INSERT INTO t1 VALUES (1, 1), (3, 3), (5, 5);
+ BEGIN;
+ INSERT OR ROLLBACK INTO t2 SELECT * FROM t1;
+ ]], {
+ -- <xfer-optimization-1.30>
+ 0
+ -- <xfer-optimization-1.30>
+ })
+
+test:do_execsql_test(
+ "xfer-optimization-1.31",
+ [[
+ INSERT INTO t2 VALUES (10, 10);
+ COMMIT;
+ SELECT * FROM t2;
+ ]], {
+ -- <xfer-optimization-1.31>
+ 1, 1, 3, 3, 5, 5, 10, 10
+ -- <xfer-optimization-1.31>
+ })
+
+aftr = box.sql.debug().sql_xfer_count
+
+do_xfer_test(32, 1)
+
+-- 3.1) insert with rollback (into non-empty table)
+------------------------------------------------------------------------------
+
+bfr = box.sql.debug().sql_xfer_count
+
+test:do_catchsql_test(
+ "xfer-optimization-1.33",
+ [[
+ DROP TABLE t1;
+ DROP TABLE t2;
+ CREATE TABLE t1(a INTEGER PRIMARY KEY, b);
+ CREATE TABLE t2(a INTEGER PRIMARY KEY, b);
+ INSERT INTO t1 VALUES (1, 1), (3, 3), (5, 5);
+ INSERT INTO t2 VALUES (2, 2), (3, 4);
+ BEGIN;
+ INSERT INTO t2 VALUES (4, 4);
+ INSERT OR ROLLBACK INTO t2 SELECT * FROM t1;
+ ]], {
+ -- <xfer-optimization-1.33>
+ 1, "UNIQUE constraint failed: T2.A"
+ -- <xfer-optimization-1.33>
+ })
+
+test:do_execsql_test(
+ "xfer-optimization-1.34",
+ [[
+ SELECT * FROM t2;
+ ]], {
+ -- <xfer-optimization-1.34>
+ 2, 2, 3, 4
+ -- <xfer-optimization-1.34>
+ })
+
+aftr = box.sql.debug().sql_xfer_count
+
+do_xfer_test(35, 0)
+
+-- 4) insert with replace
+------------------------------------------------------------------------------
+
+bfr = box.sql.debug().sql_xfer_count
+
+test:do_catchsql_test(
+ "xfer-optimization-1.36",
+ [[
+ DROP TABLE t1;
+ DROP TABLE t2;
+ CREATE TABLE t1(a INTEGER PRIMARY KEY, b);
+ CREATE TABLE t2(a INTEGER PRIMARY KEY, b);
+ INSERT INTO t1 VALUES (1, 1), (3, 3), (5, 5);
+ INSERT INTO t2 VALUES (2, 2), (3, 4);
+ BEGIN;
+ INSERT INTO t2 VALUES (4, 4);
+ INSERT OR REPLACE INTO t2 SELECT * FROM t1;
+ ]], {
+ -- <xfer-optimization-1.36>
+ 0
+ -- <xfer-optimization-1.36>
+ })
+
+test:do_execsql_test(
+ "xfer-optimization-1.37",
+ [[
+ INSERT INTO t2 VALUES (10, 10);
+ COMMIT;
+ SELECT * FROM t2;
+ ]], {
+ -- <xfer-optimization-1.37>
+ 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 10, 10
+ -- <xfer-optimization-1.37>
+ })
+
+aftr = box.sql.debug().sql_xfer_count
+
+do_xfer_test(38, 0)
+
+-- 5) insert with fail
+------------------------------------------------------------------------------
+
+bfr = box.sql.debug().sql_xfer_count
+
+test:do_catchsql_test(
+ "xfer-optimization-1.39",
+ [[
+ DROP TABLE t1;
+ DROP TABLE t2;
+ CREATE TABLE t1(a INTEGER PRIMARY KEY, b);
+ CREATE TABLE t2(a INTEGER PRIMARY KEY, b);
+ INSERT INTO t1 VALUES (1, 1), (3, 3), (5, 5);
+ INSERT INTO t2 VALUES (2, 2), (3, 4);
+ BEGIN;
+ INSERT INTO t2 VALUES (4, 4);
+ INSERT OR FAIL INTO t2 SELECT * FROM t1;
+ ]], {
+ -- <xfer-optimization-1.39>
+ 1, "Duplicate key exists in unique index 'sqlite_autoindex_T2_1' in space
'T2'"
+ -- <xfer-optimization-1.39>
+ })
+
+test:do_execsql_test(
+ "xfer-optimization-1.40",
+ [[
+ INSERT INTO t2 VALUES (10, 10);
+ COMMIT;
+ SELECT * FROM t2;
+ ]], {
+ -- <xfer-optimization-1.40>
+ 1, 1, 2, 2, 3, 4, 4, 4, 10, 10
+ -- <xfer-optimization-1.40>
+ })
+
+aftr = box.sql.debug().sql_xfer_count
+
+do_xfer_test(41, 0)
+
+-- 6) insert with ignore
+------------------------------------------------------------------------------
+
+bfr = box.sql.debug().sql_xfer_count
+
+test:do_catchsql_test(
+ "xfer-optimization-1.42",
+ [[
+ DROP TABLE t1;
+ DROP TABLE t2;
+ CREATE TABLE t1(a INTEGER PRIMARY KEY, b);
+ CREATE TABLE t2(a INTEGER PRIMARY KEY, b);
+ INSERT INTO t1 VALUES (1, 1), (3, 3), (5, 5);
+ INSERT INTO t2 VALUES (2, 2), (3, 4);
+ BEGIN;
+ INSERT INTO t2 VALUES (4, 4);
+ INSERT OR IGNORE INTO t2 SELECT * FROM t1;
+ ]], {
+ -- <xfer-optimization-1.42>
+ 0
+ -- <xfer-optimization-1.42>
+ })
+
+test:do_execsql_test(
+ "xfer-optimization-1.43",
+ [[
+ INSERT INTO t2 VALUES (10, 10);
+ COMMIT;
+ SELECT * FROM t2;
+ ]], {
+ -- <xfer-optimization-1.43>
+ 1, 1, 2, 2, 3, 4, 4, 4, 5, 5, 10, 10
+ -- <xfer-optimization-1.43>
+ })
+
+aftr = box.sql.debug().sql_xfer_count
+
+do_xfer_test(44, 0)
+
+test:finish_test()

Other related posts: