Tarantool development patches archive
 help / color / mirror / Atom feed
* [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

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