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 EA5692A6BF for ; Wed, 27 Mar 2019 09:44:53 -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 T_m1WUhcArxC for ; Wed, 27 Mar 2019 09:44:53 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 9D9FC29218 for ; Wed, 27 Mar 2019 09:44:53 -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 1/5] sql: introduce structs assembling DDL arguments during parsing From: "n.pettik" In-Reply-To: <29be7940-0421-9c09-df78-be9d3977f8d7@tarantool.org> Date: Wed, 27 Mar 2019 16:44:50 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <1B451F1D-39CE-410E-8EAE-55D2DB822C49@tarantool.org> References: <0fcc585532a1f1200a7dfd4a8e911ecf9f2c94aa.1548265148.git.korablev@tarantool.org> <45655E85-4DBB-4AC4-8161-5B783C66C688@tarantool.org> <6e4d0aae-5dbe-f8d6-3bfe-7723ea2f3d9d@tarantool.org> <78138FFB-162A-4703-9A8F-CB88FD0570EE@tarantool.org> <4841a3f7-5b21-10ea-4f58-160a5639b7d9@tarantool.org> <89C4A9F1-CB69-4F7B-95FF-9FEF751F1BB6@tarantool.org> <611D6548-9882-4FB0-9D2A-828E2CAFF83D@tarantool.org> <573b3a69-3b68-c40d-e0b5-2d36a45964fb@tarantool.org> <39DC5EC0-E609-4DB9-B6A3-9B5824B88C96@tarantool.org> <29be7940-0421-9c09-df78-be9d3977f8d7@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 > On 27 Mar 2019, at 16:29, Vladislav Shpilevoy = wrote: > On 27/03/2019 16:00, n.pettik wrote: >>=20 >>> On 26 Mar 2019, at 21:06, Vladislav Shpilevoy = wrote: >>>=20 >>> Thanks for the fixes! This commit LGTM. >>> Lets proceed to the next patches, and start >>> with a rebase, which is going to be hard. >>=20 >> Ok. Then I would like to clarify some details to avoid wasting time. >> In previous patch version, I used next (reworked) grammar to add >> FK constraints using ALTER: >>=20 >> cmd ::=3D alter_table_start alter_table_action . >>=20 >> alter_table_start ::=3D ALTER TABLE fullname(Z) . (1) >>=20 >> alter_table_action ::=3D add_constraint_def. >> alter_table_action ::=3D drop_constraint_def. >> alter_table_action ::=3D rename. >>=20 >> add_constraint_def ::=3D add_constraint_start constraint_def. >>=20 >> add_constraint_start(N) ::=3D ADD CONSTRAINT nm(Z). (2) >> constraint_def ::=3D foreign_key_def. >>=20 >> foreign_key_def ::=3D FOREIGN KEY LP eidlist(FA) RP REFERENCES nm(T) >> eidlist_opt(TA) matcharg(M) refargs(R) = defer_subclause_opt(D). >>=20 >> Now obviously I can=E2=80=99t use it since foreign_key_def should = call >> create_fk_def_init() which in turn requires table name and name >> of constraint defined in rules (1) and (2). >>=20 >> Why I want to use grammar mentioned above: it allows to remove >> code duplication. Rules to parse constraints are defined three times: >>=20 >> 1. ccons rule - that is part of column definition: =E2=80=A6, a INT = REFERENCES t1); >> 2. tcons rule - that is part of CREATE TABLE: =E2=80=A6, CONSTRAINT c = FOREIGN KEY =E2=80=A6); >> 3. ALTER TABLE statement >>=20 >> All of them use the same grammar to parse statement starting from >> REFERENCES keyword. The same applies to UNIQUE and CHECK >> constraints.=20 >>=20 >> IDK how to avoid using alter_entity_def_init() and = create_constraint_def_init() >> and at the same time divide constraint definition into several = stages. >>=20 >> Ofc, we can still use simple approach like: >>=20 >> cmd ::=3D ALTER TABLE fullname(Z) ADD CONSTRAINT nm(Z) FOREIGN KEY >> LP eidlist(FA) RP REFERENCES nm(T) eidlist_opt(TA) = matcharg(M) >> refargs(R) defer_subclause_opt(D) >>=20 >> cmd ::=3D ALTER TABLE fullname(Z) ADD CONSTRAINT nm(Z) UNIQUE >> LP sortlist(X) RP >>=20 >> cmd ::=3D ALTER TABLE fullname(Z) ADD CONSTRAINT nm(Z) PRIMARY KEY >> LP sortlist(X) RP >>=20 >> cmd ::=3D ALTER TABLE fullname(Z) ADD CONSTRAINT nm(Z) CHECK =E2=80=A6 >>=20 >> cmd ::=3D ALTER TABLE fullname(Z) RENAME TO nm(N) . >>=20 >> Is this OK? >>=20 >=20 > Obviously, it is not. Why can't you define this? >=20 > alter_table_start(T) ::=3D ALTER TABLE fullname(T) > alter_add_constraint(T, N) ::=3D alter_table_start(T) ADD CONSTRAINT = nm(N). Lemon can=E2=80=99t use two aliases as rule parameters at the same time. Instead we can introduce *another one* local struct to hold these names. Anyway my initial worry was not about duplication of ALTER TABLE CREATE = CONSTRAINT, but rather of constraints grammar (i.e. starting from FOREIGN KEY=E2=80=A6= ). > cmd ::=3D alter_add_constraint(T, N) FOREIGN KEY ... > cmd ::=3D alter_add_constraint(T, N) UNIQUE LP sortlist(X) RP > cmd ::=3D alter_add_constraint(T, N) PRIMARY KEY LP sortlist(X) RP > cmd ::=3D alter_add_constraint(T, N) CHECK ... > cmd ::=3D alter_table_start RENAME TO nm(N) . >=20 > Then inside each cmd you have both table and constraint names.