[tarantool-patches] Re: [PATCH] sql: xfer optimization issue
Nikita Tatunov
hollow653 at gmail.com
Wed Jul 18 23:18:04 MSK 2018
Hello, Nikita! Here's newer version of the patch.
Diff can be found at the end.
ср, 18 июл. 2018 г. в 18:13, n.pettik <korablev at tarantool.org>:
> 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 at gmail.com> 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()
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20180718/5d755a85/attachment.html>
More information about the Tarantool-patches
mailing list