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

[n.pettik]

>The bug was fixed so the data should now insert
>correctly.

Please, instead of mentioning that you just fixed bug (it is obvious),
provide brief information (without digging in details) how the problem was solved. 
(e.g. 'now only PK is used to handle insertion').

Overall, the idea is OK, but implementation could be more elegant.
You don’t need to iterate through all dest/source indexes:
it is possible to get PK using function sqlite3PrimaryKeyIndex();
Thus, complexity reduces from O(n^2) to O(n), where n - number of indexes.

But, there is even better approach: in Tarantool PK always comes with 0 ordinal
number. So, you can do space lookup by id (there is macros, which converts
table->tnum to space id: SQLITE_PAGENO_TO_SPACEID) and fetch real PK
index with O(1) complexity: space_index(space, 0 /* PK */);
It is not mandatory now, only if you are willing to do it.

Also, as we have discussed, remove pls redundant uniqueness check.

>  	}
>  	if (emptySrcTest)
>  		sqlite3VdbeJumpHere(v, emptySrcTest);
> 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..3b2bcc6
> --- /dev/null
> +++ b/test/sql-tap/gh-3307-xfer-optimization-issue.test.lua
> @@ -0,0 +1,52 @@
> +#!/usr/bin/env tarantool
> +test = require("sqltester")
> +test:plan(3)
> +
> +test:do_execsql_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;
> +		DROP TABLE t1;
> +		DROP TABLE t2;
> +	]], {
> +		-- <xfer-optimization-1.1>
> +
> +		-- <xfer-optimization-1.1>
> +	})

do_execsql_test() returns result of last executed query.
In this case, it is ‘DROP TABLE’, which always (in this particular case)
will return nothing (i.e. table will be successfully dropped).
To catch some error, you can use do_catchsql_test() function.
After you check that insertion occurs without errors, you need
to check that all rows have been transferred from one table to another.
So, you just use do_execsql_test() to test 'SELECT * FROM t2;’.
After all, you may drop tables in the beginning of next test,
since it won’t affect result of last executed statement.

Moreover, I would add more test cases to verify that xfer
optimization in general works: try to rearrange columns/indexes
order, add different ON CONFLICT clauses etc.