Please, add to commit message results of benchmark to indicate
that this optimisation really matters.
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;
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;
+ 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));
+ 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.
+ */
+
+ 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++;
+#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);
+ 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;
+ ]], {
+ -- <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}
+ 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