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 B29252682C for ; Tue, 5 Feb 2019 08:44:38 -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 HvGVFouSQAVw for ; Tue, 5 Feb 2019 08:44:38 -0500 (EST) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 3FA1C2672D for ; Tue, 5 Feb 2019 08:44:38 -0500 (EST) From: "n.pettik" Message-Id: <913D8DDB-15FE-48BA-8D6D-626DFBCE89E8@tarantool.org> Content-Type: multipart/alternative; boundary="Apple-Mail=_5A4D4480-45F7-4FCD-9FC4-EC5A8CD510CD" Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: [tarantool-patches] Re: [PATCH] sql: raise err on CHECK + ON CONFLICT REPLACE Date: Tue, 5 Feb 2019 16:44:35 +0300 In-Reply-To: References: <821d032b-9adf-6496-c0bc-f0f9497c5f83@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: Ivan Koptelov --Apple-Mail=_5A4D4480-45F7-4FCD-9FC4-EC5A8CD510CD Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii Please, fix commit message subject. Then LGTM. > On 4 Feb 2019, at 20:05, Ivan Koptelov = wrote: >=20 > On 04/02/2019 18:28, n.pettik wrote: >>> On 4 Feb 2019, at 17:44, Ivan Koptelov = wrote: >>>=20 >>> Adds error raise in case of CHECK constraint declared with ON >>> CONFLICT REPLACE action. Before the patch such option was >>> forbidden, but if a user tried to create space with such constraint, >>> this attempt failed silently. >>>=20 >> What is more, now all on conflict actions are ignored for check >> constraints. So lets simply fix parser to disallow this clause. >> It is quite easy: all you need is to remove onconf rule after CHECK >> keyword: >>=20 >> 319 tcons ::=3D CHECK LP expr(E) RP onconf. >> 320 = {sql_add_check_constraint(pParse,&E);} > Thank you for the idea, done. >=20 > -- > src/box/sql/parse.y | 2 +- > test/sql-tap/check.test.lua | 37 = ++++++++++++++++++++++++++++++++++++- > 2 files changed, 37 insertions(+), 2 deletions(-) >=20 > diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y > index 8e21b6fca..a5fe7c0a7 100644 > --- a/src/box/sql/parse.y > +++ b/src/box/sql/parse.y > @@ -316,7 +316,7 @@ tcons ::=3D UNIQUE LP sortlist(X) RP. > {sql_create_index(pParse,0,0,X,0, > = SORT_ORDER_ASC,false, > = SQL_INDEX_TYPE_CONSTRAINT_UNIQUE);} > -tcons ::=3D CHECK LP expr(E) RP onconf. > +tcons ::=3D CHECK LP expr(E) RP . > = {sql_add_check_constraint(pParse,&E);} > tcons ::=3D FOREIGN KEY LP eidlist(FA) RP > REFERENCES nm(T) eidlist_opt(TA) refargs(R) = defer_subclause_opt(D). { > diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua > index 1f369fb02..7960204bc 100755 > --- a/test/sql-tap/check.test.lua > +++ b/test/sql-tap/check.test.lua > @@ -1,6 +1,6 @@ > #!/usr/bin/env tarantool > test =3D require("sqltester") > -test:plan(58) > +test:plan(61) > =20 > --!./tcltestrunner.lua > -- 2005 November 2 > @@ -772,5 +772,40 @@ test:do_execsql_test( > -- > }) > =20 > +-- gh-3345 : the test checks that ON CONFLICT REPLACE > +-- is not allowed for CHECK constraint. > +test:do_catchsql_test( > + 9.1, > + [[ > + CREATE TABLE t101 (a INT primary key, b INT, CHECK(b < 10) > + ON CONFLICT REPLACE) > + ]], { > + -- <9.1> > + 1, "keyword \"ON\" is reserved" > + -- > + }) > + > +test:do_catchsql_test( > + 9.2, > + [[ > + CREATE TABLE t101 (a INT primary key, b INT, CHECK(b < 10) > + ON CONFLICT ABORT) > + ]], { > + -- <9.2> > + 1, "keyword \"ON\" is reserved" > + -- > + }) > + > +test:do_catchsql_test( > + 9.3, > + [[ > + CREATE TABLE t101 (a INT primary key, b INT, CHECK(b < 10) > + ON CONFLICT ROLLBACK) > + ]], { > + -- <9.3> > + 1, "keyword \"ON\" is reserved" > + -- > + }) > + > test:finish_test() > =20 > --=20 > 2.14.3 (Apple Git-98) >=20 >=20 --Apple-Mail=_5A4D4480-45F7-4FCD-9FC4-EC5A8CD510CD Content-Transfer-Encoding: 7bit Content-Type: text/html; charset=us-ascii Please, fix commit message subject. Then LGTM.

On 4 Feb 2019, at 20:05, Ivan Koptelov <ivan.koptelov@tarantool.org> wrote:

On 04/02/2019 18:28, n.pettik wrote:

        

          
On 4 Feb 2019, at 17:44, Ivan Koptelov <ivan.koptelov@tarantool.org> wrote:

Adds error raise in case of CHECK constraint declared with ON
CONFLICT REPLACE action. Before the patch such option was
forbidden, but if a user tried to create space with such constraint,
this attempt failed silently.



        

        
What is more, now all on conflict actions are ignored for check
constraints. So lets simply fix parser to disallow this clause.
It is quite easy: all you need is to remove onconf rule after CHECK
keyword:

   319	tcons ::= CHECK LP expr(E) RP onconf.
   320	                                 {sql_add_check_constraint(pParse,&E);}


      

      Thank you for the idea, done.

      

      --

      
 src/box/sql/parse.y         |  2 +-
 test/sql-tap/check.test.lua | 37 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 8e21b6fca..a5fe7c0a7 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -316,7 +316,7 @@ tcons ::= UNIQUE LP sortlist(X) RP.
                                  {sql_create_index(pParse,0,0,X,0,
                                                    SORT_ORDER_ASC,false,
                                                    SQL_INDEX_TYPE_CONSTRAINT_UNIQUE);}
-tcons ::= CHECK LP expr(E) RP onconf.
+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). {
diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua
index 1f369fb02..7960204bc 100755
--- a/test/sql-tap/check.test.lua
+++ b/test/sql-tap/check.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(58)
+test:plan(61)
 
 --!./tcltestrunner.lua
 -- 2005 November 2
@@ -772,5 +772,40 @@ test:do_execsql_test(
         -- </8.1>
     })
 
+-- gh-3345 : the test checks that ON CONFLICT REPLACE
+-- is not allowed for CHECK constraint.
+test:do_catchsql_test(
+    9.1,
+    [[
+        CREATE TABLE t101 (a INT primary key, b INT, CHECK(b < 10)
+        ON CONFLICT REPLACE)
+    ]], {
+        -- <9.1>
+        1, "keyword \"ON\" is reserved"
+        -- </9.1>
+    })
+
+test:do_catchsql_test(
+    9.2,
+    [[
+        CREATE TABLE t101 (a INT primary key, b INT, CHECK(b < 10)
+        ON CONFLICT ABORT)
+    ]], {
+        -- <9.2>
+        1, "keyword \"ON\" is reserved"
+        -- </9.2>
+    })
+
+test:do_catchsql_test(
+    9.3,
+    [[
+        CREATE TABLE t101 (a INT primary key, b INT, CHECK(b < 10)
+        ON CONFLICT ROLLBACK)
+    ]], {
+        -- <9.3>
+        1, "keyword \"ON\" is reserved"
+        -- </9.3>
+    })
+
 test:finish_test()
 
-- 
2.14.3 (Apple Git-98)




    

  






--Apple-Mail=_5A4D4480-45F7-4FCD-9FC4-EC5A8CD510CD--