[tarantool-patches] Re: [PATCH] sql: constraints definition among columns in CREATE TABLE()

["n.pettik" <korablev@tarantool.org>]

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