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 2E6A52F4FF for ; Wed, 21 Nov 2018 06:01:29 -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 KN7usuyES7C1 for ; Wed, 21 Nov 2018 06:01:29 -0500 (EST) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 BB70E2F495 for ; Wed, 21 Nov 2018 06:01:28 -0500 (EST) Subject: [tarantool-patches] Re: [PATCH] sql: constraints definition among columns in CREATE TABLE() References: <20181118143151.24016-1-roman.habibov1@yandex.ru> <8d84eb1f-95c4-0043-abc7-dc19f663a0df@tarantool.org> <1653371542776687@myt6-add70abb4f02.qloud-c.yandex.net> From: Vladislav Shpilevoy Message-ID: <99ea3d29-8701-f76b-b28b-4ac7a4ed2421@tarantool.org> Date: Wed, 21 Nov 2018 14:01:25 +0300 MIME-Version: 1.0 In-Reply-To: <1653371542776687@myt6-add70abb4f02.qloud-c.yandex.net> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Language: en-US Content-Transfer-Encoding: 8bit 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: roman.habibov1@yandex.ru, "tarantool-patches@freelists.org" , 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 ' >> in a row. >> >>>        -- >>>    }) > Done. > > commit 5bcc130bcabaa1f8efe8b12af35be5f2a4a05e6e > Author: Roman Khabibov > 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( > -- > }) > > --- 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 > ); > ]], { > -- > - > + 1, "near \")\": syntax error" > -- > }) > > test:do_catchsql_test( > "check-2.11", > - [[ > - INSERT INTO t2b VALUES('xyzzy','hi',5); > - ]], { > - -- > - 1, "CHECK constraint failed: T2B" > - -- > - }) > - > -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 > ); > ]], { > - -- > - > - -- > - }) > - > -test:do_catchsql_test( > - "check-2.13", > - [[ > - INSERT INTO t2c VALUES('xyzzy',7,8); > - ]], { > - -- > - 1, "CHECK constraint failed: X_TWO" > - -- > + -- > + 1, "near \")\": syntax error" > + -- > }) > > 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( > > -- > }) > + > +-- 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) > + ); > + ]], { > + -- > + > + -- > + }) > + > +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. > + > +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 > + ); > + ]], { > + -- > + > + -- > + }) > + > +test:do_catchsql_test( > + "table-22.3", > + [[ > + INSERT INTO t22 VALUES(1, 1, 1); > + INSERT INTO t22 VALUES(2, 1, 1); > + ]], { > + -- > + 1,"Duplicate key exists in unique index 'unique_ONE_2' in space 'T22'" > + -- > + }) > + > +test:do_execsql_test( > + "table-22.4", > + [[ > + CREATE TABLE t24( > + a integer primary key, > + b integer constraint two unique, > + c integer > + ); > + ]], { > + -- > + > + -- > + }) > + > +test:do_catchsql_test( > + "table-22.5", > + [[ > + INSERT INTO t24 VALUES(1, 1, 1); > + INSERT INTO t24 VALUES(2, 1, 1); > + ]], { > + -- > + 1, "Duplicate key exists in unique index 'unique_TWO_2' in space 'T24'" > + -- > + }) > + > +test:do_catchsql_test( > + "table-22.6", > + [[ > + CREATE TABLE t26( > + a integer primary key, > + b integer constraint one constraint one unique, > + c integer > + ); > + ]], { > + -- > + 1,"keyword \"constraint\" is reserved" > + -- > + }) > + > +test:do_catchsql_test( > + "table-22.7", > + [[ > + CREATE TABLE t27( > + a integer primary key, > + b integer constraint one constraint two unique, > + c integer > + ); > + ]], { > + -- > + 1,"keyword \"constraint\" is reserved" > + -- > + }) > + > +test:do_execsql_test( > + "check-22.cleanup", > + [[ > + DROP TABLE IF EXISTS t22; > + DROP TABLE IF EXISTS t24; > + ]], { > + -- > + > + -- > + }) > test:finish_test() >