From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 0DE32279BA for ; Wed, 18 Jul 2018 20:20:57 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id sF0LNa6MPksO for ; Wed, 18 Jul 2018 20:20:56 -0400 (EDT) Received: from smtp14.mail.ru (smtp14.mail.ru [94.100.181.95]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 4BDC6279AB for ; Wed, 18 Jul 2018 20:20:56 -0400 (EDT) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: [tarantool-patches] Re: [PATCH] sql: xfer optimization issue From: "n.pettik" In-Reply-To: Date: Thu, 19 Jul 2018 03:20:48 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <12B62C73-9BEC-49FA-B3FD-590C445CF25B@tarantool.org> References: <5BB99B27-5F86-4664-AAD5-57A22ECED854@tarantool.org> <93E4DAEA-EF90-479D-9F62-3D1CEB3CBE3F@tarantool.org> <20180628101839.fhnijezdpwviohop@tkn_work_nb> <20180709155006.fwrikbznqk23ger5@tkn_work_nb> <79D03E96-0BD0-418D-9DB2-45318C734628@tarantool.org> <8B8D5501-075D-4BEB-B282-35B0B81CD555@tarantool.org> <605B15EF-BD1C-4B03-8A9F-6E6225076812@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: "N. Tatunov" When answering on review, include only chunks related to your answers. Otherwise, letter becomes really long.. >=20 > > @@ -1725,9 +1727,9 @@ xferOptimization(Parse * pParse, /* = Parser context */ > > int emptyDestTest =3D 0; /* Address of test for empty pDest = */ > > int emptySrcTest =3D 0; /* Address of test for empty pSrc */ > > Vdbe *v; /* The VDBE we are building */ > > - int destHasUniqueIdx =3D 0; /* True if pDest has a = UNIQUE index */ > > int regData, regTupleid; /* Registers holding data and = tupleid */ > > struct session *user_session =3D current_session(); > > + bool is_err_action_default =3D false; >=20 > Again: why do you need this flag? Default action is just synonym for = ABORT, > so why should we care about it? >=20 >=20 > It's all about conflict action priorities as I said before. > Consider the following example: > ``` > CREATE TABLE t1(a INTEGER PRIMARY KEY ON CONFLICT = REPLACE, b); > CREATE TABLE t2(a INTEGER PRIMARY KEY ON CONFLICT = REPLACE, b); > INSERT INTO t1 VALUES (1, 1), (3, 3), (5, 5); > INSERT INTO t2 VALUES (2, 2), (3, 4); > BEGIN; > INSERT INTO t2 VALUES (4, 4); > INSERT INTO t2 SELECT * FROM t1; > INSERT INTO t2 VALUES (10, 10); > COMMIT; onError is an action of whole statement, not of index. In your example onError =3D=3D ABORT =3D=3D DEFAULT. Replace action of index is not involved in code you wrote. >+ uint32_t src_space_id =3D SQLITE_PAGENO_TO_SPACEID(pSrc->tnum); >+ struct space *src_space =3D space_by_id(src_space_id); >+ uint32_t dest_space_id =3D = SQLITE_PAGENO_TO_SPACEID(pDest->tnum); >+ struct space *dest_space =3D space_by_id(dest_space_id); Move here also assert: assert(src_space !=3D NULL && dest_space !=3D NULL); > > 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..e75fabc > > --- /dev/null > > +test:do_catchsql_test( > > + "xfer-optimization-1.15", > > + [[ > > + DROP TABLE t1; > > + DROP TABLE t2; > > + CREATE TABLE t1(a INTEGER PRIMARY KEY, b UNIQUE); > > + CREATE TABLE t2(a INTEGER PRIMARY KEY, b UNIQUE); > > + INSERT INTO t1 VALUES (2, 2), (3, 3), (5, 5); > > + INSERT INTO t2 VALUES (1, 1), (4, 4); > > + INSERT OR ROLLBACK INTO t2 SELECT * FROM t1; >=20 > INSERT OT ROLLBACK outside transaction works the same as ABORT and = DEFAULT. > So, surround it with transaction and check that it really rollbacks. >=20 > There are basically almost the same tests surrounded by transactions = (1.30 - 1.35). If so, remove redundant tests pls. > -/* Opcode: RowData P1 P2 * * * > +/* Opcode: RowData P1 P2 * * P5 > * Synopsis: r[P2]=3Ddata > * > * Write into register P2 the complete row content for the row at > @@ -3984,6 +3994,8 @@ case OP_SorterData: { > * There is no interpretation of the data. > * It is just copied onto the P2 register exactly as > * it is found in the database file. > + * P5 can be used in debug mode to check if xferOptimization has > + * actually started processing. > * > * If cursor P1 is an index, then the content is the key of the row. > * If cursor P2 is a table, then the content extracted is the data. > @@ -3996,6 +4008,13 @@ case OP_RowData: { > BtCursor *pCrsr; > u32 n; > =20 > +#ifdef SQLITE_TEST > + if (pOp->p5 =3D=3D 1) { Use named value (i.e. OPFLAG_XFER_OPT) even if they are the same. > + > +test:do_execsql_test( > + "xfer-optimization-1.37", > + [[ > + INSERT INTO t2 VALUES (10, 10); Here (and in some other places) smth wrong with indentation, fix it pls.