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 8BFDF2FC62 for ; Wed, 14 Nov 2018 11:20:35 -0500 (EST) 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 TdBPY9DlUxFe for ; Wed, 14 Nov 2018 11:20:35 -0500 (EST) Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58]) (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 D6A8C2A570 for ; Wed, 14 Nov 2018 11:20:34 -0500 (EST) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 12.0 \(3445.100.39\)) Subject: [tarantool-patches] Re: [PATCH v2 1/4] sql: don't increment row count on FK creation within CREATE TABLE From: "n.pettik" In-Reply-To: Date: Wed, 14 Nov 2018 19:20:31 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <97C74A82-4630-4F8A-9E10-4B333F0D991F@tarantool.org> References: <6c6636af9339de4965f13cf56496ea2e05525d38.1542124689.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 > Hi! Thanks for the patch! >=20 > See 1 comment below, my fixes at the end of the email > and on the branch. >=20 > On 13/11/2018 19:11, Nikita Pettik wrote: >> We have agreement that each successful DDL operation returns 1 (one) = as >> a row count (via IProto protocol or changes() SQL function), despite >> the number of other created objects (e.g. indexes, sequences, FK >> constraints etc). >> Needed for #2181 >> --- >> src/box/sql/build.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> diff --git a/src/box/sql/build.c b/src/box/sql/build.c >> index 5b3348bd2..e28856e26 100644 >> --- a/src/box/sql/build.c >> +++ b/src/box/sql/build.c >> @@ -1415,7 +1415,8 @@ vdbe_emit_fkey_create(struct Parse = *parse_context, const struct fkey_def *fk) >> constr_tuple_reg + 9); >> sqlite3VdbeAddOp3(vdbe, OP_SInsert, BOX_FK_CONSTRAINT_ID, 0, >> constr_tuple_reg + 9); >> - sqlite3VdbeChangeP5(vdbe, OPFLAG_NCHANGE); >> + if (parse_context->pNewTable =3D=3D NULL) >> + sqlite3VdbeChangeP5(vdbe, OPFLAG_NCHANGE); >> save_record(parse_context, BOX_FK_CONSTRAINT_ID, = constr_tuple_reg, 2, >> vdbe->nOp - 1); >> sqlite3ReleaseTempRange(parse_context, constr_tuple_reg, 10); >=20 > I added a test to this, and noticed, that 'drop table' returns > rowcount 2 when it has a foreign key. Please, fix. Now my test > fails on it. Thanks for provided test. Fixed: diff --git a/src/box/sql/build.c b/src/box/sql/build.c index e28856e26..9c9a6d9e1 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -1810,7 +1810,6 @@ vdbe_emit_fkey_drop(struct Parse *parse_context, = char *constraint_name, } sqlite3VdbeAddOp3(vdbe, OP_MakeRecord, key_reg, 2, key_reg + 2); sqlite3VdbeAddOp2(vdbe, OP_SDelete, BOX_FK_CONSTRAINT_ID, = key_reg + 2); - sqlite3VdbeChangeP5(vdbe, OPFLAG_NCHANGE); VdbeComment((vdbe, "Delete FK constraint %s", constraint_name)); sqlite3ReleaseTempRange(parse_context, key_reg, 3); } @@ -2269,6 +2268,13 @@ sql_drop_foreign_key(struct Parse *parse_context, = struct SrcList *table, if (constraint_name !=3D NULL) vdbe_emit_fkey_drop(parse_context, constraint_name, child->def->id); + /* + * We account changes to row count only if drop of + * foreign keys take place in a separate + * ALTER TABLE DROP CONSTRAINT statement, since whole + * DROP TABLE always returns 1 (one) as a row count. + */ + sqlite3VdbeChangeP5(sqlite3GetVdbe(parse_context), = OPFLAG_NCHANGE);=