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 58AEB2B15A for ; Fri, 29 Mar 2019 14:16:04 -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 98G5kYHMHTpW for ; Fri, 29 Mar 2019 14:16:04 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 11BF627AAB for ; Fri, 29 Mar 2019 14:16:04 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: [tarantool-patches] Re: [PATCH v2 3/4] sql: fix error message for improperly created index From: "n.pettik" In-Reply-To: Date: Fri, 29 Mar 2019 21:16:01 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <1e34e241c5647b34a59ae0703eea3bcdc39c8fac.1553729426.git.korablev@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: Vladislav Shpilevoy , Konstantin Osipov > On 28 Mar 2019, at 18:11, Vladislav Shpilevoy = wrote: >=20 > Thanks for the patch! See my review fix at the bottom > of the email, and on the branch. In my fixes I've accounted > Kostja's comment about a proper error code. I guess Kostja meant to remove this check and use means of on_replace_dd_index() trigger. In other words, generate creation of secondary index without HALT and index id =3D=3D 1 to delegate verification of index correctness to on replace trigger. Here=E2=80=99s what I did: (I put this version on separate branch: = https://github.com/tarantool/tarantool/tree/np/gh-3914-fix-create-index-v3= ) diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 78cd3cee6..ef19ac926 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -1924,9 +1924,14 @@ sql_drop_foreign_key(struct Parse *parse_context) * following: * * 1 Seek for space id in _index, goto l1 if seeks fails. - * 2 Goto l2. - * 3 l1: Halt. - * 4 l2: Fetch index id from _index record. + * 2 Fetch index id from _index record. + * 3 Goto l2 + * 4 l1: Generate iid =3D=3D 1.. + * 6 l2: Continue index creation. + * + * Note that we generate iid =3D=3D 1 in case of index search on + * purpose: it allows on_replace_dd_index() raise correct + * error - "can not add a secondary key before primary". * * @return Register holding a new index id. */ @@ -1941,24 +1946,20 @@ vdbe_emit_new_sec_index_id(struct Parse *parse, = uint32_t space_id, int seek_adr =3D sqlVdbeAddOp4Int(v, OP_SeekLE, _index_cursor, = 0, key_reg, 1); sqlVdbeAddOp4Int(v, OP_IdxLT, _index_cursor, 0, key_reg, 1); - /* Jump over Halt block. */ + int iid_reg =3D ++parse->nMem; + /* Fetch iid from the row and increment it. */ + sqlVdbeAddOp3(v, OP_Column, _index_cursor, BOX_INDEX_FIELD_ID, = iid_reg); + sqlVdbeAddOp2(v, OP_AddImm, iid_reg, 1); int goto_succ_addr =3D sqlVdbeAddOp0(v, OP_Goto); + sqlVdbeJumpHere(v, seek_adr); + sqlVdbeJumpHere(v, seek_adr + 1); /* * Absence of any records in _index for that space is - * handled here. + * handled here: to indicate that secondary index can't + * be created before primary. */ - sqlVdbeJumpHere(v, seek_adr); - sqlVdbeJumpHere(v, seek_adr + 1); - sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, = ON_CONFLICT_ACTION_FAIL, - 0, "can not add a secondary key before primary", - P4_STATIC); - sqlVdbeChangeP5(v, ER_ALTER_SPACE); - + sqlVdbeAddOp2(v, OP_Integer, 1, iid_reg); sqlVdbeJumpHere(v, goto_succ_addr); - /* Fetch iid from the row and increment it. */ - int iid_reg =3D ++parse->nMem; - sqlVdbeAddOp3(v, OP_Column, _index_cursor, BOX_INDEX_FIELD_ID, = iid_reg); - sqlVdbeAddOp2(v, OP_AddImm, iid_reg, 1); return iid_reg; } =20 diff --git a/test/sql-tap/index1.test.lua b/test/sql-tap/index1.test.lua index 5f6706a40..63c98e206 100755 --- a/test/sql-tap/index1.test.lua +++ b/test/sql-tap/index1.test.lua @@ -1031,7 +1031,7 @@ test:do_catchsql_test( [[ CREATE UNIQUE INDEX pk ON t("id"); ]], { - 1, "can not add a secondary key before primary" + 1, "Can't modify space 'T': can not add a secondary key before = primary" })