* [tarantool-patches] [PATCH] sql: fix grammar for foreign key actions
@ 2019-02-22 0:52 Nikita Pettik
2019-02-22 6:48 ` [tarantool-patches] " Kirill Yukhin
0 siblings, 1 reply; 2+ messages in thread
From: Nikita Pettik @ 2019-02-22 0:52 UTC (permalink / raw)
To: tarantool-patches; +Cc: kyukhin, 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
^ permalink raw reply [flat|nested] 2+ messages in thread* [tarantool-patches] Re: [PATCH] sql: fix grammar for foreign key actions
2019-02-22 0:52 [tarantool-patches] [PATCH] sql: fix grammar for foreign key actions Nikita Pettik
@ 2019-02-22 6:48 ` Kirill Yukhin
0 siblings, 0 replies; 2+ messages in thread
From: Kirill Yukhin @ 2019-02-22 6:48 UTC (permalink / raw)
To: Nikita Pettik; +Cc: tarantool-patches
Hello,
On 22 Feb 03:52, Nikita Pettik wrote:
> 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
I've checked your patch into 2.1 branch.
--
Regards, Kirill Yukhin
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-02-22 6:48 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-22 0:52 [tarantool-patches] [PATCH] sql: fix grammar for foreign key actions Nikita Pettik
2019-02-22 6:48 ` [tarantool-patches] " Kirill Yukhin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox