Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH] sql: introduce ADD CONSTRAINT CHECK statement
@ 2019-07-08 18:57 Nikita Pettik
  2019-07-11 21:32 ` [tarantool-patches] " Vladislav Shpilevoy
  2019-07-13 14:16 ` Kirill Yukhin
  0 siblings, 2 replies; 5+ messages in thread
From: Nikita Pettik @ 2019-07-08 18:57 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy, Nikita Pettik

This patch extends parser's grammar to allow to create CHECK constraints
on already existent tables via SQL facilities. Syntax is following:

ALTER TABLE <table> ADD CONSTRAINT <name> CHECK (<expr>);

Closes #3097
---
Branch: https://github.com/tarantool/tarantool/tree/np/sql-add-create-ck-syntax
Issue: https://github.com/tarantool/tarantool/issues/3097

 src/box/sql/build.c          | 26 +++++++++++++++++++++---
 src/box/sql/parse.y          |  7 ++++++-
 src/box/sql/parse_def.h      |  6 +++---
 test/sql-tap/alter2.test.lua | 48 +++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 79 insertions(+), 8 deletions(-)

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 292168f88..396de63fd 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -668,6 +668,11 @@ trim_space_snprintf(char *wptr, const char *str, uint32_t str_len)
 	*wptr = '\0';
 }
 
+static void
+vdbe_emit_ck_constraint_create(struct Parse *parser,
+			       const struct ck_constraint_def *ck_def,
+			       uint32_t reg_space_id);
+
 void
 sql_create_check_contraint(struct Parse *parser)
 {
@@ -680,7 +685,7 @@ sql_create_check_contraint(struct Parse *parser)
 	assert(alter_def->entity_type == ENTITY_TYPE_CK);
 	(void) alter_def;
 	struct space *space = parser->create_table_def.new_space;
-	assert(space != NULL);
+	bool is_alter = space == NULL;
 
 	/* Prepare payload for ck constraint definition. */
 	struct region *region = &parser->region;
@@ -694,6 +699,7 @@ sql_create_check_contraint(struct Parse *parser)
 			return;
 		}
 	} else {
+		assert(! is_alter);
 		uint32_t ck_idx = ++parser->create_table_def.check_count;
 		name = tt_sprintf("CK_CONSTRAINT_%d_%s", ck_idx,
 				  space->def->name);
@@ -734,8 +740,22 @@ sql_create_check_contraint(struct Parse *parser)
 	trim_space_snprintf(ck_def->expr_str, expr_str, expr_str_len);
 	memcpy(ck_def->name, name, name_len);
 	ck_def->name[name_len] = '\0';
-
-	rlist_add_entry(&parser->create_table_def.new_check, ck_parse, link);
+	if (is_alter) {
+		const char *space_name = alter_def->entity_name->a[0].zName;
+		struct space *space = space_by_name(space_name);
+		if (space == NULL) {
+			diag_set(ClientError, ER_NO_SUCH_SPACE, space_name);
+			parser->is_aborted = true;
+			return;
+		}
+		int space_id_reg = ++parser->nMem;
+		sqlVdbeAddOp2(sqlGetVdbe(parser), OP_Integer, space->def->id,
+			      space_id_reg);
+		vdbe_emit_ck_constraint_create(parser, ck_def, space_id_reg);
+	} else {
+		rlist_add_entry(&parser->create_table_def.new_check, ck_parse,
+				link);
+	}
 }
 
 /*
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 010feffd4..2a60ad25b 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -283,7 +283,7 @@ ccons ::= cconsname(N) UNIQUE. {
 ccons ::= check_constraint_def .
 
 check_constraint_def ::= cconsname(N) CHECK LP expr(X) RP. {
-  create_ck_def_init(&pParse->create_ck_def, &N, &X);
+  create_ck_def_init(&pParse->create_ck_def, NULL, &N, &X);
   sql_create_check_contraint(pParse);
 }
 
@@ -1680,6 +1680,11 @@ cmd ::= alter_add_constraint(N) FOREIGN KEY LP eidlist(FA) RP REFERENCES
   sql_create_foreign_key(pParse);
 }
 
+cmd ::= alter_add_constraint(N) CHECK LP expr(X) RP. {
+    create_ck_def_init(&pParse->create_ck_def, N.table_name, &N.name, &X);
+    sql_create_check_contraint(pParse);
+}
+
 cmd ::= alter_add_constraint(N) unique_spec(U) LP sortlist(X) RP. {
   create_index_def_init(&pParse->create_index_def, N.table_name, &N.name, X, U,
                         SORT_ORDER_ASC, false);
diff --git a/src/box/sql/parse_def.h b/src/box/sql/parse_def.h
index 6c1b6fddd..557e41529 100644
--- a/src/box/sql/parse_def.h
+++ b/src/box/sql/parse_def.h
@@ -417,10 +417,10 @@ create_trigger_def_init(struct create_trigger_def *trigger_def,
 }
 
 static inline void
-create_ck_def_init(struct create_ck_def *ck_def, struct Token *name,
-		   struct ExprSpan *expr)
+create_ck_def_init(struct create_ck_def *ck_def, struct SrcList *table_name,
+		   struct Token *name, struct ExprSpan *expr)
 {
-	create_constraint_def_init(&ck_def->base, NULL, name, false,
+	create_constraint_def_init(&ck_def->base, table_name, name, false,
 				   false, ENTITY_TYPE_CK);
 	ck_def->expr = expr;
 }
diff --git a/test/sql-tap/alter2.test.lua b/test/sql-tap/alter2.test.lua
index 8969dfab2..4dc8f8255 100755
--- a/test/sql-tap/alter2.test.lua
+++ b/test/sql-tap/alter2.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(21)
+test:plan(26)
 
 -- This suite is aimed to test ALTER TABLE ADD CONSTRAINT statement.
 --
@@ -260,4 +260,50 @@ test:do_catchsql_test(
         -- </alter2-5.2>
     })
 
+-- Test ADD CONSTRAINT CHECK functionality. CHECK constraints are
+-- intagrated into Tarantool's core, so basically we whould test
+-- only grammar and validate correctness of raised errors.
+--
+test:do_catchsql_test(
+    "alter2-6.1",
+    [[
+        CREATE TABLE t1 (id INT PRIMARY KEY);
+        ALTER TABLE t1 ADD CONSTRAINT ck CHECK(id > 0);
+        INSERT INTO t1 VALUES (-1);
+    ]], { 1, "Check constraint failed 'CK': id > 0" })
+
+-- Make sure that one can't create constraint with the same name twice.
+--
+test:do_catchsql_test(
+    "alter2-6.2",
+    [[
+        ALTER TABLE t1 ADD CONSTRAINT ck CHECK(id > 0);
+    ]], { 1, "Constraint CK already exists" })
+
+-- Make sure that CHECK constraint can be created only on empty space.
+--
+test:do_catchsql_test(
+    "alter2-6.3",
+    [[
+        INSERT INTO t1 VALUES (1);
+        ALTER TABLE t1 ADD CONSTRAINT ck1 CHECK(id > 0);
+    ]], { 1, "Failed to create check constraint 'CK1': referencing space must be empty" })
+
+-- "Non-existant" space error is raised correctly.
+--
+test:do_catchsql_test(
+    "alter2-6.4",
+    [[
+        ALTER TABLE fake ADD CONSTRAINT ck CHECK(id > 0);
+    ]], { 1, "Space 'FAKE' does not exist" })
+
+-- "Non-existant" column error is raised correctly.
+--
+test:do_catchsql_test(
+    "alter2-6.5",
+    [[
+        CREATE TABLE t2 (id INT PRIMARY KEY);
+        ALTER TABLE t2 ADD CONSTRAINT ck CHECK(fake_col > 0);
+    ]], { 1, "Failed to create check constraint 'CK': Can’t resolve field 'FAKE_COL'" })
+
 test:finish_test()
-- 
2.15.1

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

* [tarantool-patches] Re: [PATCH] sql: introduce ADD CONSTRAINT CHECK statement
  2019-07-08 18:57 [tarantool-patches] [PATCH] sql: introduce ADD CONSTRAINT CHECK statement Nikita Pettik
@ 2019-07-11 21:32 ` Vladislav Shpilevoy
  2019-07-12  9:24   ` n.pettik
  2019-07-13 14:16 ` Kirill Yukhin
  1 sibling, 1 reply; 5+ messages in thread
From: Vladislav Shpilevoy @ 2019-07-11 21:32 UTC (permalink / raw)
  To: Nikita Pettik, tarantool-patches

Hi! Thanks for the patch!

On 08/07/2019 20:57, Nikita Pettik wrote:
> This patch extends parser's grammar to allow to create CHECK constraints
> on already existent tables via SQL facilities. Syntax is following:
> 
> ALTER TABLE <table> ADD CONSTRAINT <name> CHECK (<expr>);
> 
> Closes #3097
> ---

TarantoolBot request?

> diff --git a/test/sql-tap/alter2.test.lua b/test/sql-tap/alter2.test.lua
> index 8969dfab2..4dc8f8255 100755
> --- a/test/sql-tap/alter2.test.lua
> +++ b/test/sql-tap/alter2.test.lua
> @@ -1,6 +1,6 @@
>  #!/usr/bin/env tarantool
>  test = require("sqltester")
> -test:plan(21)
> +test:plan(26)
>  
>  -- This suite is aimed to test ALTER TABLE ADD CONSTRAINT statement.
>  --
> @@ -260,4 +260,50 @@ test:do_catchsql_test(
>          -- </alter2-5.2>
>      })
>  
> +-- Test ADD CONSTRAINT CHECK functionality. CHECK constraints are
> +-- intagrated into Tarantool's core, so basically we whould test

Typos. 'intagrated', 'whould'.

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

* [tarantool-patches] Re: [PATCH] sql: introduce ADD CONSTRAINT CHECK statement
  2019-07-11 21:32 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2019-07-12  9:24   ` n.pettik
  2019-07-12 19:00     ` Vladislav Shpilevoy
  0 siblings, 1 reply; 5+ messages in thread
From: n.pettik @ 2019-07-12  9:24 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Vladislav Shpilevoy


> On 08/07/2019 20:57, Nikita Pettik wrote:
>> This patch extends parser's grammar to allow to create CHECK constraints
>> on already existent tables via SQL facilities. Syntax is following:
>> 
>> ALTER TABLE <table> ADD CONSTRAINT <name> CHECK (<expr>);
>> 
>> Closes #3097
>> ---
> 
> TarantoolBot request?

    sql: introduce ADD CONSTRAINT CHECK statement
    
    This patch extends parser's grammar to allow to create CHECK constraints
    on already existent tables via SQL facilities.
    
    Closes #3097
    
    @TarantoolBot document
    Title: Document ADD CONSTRAINT CHECK statement
    
    Now it is possible to add CHECK constraints to already existent table
    via SQL means. To achieve this one must use following syntax:
    
    ALTER TABLE <table> ADD CONSTRAINT <name> CHECK (<expr>);

>> diff --git a/test/sql-tap/alter2.test.lua b/test/sql-tap/alter2.test.lua
>> index 8969dfab2..4dc8f8255 100755
>> --- a/test/sql-tap/alter2.test.lua
>> +++ b/test/sql-tap/alter2.test.lua
>> @@ -1,6 +1,6 @@
>> #!/usr/bin/env tarantool
>> test = require("sqltester")
>> -test:plan(21)
>> +test:plan(26)
>> 
>> -- This suite is aimed to test ALTER TABLE ADD CONSTRAINT statement.
>> --
>> @@ -260,4 +260,50 @@ test:do_catchsql_test(
>>         -- </alter2-5.2>
>>     })
>> 
>> +-- Test ADD CONSTRAINT CHECK functionality. CHECK constraints are
>> +-- intagrated into Tarantool's core, so basically we whould test
> 
> Typos. 'intagrated', 'whould’.

Thanks, these typos have been fixed.

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

* [tarantool-patches] Re: [PATCH] sql: introduce ADD CONSTRAINT CHECK statement
  2019-07-12  9:24   ` n.pettik
@ 2019-07-12 19:00     ` Vladislav Shpilevoy
  0 siblings, 0 replies; 5+ messages in thread
From: Vladislav Shpilevoy @ 2019-07-12 19:00 UTC (permalink / raw)
  To: n.pettik, tarantool-patches, Kirill Yukhin

Hi!

Thanks for the fixes! LGTM.

On 12/07/2019 11:24, n.pettik wrote:
> 
>> On 08/07/2019 20:57, Nikita Pettik wrote:
>>> This patch extends parser's grammar to allow to create CHECK constraints
>>> on already existent tables via SQL facilities. Syntax is following:
>>>
>>> ALTER TABLE <table> ADD CONSTRAINT <name> CHECK (<expr>);
>>>
>>> Closes #3097
>>> ---
>>
>> TarantoolBot request?
> 
>     sql: introduce ADD CONSTRAINT CHECK statement
>     
>     This patch extends parser's grammar to allow to create CHECK constraints
>     on already existent tables via SQL facilities.
>     
>     Closes #3097
>     
>     @TarantoolBot document
>     Title: Document ADD CONSTRAINT CHECK statement
>     
>     Now it is possible to add CHECK constraints to already existent table
>     via SQL means. To achieve this one must use following syntax:
>     
>     ALTER TABLE <table> ADD CONSTRAINT <name> CHECK (<expr>);
> 
>>> diff --git a/test/sql-tap/alter2.test.lua b/test/sql-tap/alter2.test.lua
>>> index 8969dfab2..4dc8f8255 100755
>>> --- a/test/sql-tap/alter2.test.lua
>>> +++ b/test/sql-tap/alter2.test.lua
>>> @@ -1,6 +1,6 @@
>>> #!/usr/bin/env tarantool
>>> test = require("sqltester")
>>> -test:plan(21)
>>> +test:plan(26)
>>>
>>> -- This suite is aimed to test ALTER TABLE ADD CONSTRAINT statement.
>>> --
>>> @@ -260,4 +260,50 @@ test:do_catchsql_test(
>>>         -- </alter2-5.2>
>>>     })
>>>
>>> +-- Test ADD CONSTRAINT CHECK functionality. CHECK constraints are
>>> +-- intagrated into Tarantool's core, so basically we whould test
>>
>> Typos. 'intagrated', 'whould’.
> 
> Thanks, these typos have been fixed.
> 

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

* [tarantool-patches] Re: [PATCH] sql: introduce ADD CONSTRAINT CHECK statement
  2019-07-08 18:57 [tarantool-patches] [PATCH] sql: introduce ADD CONSTRAINT CHECK statement Nikita Pettik
  2019-07-11 21:32 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2019-07-13 14:16 ` Kirill Yukhin
  1 sibling, 0 replies; 5+ messages in thread
From: Kirill Yukhin @ 2019-07-13 14:16 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy, Nikita Pettik

Hello,

On 08 Jul 21:57, Nikita Pettik wrote:
> This patch extends parser's grammar to allow to create CHECK constraints
> on already existent tables via SQL facilities. Syntax is following:
> 
> ALTER TABLE <table> ADD CONSTRAINT <name> CHECK (<expr>);
> 
> Closes #3097
> ---
> Branch: https://github.com/tarantool/tarantool/tree/np/sql-add-create-ck-syntax
> Issue: https://github.com/tarantool/tarantool/issues/3097

I've checked your patch into master.

--
Regards, Kirill Yukhin

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

end of thread, other threads:[~2019-07-13 14:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-08 18:57 [tarantool-patches] [PATCH] sql: introduce ADD CONSTRAINT CHECK statement Nikita Pettik
2019-07-11 21:32 ` [tarantool-patches] " Vladislav Shpilevoy
2019-07-12  9:24   ` n.pettik
2019-07-12 19:00     ` Vladislav Shpilevoy
2019-07-13 14:16 ` Kirill Yukhin

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