[tarantool-patches] Re: [PATCH] sql: prohibit duplication of FK action clause
Kirill Yukhin
kyukhin at tarantool.org
Thu Feb 14 11:56:40 MSK 2019
Hello Nikita,
On 13 Feb 22:43, n.pettik wrote:
>
> > On 13 Feb 2019, at 09:08, Kirill Yukhin <kyukhin at tarantool.org> wrote:
> >
> > The patch prohibits duplication of action clause for foreign
> > key constraints. It is now syntax error to specify, say,
> > ON UPDATE clause for the same FK twice.
> > The patch also remove ON INSERT caluse at all, since it
> > ultimately is no-op and leads to nothing but confusion.
> >
> > Closes #3475
> > ---
> > https://github.com/tarantool/tarantool/issues/3475
> > https://github.com/tarantool/tarantool/commits/kyukhin/gh-3475-fk-duplicate-action-clause
>
> Travis status is completely negative. Please, make sure it is green
> before sending patch.
Travis is completely positive w/ this patch. Please, see [1].
Failing case is the branch after rebase.
Anyway, rebased version is fixed as well.
> > diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
> > index 32ef685..7938afd 100644
> > --- a/src/box/sql/parse.y
> > +++ b/src/box/sql/parse.y
> > @@ -283,10 +283,16 @@ autoinc(X) ::= AUTOINCR. {X = 1;}
> > //
> > %type refargs {int}
> > refargs(A) ::= . { A = FKEY_NO_ACTION; }
> > -refargs(A) ::= refargs(A) refarg(Y). { A = (A & ~Y.mask) | Y.value; }
> > +refargs(A) ::= refargs(A) refarg(Y). { if ((A & Y.mask) == 0) {
> > + A = (A & ~Y.mask) | Y.value;
> > + } else {
> > + sqlite3ErrorMsg(pParse,
> > + "near \"%T\": duplicate FK action clause.",
> > + &pParse->sLastToken);
> > + }
> > + }
>
> Please, fix indentation, it looks extremely awful.
> Lets use 2 spaces as a default indent (as almost
> everywhere in parse.y).
I've replaced 4 spaces w/ 2 for tab.
> > diff --git a/test/sql/gh-3475-fk-duplicate-clause.test.lua b/test/sql/gh-3475-fk-duplicate-clause.test.lua
> > new file mode 100644
> > index 0000000..447c104
> > --- /dev/null
> > +++ b/test/sql/gh-3475-fk-duplicate-clause.test.lua
> > @@ -0,0 +1,13 @@
> > +test_run = require('test_run').new()
> > +engine = test_run:get_cfg('engine')
> > +box.sql.execute('pragma sql_default_engine=\''..engine..'\’')
>
> Should this test be engine-dependent?
I'd test everything on boath engines. But you're reviewer, so removed.
> > +
> > +box.cfg{}
> > +
> > +box.sql.execute("CREATE TABLE t1(a INT PRIMARY KEY, b INT UNIQUE)")
> > +box.sql.execute("CREATE TABLE t2(a INT PRIMARY KEY, b INT REFERENCES t1(b) ON UPDATE CASCADE ON UPDATE CASCADE)")
> > +box.sql.execute("CREATE TABLE t2(a INT PRIMARY KEY, b INT REFERENCES t1(b) ON DELETE CASCADE ON DELETE CASCADE)")
> > +box.sql.execute("CREATE TABLE t2(a INT PRIMARY KEY, b INT REFERENCES t1(b) ON DELETE CASCADE ON UPDATE CASCADE)”)
>
> What about next cases:
>
> box.sql.execute("CREATE TABLE t2(a INT PRIMARY KEY, b INT REFERENCES t1(b) ON DELETE CASCADE ON DELETE RESTRICT)”)
> box.sql.execute("CREATE TABLE t2(a INT PRIMARY KEY, b INT REFERENCES t1(b) ON DELETE CASCADE ON UPDATE CASCADE ON DELETE RESTRICT )”)
>
> Sorry, can’t check myself since can’t build your branch.
I've added them as well.
[1] - https://travis-ci.org/tarantool/tarantool/builds/492546000
--
Regards, Kirill Yukhin
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 8c8fdc2..7ee8dc2 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -284,11 +284,11 @@ autoinc(X) ::= AUTOINCR. {X = 1;}
%type refargs {int}
refargs(A) ::= . { A = FKEY_NO_ACTION; }
refargs(A) ::= refargs(A) refarg(Y). { if ((A & Y.mask) == 0) {
- A = (A & ~Y.mask) | Y.value;
+ A = (A & ~Y.mask) | Y.value;
} else {
- sqlite3ErrorMsg(pParse,
- "near \"%T\": duplicate FK action clause.",
- &pParse->sLastToken);
+ sqlErrorMsg(pParse,
+ "near \"%T\": duplicate FK action clause.",
+ &pParse->sLastToken);
}
}
%type refarg {struct {int value; int mask;}}
diff --git a/test/sql/engine.cfg b/test/sql/engine.cfg
index 0007d8d..4ec39ac 100644
--- a/test/sql/engine.cfg
+++ b/test/sql/engine.cfg
@@ -1,4 +1,5 @@
{
+ "gh-3475-fk-duplicate-clause.test.lua": {},
"*": {
"memtx": {"engine": "memtx"},
"vinyl": {"engine": "vinyl"}
diff --git a/test/sql/gh-3475-fk-duplicate-clause.result b/test/sql/gh-3475-fk-duplicate-clause.result
index 7bcd62b..5ce556f 100644
--- a/test/sql/gh-3475-fk-duplicate-clause.result
+++ b/test/sql/gh-3475-fk-duplicate-clause.result
@@ -1,12 +1,6 @@
test_run = require('test_run').new()
---
...
-engine = test_run:get_cfg('engine')
----
-...
-box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
----
-...
box.cfg{}
---
...
@@ -21,6 +15,14 @@ box.sql.execute("CREATE TABLE t2(a INT PRIMARY KEY, b INT REFERENCES t1(b) ON DE
---
- error: 'near ")": duplicate FK action clause.'
...
+box.sql.execute("CREATE TABLE t2(a INT PRIMARY KEY, b INT REFERENCES t1(b) ON DELETE CASCADE ON DELETE RESTRICT)")
+---
+- error: 'near ")": duplicate FK action clause.'
+...
+box.sql.execute("CREATE TABLE t2(a INT PRIMARY KEY, b INT REFERENCES t1(b) ON DELETE CASCADE ON UPDATE CASCADE ON DELETE RESTRICT )")
+---
+- error: 'near ")": duplicate FK action clause.'
+...
box.sql.execute("CREATE TABLE t2(a INT PRIMARY KEY, b INT REFERENCES t1(b) ON DELETE CASCADE ON UPDATE CASCADE)")
---
...
diff --git a/test/sql/gh-3475-fk-duplicate-clause.test.lua b/test/sql/gh-3475-fk-duplicate-clause.test.lua
index 447c104..2c2b55c 100644
--- a/test/sql/gh-3475-fk-duplicate-clause.test.lua
+++ b/test/sql/gh-3475-fk-duplicate-clause.test.lua
@@ -1,12 +1,12 @@
test_run = require('test_run').new()
-engine = test_run:get_cfg('engine')
-box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
box.cfg{}
box.sql.execute("CREATE TABLE t1(a INT PRIMARY KEY, b INT UNIQUE)")
box.sql.execute("CREATE TABLE t2(a INT PRIMARY KEY, b INT REFERENCES t1(b) ON UPDATE CASCADE ON UPDATE CASCADE)")
box.sql.execute("CREATE TABLE t2(a INT PRIMARY KEY, b INT REFERENCES t1(b) ON DELETE CASCADE ON DELETE CASCADE)")
+box.sql.execute("CREATE TABLE t2(a INT PRIMARY KEY, b INT REFERENCES t1(b) ON DELETE CASCADE ON DELETE RESTRICT)")
+box.sql.execute("CREATE TABLE t2(a INT PRIMARY KEY, b INT REFERENCES t1(b) ON DELETE CASCADE ON UPDATE CASCADE ON DELETE RESTRICT )")
box.sql.execute("CREATE TABLE t2(a INT PRIMARY KEY, b INT REFERENCES t1(b) ON DELETE CASCADE ON UPDATE CASCADE)")
box.sql.execute("DROP TABLE t2")
More information about the Tarantool-patches
mailing list