From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 1AA1B2F789 for ; Wed, 21 Nov 2018 14:29:08 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id E6sOMWiuRH4j for ; Wed, 21 Nov 2018 14:29:08 -0500 (EST) Received: from smtp58.i.mail.ru (smtp58.i.mail.ru [217.69.128.38]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id C50862EDC0 for ; Wed, 21 Nov 2018 14:29:07 -0500 (EST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.0 \(3445.100.39\)) Subject: [tarantool-patches] Re: [PATCH] sql: constraints definition among columns in CREATE TABLE() From: "n.pettik" In-Reply-To: <1653371542776687@myt6-add70abb4f02.qloud-c.yandex.net> Date: Wed, 21 Nov 2018 22:29:03 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <0E11C6B2-9BF0-411F-80F8-AA20A1DF63FB@tarantool.org> References: <20181118143151.24016-1-roman.habibov1@yandex.ru> <8d84eb1f-95c4-0043-abc7-dc19f663a0df@tarantool.org> <1653371542776687@myt6-add70abb4f02.qloud-c.yandex.net> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Vladislav Shpilevoy , roman.habibov1@yandex.ru > commit 5bcc130bcabaa1f8efe8b12af35be5f2a4a05e6e > Author: Roman Khabibov > Date: Sun Nov 18 15:54:53 2018 +0300 >=20 > 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 =3D require("sqltester") > -test:plan(60) > +test:plan(58) >=20 > --!./tcltestrunner.lua > -- 2005 November 2 > @@ -270,59 +270,31 @@ test:do_catchsql_test( > -- > }) >=20 > -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))=3D=3D'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))=3D=3D'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 =3D require("sqltester") > -test:plan(57) > +test:plan(70) >=20 > --!./tcltestrunner.lua > -- 2001 September 15 > @@ -1180,4 +1180,171 @@ test:do_test( >=20 > -- > }) > + > +-- gh-3504 Check the possibility appear constraints along with = columns > +-- definitions. Re-phrase like: =E2=80=9CConstraints 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. > + ]], { > + -- > + > + -- > + }) > + > +test:do_catchsql_test( > + "table-21.2", > + [[ > + INSERT INTO t21 VALUES(1, 1, 1); > + INSERT INTO t21 VALUES(1, 2, 2); > + ]], { > + -- > + 1, "Duplicate key exists in unique index 'pk_unnamed_T21_1' = in space 'T21'" > + -- > + }) > + > +test:do_catchsql_test( > + "table-21.3", > + [[ > + INSERT INTO t21 VALUES(1, -1, 1); > + ]], { > + -- > + 1, "CHECK constraint failed: T21" > + -- > + }) > + > +test:do_catchsql_test( > + "table-21.4", > + [[ > + INSERT INTO t21 VALUES(1, 1, -1); > + ]], { > + -- > + 1, "CHECK constraint failed: T21" > + -- > + }) > + > +test:do_execsql_test( > + "check-21.cleanup", > + [[ > + DROP TABLE IF EXISTS t21; > + ]], { > + -- > + > + -- > + }) > + > +-- gh-3504: Check the CONSTRAINT name clause can't follow a = constraint > +-- only before and once or missing. Can=E2=80=99t 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 > + ); > + ]], { > + -- > + 1,"keyword \"constraint\" is reserved" > + -- > + }) > + > +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 !=3D = 0), CONSTRAINT pk2 CHECK(id > 10));