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

[Nikita Tatunov <hollow653@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 30826 bytes --]

вс, 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, {
> > +                       -- <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@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, {
- -- <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).

[-- Attachment #2: Type: text/html, Size: 66886 bytes --]