[tarantool-patches] Re: [PATCH] sql: prohibit duplication of FK action clause

["n.pettik" <korablev@tarantool.org>]

> 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);
>                                        }
>                                      }

Doesn’t look better. What I really meant is:

diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 7ee8dc292..cc4afaf71 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -283,14 +283,14 @@ 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;
-                                       } else {
-                                         sqlErrorMsg(pParse,
-                                                     "near \"%T\": duplicate FK action clause.",
-                                                     &pParse->sLastToken);
-                                       }
-                                     }
+refargs(A) ::= refargs(A) refarg(Y). {
+  if ((A & Y.mask) == 0) {
+    A = (A & ~Y.mask) | Y.value;
+  } else {
+    sqlErrorMsg(pParse, "near \"%T\": duplicate FK action clause.",
+                &pParse->sLastToken);
+  }
+}

What is more, I found this:

tarantool> box.sql.execute("CREATE TABLE t3(a INT PRIMARY KEY, b INT REFERENCES t1(b) ON DELETE NO ACTION ON DELETE NO ACTION)”)

Is executed correctly.

The same for MATCH:

tarantool> box.sql.execute("CREATE TABLE t4(a INT PRIMARY KEY, b INT REFERENCES t1(b) MATCH SIMPLE ON DELETE NO ACTION ON UPDATE SET NULL MATCH FULL)”)

Obviously, it happens since NO ACTION and SIMPLE are represented as
zero valued members of enum. I don’t like idea to start enum from value 1.

Firstly, according to ANSI grammar, MATCH clause always comes before
triggered actions:

<references specification> ::=
REFERENCES <referenced table and columns>
 [ MATCH <match type> ] [ <referential triggered action> ]

So, we can simplify grammar to smth like this:

refagrs(A) :: = matcharg(X) refarg(Y) refarg(Z) . // Note that refargs is no longer recursive rule.

Then, we can got rid of mask at all:

%type refarg {struct {int value; bool is_delete_action_set; bool is_update_action_set ;}}
refarg(A) :: = ON DELETE reface(X). {
  if(A.is_delete_action_set)
   //raise an error
  A.is_delete_action_set = true;
  A.value = X;
}
refarg(A) :: = ON UPDATE reface(X). {A.is_update_action_set = true; A.value = X; }
...

That is my suggestion. I didn’t test it, but idea seems to be valid.
Alternatively, you can come up with better solution.