Hello. I considered your remarks and the following changes were made: diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index ae8dafb..734ff34 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -1645,6 +1645,12 @@ xferCompatibleIndex(Index * pDest, Index * pSrc) assert(pDest->pTable != pSrc->pTable); uint32_t nDestCol = index_column_count(pDest); uint32_t nSrcCol = index_column_count(pSrc); + /* One of them is PK while the other isn't. */ + if ((pDest->idxType == SQLITE_IDXTYPE_PRIMARYKEY && + pSrc->idxType != SQLITE_IDXTYPE_PRIMARYKEY) || + (pDest->idxType != SQLITE_IDXTYPE_PRIMARYKEY && + pSrc->idxType == SQLITE_IDXTYPE_PRIMARYKEY)) + return 0; if (nDestCol != nSrcCol) { return 0; /* Different number of columns */ } @@ -1718,9 +1724,10 @@ 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(); + int dest_has_replace_action = 0; + int confl_action_default = 0; if (pSelect == NULL) return 0; /* Must be of the form INSERT INTO ... SELECT ... */ @@ -1737,8 +1744,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; + confl_action_default = 1; + } } assert(pSelect->pSrc); /* allocated even if there is no FROM clause */ if (pSelect->pSrc->nSrc != 1) { @@ -1828,15 +1837,16 @@ xferOptimization(Parse * pParse, /* Parser context */ return 0; /* Default values must be the same for all columns */ } } + if (pDestCol->notNull == ON_CONFLICT_ACTION_REPLACE) + dest_has_replace_action = 1; } 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; } + if (pDestIdx->onError != ON_CONFLICT_ACTION_REPLACE) + dest_has_replace_action = 1; if (pSrcIdx == 0) { return 0; /* pDestIdx has no corresponding index in pSrc */ } @@ -1875,58 +1885,63 @@ xferOptimization(Parse * pParse, /* Parser context */ 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. - */ + + /* Xfer optimization is unable to correctly insert + * data in case there's a conflict action + * other than ON_CONFLICT_ACTION_ROLLBACK or there's + * ON_CONFLICT_ACTION_DEFAULT which was transformed into + * ON_CONFLICT_ACTION_ABORT for insertion while we have a + * ON_CONFLICT_ACTION_REPLACE for any of constraints. + * 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_ROLLBACK || + (confl_action_default == 1 && + dest_has_replace_action == 1)) { 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); - emit_open_cursor(pParse, iSrc, pSrcIdx->tnum); - sqlite3VdbeSetP4KeyInfo(pParse, pSrcIdx); - VdbeComment((v, "%s", pSrcIdx->zName)); - emit_open_cursor(pParse, iDest, pDestIdx->tnum); - sqlite3VdbeSetP4KeyInfo(pParse, pDestIdx); - sqlite3VdbeChangeP5(v, OPFLAG_BULKCSR); - 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); - } + int space_ptr_reg = ++pParse->nMem; + 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; + + pDestIdx = sqlite3PrimaryKeyIndex(pDest); + pSrcIdx = sqlite3PrimaryKeyIndex(pSrc); + space_ptr_reg = ++pParse->nMem; + sqlite3VdbeAddOp4Ptr(v, OP_LoadPtr, 0, space_ptr_reg, 0, + (void *) src_space); + sqlite3VdbeAddOp3(v, OP_OpenWrite, iSrc, + pSrc->tnum, space_ptr_reg); + VdbeComment((v, "%s", src_idx->def->name)); + + if((dest_idx = space_index(dest_space, 0)) == NULL) + return 0; + space_ptr_reg = ++pParse->nMem; + sqlite3VdbeAddOp4Ptr(v, OP_LoadPtr, 0, space_ptr_reg, 0, + (void *) dest_space); + sqlite3VdbeAddOp3(v, OP_OpenWrite, iDest, + pSrc->tnum, space_ptr_reg); + sqlite3VdbeChangeP5(v, OPFLAG_BULKCSR); + 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..860b4c3 --- /dev/null +++ b/test/sql-tap/gh-3307-xfer-optimization-issue.test.lua @@ -0,0 +1,233 @@ +#!/usr/bin/env tarantool +test = require("sqltester") +test:plan(18) + +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; + ]], { + -- + 0 + -- + }) + +test:do_execsql_test( + "xfer-oprimization-1.2", + [[ + SELECT * FROM t2; + ]], { + -- + 1, 1, 2, 2, 3, 3 + -- + }) + +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; + ]], { + -- + 0 + -- + }) + +test:do_execsql_test( + "xfer-oprimization-1.4", + [[ + SELECT * FROM t2; + ]], { + -- + 1, 1, 2, 2, 3, 3 + -- + }) + +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; + ]], { + -- + 1, "table T2 has 2 columns but 3 values were supplied" + -- + }) + +test:do_execsql_test( + "xfer-oprimization-1.6", + [[ + SELECT * FROM t2; + ]], { + -- + + -- + }) + +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; + ]], { + -- + 0 + -- + }) + +test:do_execsql_test( + "xfer-oprimization-1.8", + [[ + SELECT * FROM t2; + ]], { + -- + 1, 1, 2, 2, 3, 3 + -- + }) + +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; + ]], { + -- + 1, "Duplicate key exists in unique index 'sqlite_autoindex_T2_1' in space 'T2'" + -- + }) + +test:do_execsql_test( + "xfer-oprimization-1.10", + [[ + SELECT * FROM t2; + ]], { + -- + + -- + }) + +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; + ]], { + -- + 0 + -- + }) + +test:do_execsql_test( + "xfer-oprimization-1.12", + [[ + SELECT * FROM t2; + ]], { + -- + 1, 1, 2, 2, 3, 2 + -- + }) + +test:do_catchsql_test( + "xfer-optimization-1.13", + [[ + DROP TABLE t1; + DROP TABLE t2; + CREATE TABLE t1(a INTEGER PRIMARY KEY, b UNIQUE ON CONFLICT REPLACE); + CREATE TABLE t2(a INTEGER PRIMARY KEY, b UNIQUE ON CONFLICT REPLACE); + INSERT INTO t1 VALUES (3, 3), (4, 4), (5, 5); + INSERT INTO t2 VALUES (1, 1), (2, 2); + INSERT INTO t2 SELECT * FROM t1; + ]], { + -- + 0 + -- + }) + +test:do_execsql_test( + "xfer-oprimization-1.14", + [[ + SELECT * FROM t2; + ]], { + -- + 1, 1, 2, 2, 3, 3, 4, 4, 5, 5 + -- + }) + +test:do_catchsql_test( + "xfer-optimization-1.15", + [[ + DROP TABLE t1; + DROP TABLE t2; + CREATE TABLE t1(a INTEGER PRIMARY KEY, b UNIQUE ON CONFLICT REPLACE); + CREATE TABLE t2(a INTEGER PRIMARY KEY, b UNIQUE ON CONFLICT REPLACE); + 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; + ]], { + -- + 0 + -- + }) + +test:do_execsql_test( + "xfer-oprimization-1.16", + [[ + SELECT * FROM t2; + ]], { + -- + 1, 1, 2, 2, 3, 3, 4, 4, 5, 5 + -- + }) + +test:do_catchsql_test( + "xfer-optimization-1.17", + [[ + DROP TABLE t1; + DROP TABLE t2; + CREATE TABLE t1(a INTEGER PRIMARY KEY, b UNIQUE ON CONFLICT REPLACE); + CREATE TABLE t2(a INTEGER PRIMARY KEY, b UNIQUE ON CONFLICT REPLACE); + INSERT INTO t1 VALUES (1, 2), (3, 3), (5, 5); + INSERT INTO t2 VALUES (1, 1), (4, 4); + INSERT OR REPLACE INTO t2 SELECT * FROM t1; + ]], { + -- + 0 + -- + }) + +test:do_execsql_test( + "xfer-oprimization-1.18", + [[ + SELECT * FROM t2; + ]], { + -- + 1, 2, 3, 3, 4, 4, 5, 5 + -- + }) + +test:finish_test()