вс, 29 июл. 2018 г. в 4:12, Alexander Turenko < alexander.turenko@tarantool.org>: > Hi! > > Please consider my comments and questions below. > > WBR, Alexander Turenko. > > > + /* > > + * Xfer optimization is unable to correctly insert data > > + * in case there's a conflict action other than > > + * explicit *_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)) { > > Do you mean that: > > 1. The optimization non-empty table case correctly works only with ABORT > conflict action (default or explicit). > 2. The default conflict action can be overwritten on per-column basis, so > 'default abort' can really be replace or other conflict action. > > If so, your description doesn't give this information. > > To more on that, can we check per-column conflict actions instead of check > whether the conflict action default or explicit? This would enable more > cases > with non-empty tables for the optimization. And this would look less > confusing, > IMHO. > Well, basically, you're right. But the thing is that we're going to remove column conflict actions, thus, I suppose, it doesn't make sense. Nikita, Correct me if I'm wrong please. > > I have one more question on that. It seems that SQLite has this > optimization > working with ROLLBACK conflict action. We cannot doing so, because of some > problem? Did this problem described / trackerized somewhere? Or something > changes underhood and makes this impossible? Are we know exact reason or > just > observing it does not work? > I investigated that a little. OP_IdxInsert was changed so that we can process ABORT, FAIL or IGNORE. I took it into account in newer version hence it is also used in these cases even when the destination table is not empty. > > > +#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 > > I think it would be good to mention the opcode where the counter is > incremented. You can follow the style in which other counters are > described (they are mostly mention opcodes). > Added. > > > -/* Opcode: RowData P1 P2 * * * > > +/* Opcode: RowData P1 P2 * * P5 > > We can increase the counter on per-operation basis instead of per-row by > adding the flag to OP_OpenWrite. It will save some CPU cycles :) > > +#ifdef SQLITE_TEST > > + if ((pOp->p5 & OPFLAG_XFER_OPT) != 0) { > > + pOp->p5 = 0; > > + sql_xfer_count++; > > + } > > +#endif > > 1. Not actual due to 2, but it would be better to use > `pOp->p5 &= ~OPFLAG_XFER_OPT` to drop just that flag. > 2. It is counter-intuitive, IMHO, to change operation flags during that > operation. So, said above, vote to move it to OP_OpenWrite. > > Made the first. > > +local bfr, aftr > > + > > What do you plan to do with saved letters? :) Really, such abbreviations > just makes reading harder with no gains. > I made dis :( (p.s. Changed it) > > > +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() > > + return {aftr - bfr} > > + end, { > > + -- > > + return_code > > + -- > > + }) > > +end > > That code can be written simpler (consider tap module documentation): > > test:is(after - before, exp, test_name) > > I suggest to create wrappers like so (I didn't test it): > > local function do_xfer_test(test_func, test, test_name, func, exp, opts) > local opts = opts or {} > local exp_xfer_count = opts.exp_xfer_count > local before = box.sql.debug().sql_xfer_count > local ok = test_func(test, test_name, func, exp) > local after = box.sql.debug().sql_xfer_count > if exp_xfer_count ~= nil then > ok = ok and test:is(after - before, exp_xfer_count, test_name .. > '_xfer_count') > end > return ok > end > > test.do_execsql_xfer_test = function(test, test_name, func, exp, opts) > return do_xfer_test(test.do_execsql_test, test, test_name, func, exp, > opts) > end > > test.do_catchsql_xfer_test = function(test, test_name, func, exp, opts) > return do_xfer_test(test.do_catchsql_test, test, test_name, func, exp, > opts) > end > > And use it like so: > > test:do_catchsql_xfer_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 > -- > }), { > exp_xfer_count = 1 > } > ) > > Thank you for suggestion. Added that implementation. (Needed to change it a little bit) > By the way, you can revive xfer cases in test/sql-tap/with2.test.lua. Or > drop > it if your new test includes all related cases from with2. > > They're alive now! > On Fri, Jul 20, 2018 at 07:58:48PM +0300, Nikita Tatunov wrote: > > Ooops. Thank you! fixed it and pushed. > > > > пт, 20 июл. 2018 г. в 19:43, n.pettik <[1]korablev@tarantool.org>: > > > > LGTM. > > > > diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c > > index 3c3bf37..4f52fa5 100644 > > --- a/src/box/sql/insert.c > > +++ b/src/box/sql/insert.c > > @@ -1869,7 +1869,7 @@ xferOptimization(Parse * pParse, /* Parser > > context */ > > * table (tab1) is initially empty. > > */ > > > > - /* The Vdbe we're building*/ > > + /* The Vdbe struct we're building. */ > > > > You misunderstood me. What I mean is: > > struct Vibe *v = …; > > > > Vdbe *v = sqlite3GetVdbe(pParse); > > > > References > > > > 1. mailto:korablev@tarantool.org Diff: diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index 3c45920..c3cf94a 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -1715,7 +1715,6 @@ xferOptimization(Parse * pParse, /* Parser context */ int iSrc, iDest; /* Cursors from source and destination */ int addr1; /* Loop addresses */ int emptyDestTest = 0; /* Address of test for empty pDest */ - int emptySrcTest = 0; /* Address of test for empty pSrc */ int regData, regTupleid; /* Registers holding data and tupleid */ struct session *user_session = current_session(); bool is_err_action_default = false; @@ -1881,13 +1880,15 @@ xferOptimization(Parse * pParse, /* Parser context */ /* * Xfer optimization is unable to correctly insert data - * in case there's a conflict action other than - * explicit *_ABORT. This is the reason we want to only + * in case there's a conflict action other than *_ABORT, + * *_FAIL or *_IGNORE. 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)) { + if (!(onError == ON_CONFLICT_ACTION_ABORT || + onError == ON_CONFLICT_ACTION_FAIL || + onError == ON_CONFLICT_ACTION_IGNORE) || + is_err_action_default) { addr1 = sqlite3VdbeAddOp2(v, OP_Rewind, iDest, 0); VdbeCoverage(v); emptyDestTest = sqlite3VdbeAddOp0(v, OP_Goto); @@ -1905,15 +1906,23 @@ xferOptimization(Parse * pParse, /* Parser context */ #endif sqlite3VdbeAddOp2(v, OP_IdxInsert, iDest, regData); - sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE); + switch (onError) { + case ON_CONFLICT_ACTION_IGNORE: + sqlite3VdbeChangeP5(v, OPFLAG_OE_IGNORE); + break; + case ON_CONFLICT_ACTION_FAIL: + sqlite3VdbeChangeP5(v, OPFLAG_OE_FAIL); + break; + default: + sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE); + break; + } 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); sqlite3ReleaseTempReg(pParse, regData); if (emptyDestTest) { diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index aa7f250..ce01039 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -79,10 +79,10 @@ int sql_search_count = 0; #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. + * The following global variable is incremented in OP_RowData + * whenever the xfer optimization is used. This is used on + * testing purposes only - to make sure the transfer optimization + * really is happening when it is supposed to. */ int sql_xfer_count = 0; #endif @@ -4008,9 +4008,13 @@ case OP_RowData: { BtCursor *pCrsr; u32 n; +/* + * Flag P5 is cleared after the first insertion using xfer + * optimization. + */ #ifdef SQLITE_TEST if ((pOp->p5 & OPFLAG_XFER_OPT) != 0) { - pOp->p5 = 0; + pOp->p5 &= ~OPFLAG_XFER_OPT; sql_xfer_count++; } #endif diff --git a/test/sql-tap/gh-3307-xfer-optimization-issue.test.lua b/test/sql-tap/gh-3307-xfer-optimization-issue.test.lua index 05d30c6..b99de2b 100755 --- a/test/sql-tap/gh-3307-xfer-optimization-issue.test.lua +++ b/test/sql-tap/gh-3307-xfer-optimization-issue.test.lua @@ -2,24 +2,28 @@ test = require("sqltester") test:plan(46) -local bfr, aftr - -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() - return {aftr - bfr} - end, { - -- - return_code - -- - }) +local function do_xfer_test(test, test_func, test_name, func, exp, opts) + local opts = opts or {} + local exp_xfer_count = opts.exp_xfer_count + local before = box.sql.debug().sql_xfer_count + local ok, result = pcall(test_func, test, test_name, func, exp) + local after = box.sql.debug().sql_xfer_count + if exp_xfer_count ~= nil then + ok = ok and test:is(after - before, exp_xfer_count, + test_name .. '-xfer-count') + end + return ok end -bfr = box.sql.debug().sql_xfer_count +test.do_execsql_xfer_test = function(test, test_name, func, exp, opts) + return do_xfer_test(test, test.do_execsql_test, test_name, func, exp, opts) +end + +test.do_catchsql_xfer_test = function(test, test_name, func, exp, opts) + return do_xfer_test(test, test.do_catchsql_test, test_name, func, exp, opts) +end -test:do_catchsql_test( +test:do_catchsql_xfer_test( "xfer-optimization-1.1", [[ CREATE TABLE t1(a INTEGER PRIMARY KEY, b INTEGER UNIQUE); @@ -30,10 +34,10 @@ test:do_catchsql_test( -- 0 -- + }, { + exp_xfer_count = 1 }) -aftr = box.sql.debug().sql_xfer_count - test:do_execsql_test( "xfer-optimization-1.2", [[ @@ -44,12 +48,8 @@ test:do_execsql_test( -- }) -do_xfer_test(3, 1) - -bfr = box.sql.debug().sql_xfer_count - -test:do_catchsql_test( - "xfer-optimization-1.4", +test:do_catchsql_xfer_test( + "xfer-optimization-1.3", [[ DROP TABLE t1; DROP TABLE t2; @@ -60,15 +60,15 @@ test:do_catchsql_test( INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 3); INSERT INTO t2 SELECT * FROM t1; ]], { - -- + -- 0 - -- + -- + }, { + exp_xfer_count = 1 }) -aftr = box.sql.debug().sql_xfer_count - test:do_execsql_test( - "xfer-optimization-1.5", + "xfer-optimization-1.4", [[ SELECT * FROM t2; ]], { @@ -77,12 +77,8 @@ test:do_execsql_test( -- }) -do_xfer_test(6, 1) - -bfr = box.sql.debug().sql_xfer_count - -test:do_catchsql_test( - "xfer-optimization-1.7", +test:do_catchsql_xfer_test( + "xfer-optimization-1.5", [[ DROP TABLE t1; DROP TABLE t2; @@ -91,29 +87,25 @@ test:do_catchsql_test( 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" - -- + -- + }, { + exp_xfer_count = 0 }) -aftr = box.sql.debug().sql_xfer_count - test:do_execsql_test( - "xfer-optimization-1.8", + "xfer-optimization-1.6", [[ SELECT * FROM t2; ]], { - -- + -- - -- + -- }) -do_xfer_test(9, 0) - -bfr = box.sql.debug().sql_xfer_count - -test:do_catchsql_test( - "xfer-optimization-1.10", +test:do_catchsql_xfer_test( + "xfer-optimization-1.7", [[ DROP TABLE t1; DROP TABLE t2; @@ -122,29 +114,25 @@ test:do_catchsql_test( CREATE TABLE t2(a INTEGER PRIMARY KEY, b INTEGER); INSERT INTO t2 SELECT * FROM t1; ]], { - -- + -- 0 - -- + -- + }, { + exp_xfer_count = 1 }) -aftr = box.sql.debug().sql_xfer_count - test:do_execsql_test( - "xfer-optimization-1.11", + "xfer-optimization-1.8", [[ SELECT * FROM t2; ]], { - -- + -- 1, 1, 2, 2, 3, 3 - -- + -- }) -do_xfer_test(12, 1); - -bfr = box.sql.debug().sql_xfer_count - -test:do_catchsql_test( - "xfer-optimization-1.13", +test:do_catchsql_xfer_test( + "xfer-optimization-1.9", [[ DROP TABLE t1; DROP TABLE t2; @@ -153,29 +141,25 @@ test:do_catchsql_test( 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'" - -- + -- + }, { + exp_xfer_count = 0 }) -aftr = box.sql.debug().sql_xfer_count - test:do_execsql_test( - "xfer-optimization-1.14", + "xfer-optimization-1.10", [[ SELECT * FROM t2; ]], { - -- + -- - -- + -- }) -do_xfer_test(15, 0) - -bfr = box.sql.debug().sql_xfer_count - -test:do_catchsql_test( - "xfer-optimization-1.16", +test:do_catchsql_xfer_test( + "xfer-optimization-1.11", [[ DROP TABLE t1; DROP TABLE t2; @@ -184,25 +168,23 @@ test:do_catchsql_test( CREATE TABLE t2(b INTEGER PRIMARY KEY, a INTEGER); INSERT INTO t2 SELECT * FROM t1; ]], { - -- + -- 0 - -- + -- + }, { + exp_xfer_count = 1 }) -aftr = box.sql.debug().sql_xfer_count - test:do_execsql_test( - "xfer-optimization-1.17", + "xfer-optimization-1.12", [[ SELECT * FROM t2; ]], { - -- + -- 1, 1, 2, 2, 3, 2 - -- + -- }) -do_xfer_test(18, 1) - -- 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 @@ -221,10 +203,8 @@ do_xfer_test(18, 1) -- 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.19", +test:do_catchsql_xfer_test( + "xfer-optimization-1.13", [[ DROP TABLE t1; DROP TABLE t2; @@ -236,35 +216,31 @@ test:do_catchsql_test( INSERT INTO t2 VALUES (4, 4); INSERT INTO t2 SELECT * FROM t1; ]], { - -- + -- 1, "Duplicate key exists in unique index 'sqlite_autoindex_T2_1' in space 'T2'" - -- + -- + }, { + exp_xfer_count = 0 }) test:do_execsql_test( - "xfer-optimization-1.20", + "xfer-optimization-1.14", [[ INSERT INTO t2 VALUES (10, 10); COMMIT; SELECT * FROM t2; ]], { - -- + -- 2, 2, 3, 4, 4, 4, 10, 10 - -- + -- }) -aftr = box.sql.debug().sql_xfer_count - -do_xfer_test(21, 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.22", +test:do_catchsql_xfer_test( + "xfer-optimization-1.15", [[ DROP TABLE t1; DROP TABLE t2; @@ -276,45 +252,41 @@ test:do_catchsql_test( INSERT INTO t3 VALUES (1); INSERT INTO t2 SELECT * FROM t1; ]], { - -- + -- 0 - -- + -- + }, { + exp_xfer_count = 1 }) test:do_execsql_test( - "xfer-optimization-1.23", + "xfer-optimization-1.16", [[ INSERT INTO t2 VALUES (10, 10); COMMIT; SELECT * FROM t2; ]], { - -- + -- 1, 1, 3, 3, 5, 5, 10, 10 - -- + -- }) -aftr = box.sql.debug().sql_xfer_count - test:do_execsql_test( - "xfer-optimization-1.24", + "xfer-optimization-1.17", [[ SELECT * FROM t3; ]], { - -- + -- 1 - -- + -- }) -do_xfer_test(25, 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.26", +test:do_catchsql_xfer_test( + "xfer-optimization-1.18", [[ DROP TABLE t1; DROP TABLE t2; @@ -327,34 +299,30 @@ test:do_catchsql_test( INSERT INTO t2 VALUES (4, 4); INSERT INTO t2 SELECT * FROM t1; ]], { - -- + -- 0 - -- + -- + }, { + exp_xfer_count = 0 }) test:do_execsql_test( - "xfer-optimization-1.27", + "xfer-optimization-1.19", [[ INSERT INTO t2 VALUES (10, 10); COMMIT; SELECT * FROM t2; ]], { - -- + -- 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 10, 10 - -- + -- }) -aftr = box.sql.debug().sql_xfer_count - -do_xfer_test(28, 0) - -- 2) insert with abort ------------------------------------------------------------------------------ -bfr = box.sql.debug().sql_xfer_count - -test:do_catchsql_test( - "xfer-optimization-1.29", +test:do_catchsql_xfer_test( + "xfer-optimization-1.20", [[ DROP TABLE t1; DROP TABLE t2; @@ -366,34 +334,30 @@ test:do_catchsql_test( INSERT INTO t2 VALUES (4, 4); INSERT OR ABORT INTO t2 SELECT * FROM t1; ]], { - -- + -- 1, "Duplicate key exists in unique index 'sqlite_autoindex_T2_1' in space 'T2'" - -- + -- + }, { + exp_xfer_count = 1 }) test:do_execsql_test( - "xfer-optimization-1.30", + "xfer-optimization-1.21", [[ INSERT INTO t2 VALUES (10, 10); COMMIT; SELECT * FROM t2; ]], { - -- + -- 2, 2, 3, 4, 4, 4, 10, 10 - -- + -- }) -aftr = box.sql.debug().sql_xfer_count - -do_xfer_test(31, 1) - -- 3.0) insert with rollback (into empty table) ------------------------------------------------------------------------------ -bfr = box.sql.debug().sql_xfer_count - -test:do_catchsql_test( - "xfer-optimization-1.32", +test:do_catchsql_xfer_test( + "xfer-optimization-1.22", [[ DROP TABLE t1; DROP TABLE t2; @@ -403,34 +367,30 @@ test:do_catchsql_test( BEGIN; INSERT OR ROLLBACK INTO t2 SELECT * FROM t1; ]], { - -- + -- 0 - -- + -- + }, { + exp_xfer_count = 1 }) test:do_execsql_test( - "xfer-optimization-1.33", + "xfer-optimization-1.23", [[ INSERT INTO t2 VALUES (10, 10); COMMIT; SELECT * FROM t2; ]], { - -- + -- 1, 1, 3, 3, 5, 5, 10, 10 - -- + -- }) -aftr = box.sql.debug().sql_xfer_count - -do_xfer_test(34, 1) - -- 3.1) insert with rollback (into non-empty table) ------------------------------------------------------------------------------ -bfr = box.sql.debug().sql_xfer_count - -test:do_catchsql_test( - "xfer-optimization-1.35", +test:do_catchsql_xfer_test( + "xfer-optimization-1.24", [[ DROP TABLE t1; DROP TABLE t2; @@ -442,32 +402,28 @@ test:do_catchsql_test( INSERT INTO t2 VALUES (4, 4); INSERT OR ROLLBACK INTO t2 SELECT * FROM t1; ]], { - -- + -- 1, "UNIQUE constraint failed: T2.A" - -- + -- + }, { + exp_xfer_count = 0 }) test:do_execsql_test( - "xfer-optimization-1.36", + "xfer-optimization-1.25", [[ SELECT * FROM t2; ]], { - -- + -- 2, 2, 3, 4 - -- + -- }) -aftr = box.sql.debug().sql_xfer_count - -do_xfer_test(37, 0) - -- 4) insert with replace ------------------------------------------------------------------------------ -bfr = box.sql.debug().sql_xfer_count - -test:do_catchsql_test( - "xfer-optimization-1.38", +test:do_catchsql_xfer_test( + "xfer-optimization-1.26", [[ DROP TABLE t1; DROP TABLE t2; @@ -479,34 +435,30 @@ test:do_catchsql_test( INSERT INTO t2 VALUES (4, 4); INSERT OR REPLACE INTO t2 SELECT * FROM t1; ]], { - -- + -- 0 - -- + -- + }, { + exp_xfer_count = 0 }) test:do_execsql_test( - "xfer-optimization-1.39", + "xfer-optimization-1.27", [[ INSERT INTO t2 VALUES (10, 10); COMMIT; SELECT * FROM t2; ]], { - -- + -- 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 10, 10 - -- + -- }) -aftr = box.sql.debug().sql_xfer_count - -do_xfer_test(40, 0) - -- 5) insert with fail ------------------------------------------------------------------------------ -bfr = box.sql.debug().sql_xfer_count - -test:do_catchsql_test( - "xfer-optimization-1.41", +test:do_catchsql_xfer_test( + "xfer-optimization-1.28", [[ DROP TABLE t1; DROP TABLE t2; @@ -518,34 +470,30 @@ test:do_catchsql_test( INSERT INTO t2 VALUES (4, 4); INSERT OR FAIL INTO t2 SELECT * FROM t1; ]], { - -- + -- 1, "Duplicate key exists in unique index 'sqlite_autoindex_T2_1' in space 'T2'" - -- + -- + }, { + exp_xfer_count = 1 }) test:do_execsql_test( - "xfer-optimization-1.42", + "xfer-optimization-1.29", [[ INSERT INTO t2 VALUES (10, 10); COMMIT; SELECT * FROM t2; ]], { - -- + -- 1, 1, 2, 2, 3, 4, 4, 4, 10, 10 - -- + -- }) -aftr = box.sql.debug().sql_xfer_count - -do_xfer_test(43, 0) - -- 6) insert with ignore ------------------------------------------------------------------------------ -bfr = box.sql.debug().sql_xfer_count - -test:do_catchsql_test( - "xfer-optimization-1.44", +test:do_catchsql_xfer_test( + "xfer-optimization-1.30", [[ DROP TABLE t1; DROP TABLE t2; @@ -557,25 +505,23 @@ test:do_catchsql_test( INSERT INTO t2 VALUES (4, 4); INSERT OR IGNORE INTO t2 SELECT * FROM t1; ]], { - -- + -- 0 - -- + -- + }, { + exp_xfer_count = 1 }) test:do_execsql_test( - "xfer-optimization-1.45", + "xfer-optimization-1.31", [[ INSERT INTO t2 VALUES (10, 10); COMMIT; SELECT * FROM t2; ]], { - -- + -- 1, 1, 2, 2, 3, 4, 4, 4, 5, 5, 10, 10 - -- + -- }) -aftr = box.sql.debug().sql_xfer_count - -do_xfer_test(46, 0) - test:finish_test() diff --git a/test/sql-tap/with2.test.lua b/test/sql-tap/with2.test.lua index fbd1f4e..bd0187f 100755 --- a/test/sql-tap/with2.test.lua +++ b/test/sql-tap/with2.test.lua @@ -1,6 +1,6 @@ #!/usr/bin/env tarantool test = require("sqltester") -test:plan(44) +test:plan(59) --!./tcltestrunner.lua -- 2014 January 11 @@ -382,43 +382,136 @@ genstmt(255), { -- -- -- }) ------------------------------------------------------------------------------ --- Check that adding a WITH clause to an INSERT disables the xfer +----------------------------------------------------------------- +-- Check that adding a WITH clause to an INSERT disables the xfer -- optimization. --- --- Tarantool: `sqlite3_xferopt_count` is not exported --- Need to understand if this optimization works at all and if we really need it. --- Commented so far. --- function do_xfer_test(tn, bXfer, sql, res) --- res = res or "" --- sqlite3_xferopt_count = 0 --- X(267, "X!cmd", [=[["uplevel",[["list","do_test",["tn"],["\n set dres [db eval {",["sql"],"}]\n list [set ::sqlite3_xferopt_count] [set dres]\n "],[["list",["bXfer"],["res"]]]]]]]=]) --- end --- test:do_execsql_test( --- 5.1, --- [[ --- DROP TABLE IF EXISTS t1; --- DROP TABLE IF EXISTS t2; --- CREATE TABLE t1(a PRIMARY KEY, b); --- CREATE TABLE t2(a PRIMARY KEY, b); --- ]]) - --- do_xfer_test(5.2, 1, " INSERT INTO t1 SELECT * FROM t2 ") --- do_xfer_test(5.3, 0, " INSERT INTO t1 SELECT a, b FROM t2 ") --- do_xfer_test(5.4, 0, " INSERT INTO t1 SELECT b, a FROM t2 ") --- do_xfer_test(5.5, 0, [[ --- WITH x AS (SELECT a, b FROM t2) INSERT INTO t1 SELECT * FROM x --- ]]) --- do_xfer_test(5.6, 0, [[ --- WITH x AS (SELECT a, b FROM t2) INSERT INTO t1 SELECT * FROM t2 --- ]]) --- do_xfer_test(5.7, 0, [[ --- INSERT INTO t1 WITH x AS ( SELECT * FROM t2 ) SELECT * FROM x --- ]]) --- do_xfer_test(5.8, 0, [[ --- INSERT INTO t1 WITH x(a,b) AS ( SELECT * FROM t2 ) SELECT * FROM x --- ]]) +local function do_xfer_test(test, test_func, test_name, func, exp, opts) + local opts = opts or {} + local exp_xfer_count = opts.exp_xfer_count + local before = box.sql.debug().sql_xfer_count + local ok, result = pcall(test_func, test, test_name, func, exp) + local after = box.sql.debug().sql_xfer_count + if exp_xfer_count ~= nil then + ok = ok and test:is(after - before, exp_xfer_count, + test_name .. '-xfer-count') + end + return ok +end + +test.do_execsql_xfer_test = function(test, test_name, func, exp, opts) + return do_xfer_test(test, test.do_execsql_test, test_name, func, exp, opts) +end + +test.do_catchsql_xfer_test = function(test, test_name, func, exp, opts) + return do_xfer_test(test, test.do_catchsql_test, test_name, func, exp, opts) +end + +test:do_execsql_test( + 5.1, + [[ + DROP TABLE IF EXISTS t1; + DROP TABLE IF EXISTS t2; + CREATE TABLE t1(a PRIMARY KEY, b); + CREATE TABLE t2(a PRIMARY KEY, b); + INSERT INTO t2 VALUES (1, 1), (2, 2); + ]], { + -- <5.1> + + -- <5.1> + }) + +test:do_execsql_xfer_test( + 5.2, + [[ + INSERT INTO t1 SELECT * FROM t2; + DELETE FROM t1; + ]], { + -- <5.2> + + -- <5.2> + }, { + exp_xfer_count = 1 + }) + +test:do_execsql_xfer_test( + 5.3, + [[ + INSERT INTO t1 SELECT a, b FROM t2; + DELETE FROM t1; + ]], { + -- <5.3> + + -- <5.3> + }, { + exp_xfer_count = 0 + }) + +test:do_execsql_xfer_test( + 5.4, + [[ + INSERT INTO t1 SELECT b, a FROM t2; + DELETE FROM t1; + ]], { + -- <5.4> + + -- <5.4> + }, { + exp_xfer_count = 0 + }) + +test:do_execsql_xfer_test( + 5.5, + [[ + WITH x AS (SELECT a, b FROM t2) INSERT INTO t1 SELECT * FROM x; + DELETE FROM t1; + ]], { + -- <5.5> + + -- <5.5> + }, { + exp_xfer_count = 0 + }) + +test:do_execsql_xfer_test( + 5.6, + [[ + WITH x AS (SELECT a, b FROM t2) INSERT INTO t1 SELECT * FROM t2; + DELETE FROM t1; + ]], { + -- <5.6> + + -- <5.6> + }, { + exp_xfer_count = 0 + }) + +test:do_execsql_xfer_test( + 5.7, + [[ + INSERT INTO t1 WITH x AS (SELECT * FROM t2) SELECT * FROM x; + DELETE FROM t1; + ]], { + -- <5.7> + + -- <5.7> + }, { + exp_xfer_count = 0 + }) + +test:do_execsql_xfer_test( + 5.8, + [[ + INSERT INTO t1 WITH x(a,b) AS (SELECT * FROM t2) SELECT * FROM x; + DELETE FROM t1; + ]], { + -- <5.8> + + -- <5.8> + }, { + exp_xfer_count = 0 + }) + ----------------------------------------------------------------------------- -- Check that syntax (and other) errors in statements with WITH clauses -- attached to them do not cause problems (e.g. memory leaks).