Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH] sql: raise err on CHECK + ON CONFLICT REPLACE
@ 2019-02-04 14:44 Ivan Koptelov
  2019-02-04 15:28 ` [tarantool-patches] " n.pettik
  0 siblings, 1 reply; 5+ messages in thread
From: Ivan Koptelov @ 2019-02-04 14:44 UTC (permalink / raw)
  To: tarantool-patches; +Cc: korablev, Ivan Koptelov

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.

Closes #3345
---
Branch https://github.com/tarantool/tarantool/tree/sudobobo/gh-3345-raise-err-on-CHECK-w-ON-CONFLICT-REPLACE
Issue https://github.com/tarantool/tarantool/issues/3345

 src/box/sql/build.c         | 12 ++++++++++++
 test/sql-tap/check.test.lua | 16 +++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 49b90b5d0..e43694e0b 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -761,6 +761,18 @@ sql_add_check_constraint(struct Parse *parser, struct ExprSpan *span)
 {
 	struct Expr *expr = span->pExpr;
 	struct Table *table = parser->pNewTable;
+
+	if (strcasestr(span->zEnd, "on conflict replace") != NULL) {
+		diag_set(ClientError, ER_CREATE_SPACE, table->def->name,
+			 "ON CONFLICT REPLACE action is not allowed for "
+			 "CHECK constraint");
+		parser->rc = SQL_TARANTOOL_ERROR;
+		parser->nErr++;
+		sqlite3DbFree(parser->db, expr->u.zToken);
+
+		goto release_expr;
+	}
+
 	if (table != NULL) {
 		expr->u.zToken =
 			sqlite3DbStrNDup(parser->db, (char *)span->zStart,
diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua
index 1f369fb02..b6e17c059 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(59)
 
 --!./tcltestrunner.lua
 -- 2005 November 2
@@ -772,5 +772,19 @@ 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, "Failed to create space 'T101': ON CONFLICT REPLACE " ..
+                    "action is not allowed for CHECK constraint"
+            -- </9.1>
+        })
+
 test:finish_test()
 
-- 
2.14.3 (Apple Git-98)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tarantool-patches] Re: [PATCH] sql: raise err on CHECK + ON CONFLICT REPLACE
  2019-02-04 14:44 [tarantool-patches] [PATCH] sql: raise err on CHECK + ON CONFLICT REPLACE Ivan Koptelov
@ 2019-02-04 15:28 ` n.pettik
  0 siblings, 0 replies; 5+ messages in thread
From: n.pettik @ 2019-02-04 15:28 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Ivan Koptelov



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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tarantool-patches] Re: [PATCH] sql: raise err on CHECK + ON CONFLICT REPLACE
  2019-02-04 17:05 ` Ivan Koptelov
  2019-02-05 13:44   ` n.pettik
@ 2019-02-08 14:22   ` Kirill Yukhin
  1 sibling, 0 replies; 5+ messages in thread
From: Kirill Yukhin @ 2019-02-08 14:22 UTC (permalink / raw)
  To: tarantool-patches

Hello,

On 04 Feb 20:05, Ivan Koptelov wrote:
>    On 04/02/2019 18:28, n.pettik wrote:
> 
> 
> On 4 Feb 2019, at 17:44, Ivan Koptelov [1]<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(-)

I've checked your patch into 2.1 branch.

--
Regards, Kirill Yukhin

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tarantool-patches] Re: [PATCH] sql: raise err on CHECK + ON CONFLICT REPLACE
  2019-02-04 17:05 ` Ivan Koptelov
@ 2019-02-05 13:44   ` n.pettik
  2019-02-08 14:22   ` Kirill Yukhin
  1 sibling, 0 replies; 5+ messages in thread
From: n.pettik @ 2019-02-05 13:44 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Ivan Koptelov

[-- Attachment #1: Type: text/plain, Size: 3276 bytes --]

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> <mailto: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)
> 
> 


[-- Attachment #2: Type: text/html, Size: 4126 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tarantool-patches] Re: [PATCH] sql: raise err on CHECK + ON CONFLICT REPLACE
       [not found] <821d032b-9adf-6496-c0bc-f0f9497c5f83@tarantool.org>
@ 2019-02-04 17:05 ` Ivan Koptelov
  2019-02-05 13:44   ` n.pettik
  2019-02-08 14:22   ` Kirill Yukhin
  0 siblings, 2 replies; 5+ messages in thread
From: Ivan Koptelov @ 2019-02-04 17:05 UTC (permalink / raw)
  To: tarantool-patches

[-- Attachment #1: Type: text/html, Size: 3636 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-02-08 14:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-04 14:44 [tarantool-patches] [PATCH] sql: raise err on CHECK + ON CONFLICT REPLACE Ivan Koptelov
2019-02-04 15:28 ` [tarantool-patches] " n.pettik
     [not found] <821d032b-9adf-6496-c0bc-f0f9497c5f83@tarantool.org>
2019-02-04 17:05 ` Ivan Koptelov
2019-02-05 13:44   ` n.pettik
2019-02-08 14:22   ` Kirill Yukhin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox