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 A150227B55 for ; Thu, 21 Feb 2019 19:52:07 -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 6ZlnZXFrMId6 for ; Thu, 21 Feb 2019 19:52:07 -0500 (EST) Received: from smtp44.i.mail.ru (smtp44.i.mail.ru [94.100.177.104]) (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 BE7CB255E0 for ; Thu, 21 Feb 2019 19:52:06 -0500 (EST) From: Nikita Pettik Subject: [tarantool-patches] [PATCH] sql: fix grammar for foreign key actions Date: Fri, 22 Feb 2019 03:52:00 +0300 Message-Id: <20190222005200.26374-1-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: kyukhin@tarantool.org, Nikita Pettik SQLite grammar implementing foreign keys parsing is quite compact, but on the other hand it allows to define ON DELETE and ON UPDATE actions multiple times. For instance: ... REFERENCES t ON DELETE CASCADE ON DELETE RESTRICT; It makes no sense and contradicts original ANSI syntax. So, lets rework it a bit. Firstly, MATCH clause must come first, so we place it in independent rule. Then we remove ON INSERT clause, since there is no such opportunity at all. Finally, we have only 4 options to expose refargs (i.e. grammar rule to parse FK actions): ON UPDATE, ON DELETE, ON UPDATE ON DELETE, ON DELETE ON UPDATE. That's it. Closes #3475 --- Branch: https://github.com/tarantool/tarantool/commits/np/gh-3475-fix-fk-grammar Issue: https://github.com/tarantool/tarantool/issues/3475 src/box/sql/build.c | 4 ++-- src/box/sql/parse.y | 40 +++++++++++++++++++++++----------------- src/box/sql/sqlInt.h | 7 ++++--- test/sql-tap/alter2.test.lua | 4 ++-- test/sql/foreign-keys.result | 26 +++++++++++++++++++++++++- test/sql/foreign-keys.test.lua | 11 ++++++++++- 6 files changed, 66 insertions(+), 26 deletions(-) diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 03bfbfb90..23d3eca1b 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -1690,7 +1690,7 @@ void sql_create_foreign_key(struct Parse *parse_context, struct SrcList *child, struct Token *constraint, struct ExprList *child_cols, struct Token *parent, struct ExprList *parent_cols, - bool is_deferred, int actions) + bool is_deferred, int match, int actions) { struct sql *db = parse_context->db; /* @@ -1819,7 +1819,7 @@ sql_create_foreign_key(struct Parse *parse_context, struct SrcList *child, fk->child_id = child_space != NULL ? child_space->def->id : 0; fk->parent_id = parent_space != NULL ? parent_space->def->id : 0; fk->is_deferred = is_deferred; - fk->match = (enum fkey_match) ((actions >> 16) & 0xff); + fk->match = (enum fkey_match) match; fk->on_update = (enum fkey_action) ((actions >> 8) & 0xff); fk->on_delete = (enum fkey_action) (actions & 0xff); fk->links = (struct field_link *) ((char *) fk->name + name_len + 1); diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index 661e69584..4b53cd485 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -266,8 +266,8 @@ ccons ::= UNIQUE. {sql_create_index(pParse,0,0,0,0, SORT_ORDER_ASC, false, SQL_INDEX_TYPE_CONSTRAINT_UNIQUE);} ccons ::= CHECK LP expr(X) RP. {sql_add_check_constraint(pParse,&X);} -ccons ::= REFERENCES nm(T) eidlist_opt(TA) refargs(R). - {sql_create_foreign_key(pParse, NULL, NULL, NULL, &T, TA, false, R);} +ccons ::= REFERENCES nm(T) eidlist_opt(TA) matcharg(M) refargs(R). + {sql_create_foreign_key(pParse, NULL, NULL, NULL, &T, TA, false, M, R);} ccons ::= defer_subclause(D). {fkey_change_defer_mode(pParse, D);} ccons ::= COLLATE id(C). {sqlAddCollateType(pParse, &C);} @@ -282,17 +282,23 @@ autoinc(X) ::= AUTOINCR. {X = 1;} // check fails. // %type refargs {int} -refargs(A) ::= . { A = FKEY_NO_ACTION; } -refargs(A) ::= refargs(A) refarg(Y). { A = (A & ~Y.mask) | Y.value; } -%type refarg {struct {int value; int mask;}} -refarg(A) ::= MATCH matcharg(X). { A.value = X<<16; A.mask = 0xff0000; } -refarg(A) ::= ON INSERT refact. { A.value = 0; A.mask = 0x000000; } -refarg(A) ::= ON DELETE refact(X). { A.value = X; A.mask = 0x0000ff; } -refarg(A) ::= ON UPDATE refact(X). { A.value = X<<8; A.mask = 0x00ff00; } +refargs(A) ::= refact_update(X) . { A = (X << 8); } +refargs(A) ::= refact_delete(X) . { A = X; } +refargs(A) ::= refact_delete(X) refact_update(Y) . { A = (Y << 8) | (X) ; } +refargs(A) ::= refact_update(X) refact_delete(Y) . { A = (X << 8) | (Y) ; } +refargs(A) ::= . { A = 0; } + +%type refact_update {int} +refact_update(A) ::= ON UPDATE refact(X). { A = X; } +%type refact_delete {int} +refact_delete(A) ::= ON DELETE refact(X). { A = X; } + %type matcharg {int} -matcharg(A) ::= SIMPLE. { A = FKEY_MATCH_SIMPLE; } -matcharg(A) ::= PARTIAL. { A = FKEY_MATCH_PARTIAL; } -matcharg(A) ::= FULL. { A = FKEY_MATCH_FULL; } +matcharg(A) ::= MATCH SIMPLE. { A = FKEY_MATCH_SIMPLE; } +matcharg(A) ::= MATCH PARTIAL. { A = FKEY_MATCH_PARTIAL; } +matcharg(A) ::= MATCH FULL. { A = FKEY_MATCH_FULL; } +matcharg(A) ::= . { A = FKEY_MATCH_SIMPLE; } + %type refact {int} refact(A) ::= SET NULL. { A = FKEY_ACTION_SET_NULL; } refact(A) ::= SET DEFAULT. { A = FKEY_ACTION_SET_DEFAULT; } @@ -319,8 +325,8 @@ tcons ::= UNIQUE LP sortlist(X) RP. tcons ::= CHECK LP expr(E) RP . {sql_add_check_constraint(pParse,&E);} tcons ::= FOREIGN KEY LP eidlist(FA) RP - REFERENCES nm(T) eidlist_opt(TA) refargs(R) defer_subclause_opt(D). { - sql_create_foreign_key(pParse, NULL, NULL, FA, &T, TA, D, R); + REFERENCES nm(T) eidlist_opt(TA) matcharg(M) refargs(R) defer_subclause_opt(D). { + sql_create_foreign_key(pParse, NULL, NULL, FA, &T, TA, D, M, R); } %type defer_subclause_opt {int} defer_subclause_opt(A) ::= . {A = 0;} @@ -1448,9 +1454,9 @@ cmd ::= ALTER TABLE fullname(X) RENAME TO nm(Z). { } cmd ::= ALTER TABLE fullname(X) ADD CONSTRAINT nm(Z) FOREIGN KEY - LP eidlist(FA) RP REFERENCES nm(T) eidlist_opt(TA) refargs(R) - defer_subclause_opt(D). { - sql_create_foreign_key(pParse, X, &Z, FA, &T, TA, D, R); + LP eidlist(FA) RP REFERENCES nm(T) eidlist_opt(TA) matcharg(M) + refargs(R) defer_subclause_opt(D). { + sql_create_foreign_key(pParse, X, &Z, FA, &T, TA, D, M, R); } cmd ::= ALTER TABLE fullname(X) DROP CONSTRAINT nm(Z). { diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 2830ab639..cd78c9e26 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -4121,14 +4121,15 @@ fkey_change_defer_mode(struct Parse *parse_context, bool is_deferred); * @param parent_cols List of referenced columns. If NULL, columns * which make up PK of referenced table are used. * @param is_deferred Is FK constraint initially deferred. - * @param actions ON DELETE, UPDATE and INSERT resolution - * algorithms (e.g. CASCADE, RESTRICT etc). + * @param match Value of MATCH clause: SIMPLE, PARTIAL, FULL. + * @param actions ON DELETE and ON UPDATE resolution algorithms + * (e.g. CASCADE, RESTRICT etc). */ void sql_create_foreign_key(struct Parse *parse_context, struct SrcList *child, struct Token *constraint, struct ExprList *child_cols, struct Token *parent, struct ExprList *parent_cols, - bool is_deferred, int actions); + bool is_deferred, int match, int actions); /** * Function called from parser to handle diff --git a/test/sql-tap/alter2.test.lua b/test/sql-tap/alter2.test.lua index e1dd0ff1b..e219a36bd 100755 --- a/test/sql-tap/alter2.test.lua +++ b/test/sql-tap/alter2.test.lua @@ -188,7 +188,7 @@ test:do_execsql_test( DROP TABLE parent; CREATE TABLE child (id INT PRIMARY KEY, a INT, b INT); CREATE TABLE parent (id INT PRIMARY KEY, c INT, d INT); - ALTER TABLE child ADD CONSTRAINT fk FOREIGN KEY (id) REFERENCES parent ON DELETE CASCADE MATCH FULL; + ALTER TABLE child ADD CONSTRAINT fk FOREIGN KEY (id) REFERENCES parent MATCH FULL ON DELETE CASCADE; INSERT INTO parent VALUES(1, 2, 3), (3, 4, 5), (6, 7, 8); INSERT INTO child VALUES(1, 1, 1), (3, 2, 2); DELETE FROM parent WHERE id = 1; @@ -206,7 +206,7 @@ test:do_execsql_test( DROP TABLE parent; CREATE TABLE child (id INT UNIQUE, a INT, b INT, z INT PRIMARY KEY AUTOINCREMENT); CREATE TABLE parent (id INT UNIQUE, c INT, d INT, z INT PRIMARY KEY AUTOINCREMENT); - ALTER TABLE child ADD CONSTRAINT fk FOREIGN KEY (id) REFERENCES parent(id) ON UPDATE CASCADE MATCH PARTIAL; + ALTER TABLE child ADD CONSTRAINT fk FOREIGN KEY (id) REFERENCES parent(id) MATCH PARTIAL ON UPDATE CASCADE; INSERT INTO parent(id, c, d) VALUES(1, 2, 3), (3, 4, 5), (6, 7, 8); INSERT INTO child(id, a, b) VALUES(1, 1, 1), (3, 2, 2); UPDATE parent SET id = 5 WHERE id = 1; diff --git a/test/sql/foreign-keys.result b/test/sql/foreign-keys.result index b6d23a554..56b695b92 100644 --- a/test/sql/foreign-keys.result +++ b/test/sql/foreign-keys.result @@ -333,7 +333,7 @@ box.space.PARENT:drop() box.sql.execute('CREATE TABLE tp (id INT PRIMARY KEY, a INT UNIQUE)') --- ... -box.sql.execute('CREATE TABLE tc (id INT PRIMARY KEY, a INT REFERENCES tp(a) ON DELETE SET NULL MATCH FULL)') +box.sql.execute('CREATE TABLE tc (id INT PRIMARY KEY, a INT REFERENCES tp(a) MATCH FULL ON DELETE SET NULL)') --- ... box.sql.execute('ALTER TABLE tc ADD CONSTRAINT fk1 FOREIGN KEY (id) REFERENCES tp(id) MATCH PARTIAL ON DELETE CASCADE ON UPDATE SET NULL') @@ -351,5 +351,29 @@ box.sql.execute('DROP TABLE tc') box.sql.execute('DROP TABLE tp') --- ... +-- gh-3475: ON UPDATE and ON DELETE clauses must appear once; +-- MATCH clause must come first. +box.sql.execute('CREATE TABLE t1 (id INT PRIMARY KEY);') +--- +... +box.sql.execute('CREATE TABLE t2 (id INT PRIMARY KEY REFERENCES t2 ON DELETE CASCADE ON DELETE RESTRICT);') +--- +- error: keyword "DELETE" is reserved +... +box.sql.execute('CREATE TABLE t2 (id INT PRIMARY KEY REFERENCES t2 ON DELETE CASCADE ON DELETE CASCADE);') +--- +- error: keyword "DELETE" is reserved +... +box.sql.execute('CREATE TABLE t2 (id INT PRIMARY KEY REFERENCES t2 ON DELETE CASCADE ON UPDATE RESTRICT ON DELETE RESTRICT);') +--- +- error: keyword "ON" is reserved +... +box.sql.execute('CREATE TABLE t2 (id INT PRIMARY KEY REFERENCES t2 ON DELETE CASCADE MATCH FULL);') +--- +- error: keyword "MATCH" is reserved +... +box.space.T1:drop() +--- +... --- Clean-up SQL DD hash. -test_run:cmd('restart server default with cleanup=1') diff --git a/test/sql/foreign-keys.test.lua b/test/sql/foreign-keys.test.lua index 677f3b1f4..3fb7cab18 100644 --- a/test/sql/foreign-keys.test.lua +++ b/test/sql/foreign-keys.test.lua @@ -154,11 +154,20 @@ box.space.PARENT:drop() -- ON UPDATE clauses. -- box.sql.execute('CREATE TABLE tp (id INT PRIMARY KEY, a INT UNIQUE)') -box.sql.execute('CREATE TABLE tc (id INT PRIMARY KEY, a INT REFERENCES tp(a) ON DELETE SET NULL MATCH FULL)') +box.sql.execute('CREATE TABLE tc (id INT PRIMARY KEY, a INT REFERENCES tp(a) MATCH FULL ON DELETE SET NULL)') box.sql.execute('ALTER TABLE tc ADD CONSTRAINT fk1 FOREIGN KEY (id) REFERENCES tp(id) MATCH PARTIAL ON DELETE CASCADE ON UPDATE SET NULL') box.space._fk_constraint:select{} box.sql.execute('DROP TABLE tc') box.sql.execute('DROP TABLE tp') +-- gh-3475: ON UPDATE and ON DELETE clauses must appear once; +-- MATCH clause must come first. +box.sql.execute('CREATE TABLE t1 (id INT PRIMARY KEY);') +box.sql.execute('CREATE TABLE t2 (id INT PRIMARY KEY REFERENCES t2 ON DELETE CASCADE ON DELETE RESTRICT);') +box.sql.execute('CREATE TABLE t2 (id INT PRIMARY KEY REFERENCES t2 ON DELETE CASCADE ON DELETE CASCADE);') +box.sql.execute('CREATE TABLE t2 (id INT PRIMARY KEY REFERENCES t2 ON DELETE CASCADE ON UPDATE RESTRICT ON DELETE RESTRICT);') +box.sql.execute('CREATE TABLE t2 (id INT PRIMARY KEY REFERENCES t2 ON DELETE CASCADE MATCH FULL);') +box.space.T1:drop() + --- Clean-up SQL DD hash. -test_run:cmd('restart server default with cleanup=1') -- 2.15.1