> Currently insertion from the table to another one
> with the same schema using SELECT works wrong.
> The problem lies in xfer optimization which opens cursors for
> all of the indexes and inserts data excessively.
> Now only PRIMARY KEY is used to handle insertion.
> Moreover analyzing xfer optimization I have noticed
Nitpicking: it is not worth mentioning process of exploration,
just use (in this particular case) statement of facts.
> diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
> index ae8dafb..ed134f4 100644
> --- a/src/box/sql/insert.c
> +++ b/src/box/sql/insert.c
> @@ -1711,6 +1711,7 @@ xferOptimization(Parse * pParse, /* Parser context */
> ExprList *pEList; /* The result set of the SELECT */
> Table *pSrc; /* The table in the FROM clause of SELECT */
> Index *pSrcIdx, *pDestIdx; /* Source and destination indices */
> + struct index *src_idx, *dest_idx; /* Source and destination indices */
We don’t use forward var declaration: it is obsolete rule from ancient C standards.
Moreover, we place comments above the code to be commented.
> + struct space *space; /* Space pointer for pDest and pSrc */
The same is here.
> - assert(destHasUniqueIdx);
> - if ((pDest->iPKey < 0 && pDest->pIndex != 0) /* (1) */
> - ||destHasUniqueIdx /* (2) */
> - || (onError != ON_CONFLICT_ACTION_ABORT
> - && onError != ON_CONFLICT_ACTION_ROLLBACK) /* (3) */
> - ) {
> - /* In some circumstances, we are able to run the xfer optimization
> - * only if the destination table is initially empty.
> - * This block generates code to make
> - * that determination.
> - *
> - * Conditions under which the destination must be empty:
> - *
> - * (1) There is no INTEGER PRIMARY KEY but there are indices.
> - *
> - * (2) The destination has a unique index. (The xfer optimization
> - * is unable to test uniqueness.)
> - *
> - * (3) onError is something other than ON_CONFLICT_ACTION_ABORT and _ROLLBACK.
> - */
> - addr1 = sqlite3VdbeAddOp2(v, OP_Rewind, iDest, 0);
> - VdbeCoverage(v);
> - emptyDestTest = sqlite3VdbeAddOp0(v, OP_Goto);
> - sqlite3VdbeJumpHere(v, addr1);
> - }
Why did you delete if-clause? The code which checks that
table is empty is needed only in the cases in if-clauses,
i.e. in some cases xfer optimisation can be applied even if
table is not initially empty (as far as I understand).
Yes, that's true this code is needed if any of the
cases in if-clause != 0. But the point is that in Tarantool
there's always a PRIMARY KEY that is obviously unique
which means that the second case is always != 0 and
the result of the boolean expression is always not 0,
hence there's no reason for this if-check.
> + /* The xfer optimization is unable to test uniqueness
> + * while we have a unique PRIMARY KEY in any existing table.
> + * This is the reason we can only run it if the destination table
Nitpicking: comments should fit into 66 chars.
> + * is initially empty.
> + * This block generates code to make that determination.
> + */
> + addr1 = sqlite3VdbeAddOp2(v, OP_Rewind, iDest, 0);
> + VdbeCoverage(v);
> + emptyDestTest = sqlite3VdbeAddOp0(v, OP_Goto);
> + sqlite3VdbeJumpHere(v, addr1);
> +
> + space = space_by_id(SQLITE_PAGENO_TO_SPACEID(pDest->tnum));
> + dest_idx = space_index(space, 0 /* PK */);
> + space = space_by_id(SQLITE_PAGENO_TO_SPACEID(pSrc->tnum));
> + src_idx = space_index(space, 0 /* PK */);
> + assert(src_idx);
> + assert(dest_idx);
We use explicit != NULL checks.
> + pDestIdx = sqlite3PrimaryKeyIndex(pDest);
> + pSrcIdx = sqlite3PrimaryKeyIndex(pSrc);
In fact, you don’t need these indexes at all.
Instead of calling emit_open_cursor() you
can simply add two opcodes: OP_LoadPtr and OP_OpenWrite
(as it happens inside that function). Or, you can use pSrc->tnum.
Table’s tnum exactly encodes PK space id (index id is 0).
As for KeyInfo - you are able to remove it at all, since opcodes
below don’t rely on key info (AFAIK, better check it yourself).
The only issue remained - how to check PK compatibility.
I would add to xferCompatibleIndex() another one check:
If first index is PK, then second also must be PK.
It is easy to implement, since Index struct contains appropriate flag.