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 8879F26319 for ; Thu, 17 Jan 2019 12:14:53 -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 j23Ea7zNcrrZ for ; Thu, 17 Jan 2019 12:14:53 -0500 (EST) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 45525261A3 for ; Thu, 17 Jan 2019 12:14:53 -0500 (EST) 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 2/6] sql: rework ALTER TABLE grammar From: "n.pettik" In-Reply-To: <20190117115100.GM28204@chai> Date: Thu, 17 Jan 2019 20:14:50 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <0117c011c631182ddd64cff7a46e2b3e940bf03c.1547035183.git.korablev@tarantool.org> <20190117115100.GM28204@chai> 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: Konstantin Osipov , Vladislav Shpilevoy > On 17 Jan 2019, at 14:51, Konstantin Osipov = wrote: >=20 > * Nikita Pettik [19/01/09 15:17]: >> +alter_table_start ::=3D ALTER TABLE fullname(Z) . { >> + pParse->constraint->table_name =3D Z; >> + pParse->constraint->name.n =3D 0; >> +} >=20 > It's bikeshed at this point, but in future you will have other > ALTER TABLE clauses: add/drop column, add/drop index, Indexes out of scope: they are part of constraint routines. > add/drop > constraint. It's unclear why you initialize a specific parsing > context (constraint definition) right in alter_table_start rule. > OK for now. We must save name of table before any other processing. So, at the moment of parsing table=E2=80=99s name we don=E2=80=99t have any notion about next actions yet. We can use the way you suggested in previous letter: introduce some hierarchical structures. For instance ALTER can be implemented in the next way: diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index e38e0da08..f1a5e1974 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -2707,6 +2707,105 @@ struct constraint_parse { struct ExprList *cols; }; =20 +enum entity_type { + ENTITY_TYPE_TABLE =3D 0, + ENTITY_TYPE_INDEX, + ENTITY_TYPE_TRIGGER, + ENTITY_TYPE_CONSTRAINT, + ENTITY_TYPE_COLUMN +}; + +/*** + * Hierarchy is following: + * + * Base structure is ALTER. + * ALTER is omitted only for CREATE TABLE. + * + * DROP is general for all existing objects and includes + * name of object itself, name of parent object (table), + * IF EXISTS clause and may contain on-drop behaviour. + * Hence, it terms of grammar - it is terminal symbol. + * + * RENAME can be applied only to table, so it is also + * terminal symbol. + * + * CREATE in turn can be expanded to nonterminal symbol + * CREATE CONSTRAINT or to terminal CREATE TABLE/INDEX/TRIGGER. + * CREATE CONSTRAINT unfolds to FOREIGN KEY or UNIQUE/PRIMARY KEY. + * + * For instance, ALTER TABLE t ADD CONSTRAINT c FOREIGN KEY + * REFERENCES t2(id); + * ALTER *TABLE* -> CREATE ENTITY -> CREATE CONSTRAINT -> CREATE FK + * + * CREATE TRIGGER tr1 ... + * ALTER *TABLE* -> CREATE ENTITY -> CREATE TRIGGER + */ +struct alter_entity_def { + enum entity_type entity_type; + struct SrcList *entity_name; +}; + +struct rename_entity_def { + struct alter_entity_def *base; + struct Token new_name; +}; + +struct create_entity_def { + struct alter_entity_def *base; + /** Statement comes with IF NOT EXISTS clause. */ + bool if_not_exist; + struct Token *name; +}; + +struct drop_entity_def { + struct alter_entity_def *base; + /** Statement comes with IF EXISTS clause. */ + bool if_exist; + struct Token entity_name; + /** One of CASCADE, RESTRICT */ + int drop_behaviour; +}; + +struct create_trigger_def { + struct create_entity_def *base; + /** One of TK_BEFORE, TK_AFTER, TK_INSTEAD. */ + int tr_tm; + /** One of TK_INSERT, TK_UPDATE, TK_DELETE. */ + int op; + /** Column list if this is an UPDATE OF trigger. */ + struct IdList *columns; + /** When clause. */ + struct Expr *when; + bool if_not_exist; + +}; +struct create_table_def { + struct create_entity_def *base; + struct space_def *table_def; + struct Select *select; +}; + +struct create_index_def { + struct create_entity_def *base; + enum sort_order sort_order; + enum sql_index_type idx_type; + struct ExprList *col_list; +}; + +struct create_constraint_def { + struct create_entity_def *base; + /** One of DEFERRED, IMMEDIATE. */ + int check_time; +}; + +struct create_fk_def { + struct create_constraint_def *base; + struct ExprList *child_cols; + struct ExprList *parent_cols; + struct Token *parent_name; + enum fkey_action action; +}; + diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index de5429498..84597bb91 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -1452,8 +1452,11 @@ cmd ::=3D alter_table_start alter_table_action . * the separate rule. */ alter_table_start ::=3D ALTER TABLE fullname(Z) . { - pParse->constraint.table =3D Z; - pParse->constraint.name.n =3D 0; + pParse->alter_entity_def =3D + region_alloc(pParse->region, sizeof(struct alter_entity_def)); + pParse->alter_entity_def->entity_type =3D ENTITY_TYPE_TABLE; + pParse->alter_entity_def->parent_name =3D Z; + pParse->alter_entity_def->entity_name.n =3D 0; } =20 alter_table_action ::=3D add_constraint_def. @@ -1461,25 +1464,44 @@ alter_table_action ::=3D drop_constraint_def. alter_table_action ::=3D rename. =20 rename ::=3D RENAME TO nm(Z). { - sql_alter_table_rename(pParse, &Z); + struct alter_entity_rename *entity_rename =3D + region_alloc(pParse->region, sizeof(struct rename_entity_def)); + entity_rename->base =3D pParse->alter_entity_def; + entity_rename->new_name =3D Z; + pParse->alter_entity_def =3D (struct alter_entity_def *) = entity_rename; + sql_alter_table_rename(pParse); } =20 add_constraint_def ::=3D add_constraint_start constraint_def. =20 add_constraint_start ::=3D ADD CONSTRAINT nm(Z). { - pParse->constraint.name =3D Z; + struct create_entity_def *entity_create =3D + region_alloc(pParse->region, sizeof(struct create_entity_def)); + entity_create->base =3D pParse->alter_entity_def; + entity_create->name =3D Z; + entity_create->if_not_exist =3D false; + pParse->alter_entity_def =3D (struct alter_entity_def = *)entity_create; } =20 constraint_def ::=3D foreign_key_def. =20 foreign_key_def ::=3D FOREIGN KEY LP eidlist(FA) RP REFERENCES nm(T) eidlist_opt(TA) refargs(R) defer_subclause_opt(D). = { - pParse->constraint.cols =3D FA; - sql_create_foreign_key(pParse, &T, TA, D, R); + struct create_fk_def *fk_def =3D + region_alloc(pParse->region, sizeof(struct create_fk_def)); + fk_def->child_cols =3D FA; + fk_def->parent_cols =3D TA; + fk_def->action =3D R; + fk_def->parent_name =3D T; + sql_create_foreign_key(pParse); } =20 drop_constraint_def ::=3D DROP CONSTRAINT nm(Z). { - sql_drop_foreign_key(pParse, &Z); + struct drop_entity_rename *entity_drop =3D + region_alloc(pParse->region, sizeof(struct drop_entity_def)); + entity_drop->base =3D pParse->alter_entity_def; + entity_drop->new_name =3D Z; + sql_drop_foreign_key(pParse); } It is easy to fix other create/drop statements to use these structures. If it is OK to you and Vlad, I can rework whole patch. >> + >> +alter_table_action ::=3D add_constraint_def. >> +alter_table_action ::=3D drop_constraint_def. >=20 > Why use the same data structure for create and drop? For the sake of simplicity. > When I drop a > constraint I specify its name and (perhaps) table name, none of > other properties. Not quite: drop constraint clause also may include drop behaviour: one of CASCADE or RESTRICT. In our implementation it is always RESTRICT. > To capture drop context one could use: >=20 > struct drop_object_def { > enum object_type object_type; > const char *object_name; > const char *parent_object_name; > }; >=20 > rename of an object could be captured in a similar data structure. Rename and drop are the easiest cases.