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

  • From: "n.pettik" <korablev@xxxxxxxxxxxxx>
  • To: tarantool-patches@xxxxxxxxxxxxx
  • Date: Fri, 20 Jul 2018 06:20:41 +0300

Several minor remarks.

@@ -1885,12 +1881,11 @@ xferOptimization(Parse * pParse,      /* Parser 
context */
 
      /*
       * Xfer optimization is unable to correctly insert data
-      * in case there's a conflict action other than *_ABORT.
-      * This is the reason we want to only run it if the
-      * destination table is initially empty.
+      * 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 == false)) {

AFAIK we don’t use == false comparison. Instead simply use ! 
is_rr_action_default.

              addr1 = sqlite3VdbeAddOp2(v, OP_Rewind, iDest, 0);
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 5f9bc13..bc169d9 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4009,7 +4009,7 @@ case OP_RowData: {
      u32 n;
 
 #ifdef SQLITE_TEST
-     if (pOp->p5 == 1) {
+     if (pOp->p5 == OPFLAG_XFER_OPT) {

Flags are usually tested with & operation:

if (pOp->p5 & OPFLAG_XFER_OPT != 0)

              pOp->p5 = 0;
              sql_xfer_count++;
      }
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 34f603f..29f0efe 100755
--- a/test/sql-tap/gh-3307-xfer-optimization-issue.test.lua
+++ b/test/sql-tap/gh-3307-xfer-optimization-issue.test.lua
@@ -1,9 +1,29 @@
 
+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()
+                     if (aftr - bfr == 1) then
+                             return {1}
+                     end
+                     if (aftr == bfr) then
+                             return {0}
+                     end

You can simplify it to:

return aftr - bfr;

:)

Also

+      /* The Vdbe we're building*/
+      Vdbe *v = sqlite3GetVdbe(pParse);

Use ’struct’ modifier for complex types.
And put dot at the end of comment.


Other related posts:

  1. Home
  2. tarantool-patches
  3. Archive
  4. 07-2018