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

  • From: Nikita Tatunov <hollow653@xxxxxxxxx>
  • To: Alexander Turenko <alexander.turenko@xxxxxxxxxxxxx>
  • Date: Tue, 31 Jul 2018 14:48:04 +0300

вс, 29 июл. 2018 г. в 4:12, Alexander Turenko <
alexander.turenko@xxxxxxxxxxxxx>:

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, {
+                       -- <test_name>
+                       return_code
+                       -- <test_name>
+               })
+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;
    ]], {
        -- <xfer-optimization-1.1>
        0
        -- <xfer-optimization-1.1>
    }), {
        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@xxxxxxxxxxxxx>:

   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@xxxxxxxxxxxxx


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, {
- -- <test_name>
- return_code
- -- <test_name>
- })
+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(
  -- <xfer-optimization-1.1>
  0
  -- <xfer-optimization-1.1>
+ }, {
+ 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(
  -- <xfer-optimization-1.2>
  })

-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;
  ]], {
- -- <xfer-optimization-1.4>
+ -- <xfer-optimization-1.3>
  0
- -- <xfer-optimization-1.4>
+ -- <xfer-optimization-1.3>
+ }, {
+ 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(
  -- <xfer-optimization-1.5>
  })

-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;
  ]], {
- -- <xfer-optimization-1.7>
+ -- <xfer-optimization-1.5>
  1, "table T2 has 2 columns but 3 values were supplied"
- -- <xfer-optimization-1.7>
+ -- <xfer-optimization-1.5>
+ }, {
+ 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;
  ]], {
- -- <xfer-optimization-1.8>
+ -- <xfer-optimization-1.6>

- -- <xfer-optimization-1.8>
+ -- <xfer-optimization-1.6>
  })

-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;
  ]], {
- -- <xfer-optimization-1.10>
+ -- <xfer-optimization-1.7>
  0
- -- <xfer-optimization-1.10>
+ -- <xfer-optimization-1.7>
+ }, {
+ 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;
  ]], {
- -- <xfer-optimization-1.11>
+ -- <xfer-optimization-1.8>
  1, 1, 2, 2, 3, 3
- -- <xfer-optimization-1.11>
+ -- <xfer-optimization-1.8>
  })

-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;
  ]], {
- -- <xfer-optimization-1.13>
+ -- <xfer-optimization-1.9>
  1, "Duplicate key exists in unique index 'sqlite_autoindex_T2_1' in space
'T2'"
- -- <xfer-optimization-1.13>
+ -- <xfer-optimization-1.9>
+ }, {
+ 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;
  ]], {
- -- <xfer-optimization-1.14>
+ -- <xfer-optimization-1.10>

- -- <xfer-optimization-1.14>
+ -- <xfer-optimization-1.10>
  })

-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;
  ]], {
- -- <xfer-optimization-1.16>
+ -- <xfer-optimization-1.11>
  0
- -- <xfer-optimization-1.16>
+ -- <xfer-optimization-1.11>
+ }, {
+ 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;
  ]], {
- -- <xfer-optimization-1.17>
+ -- <xfer-optimization-1.12>
  1, 1, 2, 2, 3, 2
- -- <xfer-optimization-1.17>
+ -- <xfer-optimization-1.12>
  })

-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;
  ]], {
- -- <xfer-optimization-1.19>
+ -- <xfer-optimization-1.13>
  1, "Duplicate key exists in unique index 'sqlite_autoindex_T2_1' in space
'T2'"
- -- <xfer-optimization-1.19>
+ -- <xfer-optimization-1.13>
+ }, {
+ 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;
  ]], {
- -- <xfer-optimization-1.20>
+ -- <xfer-optimization-1.14>
  2, 2, 3, 4, 4, 4, 10, 10
- -- <xfer-optimization-1.20>
+ -- <xfer-optimization-1.14>
  })

-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;
  ]], {
- -- <xfer-optimization-1.22>
+ -- <xfer-optimization-1.15>
  0
- -- <xfer-optimization-1.22>
+ -- <xfer-optimization-1.15>
+ }, {
+ 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;
  ]], {
- -- <xfer-optimization-1.23>
+ -- <xfer-optimization-1.16>
  1, 1, 3, 3, 5, 5, 10, 10
- -- <xfer-optimization-1.23>
+ -- <xfer-optimization-1.16>
  })

-aftr = box.sql.debug().sql_xfer_count
-
 test:do_execsql_test(
- "xfer-optimization-1.24",
+ "xfer-optimization-1.17",
  [[
  SELECT * FROM t3;
  ]], {
- -- <xfer-optimization-1.24>
+ -- <xfer-optimization-1.17>
  1
- -- <xfer-optimization-1.24>
+ -- <xfer-optimization-1.17>
  })

-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;
  ]], {
- -- <xfer-optimization-1.26>
+ -- <xfer-optimization-1.18>
  0
- -- <xfer-optimization-1.26>
+ -- <xfer-optimization-1.18>
+ }, {
+ 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;
  ]], {
- -- <xfer-optimization-1.27>
+ -- <xfer-optimization-1.19>
  1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 10, 10
- -- <xfer-optimization-1.27>
+ -- <xfer-optimization-1.19>
  })

-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;
  ]], {
- -- <xfer-optimization-1.29>
+ -- <xfer-optimization-1.20>
  1, "Duplicate key exists in unique index 'sqlite_autoindex_T2_1' in space
'T2'"
- -- <xfer-optimization-1.29>
+ -- <xfer-optimization-1.20>
+ }, {
+ 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;
  ]], {
- -- <xfer-optimization-1.30>
+ -- <xfer-optimization-1.21>
  2, 2, 3, 4, 4, 4, 10, 10
- -- <xfer-optimization-1.30>
+ -- <xfer-optimization-1.21>
  })

-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;
  ]], {
- -- <xfer-optimization-1.32>
+ -- <xfer-optimization-1.22>
  0
- -- <xfer-optimization-1.32>
+ -- <xfer-optimization-1.22>
+ }, {
+ 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;
  ]], {
- -- <xfer-optimization-1.33>
+ -- <xfer-optimization-1.23>
  1, 1, 3, 3, 5, 5, 10, 10
- -- <xfer-optimization-1.33>
+ -- <xfer-optimization-1.23>
  })

-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;
  ]], {
- -- <xfer-optimization-1.35>
+ -- <xfer-optimization-1.24>
  1, "UNIQUE constraint failed: T2.A"
- -- <xfer-optimization-1.35>
+ -- <xfer-optimization-1.24>
+ }, {
+ exp_xfer_count = 0
  })

 test:do_execsql_test(
- "xfer-optimization-1.36",
+ "xfer-optimization-1.25",
  [[
  SELECT * FROM t2;
  ]], {
- -- <xfer-optimization-1.36>
+ -- <xfer-optimization-1.25>
  2, 2, 3, 4
- -- <xfer-optimization-1.36>
+ -- <xfer-optimization-1.25>
  })

-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;
  ]], {
- -- <xfer-optimization-1.38>
+ -- <xfer-optimization-1.26>
  0
- -- <xfer-optimization-1.38>
+ -- <xfer-optimization-1.26>
+ }, {
+ 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;
  ]], {
- -- <xfer-optimization-1.39>
+ -- <xfer-optimization-1.27>
  1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 10, 10
- -- <xfer-optimization-1.39>
+ -- <xfer-optimization-1.27>
  })

-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;
  ]], {
- -- <xfer-optimization-1.41>
+ -- <xfer-optimization-1.28>
  1, "Duplicate key exists in unique index 'sqlite_autoindex_T2_1' in space
'T2'"
- -- <xfer-optimization-1.41>
+ -- <xfer-optimization-1.28>
+ }, {
+ 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;
  ]], {
- -- <xfer-optimization-1.42>
+ -- <xfer-optimization-1.29>
  1, 1, 2, 2, 3, 4, 4, 4, 10, 10
- -- <xfer-optimization-1.42>
+ -- <xfer-optimization-1.29>
  })

-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;
  ]], {
- -- <xfer-optimization-1.44>
+ -- <xfer-optimization-1.30>
  0
- -- <xfer-optimization-1.44>
+ -- <xfer-optimization-1.30>
+ }, {
+ 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;
  ]], {
- -- <xfer-optimization-1.45>
+ -- <xfer-optimization-1.31>
  1, 1, 2, 2, 3, 4, 4, 4, 5, 5, 10, 10
- -- <xfer-optimization-1.45>
+ -- <xfer-optimization-1.31>
  })

-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), {
 --     -- </4.7>
 -- })

------------------------------------------------------------------------------
--- 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).

Other related posts: