* [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 ` [tarantool-patches] " 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
* [tarantool-patches] Re: [PATCH] sql: raise err on CHECK + ON CONFLICT REPLACE 2019-02-04 17:05 ` [tarantool-patches] Re: [PATCH] sql: raise err on CHECK + ON CONFLICT REPLACE Ivan Koptelov @ 2019-02-05 13:44 ` n.pettik 2019-02-07 18:40 ` [tarantool-patches] " i.koptelov 2019-02-08 14:22 ` [tarantool-patches] " Kirill Yukhin 1 sibling, 1 reply; 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] [PATCH] sql: raise err on CHECK + ON CONFLICT REPLACE 2019-02-05 13:44 ` n.pettik @ 2019-02-07 18:40 ` i.koptelov 0 siblings, 0 replies; 5+ messages in thread From: i.koptelov @ 2019-02-07 18:40 UTC (permalink / raw) To: tarantool-patches; +Cc: n.pettik > On 5 Feb 2019, at 16:44, n.pettik <korablev@tarantool.org> wrote: > > Please, fix commit message subject. Then LGTM. Ok, done (on branch) ^ 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 ` [tarantool-patches] Re: [PATCH] sql: raise err on CHECK + ON CONFLICT REPLACE 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] [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] " 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
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 --
[not found] <821d032b-9adf-6496-c0bc-f0f9497c5f83@tarantool.org>
2019-02-04 17:05 ` [tarantool-patches] Re: [PATCH] sql: raise err on CHECK + ON CONFLICT REPLACE Ivan Koptelov
2019-02-05 13:44 ` n.pettik
2019-02-07 18:40 ` [tarantool-patches] " i.koptelov
2019-02-08 14:22 ` [tarantool-patches] " Kirill Yukhin
2019-02-04 14:44 [tarantool-patches] " Ivan Koptelov
2019-02-04 15:28 ` [tarantool-patches] " n.pettik
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox