Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH] sql: constraints definition among columns in CREATE TABLE()
@ 2018-11-18 14:31 Roman Khabibov
  2018-11-19 12:41 ` [tarantool-patches] " Vladislav Shpilevoy
  2018-11-26 10:33 ` Kirill Yukhin
  0 siblings, 2 replies; 7+ messages in thread
From: Roman Khabibov @ 2018-11-18 14:31 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy

Allow constraints to appear along with columns definitions. Disallow typing
a constraint name without specifying the constraint and after.

Closes #3504

Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3504-constraints-create-table
Issue: https://github.com/tarantool/tarantool/issues/3504
---
 src/box/sql/parse.y         |  20 ++---
 test/sql-tap/check.test.lua |  50 +++----------
 test/sql-tap/table.test.lua | 141 +++++++++++++++++++++++++++++++++++-
 3 files changed, 161 insertions(+), 50 deletions(-)

diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index abaa73736..5b9553017 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -176,15 +176,17 @@ createkw(A) ::= CREATE(A).  {disableLookaside(pParse);}
 ifnotexists(A) ::= .              {A = 0;}
 ifnotexists(A) ::= IF NOT EXISTS. {A = 1;}
 
-create_table_args ::= LP columnlist conslist_opt RP(E). {
+create_table_args ::= LP columnlist RP(E). {
   sqlite3EndTable(pParse,&E,0);
 }
 create_table_args ::= AS select(S). {
   sqlite3EndTable(pParse,0,S);
   sql_select_delete(pParse->db, S);
 }
+columnlist ::= columnlist COMMA tconsdef.
 columnlist ::= columnlist COMMA columnname carglist.
 columnlist ::= columnname carglist.
+columnlist ::= tconsdef.
 columnname(A) ::= nm(A) typedef(Y). {sqlite3AddColumn(pParse,&A,&Y);}
 
 // An IDENTIFIER can be a generic identifier, or one of several
@@ -232,9 +234,11 @@ nm(A) ::= id(A). {
 // "carglist" is a list of additional constraints that come after the
 // column name and column type in a CREATE TABLE statement.
 //
-carglist ::= carglist ccons.
+carglist ::= carglist cconsdef.
 carglist ::= .
-ccons ::= CONSTRAINT nm(X).           {pParse->constraintName = X;}
+cconsdef ::= cconsname ccons.
+cconsname ::= CONSTRAINT nm(X).           {pParse->constraintName = X;}
+cconsname ::= .                           {pParse->constraintName.n = 0;}
 ccons ::= DEFAULT term(X).            {sqlite3AddDefaultValue(pParse,&X);}
 ccons ::= DEFAULT LP expr(X) RP.      {sqlite3AddDefaultValue(pParse,&X);}
 ccons ::= DEFAULT PLUS term(X).       {sqlite3AddDefaultValue(pParse,&X);}
@@ -303,13 +307,9 @@ init_deferred_pred_opt(A) ::= .                       {A = 0;}
 init_deferred_pred_opt(A) ::= INITIALLY DEFERRED.     {A = 1;}
 init_deferred_pred_opt(A) ::= INITIALLY IMMEDIATE.    {A = 0;}
 
-conslist_opt ::= .
-conslist_opt ::= COMMA conslist.
-conslist ::= conslist tconscomma tcons.
-conslist ::= tcons.
-tconscomma ::= COMMA.            {pParse->constraintName.n = 0;}
-tconscomma ::= .
-tcons ::= CONSTRAINT nm(X).      {pParse->constraintName = X;}
+tconsdef ::= tconsname tcons.
+tconsname ::= CONSTRAINT nm(X).      {pParse->constraintName = X;}
+tconsname ::= .                      {pParse->constraintName.n = 0;}
 tcons ::= PRIMARY KEY LP sortlist(X) autoinc(I) RP.
                                  {sqlite3AddPrimaryKey(pParse,X,I,0);}
 tcons ::= UNIQUE LP sortlist(X) RP.
diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua
index 039e2291e..ebdbc5b13 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(60)
+test:plan(58)
 
 --!./tcltestrunner.lua
 -- 2005 November 2
@@ -270,59 +270,31 @@ test:do_catchsql_test(
         -- </check-2.6>
     })
 
--- Undocumented behavior:  The CONSTRAINT name clause can follow a constraint.
--- Such a clause is ignored.  But the parser must accept it for backwards
--- compatibility.
---
-test:do_execsql_test(
+-- gh-3504: Check the CONSTRAINT name clause can't follow a constraint.
+
+test:do_catchsql_test(
     "check-2.10",
     [[
         CREATE TABLE t2b(
-          x INTEGER CHECK( typeof(coalesce(x,0))=='integer' ) CONSTRAINT one,
-          y TEXT PRIMARY KEY constraint two,
-          z INTEGER,
-          UNIQUE(x,z) constraint three
+          x INTEGER CHECK( typeof(coalesce(x,0))=='integer' ) CONSTRAINT one
         );
     ]], {
         -- <check-2.10>
-
+        1, "near \")\": syntax error"
         -- </check-2.10>
     })
 
 test:do_catchsql_test(
     "check-2.11",
-    [[
-        INSERT INTO t2b VALUES('xyzzy','hi',5);
-    ]], {
-        -- <check-2.11>
-        1, "CHECK constraint failed: T2B"
-        -- </check-2.11>
-    })
-
-test:do_execsql_test(
-    "check-2.12",
     [[
         CREATE TABLE t2c(
-          x INTEGER CONSTRAINT x_one CONSTRAINT x_two primary key
-              CHECK( typeof(coalesce(x,0))=='integer' )
-              CONSTRAINT x_two CONSTRAINT x_three,
-          y INTEGER, z INTEGER,
-          CONSTRAINT u_one UNIQUE(x,y,z) CONSTRAINT u_two
+          x INTEGER CONSTRAINT one CHECK( typeof(coalesce(x,0))=='integer' )
+        CONSTRAINT two
         );
     ]], {
-        -- <check-2.12>
-
-        -- </check-2.12>
-    })
-
-test:do_catchsql_test(
-    "check-2.13",
-    [[
-        INSERT INTO t2c VALUES('xyzzy',7,8);
-    ]], {
-        -- <check-2.13>
-        1, "CHECK constraint failed: X_TWO"
-        -- </check-2.13>
+        -- <check-2.10>
+        1, "near \")\": syntax error"
+        -- </check-2.10>
     })
 
 test:do_execsql_test(
diff --git a/test/sql-tap/table.test.lua b/test/sql-tap/table.test.lua
index 8367ec016..65b45de0a 100755
--- a/test/sql-tap/table.test.lua
+++ b/test/sql-tap/table.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(57)
+test:plan(68)
 
 --!./tcltestrunner.lua
 -- 2001 September 15
@@ -1180,4 +1180,143 @@ test:do_test(
 
     -- </table-15.1>
 })
+
+-- gh-3504 Check the possibility appear constraints along with columns
+-- definitions.
+
+test:do_execsql_test(
+    "table-21.1",
+    [[
+        CREATE TABLE t21(
+           a integer,
+           primary key (a),
+           b integer,
+           check (b > 0),
+           c integer
+           check (c > 0)
+        );
+    ]], {
+        -- <table-21.1>
+
+        -- </table-21.1>
+    })
+
+test:do_catchsql_test(
+    "table-21.2",
+    [[
+        INSERT INTO t21 VALUES(1, 1, 1);
+        INSERT INTO t21 VALUES(1, 2, 2);
+    ]], {
+        -- <table-21.2>
+        1, "Duplicate key exists in unique index 'pk_unnamed_T21_1' in space 'T21'"
+        -- </table-21.2>
+    })
+
+test:do_catchsql_test(
+    "table-21.3",
+    [[
+        INSERT INTO t21 VALUES(1, -1, 1);
+    ]], {
+        -- <table-21.3>
+        1, "CHECK constraint failed: T21"
+        -- </table-21.3>
+    })
+
+test:do_catchsql_test(
+    "table-21.4",
+    [[
+        INSERT INTO t21 VALUES(1, 1, -1);
+    ]], {
+        -- <table-21.4>
+        1, "CHECK constraint failed: T21"
+        -- </table-21.4>
+    })
+
+test:do_execsql_test(
+    "check-21.cleanup",
+    [[
+        DROP TABLE IF EXISTS t21;
+    ]], {
+        -- <check-21.cleanup>
+
+        -- </check-21.cleanup>
+    })
+
+-- gh-3504: Check the CONSTRAINT name clause can't follow a constraint
+-- only before.
+
+test:do_catchsql_test(
+    "table-22.1",
+    [[
+        CREATE TABLE t22(
+           a integer,
+           primary key (a) constraint one
+        );
+    ]], {
+        -- <table-22.1>
+        1,"keyword \"constraint\" is reserved"
+        -- </table-22.1>
+    })
+
+test:do_execsql_test(
+    "table-22.2",
+    [[
+        CREATE TABLE t22(
+           a integer primary key,
+           b integer,
+           constraint one unique (b),
+           c integer
+        );
+    ]], {
+        -- <table-22.2>
+
+        -- </table-22.2>
+    })
+
+test:do_catchsql_test(
+    "table-22.3",
+    [[
+        INSERT INTO t22 VALUES(1, 1, 1);
+        INSERT INTO t22 VALUES(2, 1, 1);
+    ]], {
+        -- <table-22.3>
+        1,"Duplicate key exists in unique index 'unique_ONE_2' in space 'T22'"
+        -- </table-22.3>
+    })
+
+test:do_execsql_test(
+    "table-22.4",
+    [[
+        CREATE TABLE t24(
+           a integer primary key,
+           b integer constraint two unique,
+           c integer
+        );
+    ]], {
+        -- <table-22.4>
+
+        -- </table-22.4>
+    })
+
+test:do_catchsql_test(
+    "table-22.5",
+    [[
+        INSERT INTO t24 VALUES(1, 1, 1);
+        INSERT INTO t24 VALUES(2, 1, 1);
+    ]], {
+        -- <table-21.5>
+        1, "Duplicate key exists in unique index 'unique_TWO_2' in space 'T24'"
+        -- </table-22.5>
+    })
+
+test:do_execsql_test(
+    "check-22.cleanup",
+    [[
+        DROP TABLE IF EXISTS t22;
+        DROP TABLE IF EXISTS t24;
+    ]], {
+        -- <check-22.cleanup>
+
+        -- </check-22.cleanup>
+    })
 test:finish_test()
-- 
2.19.1

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

* [tarantool-patches] Re: [PATCH] sql: constraints definition among columns in CREATE TABLE()
  2018-11-18 14:31 [tarantool-patches] [PATCH] sql: constraints definition among columns in CREATE TABLE() Roman Khabibov
@ 2018-11-19 12:41 ` Vladislav Shpilevoy
  2018-11-21  5:04   ` roman.habibov1
  2018-11-26 10:33 ` Kirill Yukhin
  1 sibling, 1 reply; 7+ messages in thread
From: Vladislav Shpilevoy @ 2018-11-19 12:41 UTC (permalink / raw)
  To: tarantool-patches, Roman Khabibov

Hi! Thanks for the patch! Please, fix one minor comment
below.

On 18/11/2018 17:31, Roman Khabibov wrote:
> Allow constraints to appear along with columns definitions. Disallow typing
> a constraint name without specifying the constraint and after.
> 
> Closes #3504
> 
> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3504-constraints-create-table
> Issue: https://github.com/tarantool/tarantool/issues/3504
> diff --git a/test/sql-tap/table.test.lua b/test/sql-tap/table.test.lua
> index 8367ec016..65b45de0a 100755
> --- a/test/sql-tap/table.test.lua
> +++ b/test/sql-tap/table.test.lua
> @@ -1,6 +1,6 @@
>   #!/usr/bin/env tarantool
>   test = require("sqltester")
> -test:plan(57)
> +test:plan(68)
>   
>   --!./tcltestrunner.lua
>   -- 2001 September 15
> @@ -1180,4 +1180,143 @@ test:do_test(

Please, add a test that I can not write multiple 'CONSTRAINT <name>'
in a row.

>   
>       -- </table-15.1>
>   })

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

* [tarantool-patches] Re: [PATCH] sql: constraints definition among columns in CREATE TABLE()
  2018-11-19 12:41 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2018-11-21  5:04   ` roman.habibov1
  2018-11-21 11:01     ` Vladislav Shpilevoy
  2018-11-21 19:29     ` n.pettik
  0 siblings, 2 replies; 7+ messages in thread
From: roman.habibov1 @ 2018-11-21  5:04 UTC (permalink / raw)
  To: Vladislav Shpilevoy, tarantool-patches

Hi! Thanks for review.

> Hi! Thanks for the patch! Please, fix one minor comment
> below.
> Please, add a test that I can not write multiple 'CONSTRAINT <name>'
> in a row.
>
>>        -- </table-15.1>
>>    })
Done.

commit 5bcc130bcabaa1f8efe8b12af35be5f2a4a05e6e
Author: Roman Khabibov <roman.habibov1@yandex.ru>
Date:   Sun Nov 18 15:54:53 2018 +0300

    sql: constraints def among columns in CREATE TABLE()
    
    Allow constraints to appear along with columns definitions. Disallow typing
    a constraint name without specifying the constraint and after.
    
    Closes #3504

diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index abaa73736..5b9553017 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -176,15 +176,17 @@ createkw(A) ::= CREATE(A).  {disableLookaside(pParse);}
 ifnotexists(A) ::= .              {A = 0;}
 ifnotexists(A) ::= IF NOT EXISTS. {A = 1;}
 
-create_table_args ::= LP columnlist conslist_opt RP(E). {
+create_table_args ::= LP columnlist RP(E). {
   sqlite3EndTable(pParse,&E,0);
 }
 create_table_args ::= AS select(S). {
   sqlite3EndTable(pParse,0,S);
   sql_select_delete(pParse->db, S);
 }
+columnlist ::= columnlist COMMA tconsdef.
 columnlist ::= columnlist COMMA columnname carglist.
 columnlist ::= columnname carglist.
+columnlist ::= tconsdef.
 columnname(A) ::= nm(A) typedef(Y). {sqlite3AddColumn(pParse,&A,&Y);}
 
 // An IDENTIFIER can be a generic identifier, or one of several
@@ -232,9 +234,11 @@ nm(A) ::= id(A). {
 // "carglist" is a list of additional constraints that come after the
 // column name and column type in a CREATE TABLE statement.
 //
-carglist ::= carglist ccons.
+carglist ::= carglist cconsdef.
 carglist ::= .
-ccons ::= CONSTRAINT nm(X).           {pParse->constraintName = X;}
+cconsdef ::= cconsname ccons.
+cconsname ::= CONSTRAINT nm(X).           {pParse->constraintName = X;}
+cconsname ::= .                           {pParse->constraintName.n = 0;}
 ccons ::= DEFAULT term(X).            {sqlite3AddDefaultValue(pParse,&X);}
 ccons ::= DEFAULT LP expr(X) RP.      {sqlite3AddDefaultValue(pParse,&X);}
 ccons ::= DEFAULT PLUS term(X).       {sqlite3AddDefaultValue(pParse,&X);}
@@ -303,13 +307,9 @@ init_deferred_pred_opt(A) ::= .                       {A = 0;}
 init_deferred_pred_opt(A) ::= INITIALLY DEFERRED.     {A = 1;}
 init_deferred_pred_opt(A) ::= INITIALLY IMMEDIATE.    {A = 0;}
 
-conslist_opt ::= .
-conslist_opt ::= COMMA conslist.
-conslist ::= conslist tconscomma tcons.
-conslist ::= tcons.
-tconscomma ::= COMMA.            {pParse->constraintName.n = 0;}
-tconscomma ::= .
-tcons ::= CONSTRAINT nm(X).      {pParse->constraintName = X;}
+tconsdef ::= tconsname tcons.
+tconsname ::= CONSTRAINT nm(X).      {pParse->constraintName = X;}
+tconsname ::= .                      {pParse->constraintName.n = 0;}
 tcons ::= PRIMARY KEY LP sortlist(X) autoinc(I) RP.
                                  {sqlite3AddPrimaryKey(pParse,X,I,0);}
 tcons ::= UNIQUE LP sortlist(X) RP.
diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua
index 039e2291e..ebdbc5b13 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(60)
+test:plan(58)
 
 --!./tcltestrunner.lua
 -- 2005 November 2
@@ -270,59 +270,31 @@ test:do_catchsql_test(
         -- </check-2.6>
     })
 
--- Undocumented behavior:  The CONSTRAINT name clause can follow a constraint.
--- Such a clause is ignored.  But the parser must accept it for backwards
--- compatibility.
---
-test:do_execsql_test(
+-- gh-3504: Check the CONSTRAINT name clause can't follow a constraint.
+
+test:do_catchsql_test(
     "check-2.10",
     [[
         CREATE TABLE t2b(
-          x INTEGER CHECK( typeof(coalesce(x,0))=='integer' ) CONSTRAINT one,
-          y TEXT PRIMARY KEY constraint two,
-          z INTEGER,
-          UNIQUE(x,z) constraint three
+          x INTEGER CHECK( typeof(coalesce(x,0))=='integer' ) CONSTRAINT one
         );
     ]], {
         -- <check-2.10>
-
+        1, "near \")\": syntax error"
         -- </check-2.10>
     })
 
 test:do_catchsql_test(
     "check-2.11",
-    [[
-        INSERT INTO t2b VALUES('xyzzy','hi',5);
-    ]], {
-        -- <check-2.11>
-        1, "CHECK constraint failed: T2B"
-        -- </check-2.11>
-    })
-
-test:do_execsql_test(
-    "check-2.12",
     [[
         CREATE TABLE t2c(
-          x INTEGER CONSTRAINT x_one CONSTRAINT x_two primary key
-              CHECK( typeof(coalesce(x,0))=='integer' )
-              CONSTRAINT x_two CONSTRAINT x_three,
-          y INTEGER, z INTEGER,
-          CONSTRAINT u_one UNIQUE(x,y,z) CONSTRAINT u_two
+          x INTEGER CONSTRAINT one CHECK( typeof(coalesce(x,0))=='integer' )
+        CONSTRAINT two
         );
     ]], {
-        -- <check-2.12>
-
-        -- </check-2.12>
-    })
-
-test:do_catchsql_test(
-    "check-2.13",
-    [[
-        INSERT INTO t2c VALUES('xyzzy',7,8);
-    ]], {
-        -- <check-2.13>
-        1, "CHECK constraint failed: X_TWO"
-        -- </check-2.13>
+        -- <check-2.10>
+        1, "near \")\": syntax error"
+        -- </check-2.10>
     })
 
 test:do_execsql_test(
diff --git a/test/sql-tap/table.test.lua b/test/sql-tap/table.test.lua
index 8367ec016..0d70187ba 100755
--- a/test/sql-tap/table.test.lua
+++ b/test/sql-tap/table.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(57)
+test:plan(70)
 
 --!./tcltestrunner.lua
 -- 2001 September 15
@@ -1180,4 +1180,171 @@ test:do_test(
 
     -- </table-15.1>
 })
+
+-- gh-3504 Check the possibility appear constraints along with columns
+-- definitions.
+
+test:do_execsql_test(
+    "table-21.1",
+    [[
+        CREATE TABLE t21(
+           a integer,
+           primary key (a),
+           b integer,
+           check (b > 0),
+           c integer
+           check (c > 0)
+        );
+    ]], {
+        -- <table-21.1>
+
+        -- </table-21.1>
+    })
+
+test:do_catchsql_test(
+    "table-21.2",
+    [[
+        INSERT INTO t21 VALUES(1, 1, 1);
+        INSERT INTO t21 VALUES(1, 2, 2);
+    ]], {
+        -- <table-21.2>
+        1, "Duplicate key exists in unique index 'pk_unnamed_T21_1' in space 'T21'"
+        -- </table-21.2>
+    })
+
+test:do_catchsql_test(
+    "table-21.3",
+    [[
+        INSERT INTO t21 VALUES(1, -1, 1);
+    ]], {
+        -- <table-21.3>
+        1, "CHECK constraint failed: T21"
+        -- </table-21.3>
+    })
+
+test:do_catchsql_test(
+    "table-21.4",
+    [[
+        INSERT INTO t21 VALUES(1, 1, -1);
+    ]], {
+        -- <table-21.4>
+        1, "CHECK constraint failed: T21"
+        -- </table-21.4>
+    })
+
+test:do_execsql_test(
+    "check-21.cleanup",
+    [[
+        DROP TABLE IF EXISTS t21;
+    ]], {
+        -- <check-21.cleanup>
+
+        -- </check-21.cleanup>
+    })
+
+-- gh-3504: Check the CONSTRAINT name clause can't follow a constraint
+-- only before and once or missing.
+
+test:do_catchsql_test(
+    "table-22.1",
+    [[
+        CREATE TABLE t22(
+           a integer,
+           primary key (a) constraint one
+        );
+    ]], {
+        -- <table-22.1>
+        1,"keyword \"constraint\" is reserved"
+        -- </table-22.1>
+    })
+
+test:do_execsql_test(
+    "table-22.2",
+    [[
+        CREATE TABLE t22(
+           a integer primary key,
+           b integer,
+           constraint one unique (b),
+           c integer
+        );
+    ]], {
+        -- <table-22.2>
+
+        -- </table-22.2>
+    })
+
+test:do_catchsql_test(
+    "table-22.3",
+    [[
+        INSERT INTO t22 VALUES(1, 1, 1);
+        INSERT INTO t22 VALUES(2, 1, 1);
+    ]], {
+        -- <table-22.3>
+        1,"Duplicate key exists in unique index 'unique_ONE_2' in space 'T22'"
+        -- </table-22.3>
+    })
+
+test:do_execsql_test(
+    "table-22.4",
+    [[
+        CREATE TABLE t24(
+           a integer primary key,
+           b integer constraint two unique,
+           c integer
+        );
+    ]], {
+        -- <table-22.4>
+
+        -- </table-22.4>
+    })
+
+test:do_catchsql_test(
+    "table-22.5",
+    [[
+        INSERT INTO t24 VALUES(1, 1, 1);
+        INSERT INTO t24 VALUES(2, 1, 1);
+    ]], {
+        -- <table-21.5>
+        1, "Duplicate key exists in unique index 'unique_TWO_2' in space 'T24'"
+        -- </table-22.5>
+    })
+
+test:do_catchsql_test(
+    "table-22.6",
+    [[
+        CREATE TABLE t26(
+           a integer primary key,
+           b integer constraint one constraint one unique,
+           c integer
+        );
+    ]], {
+        -- <table-22.6>
+        1,"keyword \"constraint\" is reserved"
+        -- </table-22.6>
+    })
+
+test:do_catchsql_test(
+    "table-22.7",
+    [[
+        CREATE TABLE t27(
+           a integer primary key,
+           b integer constraint one constraint two unique,
+           c integer
+        );
+    ]], {
+        -- <table-22.7>
+        1,"keyword \"constraint\" is reserved"
+        -- </table-22.7>
+    })
+
+test:do_execsql_test(
+    "check-22.cleanup",
+    [[
+        DROP TABLE IF EXISTS t22;
+        DROP TABLE IF EXISTS t24;
+    ]], {
+        -- <check-22.cleanup>
+
+        -- </check-22.cleanup>
+    })
 test:finish_test()

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

* [tarantool-patches] Re: [PATCH] sql: constraints definition among columns in CREATE TABLE()
  2018-11-21  5:04   ` roman.habibov1
@ 2018-11-21 11:01     ` Vladislav Shpilevoy
  2018-11-21 19:29     ` n.pettik
  1 sibling, 0 replies; 7+ messages in thread
From: Vladislav Shpilevoy @ 2018-11-21 11:01 UTC (permalink / raw)
  To: roman.habibov1, tarantool-patches, Nikita Pettik

Thanks for the fix! LGTM. Nikita, please, review.

On 21/11/2018 08:04, roman.habibov1@yandex.ru wrote:
> Hi! Thanks for review.
> 
>> Hi! Thanks for the patch! Please, fix one minor comment
>> below.
>> Please, add a test that I can not write multiple 'CONSTRAINT <name>'
>> in a row.
>>
>>>         -- </table-15.1>
>>>     })
> Done.
> 
> commit 5bcc130bcabaa1f8efe8b12af35be5f2a4a05e6e
> Author: Roman Khabibov <roman.habibov1@yandex.ru>
> Date:   Sun Nov 18 15:54:53 2018 +0300
> 
>      sql: constraints def among columns in CREATE TABLE()
>      
>      Allow constraints to appear along with columns definitions. Disallow typing
>      a constraint name without specifying the constraint and after.
>      
>      Closes #3504
> 
> diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
> index abaa73736..5b9553017 100644
> --- a/src/box/sql/parse.y
> +++ b/src/box/sql/parse.y
> @@ -176,15 +176,17 @@ createkw(A) ::= CREATE(A).  {disableLookaside(pParse);}
>   ifnotexists(A) ::= .              {A = 0;}
>   ifnotexists(A) ::= IF NOT EXISTS. {A = 1;}
>   
> -create_table_args ::= LP columnlist conslist_opt RP(E). {
> +create_table_args ::= LP columnlist RP(E). {
>     sqlite3EndTable(pParse,&E,0);
>   }
>   create_table_args ::= AS select(S). {
>     sqlite3EndTable(pParse,0,S);
>     sql_select_delete(pParse->db, S);
>   }
> +columnlist ::= columnlist COMMA tconsdef.
>   columnlist ::= columnlist COMMA columnname carglist.
>   columnlist ::= columnname carglist.
> +columnlist ::= tconsdef.
>   columnname(A) ::= nm(A) typedef(Y). {sqlite3AddColumn(pParse,&A,&Y);}
>   
>   // An IDENTIFIER can be a generic identifier, or one of several
> @@ -232,9 +234,11 @@ nm(A) ::= id(A). {
>   // "carglist" is a list of additional constraints that come after the
>   // column name and column type in a CREATE TABLE statement.
>   //
> -carglist ::= carglist ccons.
> +carglist ::= carglist cconsdef.
>   carglist ::= .
> -ccons ::= CONSTRAINT nm(X).           {pParse->constraintName = X;}
> +cconsdef ::= cconsname ccons.
> +cconsname ::= CONSTRAINT nm(X).           {pParse->constraintName = X;}
> +cconsname ::= .                           {pParse->constraintName.n = 0;}
>   ccons ::= DEFAULT term(X).            {sqlite3AddDefaultValue(pParse,&X);}
>   ccons ::= DEFAULT LP expr(X) RP.      {sqlite3AddDefaultValue(pParse,&X);}
>   ccons ::= DEFAULT PLUS term(X).       {sqlite3AddDefaultValue(pParse,&X);}
> @@ -303,13 +307,9 @@ init_deferred_pred_opt(A) ::= .                       {A = 0;}
>   init_deferred_pred_opt(A) ::= INITIALLY DEFERRED.     {A = 1;}
>   init_deferred_pred_opt(A) ::= INITIALLY IMMEDIATE.    {A = 0;}
>   
> -conslist_opt ::= .
> -conslist_opt ::= COMMA conslist.
> -conslist ::= conslist tconscomma tcons.
> -conslist ::= tcons.
> -tconscomma ::= COMMA.            {pParse->constraintName.n = 0;}
> -tconscomma ::= .
> -tcons ::= CONSTRAINT nm(X).      {pParse->constraintName = X;}
> +tconsdef ::= tconsname tcons.
> +tconsname ::= CONSTRAINT nm(X).      {pParse->constraintName = X;}
> +tconsname ::= .                      {pParse->constraintName.n = 0;}
>   tcons ::= PRIMARY KEY LP sortlist(X) autoinc(I) RP.
>                                    {sqlite3AddPrimaryKey(pParse,X,I,0);}
>   tcons ::= UNIQUE LP sortlist(X) RP.
> diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua
> index 039e2291e..ebdbc5b13 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(60)
> +test:plan(58)
>   
>   --!./tcltestrunner.lua
>   -- 2005 November 2
> @@ -270,59 +270,31 @@ test:do_catchsql_test(
>           -- </check-2.6>
>       })
>   
> --- Undocumented behavior:  The CONSTRAINT name clause can follow a constraint.
> --- Such a clause is ignored.  But the parser must accept it for backwards
> --- compatibility.
> ---
> -test:do_execsql_test(
> +-- gh-3504: Check the CONSTRAINT name clause can't follow a constraint.
> +
> +test:do_catchsql_test(
>       "check-2.10",
>       [[
>           CREATE TABLE t2b(
> -          x INTEGER CHECK( typeof(coalesce(x,0))=='integer' ) CONSTRAINT one,
> -          y TEXT PRIMARY KEY constraint two,
> -          z INTEGER,
> -          UNIQUE(x,z) constraint three
> +          x INTEGER CHECK( typeof(coalesce(x,0))=='integer' ) CONSTRAINT one
>           );
>       ]], {
>           -- <check-2.10>
> -
> +        1, "near \")\": syntax error"
>           -- </check-2.10>
>       })
>   
>   test:do_catchsql_test(
>       "check-2.11",
> -    [[
> -        INSERT INTO t2b VALUES('xyzzy','hi',5);
> -    ]], {
> -        -- <check-2.11>
> -        1, "CHECK constraint failed: T2B"
> -        -- </check-2.11>
> -    })
> -
> -test:do_execsql_test(
> -    "check-2.12",
>       [[
>           CREATE TABLE t2c(
> -          x INTEGER CONSTRAINT x_one CONSTRAINT x_two primary key
> -              CHECK( typeof(coalesce(x,0))=='integer' )
> -              CONSTRAINT x_two CONSTRAINT x_three,
> -          y INTEGER, z INTEGER,
> -          CONSTRAINT u_one UNIQUE(x,y,z) CONSTRAINT u_two
> +          x INTEGER CONSTRAINT one CHECK( typeof(coalesce(x,0))=='integer' )
> +        CONSTRAINT two
>           );
>       ]], {
> -        -- <check-2.12>
> -
> -        -- </check-2.12>
> -    })
> -
> -test:do_catchsql_test(
> -    "check-2.13",
> -    [[
> -        INSERT INTO t2c VALUES('xyzzy',7,8);
> -    ]], {
> -        -- <check-2.13>
> -        1, "CHECK constraint failed: X_TWO"
> -        -- </check-2.13>
> +        -- <check-2.10>
> +        1, "near \")\": syntax error"
> +        -- </check-2.10>
>       })
>   
>   test:do_execsql_test(
> diff --git a/test/sql-tap/table.test.lua b/test/sql-tap/table.test.lua
> index 8367ec016..0d70187ba 100755
> --- a/test/sql-tap/table.test.lua
> +++ b/test/sql-tap/table.test.lua
> @@ -1,6 +1,6 @@
>   #!/usr/bin/env tarantool
>   test = require("sqltester")
> -test:plan(57)
> +test:plan(70)
>   
>   --!./tcltestrunner.lua
>   -- 2001 September 15
> @@ -1180,4 +1180,171 @@ test:do_test(
>   
>       -- </table-15.1>
>   })
> +
> +-- gh-3504 Check the possibility appear constraints along with columns
> +-- definitions.
> +
> +test:do_execsql_test(
> +    "table-21.1",
> +    [[
> +        CREATE TABLE t21(
> +           a integer,
> +           primary key (a),
> +           b integer,
> +           check (b > 0),
> +           c integer
> +           check (c > 0)
> +        );
> +    ]], {
> +        -- <table-21.1>
> +
> +        -- </table-21.1>
> +    })
> +
> +test:do_catchsql_test(
> +    "table-21.2",
> +    [[
> +        INSERT INTO t21 VALUES(1, 1, 1);
> +        INSERT INTO t21 VALUES(1, 2, 2);
> +    ]], {
> +        -- <table-21.2>
> +        1, "Duplicate key exists in unique index 'pk_unnamed_T21_1' in space 'T21'"
> +        -- </table-21.2>
> +    })
> +
> +test:do_catchsql_test(
> +    "table-21.3",
> +    [[
> +        INSERT INTO t21 VALUES(1, -1, 1);
> +    ]], {
> +        -- <table-21.3>
> +        1, "CHECK constraint failed: T21"
> +        -- </table-21.3>
> +    })
> +
> +test:do_catchsql_test(
> +    "table-21.4",
> +    [[
> +        INSERT INTO t21 VALUES(1, 1, -1);
> +    ]], {
> +        -- <table-21.4>
> +        1, "CHECK constraint failed: T21"
> +        -- </table-21.4>
> +    })
> +
> +test:do_execsql_test(
> +    "check-21.cleanup",
> +    [[
> +        DROP TABLE IF EXISTS t21;
> +    ]], {
> +        -- <check-21.cleanup>
> +
> +        -- </check-21.cleanup>
> +    })
> +
> +-- gh-3504: Check the CONSTRAINT name clause can't follow a constraint
> +-- only before and once or missing.
> +
> +test:do_catchsql_test(
> +    "table-22.1",
> +    [[
> +        CREATE TABLE t22(
> +           a integer,
> +           primary key (a) constraint one
> +        );
> +    ]], {
> +        -- <table-22.1>
> +        1,"keyword \"constraint\" is reserved"
> +        -- </table-22.1>
> +    })
> +
> +test:do_execsql_test(
> +    "table-22.2",
> +    [[
> +        CREATE TABLE t22(
> +           a integer primary key,
> +           b integer,
> +           constraint one unique (b),
> +           c integer
> +        );
> +    ]], {
> +        -- <table-22.2>
> +
> +        -- </table-22.2>
> +    })
> +
> +test:do_catchsql_test(
> +    "table-22.3",
> +    [[
> +        INSERT INTO t22 VALUES(1, 1, 1);
> +        INSERT INTO t22 VALUES(2, 1, 1);
> +    ]], {
> +        -- <table-22.3>
> +        1,"Duplicate key exists in unique index 'unique_ONE_2' in space 'T22'"
> +        -- </table-22.3>
> +    })
> +
> +test:do_execsql_test(
> +    "table-22.4",
> +    [[
> +        CREATE TABLE t24(
> +           a integer primary key,
> +           b integer constraint two unique,
> +           c integer
> +        );
> +    ]], {
> +        -- <table-22.4>
> +
> +        -- </table-22.4>
> +    })
> +
> +test:do_catchsql_test(
> +    "table-22.5",
> +    [[
> +        INSERT INTO t24 VALUES(1, 1, 1);
> +        INSERT INTO t24 VALUES(2, 1, 1);
> +    ]], {
> +        -- <table-21.5>
> +        1, "Duplicate key exists in unique index 'unique_TWO_2' in space 'T24'"
> +        -- </table-22.5>
> +    })
> +
> +test:do_catchsql_test(
> +    "table-22.6",
> +    [[
> +        CREATE TABLE t26(
> +           a integer primary key,
> +           b integer constraint one constraint one unique,
> +           c integer
> +        );
> +    ]], {
> +        -- <table-22.6>
> +        1,"keyword \"constraint\" is reserved"
> +        -- </table-22.6>
> +    })
> +
> +test:do_catchsql_test(
> +    "table-22.7",
> +    [[
> +        CREATE TABLE t27(
> +           a integer primary key,
> +           b integer constraint one constraint two unique,
> +           c integer
> +        );
> +    ]], {
> +        -- <table-22.7>
> +        1,"keyword \"constraint\" is reserved"
> +        -- </table-22.7>
> +    })
> +
> +test:do_execsql_test(
> +    "check-22.cleanup",
> +    [[
> +        DROP TABLE IF EXISTS t22;
> +        DROP TABLE IF EXISTS t24;
> +    ]], {
> +        -- <check-22.cleanup>
> +
> +        -- </check-22.cleanup>
> +    })
>   test:finish_test()
> 

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

* [tarantool-patches] Re: [PATCH] sql: constraints definition among columns in CREATE TABLE()
  2018-11-21  5:04   ` roman.habibov1
  2018-11-21 11:01     ` Vladislav Shpilevoy
@ 2018-11-21 19:29     ` n.pettik
  2018-11-22  0:23       ` roman.habibov1
  1 sibling, 1 reply; 7+ messages in thread
From: n.pettik @ 2018-11-21 19:29 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Vladislav Shpilevoy, roman.habibov1


> commit 5bcc130bcabaa1f8efe8b12af35be5f2a4a05e6e
> Author: Roman Khabibov <roman.habibov1@yandex.ru>
> Date:   Sun Nov 18 15:54:53 2018 +0300
> 
>    sql: constraints def among columns in CREATE TABLE()

Commit message lacks a verb. I would call it sort of:

sql: allow appearing constraint definition among columns

>    Allow constraints to appear along with columns definitions. Disallow typing
>    a constraint name without specifying the constraint and after.

Please, re-phrase last sentence, it is really hard to understand what does it mean:
after what?

You can fix several nitpickings below, but even with them patch is OK.

> diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua
> index 039e2291e..ebdbc5b13 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(60)
> +test:plan(58)
> 
> --!./tcltestrunner.lua
> -- 2005 November 2
> @@ -270,59 +270,31 @@ test:do_catchsql_test(
>         -- </check-2.6>
>     })
> 
> -test:do_execsql_test(
> -    "check-2.12",
>     [[
>         CREATE TABLE t2c(
> -          x INTEGER CONSTRAINT x_one CONSTRAINT x_two primary key
> -              CHECK( typeof(coalesce(x,0))=='integer' )
> -              CONSTRAINT x_two CONSTRAINT x_three,
> -          y INTEGER, z INTEGER,
> -          CONSTRAINT u_one UNIQUE(x,y,z) CONSTRAINT u_two
> +          x INTEGER CONSTRAINT one CHECK( typeof(coalesce(x,0))=='integer' )
> +        CONSTRAINT two

This test is copy of previous one. To diversify cases you can check
another type of constraint. For example:

CREATE TABLE t (id INT PRIMARY KEY CONSTRAINT PK);

Also, both your examples fail even without your patch since table
lacks primary key (yes, error message is different, but lets make
only one fail in this test).

> test:do_execsql_test(
> diff --git a/test/sql-tap/table.test.lua b/test/sql-tap/table.test.lua
> index 8367ec016..0d70187ba 100755
> --- a/test/sql-tap/table.test.lua
> +++ b/test/sql-tap/table.test.lua
> @@ -1,6 +1,6 @@
> #!/usr/bin/env tarantool
> test = require("sqltester")
> -test:plan(57)
> +test:plan(70)
> 
> --!./tcltestrunner.lua
> -- 2001 September 15
> @@ -1180,4 +1180,171 @@ test:do_test(
> 
>     -- </table-15.1>
> })
> +
> +-- gh-3504 Check the possibility appear constraints along with columns
> +-- definitions.

Re-phrase like: “Constraints definition can appear among columns ones."

> +
> +test:do_execsql_test(
> +    "table-21.1",
> +    [[
> +        CREATE TABLE t21(
> +           a integer,
> +           primary key (a),
> +           b integer,
> +           check (b > 0),
> +           c integer
> +           check (c > 0)
> +        );

It would be better to use uppercase for SQL keywords.

> +    ]], {
> +        -- <table-21.1>
> +
> +        -- </table-21.1>
> +    })
> +
> +test:do_catchsql_test(
> +    "table-21.2",
> +    [[
> +        INSERT INTO t21 VALUES(1, 1, 1);
> +        INSERT INTO t21 VALUES(1, 2, 2);
> +    ]], {
> +        -- <table-21.2>
> +        1, "Duplicate key exists in unique index 'pk_unnamed_T21_1' in space 'T21'"
> +        -- </table-21.2>
> +    })
> +
> +test:do_catchsql_test(
> +    "table-21.3",
> +    [[
> +        INSERT INTO t21 VALUES(1, -1, 1);
> +    ]], {
> +        -- <table-21.3>
> +        1, "CHECK constraint failed: T21"
> +        -- </table-21.3>
> +    })
> +
> +test:do_catchsql_test(
> +    "table-21.4",
> +    [[
> +        INSERT INTO t21 VALUES(1, 1, -1);
> +    ]], {
> +        -- <table-21.4>
> +        1, "CHECK constraint failed: T21"
> +        -- </table-21.4>
> +    })
> +
> +test:do_execsql_test(
> +    "check-21.cleanup",
> +    [[
> +        DROP TABLE IF EXISTS t21;
> +    ]], {
> +        -- <check-21.cleanup>
> +
> +        -- </check-21.cleanup>
> +    })
> +
> +-- gh-3504: Check the CONSTRAINT name clause can't follow a constraint
> +-- only before and once or missing.

Can’t parse this sentence as well. Re-phrase it pls.

> +
> +test:do_catchsql_test(
> +    "table-22.1",
> +    [[
> +        CREATE TABLE t22(
> +           a integer,
> +           primary key (a) constraint one
> +        );
> +    ]], {
> +        -- <table-22.1>
> +        1,"keyword \"constraint\" is reserved"
> +        -- </table-22.1>
> +    })
> +
> +test:do_execsql_test(
> +    "table-22.2",
> +    [[
> +        CREATE TABLE t22(
> +           a integer primary key,
> +           b integer,
> +           constraint one unique (b),
> +           c integer
> +        );
> +    ]], {

Also, I would add test where several constraint definitions
come one after another, like this:

CREATE TABLE t (id INT, PRIMARY KEY (id), CONSTRAINT pk1 CHECK(id != 0), CONSTRAINT pk2 CHECK(id > 10));

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

* [tarantool-patches] Re: [PATCH] sql: constraints definition among columns in CREATE TABLE()
  2018-11-21 19:29     ` n.pettik
@ 2018-11-22  0:23       ` roman.habibov1
  0 siblings, 0 replies; 7+ messages in thread
From: roman.habibov1 @ 2018-11-22  0:23 UTC (permalink / raw)
  To: n.pettik, tarantool-patches; +Cc: Vladislav Shpilevoy


Hi! Thanks for review.

> Commit message lacks a verb. I would call it sort of:
>
> sql: allow appearing constraint definition among columns
Done.

>>     Allow constraints to appear along with columns definitions. Disallow typing
>>     a constraint name without specifying the constraint and after.
>
> Please, re-phrase last sentence, it is really hard to understand what does it mean:
> after what?
Done.

> You can fix several nitpickings below, but even with them patch is OK.
>
>>  diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua
>>  index 039e2291e..ebdbc5b13 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(60)
>>  +test:plan(58)
>>
>>  --!./tcltestrunner.lua
>>  -- 2005 November 2
>>  @@ -270,59 +270,31 @@ test:do_catchsql_test(
>>          -- </check-2.6>
>>      })
>>
>>  -test:do_execsql_test(
>>  - "check-2.12",
>>      [[
>>          CREATE TABLE t2c(
>>  - x INTEGER CONSTRAINT x_one CONSTRAINT x_two primary key
>>  - CHECK( typeof(coalesce(x,0))=='integer' )
>>  - CONSTRAINT x_two CONSTRAINT x_three,
>>  - y INTEGER, z INTEGER,
>>  - CONSTRAINT u_one UNIQUE(x,y,z) CONSTRAINT u_two
>>  + x INTEGER CONSTRAINT one CHECK( typeof(coalesce(x,0))=='integer' )
>>  + CONSTRAINT two
>
> This test is copy of previous one. To diversify cases you can check
> another type of constraint. For example:
>
> CREATE TABLE t (id INT PRIMARY KEY CONSTRAINT PK);
>
> Also, both your examples fail even without your patch since table
> lacks primary key (yes, error message is different, but lets make
> only one fail in this test).
Added primary keys. I thought this file about CHECK CONSTRAINT.

>>  test:do_execsql_test(
>>  diff --git a/test/sql-tap/table.test.lua b/test/sql-tap/table.test.lua
>>  index 8367ec016..0d70187ba 100755
>>  --- a/test/sql-tap/table.test.lua
>>  +++ b/test/sql-tap/table.test.lua
>>  @@ -1,6 +1,6 @@
>>  #!/usr/bin/env tarantool
>>  test = require("sqltester")
>>  -test:plan(57)
>>  +test:plan(70)
>>
>>  --!./tcltestrunner.lua
>>  -- 2001 September 15
>>  @@ -1180,4 +1180,171 @@ test:do_test(
>>
>>      -- </table-15.1>
>>  })
>>  +
>>  +-- gh-3504 Check the possibility appear constraints along with columns
>>  +-- definitions.
>
> Re-phrase like: “Constraints definition can appear among columns ones."
Done.

>>  +
>>  +test:do_execsql_test(
>>  + "table-21.1",
>>  + [[
>>  + CREATE TABLE t21(
>>  + a integer,
>>  + primary key (a),
>>  + b integer,
>>  + check (b > 0),
>>  + c integer
>>  + check (c > 0)
>>  + );
>
> It would be better to use uppercase for SQL keywords.
Done.

>>  + ]], {
>>  + -- <table-21.1>
>>  +
>>  + -- </table-21.1>
>>  + })
>>  +
>>  +test:do_catchsql_test(
>>  + "table-21.2",
>>  + [[
>>  + INSERT INTO t21 VALUES(1, 1, 1);
>>  + INSERT INTO t21 VALUES(1, 2, 2);
>>  + ]], {
>>  + -- <table-21.2>
>>  + 1, "Duplicate key exists in unique index 'pk_unnamed_T21_1' in space 'T21'"
>>  + -- </table-21.2>
>>  + })
>>  +
>>  +test:do_catchsql_test(
>>  + "table-21.3",
>>  + [[
>>  + INSERT INTO t21 VALUES(1, -1, 1);
>>  + ]], {
>>  + -- <table-21.3>
>>  + 1, "CHECK constraint failed: T21"
>>  + -- </table-21.3>
>>  + })
>>  +
>>  +test:do_catchsql_test(
>>  + "table-21.4",
>>  + [[
>>  + INSERT INTO t21 VALUES(1, 1, -1);
>>  + ]], {
>>  + -- <table-21.4>
>>  + 1, "CHECK constraint failed: T21"
>>  + -- </table-21.4>
>>  + })
>>  +
>>  +test:do_execsql_test(
>>  + "check-21.cleanup",
>>  + [[
>>  + DROP TABLE IF EXISTS t21;
>>  + ]], {
>>  + -- <check-21.cleanup>
>>  +
>>  + -- </check-21.cleanup>
>>  + })
>>  +
>>  +-- gh-3504: Check the CONSTRAINT name clause can't follow a constraint
>>  +-- only before and once or missing.
>
> Can’t parse this sentence as well. Re-phrase it pls.
+
+-- gh-3504: Check the CONSTRAINT name clause can't follow a constraint.
+-- The name may be typed once before the constraint or not.
+

>>  +
>>  +test:do_catchsql_test(
>>  + "table-22.1",
>>  + [[
>>  + CREATE TABLE t22(
>>  + a integer,
>>  + primary key (a) constraint one
>>  + );
>>  + ]], {
>>  + -- <table-22.1>
>>  + 1,"keyword \"constraint\" is reserved"
>>  + -- </table-22.1>
>>  + })
>>  +
>>  +test:do_execsql_test(
>>  + "table-22.2",
>>  + [[
>>  + CREATE TABLE t22(
>>  + a integer primary key,
>>  + b integer,
>>  + constraint one unique (b),
>>  + c integer
>>  + );
>>  + ]], {
>
> Also, I would add test where several constraint definitions
> come one after another, like this:
>
> CREATE TABLE t (id INT, PRIMARY KEY (id), CONSTRAINT pk1 CHECK(id != 0), CONSTRAINT pk2 CHECK(id > 10));
+test:do_execsql_test(
+    "table-22.8",
+    [[
+        CREATE TABLE T28(
+           id INT,
+           PRIMARY KEY (id),
+           CONSTRAINT check1 CHECK(id != 0),
+           CONSTRAINT check2 CHECK(id > 10)
+        );
+    ]], {
+        -- <table-22.8>
+
+        -- </table-22.8>
+    })
+
+test:do_catchsql_test(
+    "table-22.9",
+    [[
+        INSERT INTO T28 VALUES(11);
+        INSERT INTO T28 VALUES(11);
+    ]], {
+        -- <table-22.9>
+        1,"Duplicate key exists in unique index 'pk_unnamed_T28_1' in space 'T28'"
+        -- </table-22.9>
+    })
+
+test:do_catchsql_test(
+    "table-22.10",
+    [[
+        INSERT INTO T28 VALUES(0);
+    ]], {
+        -- <table-22.10>
+        1, "CHECK constraint failed: CHECK1"
+        -- </table-22.10>
+    })
+
+test:do_catchsql_test(
+    "table-22.11",
+    [[
+        INSERT INTO T28 VALUES(9);
+    ]], {
+        -- <table-22.11>
+        1, "CHECK constraint failed: CHECK2"
+        -- </table-22.11>
+    })
+

commit 0cd2b50906b21d37c017b5e8468e13afb7a7c65b
Author: Roman Khabibov <roman.habibov1@yandex.ru>
Date:   Sun Nov 18 15:54:53 2018 +0300

    sql: allow appearing constraint definition among columns
    
    Allow constraints to appear along with columns definitions. Disallow typing
    a constraint name without specifying the constraint and after it.
    
    Closes #3504

diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index abaa73736..5b9553017 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -176,15 +176,17 @@ createkw(A) ::= CREATE(A).  {disableLookaside(pParse);}
 ifnotexists(A) ::= .              {A = 0;}
 ifnotexists(A) ::= IF NOT EXISTS. {A = 1;}
 
-create_table_args ::= LP columnlist conslist_opt RP(E). {
+create_table_args ::= LP columnlist RP(E). {
   sqlite3EndTable(pParse,&E,0);
 }
 create_table_args ::= AS select(S). {
   sqlite3EndTable(pParse,0,S);
   sql_select_delete(pParse->db, S);
 }
+columnlist ::= columnlist COMMA tconsdef.
 columnlist ::= columnlist COMMA columnname carglist.
 columnlist ::= columnname carglist.
+columnlist ::= tconsdef.
 columnname(A) ::= nm(A) typedef(Y). {sqlite3AddColumn(pParse,&A,&Y);}
 
 // An IDENTIFIER can be a generic identifier, or one of several
@@ -232,9 +234,11 @@ nm(A) ::= id(A). {
 // "carglist" is a list of additional constraints that come after the
 // column name and column type in a CREATE TABLE statement.
 //
-carglist ::= carglist ccons.
+carglist ::= carglist cconsdef.
 carglist ::= .
-ccons ::= CONSTRAINT nm(X).           {pParse->constraintName = X;}
+cconsdef ::= cconsname ccons.
+cconsname ::= CONSTRAINT nm(X).           {pParse->constraintName = X;}
+cconsname ::= .                           {pParse->constraintName.n = 0;}
 ccons ::= DEFAULT term(X).            {sqlite3AddDefaultValue(pParse,&X);}
 ccons ::= DEFAULT LP expr(X) RP.      {sqlite3AddDefaultValue(pParse,&X);}
 ccons ::= DEFAULT PLUS term(X).       {sqlite3AddDefaultValue(pParse,&X);}
@@ -303,13 +307,9 @@ init_deferred_pred_opt(A) ::= .                       {A = 0;}
 init_deferred_pred_opt(A) ::= INITIALLY DEFERRED.     {A = 1;}
 init_deferred_pred_opt(A) ::= INITIALLY IMMEDIATE.    {A = 0;}
 
-conslist_opt ::= .
-conslist_opt ::= COMMA conslist.
-conslist ::= conslist tconscomma tcons.
-conslist ::= tcons.
-tconscomma ::= COMMA.            {pParse->constraintName.n = 0;}
-tconscomma ::= .
-tcons ::= CONSTRAINT nm(X).      {pParse->constraintName = X;}
+tconsdef ::= tconsname tcons.
+tconsname ::= CONSTRAINT nm(X).      {pParse->constraintName = X;}
+tconsname ::= .                      {pParse->constraintName.n = 0;}
 tcons ::= PRIMARY KEY LP sortlist(X) autoinc(I) RP.
                                  {sqlite3AddPrimaryKey(pParse,X,I,0);}
 tcons ::= UNIQUE LP sortlist(X) RP.
diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua
index 039e2291e..c24ae2ff1 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(60)
+test:plan(58)
 
 --!./tcltestrunner.lua
 -- 2005 November 2
@@ -270,59 +270,33 @@ test:do_catchsql_test(
         -- </check-2.6>
     })
 
--- Undocumented behavior:  The CONSTRAINT name clause can follow a constraint.
--- Such a clause is ignored.  But the parser must accept it for backwards
--- compatibility.
---
-test:do_execsql_test(
+-- gh-3504: Check the CONSTRAINT name clause can't follow a constraint.
+
+test:do_catchsql_test(
     "check-2.10",
     [[
         CREATE TABLE t2b(
           x INTEGER CHECK( typeof(coalesce(x,0))=='integer' ) CONSTRAINT one,
-          y TEXT PRIMARY KEY constraint two,
-          z INTEGER,
-          UNIQUE(x,z) constraint three
+          PRIMARY KEY (x)
         );
     ]], {
         -- <check-2.10>
-
+        1,"near \",\": syntax error"
         -- </check-2.10>
     })
 
 test:do_catchsql_test(
     "check-2.11",
-    [[
-        INSERT INTO t2b VALUES('xyzzy','hi',5);
-    ]], {
-        -- <check-2.11>
-        1, "CHECK constraint failed: T2B"
-        -- </check-2.11>
-    })
-
-test:do_execsql_test(
-    "check-2.12",
     [[
         CREATE TABLE t2c(
-          x INTEGER CONSTRAINT x_one CONSTRAINT x_two primary key
-              CHECK( typeof(coalesce(x,0))=='integer' )
-              CONSTRAINT x_two CONSTRAINT x_three,
-          y INTEGER, z INTEGER,
-          CONSTRAINT u_one UNIQUE(x,y,z) CONSTRAINT u_two
+          x INTEGER CONSTRAINT one CHECK( typeof(coalesce(x,0))=='integer' )
+        CONSTRAINT two,
+          PRIMARY KEY (x)
         );
     ]], {
-        -- <check-2.12>
-
-        -- </check-2.12>
-    })
-
-test:do_catchsql_test(
-    "check-2.13",
-    [[
-        INSERT INTO t2c VALUES('xyzzy',7,8);
-    ]], {
-        -- <check-2.13>
-        1, "CHECK constraint failed: X_TWO"
-        -- </check-2.13>
+        -- <check-2.10>
+        1,"near \",\": syntax error"
+        -- </check-2.10>
     })
 
 test:do_execsql_test(
diff --git a/test/sql-tap/table.test.lua b/test/sql-tap/table.test.lua
index 8367ec016..7fd9bac9f 100755
--- a/test/sql-tap/table.test.lua
+++ b/test/sql-tap/table.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(57)
+test:plan(74)
 
 --!./tcltestrunner.lua
 -- 2001 September 15
@@ -1180,4 +1180,217 @@ test:do_test(
 
     -- </table-15.1>
 })
+
+-- gh-3504 Constraints definition can appear among columns ones.
+
+test:do_execsql_test(
+    "table-21.1",
+    [[
+        CREATE TABLE t21(
+           A INTEGER,
+           PRIMARY KEY (A),
+           B INTEGER,
+           CHECK (B > 0),
+           C INTEGER
+           CHECK (C > 0)
+        );
+    ]], {
+        -- <table-21.1>
+
+        -- </table-21.1>
+    })
+
+test:do_catchsql_test(
+    "table-21.2",
+    [[
+        INSERT INTO T21 VALUES(1, 1, 1);
+        INSERT INTO T21 VALUES(1, 2, 2);
+    ]], {
+        -- <table-21.2>
+        1, "Duplicate key exists in unique index 'pk_unnamed_T21_1' in space 'T21'"
+        -- </table-21.2>
+    })
+
+test:do_catchsql_test(
+    "table-21.3",
+    [[
+        INSERT INTO T21 VALUES(1, -1, 1);
+    ]], {
+        -- <table-21.3>
+        1, "CHECK constraint failed: T21"
+        -- </table-21.3>
+    })
+
+test:do_catchsql_test(
+    "table-21.4",
+    [[
+        INSERT INTO T21 VALUES(1, 1, -1);
+    ]], {
+        -- <table-21.4>
+        1, "CHECK constraint failed: T21"
+        -- </table-21.4>
+    })
+
+test:do_execsql_test(
+    "check-21.cleanup",
+    [[
+        DROP TABLE IF EXISTS T21;
+    ]], {
+        -- <check-21.cleanup>
+
+        -- </check-21.cleanup>
+    })
+
+-- gh-3504: Check the CONSTRAINT name clause can't follow a constraint.
+-- The name may be typed once before the constraint or not.
+
+test:do_catchsql_test(
+    "table-22.1",
+    [[
+        CREATE TABLE T22(
+           A INTEGER,
+           PRIMARY KEY (A) CONSTRAINT ONE
+        );
+    ]], {
+        -- <table-22.1>
+        1,"keyword \"CONSTRAINT\" is reserved"
+        -- </table-22.1>
+    })
+
+test:do_execsql_test(
+    "table-22.2",
+    [[
+        CREATE TABLE T22(
+           A INTEGER PRIMARY KEY,
+           B INTEGER,
+           CONSTRAINT ONE UNIQUE (B),
+           C INTEGER
+        );
+    ]], {
+        -- <table-22.2>
+
+        -- </table-22.2>
+    })
+
+test:do_catchsql_test(
+    "table-22.3",
+    [[
+        INSERT INTO T22 VALUES(1, 1, 1);
+        INSERT INTO T22 VALUES(2, 1, 1);
+    ]], {
+        -- <table-22.3>
+        1,"Duplicate key exists in unique index 'unique_ONE_2' in space 'T22'"
+        -- </table-22.3>
+    })
+
+test:do_execsql_test(
+    "table-22.4",
+    [[
+        CREATE TABLE T24(
+           A INTEGER PRIMARY KEY,
+           B INTEGER CONSTRAINT TWO UNIQUE,
+           C INTEGER
+        );
+    ]], {
+        -- <table-22.4>
+
+        -- </table-22.4>
+    })
+
+test:do_catchsql_test(
+    "table-22.5",
+    [[
+        INSERT INTO T24 VALUES(1, 1, 1);
+        INSERT INTO T24 VALUES(2, 1, 1);
+    ]], {
+        -- <table-22.5>
+        1, "Duplicate key exists in unique index 'unique_TWO_2' in space 'T24'"
+        -- </table-22.5>
+    })
+
+test:do_catchsql_test(
+    "table-22.6",
+    [[
+        CREATE TABLE T26(
+           A INTEGER PRIMARY KEY,
+           B INTEGER CONSTRAINT ONE CONSTRAINT ONE UNIQUE,
+           C INTEGER
+        );
+    ]], {
+        -- <table-22.6>
+        1,"keyword \"CONSTRAINT\" is reserved"
+        -- </table-22.6>
+    })
+
+test:do_catchsql_test(
+    "table-22.7",
+    [[
+        CREATE TABLE T27(
+           A INTEGER PRIMARY KEY,
+           B INTEGER CONSTRAINT ONE CONSTRAINT TWO UNIQUE,
+           C INTEGER
+        );
+    ]], {
+        -- <table-22.7>
+        1,"keyword \"CONSTRAINT\" is reserved"
+        -- </table-22.7>
+    })
+
+test:do_execsql_test(
+    "table-22.8",
+    [[
+        CREATE TABLE T28(
+           id INT,
+           PRIMARY KEY (id),
+           CONSTRAINT check1 CHECK(id != 0),
+           CONSTRAINT check2 CHECK(id > 10)
+        );
+    ]], {
+        -- <table-22.8>
+
+        -- </table-22.8>
+    })
+
+test:do_catchsql_test(
+    "table-22.9",
+    [[
+        INSERT INTO T28 VALUES(11);
+        INSERT INTO T28 VALUES(11);
+    ]], {
+        -- <table-22.9>
+        1,"Duplicate key exists in unique index 'pk_unnamed_T28_1' in space 'T28'"
+        -- </table-22.9>
+    })
+
+test:do_catchsql_test(
+    "table-22.10",
+    [[
+        INSERT INTO T28 VALUES(0);
+    ]], {
+        -- <table-22.10>
+        1, "CHECK constraint failed: CHECK1"
+        -- </table-22.10>
+    })
+
+test:do_catchsql_test(
+    "table-22.11",
+    [[
+        INSERT INTO T28 VALUES(9);
+    ]], {
+        -- <table-22.11>
+        1, "CHECK constraint failed: CHECK2"
+        -- </table-22.11>
+    })
+
+test:do_execsql_test(
+    "check-22.cleanup",
+    [[
+        DROP TABLE IF EXISTS t22;
+        DROP TABLE IF EXISTS t24;
+        DROP TABLE IF EXISTS t28;
+    ]], {
+        -- <check-22.cleanup>
+
+        -- </check-22.cleanup>
+    })
 test:finish_test()

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

* [tarantool-patches] Re: [PATCH] sql: constraints definition among columns in CREATE TABLE()
  2018-11-18 14:31 [tarantool-patches] [PATCH] sql: constraints definition among columns in CREATE TABLE() Roman Khabibov
  2018-11-19 12:41 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2018-11-26 10:33 ` Kirill Yukhin
  1 sibling, 0 replies; 7+ messages in thread
From: Kirill Yukhin @ 2018-11-26 10:33 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy

Hello,
On 18 Nov 17:31, Roman Khabibov wrote:
> Allow constraints to appear along with columns definitions. Disallow typing
> a constraint name without specifying the constraint and after.
> 
> Closes #3504
> 
> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3504-constraints-create-table
> Issue: https://github.com/tarantool/tarantool/issues/3504
I've checked your patch into 2.1 branch.

--
Regards, Kirill Yukhin

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

end of thread, other threads:[~2018-11-26 10:33 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-18 14:31 [tarantool-patches] [PATCH] sql: constraints definition among columns in CREATE TABLE() Roman Khabibov
2018-11-19 12:41 ` [tarantool-patches] " Vladislav Shpilevoy
2018-11-21  5:04   ` roman.habibov1
2018-11-21 11:01     ` Vladislav Shpilevoy
2018-11-21 19:29     ` n.pettik
2018-11-22  0:23       ` roman.habibov1
2018-11-26 10:33 ` Kirill Yukhin

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