From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: Nikita Pettik <korablev@tarantool.org>, Roman Khabibov <roman.habibov@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH v2 2/3] box: make constraint operations transactional Date: Sat, 28 Dec 2019 14:07:21 +0300 [thread overview] Message-ID: <b8edb143-c126-fe9b-893e-97ce6e8d8df3@tarantool.org> (raw) In-Reply-To: <20191228001833.GA45089@tarantool.org> Thanks for the review! On 28/12/2019 03:18, Nikita Pettik wrote: > On 17 Dec 18:03, Roman Khabibov wrote: >>>> >>>> @TarantoolBot document >>>> Title: Table constraints in SQL >>>> >>>> SQL: >>>> According to ANSI SQL, table constraint is one of the following >>>> entities: PRIMARY KEY, UNIQUE, FOREIGN KEY, CHECK. Every >>>> constraint have its own name passed by user or automatically > > have -> has > >>>> generated. And these names must be unique within one table/space. >>>> Naming in SQL is case-insensitive, so "CONSTRAINT c" and > > False. Naming in SQL is case-sensitive. It's all about uppercasing > unquoted identifiers: > > CREATE TABLE t2 (i INT, CONSTRAINT "c" CHECK (i > 0), CONSTRAINT c PRIMARY KEY (i)); > --- > - row_count: 1 > ... I rewrote the commit message, check this: ================================================================================ sql: make constraint names unique in scope of table Put constraint names into the space's hash table and drop them on insert/delete in corresponding system spaces (_index, _fk_constraint, _ck_constraint). Closes #3503 @TarantoolBot document Title: Constraint names are unique in scope of table SQL: According to ANSI SQL, table constraint is one of the following entities: PRIMARY KEY, UNIQUE, FOREIGN KEY, CHECK. (Also there is NOT NULL, but we don't consider it a constraint.) Every constraint has its own name passed by user or automatically generated. And these names must be unique within one table/space. For example: tarantool> box.execute([[CREATE TABLE test ( a INTEGER PRIMARY KEY, b INTEGER, CONSTRAINT cnstr CHECK (a >= 0) );]]) --- - row_count: 1 ... tarantool> box.execute('CREATE UNIQUE INDEX cnstr ON test(b);') --- - null - Constraint CHECK 'CNSTR' already exists in space 'TEST' ... Unique index and CHECK are different constraint types, but they share namespace, and can't have clashing names. The same for all the other constraints. ================================================================================ >>>> "CONSTRAINT C" are the same. For example, you will get error, if >>>> you try to: >>>> >>>> CREATE TABLE t2 (i INT, CONSTRAINT c CHECK (i > 0), >>>> CONSTRAINT c PRIMARY KEY (i)); >>>> >>>> or >>>> >>>> CREATE TABLE t2 (i INT CONSTRAINT c PRIMARY KEY); >>>> ALTER TABLE t2 ADD CONSTRAINT c CHECK(i > 0);'); >>>> The same is for any other constraint types. >>>> >> >> +/** >> + * Check if constraint with @a name exists within @a space. Call >> + * diag_set() if it is so. >> + */ >> +static inline int >> +space_check_constraint_existence(struct space *space, const char *name) > > IMHO this function looks a bit strange (or I'd say - contradictory) Yes, maybe the name is bad. Check this: ================================================================================ diff --git a/src/box/alter.cc b/src/box/alter.cc index 2e4fa3c41..1ce1c3858 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -1776,11 +1776,11 @@ MoveCkConstraints::rollback(struct alter_space *alter) } /** - * Check if constraint with @a name exists within @a space. Call - * diag_set() if it is so. + * Check if a constraint name is not occupied in @a space. Treat + * existence as an error. */ static inline int -space_check_constraint_existence(struct space *space, const char *name) +space_check_constraint_name_is_free(struct space *space, const char *name) { struct constraint_id *id = space_find_constraint_id(space, name); if (id == NULL) @@ -1798,7 +1798,7 @@ static int space_insert_constraint_id(struct space *space, enum constraint_type type, const char *name) { - if (space_check_constraint_existence(space, name) != 0) + if (space_check_constraint_name_is_free(space, name) != 0) return -1; struct constraint_id *id = constraint_id_new(type, name); if (id == NULL) @@ -1839,8 +1839,8 @@ public: void CreateConstraintID::prepare(struct alter_space *alter) { - if (space_check_constraint_existence(alter->old_space, - new_id->name) != 0) + if (space_check_constraint_name_is_free(alter->old_space, + new_id->name) != 0) diag_raise(); } ================================================================================ > Let's simply inline it, only 2 usages tho. I would agree if not the long diag_set. Purpose of this function is not to write it more than once. >> +{ >> + struct constraint_id *id = space_find_constraint_id(space, name); >> + if (id == NULL) >> + return 0; >> + diag_set(ClientError, ER_CONSTRAINT_EXISTS, >> + constraint_type_strs[id->type], name, space_name(space)); >> + return -1; >> +} >> + >> + >> +static inline void >> +space_delete_constraint_id(struct space *space, const char *name) > > Why do we need this wrapper? Multiple reasons: 1) it appears 5 times, so I decided to wrap it; 2) it is shorter, 3) it is consistent with other space_<action>_constraint_id() functions. But is not worth moving to space.h/.c IMO. >> +{ >> + constraint_id_delete(space_pop_constraint_id(space, name)); >> +} >> + >> diff --git a/src/box/errcode.h b/src/box/errcode.h >> index c660b1c70..094a63ee1 100644 >> --- a/src/box/errcode.h >> +++ b/src/box/errcode.h >> @@ -222,7 +222,7 @@ struct errcode_record { >> /*167 */_(ER_CREATE_FK_CONSTRAINT, "Failed to create foreign key constraint '%s': %s") \ >> /*168 */_(ER_DROP_FK_CONSTRAINT, "Failed to drop foreign key constraint '%s': %s") \ >> /*169 */_(ER_NO_SUCH_CONSTRAINT, "Constraint %s does not exist") \ >> - /*170 */_(ER_CONSTRAINT_EXISTS, "Constraint %s already exists") \ >> + /*170 */_(ER_CONSTRAINT_EXISTS, "Constraint %s '%s' already exists in space '%s'") \ > > I'd better place type of constraint at the beggining of sentence: > > Check constraint xxx already exists > Foreign key constraint ... > Unique constraint ... > > Sounds way much better, doesn't it? It is subjective. I changed it to this order on purpose. 1) Because I wanted to go from more common term to a more concrete: 'Constraint' - common, <constraint type> - more concrete, <name> - even more concrete. 2) Because when I see '<type> constraint <name> already exists ...' for me it looks like someone tried to create constraint of type <type>. But it is not always so. Type is not the most important part of the message. But I don't mind to change it back. Since I have no a proof that my point is correct, except personal taste. ================================================================================ diff --git a/src/box/errcode.h b/src/box/errcode.h index 094a63ee1..c940299b1 100644 --- a/src/box/errcode.h +++ b/src/box/errcode.h @@ -222,7 +222,7 @@ struct errcode_record { /*167 */_(ER_CREATE_FK_CONSTRAINT, "Failed to create foreign key constraint '%s': %s") \ /*168 */_(ER_DROP_FK_CONSTRAINT, "Failed to drop foreign key constraint '%s': %s") \ /*169 */_(ER_NO_SUCH_CONSTRAINT, "Constraint %s does not exist") \ - /*170 */_(ER_CONSTRAINT_EXISTS, "Constraint %s '%s' already exists in space '%s'") \ + /*170 */_(ER_CONSTRAINT_EXISTS, "%s constraint '%s' already exists in space '%s'") \ /*171 */_(ER_SQL_TYPE_MISMATCH, "Type mismatch: can not convert %s to %s") \ /*172 */_(ER_ROWID_OVERFLOW, "Rowid is overflowed: too many entries in ephemeral space") \ /*173 */_(ER_DROP_COLLATION, "Can't drop collation %s : %s") \ diff --git a/test/sql-tap/alter2.test.lua b/test/sql-tap/alter2.test.lua index ac3ed000e..f6d421424 100755 --- a/test/sql-tap/alter2.test.lua +++ b/test/sql-tap/alter2.test.lua @@ -246,7 +246,7 @@ test:do_catchsql_test( ALTER TABLE child ADD CONSTRAINT fk FOREIGN KEY (a) REFERENCES child; ]], { -- <alter2-5.1> - 1, "Constraint FOREIGN KEY 'FK' already exists in space 'CHILD'" + 1, "FOREIGN KEY constraint 'FK' already exists in space 'CHILD'" -- </alter2-5.1> }) @@ -278,7 +278,7 @@ test:do_catchsql_test( "alter2-6.2", [[ ALTER TABLE t1 ADD CONSTRAINT ck CHECK(id > 0); - ]], { 1, "Constraint CHECK 'CK' already exists in space 'T1'" }) + ]], { 1, "CHECK constraint 'CK' already exists in space 'T1'" }) -- Make sure that CHECK constraint can be created only on empty space. -- diff --git a/test/sql/checks.result b/test/sql/checks.result index 488659e84..ebdf8803a 100644 --- a/test/sql/checks.result +++ b/test/sql/checks.result @@ -260,7 +260,7 @@ s:drop() box.execute("CREATE TABLE T2(ID INT PRIMARY KEY, CONSTRAINT CK1 CHECK(ID > 0), CONSTRAINT CK1 CHECK(ID < 0))") --- - null -- Constraint CHECK 'CK1' already exists in space 'T2' +- CHECK constraint 'CK1' already exists in space 'T2' ... box.space.T2 --- diff --git a/test/sql/clear.result b/test/sql/clear.result index 988a972e7..e7c498595 100644 --- a/test/sql/clear.result +++ b/test/sql/clear.result @@ -177,7 +177,7 @@ box.execute("DROP TABLE zoobar") box.execute("CREATE TABLE t1(id INT PRIMARY KEY, CONSTRAINT ck1 CHECK(id > 0), CONSTRAINT ck1 CHECK(id < 0));") --- - null -- Constraint CHECK 'CK1' already exists in space 'T1' +- CHECK constraint 'CK1' already exists in space 'T1' ... box.space.t1 --- @@ -190,7 +190,7 @@ box.space._ck_constraint:select() box.execute("CREATE TABLE t2(id INT PRIMARY KEY, CONSTRAINT fk1 FOREIGN KEY(id) REFERENCES t2, CONSTRAINT fk1 FOREIGN KEY(id) REFERENCES t2);") --- - null -- Constraint FOREIGN KEY 'FK1' already exists in space 'T2' +- FOREIGN KEY constraint 'FK1' already exists in space 'T2' ... box.space.t2 --- @@ -207,7 +207,7 @@ box.space._fk_constraint:select() box.execute("CREATE TABLE t3(id INT PRIMARY KEY, CONSTRAINT ck1 CHECK(id > 0), CONSTRAINT ck1 FOREIGN KEY(id) REFERENCES t3, CONSTRAINT fk1 FOREIGN KEY(id) REFERENCES t3, CONSTRAINT ck1 CHECK(id < 0));") --- - null -- Constraint FOREIGN KEY 'CK1' already exists in space 'T3' +- FOREIGN KEY constraint 'CK1' already exists in space 'T3' ... box.space.t1 --- diff --git a/test/sql/constraint.result b/test/sql/constraint.result index e452e5305..fcb8c8d65 100644 --- a/test/sql/constraint.result +++ b/test/sql/constraint.result @@ -26,48 +26,48 @@ box.execute([[CREATE TABLE t2 (i INT, CONSTRAINT c CHECK (i > 0), CONSTRAINT c PRIMARY KEY (i));]]); | --- | - null - | - Constraint PRIMARY KEY 'C' already exists in space 'T2' + | - PRIMARY KEY constraint 'C' already exists in space 'T2' | ... box.execute([[CREATE TABLE t2 (i INT, CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i), CONSTRAINT c PRIMARY KEY (i));]]); | --- | - null - | - Constraint PRIMARY KEY 'C' already exists in space 'T2' + | - PRIMARY KEY constraint 'C' already exists in space 'T2' | ... box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY, CONSTRAINT c CHECK (i > 0), CONSTRAINT c UNIQUE (i));]]); | --- | - null - | - Constraint UNIQUE 'C' already exists in space 'T2' + | - UNIQUE constraint 'C' already exists in space 'T2' | ... box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY, CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i), CONSTRAINT c UNIQUE (i));]]); | --- | - null - | - Constraint UNIQUE 'C' already exists in space 'T2' + | - UNIQUE constraint 'C' already exists in space 'T2' | ... box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY, CONSTRAINT c CHECK (i > 0), CONSTRAINT c CHECK (i < 0));]]); | --- | - null - | - Constraint CHECK 'C' already exists in space 'T2' + | - CHECK constraint 'C' already exists in space 'T2' | ... box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY, CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i), CONSTRAINT c CHECK (i > 0));]]); | --- | - null - | - Constraint FOREIGN KEY 'C' already exists in space 'T2' + | - FOREIGN KEY constraint 'C' already exists in space 'T2' | ... box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY CONSTRAINT c REFERENCES t1(i), CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i))]]); | --- | - null - | - Constraint FOREIGN KEY 'C' already exists in space 'T2' + | - FOREIGN KEY constraint 'C' already exists in space 'T2' | ... test_run:cmd("setopt delimiter ''"); | --- @@ -85,7 +85,7 @@ box.execute('CREATE TABLE t2 (i INT CONSTRAINT c PRIMARY KEY);') box.execute('ALTER TABLE t2 ADD CONSTRAINT c CHECK(i > 0);') | --- | - null - | - Constraint PRIMARY KEY 'C' already exists in space 'T2' + | - PRIMARY KEY constraint 'C' already exists in space 'T2' | ... box.execute('ALTER TABLE t2 ADD CONSTRAINT c UNIQUE(i);') | --- @@ -95,7 +95,7 @@ box.execute('ALTER TABLE t2 ADD CONSTRAINT c UNIQUE(i);') box.execute('ALTER TABLE t2 ADD CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i);') | --- | - null - | - Constraint PRIMARY KEY 'C' already exists in space 'T2' + | - PRIMARY KEY constraint 'C' already exists in space 'T2' | ... -- @@ -162,7 +162,7 @@ box.space.T2.index.D:alter({name = 'E'}) box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);') | --- | - null - | - Constraint UNIQUE 'E' already exists in space 'T2' + | - UNIQUE constraint 'E' already exists in space 'T2' | ... -- @@ -178,7 +178,7 @@ box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);') | ... box.space.T2.index.E:alter({unique = true}) | --- - | - error: Constraint CHECK 'E' already exists in space 'T2' + | - error: CHECK constraint 'E' already exists in space 'T2' | ... box.space.T2.ck_constraint.E:drop() | --- @@ -189,7 +189,7 @@ box.space.T2.index.E:alter({unique = true}) box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);') | --- | - null - | - Constraint UNIQUE 'E' already exists in space 'T2' + | - UNIQUE constraint 'E' already exists in space 'T2' | ... -- Alter name and uniqueness of an unique index simultaneously. ================================================================================ >> new file mode 100755 >> index 000000000..b41e16dc2 >> --- /dev/null >> +++ b/test/sql/constraint.test.lua >> @@ -0,0 +1,93 @@ >> +test_run = require('test_run').new() >> +engine = test_run:get_cfg('engine') >> +box.execute('pragma sql_default_engine=\''..engine..'\'') >> + >> +-- Make sure, that altering of an index name affects its record >> +-- in the space's constraint hash table. >> +-- >> +box.execute('CREATE UNIQUE INDEX d ON t2(i);') >> +box.space.T2.index.D:alter({name = 'E'}) >> +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);') > > As far as I see, you missed case of altering index (or other constraint) > name to already occupied by another constraint. True. ================================================================================ diff --git a/test/sql/constraint.result b/test/sql/constraint.result index fcb8c8d65..306eb7184 100644 --- a/test/sql/constraint.result +++ b/test/sql/constraint.result @@ -165,6 +165,29 @@ box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);') | - UNIQUE constraint 'E' already exists in space 'T2' | ... +-- +-- Try rename to an already occupied name. +-- +box.execute('ALTER TABLE t2 ADD CONSTRAINT f CHECK(i > 0);') + | --- + | - row_count: 1 + | ... +box.execute('CREATE UNIQUE INDEX g ON t2(i);') + | --- + | - row_count: 1 + | ... +box.space.T2.index.G:alter({name = 'F'}) + | --- + | - error: CHECK constraint 'F' already exists in space 'T2' + | ... +box.execute('DROP INDEX g ON t2;') + | --- + | - row_count: 1 + | ... +box.space.T2.ck_constraint.F:drop() + | --- + | ... + -- -- Make sure, that altering of an index uniqueness puts/drops -- its name to/from the space's constraint hash table. diff --git a/test/sql/constraint.test.lua b/test/sql/constraint.test.lua index b41e16dc2..627421f77 100755 --- a/test/sql/constraint.test.lua +++ b/test/sql/constraint.test.lua @@ -71,6 +71,15 @@ box.execute('CREATE UNIQUE INDEX d ON t2(i);') box.space.T2.index.D:alter({name = 'E'}) box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);') +-- +-- Try rename to an already occupied name. +-- +box.execute('ALTER TABLE t2 ADD CONSTRAINT f CHECK(i > 0);') +box.execute('CREATE UNIQUE INDEX g ON t2(i);') +box.space.T2.index.G:alter({name = 'F'}) +box.execute('DROP INDEX g ON t2;') +box.space.T2.ck_constraint.F:drop() + -- -- Make sure, that altering of an index uniqueness puts/drops -- its name to/from the space's constraint hash table. ================================================================================ >> +-- >> +-- Make sure, that altering of an index uniqueness puts/drops >> +-- its name to/from the space's constraint hash table. >> +-- >> +box.space.T2.index.E:alter({unique = false}) >> +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);') >> +box.space.T2.index.E:alter({unique = true}) >> +box.space.T2.ck_constraint.E:drop() >> +box.space.T2.index.E:alter({unique = true}) >> +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);') >> + >> +-- Alter name and uniqueness of an unique index simultaneously. >> +box.space.T2.index.E:alter({name = 'D', unique = false}) >> +box.execute('CREATE UNIQUE INDEX e ON t2(i);') > > And make sure that CREATE UNIQUE INDEX d ON t2(i); fails I am not sure how that test is related to the patch, because uniqueness of index names is a different task, and it has many tests in other places. But ok. ================================================================================ diff --git a/test/sql/constraint.result b/test/sql/constraint.result index 306eb7184..65ec61e66 100644 --- a/test/sql/constraint.result +++ b/test/sql/constraint.result @@ -223,6 +223,13 @@ box.execute('CREATE UNIQUE INDEX e ON t2(i);') | --- | - row_count: 1 | ... +-- The existing D index is not unique, but regardless of +-- uniqueness, index names should be unique in a space. +box.execute('CREATE UNIQUE INDEX d ON t2(i);') + | --- + | - null + | - Index 'D' already exists in space 'T2' + | ... -- -- Cleanup. diff --git a/test/sql/constraint.test.lua b/test/sql/constraint.test.lua index 627421f77..7510a69d6 100755 --- a/test/sql/constraint.test.lua +++ b/test/sql/constraint.test.lua @@ -94,6 +94,9 @@ box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);') -- Alter name and uniqueness of an unique index simultaneously. box.space.T2.index.E:alter({name = 'D', unique = false}) box.execute('CREATE UNIQUE INDEX e ON t2(i);') +-- The existing D index is not unique, but regardless of +-- uniqueness, index names should be unique in a space. +box.execute('CREATE UNIQUE INDEX d ON t2(i);') -- -- Cleanup. ================================================================================ > What about circle renaming of consraint inside transaction which > is rolled back later? I wasn't sure what do you mean as a circle renaming. I tried this: ================================================================================ diff --git a/test/sql/constraint.result b/test/sql/constraint.result index 65ec61e66..32c42897a 100644 --- a/test/sql/constraint.result +++ b/test/sql/constraint.result @@ -148,6 +148,48 @@ test_run:cmd("setopt delimiter ''"); | - true | ... +-- +-- Circle renaming in a transaction. +-- +box.begin() \ +box.execute('CREATE UNIQUE INDEX d ON t2(i);') \ +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK (i > 0);') \ +-- D and E constraints exchange their names. Note, CHECK can't be \ +-- renamed, because CHECK name is a primary key of _ck_constraint. \ +-- It can be only dropped and created again with a new name. \ +box.space.T2.index.D:rename('F') \ +box.space.T2.ck_constraint.E:drop() \ +box.space.T2.index.F:rename('E') \ +box.execute('ALTER TABLE t2 ADD CONSTRAINT D CHECK (i > 0);') \ +-- Now an attempt to occupy the names will report, that the names \ +-- are really swapped. \ +_, err1 = box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK (i > 0);') \ +_, err2 = box.execute('CREATE UNIQUE INDEX d ON t2(i);') \ +box.rollback() + | --- + | ... +err1, err2 + | --- + | - UNIQUE constraint 'E' already exists in space 'T2' + | - CHECK constraint 'D' already exists in space 'T2' + | ... +-- After rollback ensure, that the names are free again. +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK (i > 0);') + | --- + | - row_count: 1 + | ... +box.execute('CREATE UNIQUE INDEX d ON t2(i);') + | --- + | - row_count: 1 + | ... +box.execute('DROP INDEX d ON t2;') + | --- + | - row_count: 1 + | ... +box.space.T2.ck_constraint.E:drop() + | --- + | ... + -- -- Make sure, that altering of an index name affects its record -- in the space's constraint hash table. diff --git a/test/sql/constraint.test.lua b/test/sql/constraint.test.lua index 7510a69d6..1e679b734 100755 --- a/test/sql/constraint.test.lua +++ b/test/sql/constraint.test.lua @@ -63,6 +63,31 @@ box.execute('ALTER TABLE t2 DROP CONSTRAINT d;') box.commit(); test_run:cmd("setopt delimiter ''"); +-- +-- Circle renaming in a transaction. +-- +box.begin() \ +box.execute('CREATE UNIQUE INDEX d ON t2(i);') \ +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK (i > 0);') \ +-- D and E constraints exchange their names. Note, CHECK can't be \ +-- renamed, because CHECK name is a primary key of _ck_constraint. \ +-- It can be only dropped and created again with a new name. \ +box.space.T2.index.D:rename('F') \ +box.space.T2.ck_constraint.E:drop() \ +box.space.T2.index.F:rename('E') \ +box.execute('ALTER TABLE t2 ADD CONSTRAINT D CHECK (i > 0);') \ +-- Now an attempt to occupy the names will report, that the names \ +-- are really swapped. \ +_, err1 = box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK (i > 0);') \ +_, err2 = box.execute('CREATE UNIQUE INDEX d ON t2(i);') \ +box.rollback() +err1, err2 +-- After rollback ensure, that the names are free again. +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK (i > 0);') +box.execute('CREATE UNIQUE INDEX d ON t2(i);') +box.execute('DROP INDEX d ON t2;') +box.space.T2.ck_constraint.E:drop() + -- -- Make sure, that altering of an index name affects its record -- in the space's constraint hash table. ================================================================================ Also I did some minor refactoring to the test. See the whole patch below. ================================================================================ sql: make constraint names unique in scope of table Put constraint names into the space's hash table and drop them on insert/delete in corresponding system spaces (_index, _fk_constraint, _ck_constraint). Closes #3503 @TarantoolBot document Title: Constraint names are unique in scope of table SQL: According to ANSI SQL, table constraint is one of the following entities: PRIMARY KEY, UNIQUE, FOREIGN KEY, CHECK. (Also there is NOT NULL, but we don't consider it a constraint.) Every constraint has its own name passed by user or automatically generated. And these names must be unique within one table/space. For example: tarantool> box.execute([[CREATE TABLE test ( a INTEGER PRIMARY KEY, b INTEGER, CONSTRAINT cnstr CHECK (a >= 0) );]]) --- - row_count: 1 ... tarantool> box.execute('CREATE UNIQUE INDEX cnstr ON test(b);') --- - null - Constraint CHECK 'CNSTR' already exists in space 'TEST' ... Unique index and CHECK are different constraint types, but they share namespace, and can't have clashing names. The same for all the other constraints. diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt index 5cd5cba81..19ce3d481 100644 --- a/src/box/CMakeLists.txt +++ b/src/box/CMakeLists.txt @@ -102,6 +102,7 @@ add_library(box STATIC sequence.c ck_constraint.c fk_constraint.c + constraint_id.c func.c func_def.c key_list.c diff --git a/src/box/alter.cc b/src/box/alter.cc index bef25b605..1ce1c3858 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -57,6 +57,7 @@ #include "version.h" #include "sequence.h" #include "sql.h" +#include "constraint_id.h" /* {{{ Auxiliary functions and methods. */ @@ -703,6 +704,12 @@ space_swap_fk_constraints(struct space *new_space, struct space *old_space) SWAP(new_space->fk_constraint_mask, old_space->fk_constraint_mask); } +static void +space_swap_constraint_ids(struct space *new_space, struct space *old_space) +{ + SWAP(new_space->constraint_ids, old_space->constraint_ids); +} + /** * True if the space has records identified by key 'uid'. * Uses 'iid' index. @@ -1033,10 +1040,12 @@ alter_space_rollback(struct trigger *trigger, void * /* event */) space_fill_index_map(alter->old_space); space_fill_index_map(alter->new_space); /* - * Don't forget about space triggers and foreign keys. + * Don't forget about space triggers, foreign keys and + * constraints. */ space_swap_triggers(alter->new_space, alter->old_space); space_swap_fk_constraints(alter->new_space, alter->old_space); + space_swap_constraint_ids(alter->new_space, alter->old_space); space_cache_replace(alter->new_space, alter->old_space); alter_space_delete(alter); return 0; @@ -1143,10 +1152,12 @@ alter_space_do(struct txn_stmt *stmt, struct alter_space *alter) space_fill_index_map(alter->old_space); space_fill_index_map(alter->new_space); /* - * Don't forget about space triggers and foreign keys. + * Don't forget about space triggers, foreign keys and + * constraints. */ space_swap_triggers(alter->new_space, alter->old_space); space_swap_fk_constraints(alter->new_space, alter->old_space); + space_swap_constraint_ids(alter->new_space, alter->old_space); /* * The new space is ready. Time to update the space * cache with it. @@ -1408,14 +1419,14 @@ ModifyIndex::~ModifyIndex() /** CreateIndex - add a new index to the space. */ class CreateIndex: public AlterSpaceOp { -public: - CreateIndex(struct alter_space *alter) - :AlterSpaceOp(alter), new_index(NULL), new_index_def(NULL) - {} /** New index. */ struct index *new_index; /** New index index_def. */ struct index_def *new_index_def; +public: + CreateIndex(struct alter_space *alter, struct index_def *def) + :AlterSpaceOp(alter), new_index(NULL), new_index_def(def) + {} virtual void alter_def(struct alter_space *alter); virtual void prepare(struct alter_space *alter); virtual void commit(struct alter_space *alter, int64_t lsn); @@ -1764,6 +1775,145 @@ MoveCkConstraints::rollback(struct alter_space *alter) space_swap_ck_constraint(alter->new_space, alter->old_space); } +/** + * Check if a constraint name is not occupied in @a space. Treat + * existence as an error. + */ +static inline int +space_check_constraint_name_is_free(struct space *space, const char *name) +{ + struct constraint_id *id = space_find_constraint_id(space, name); + if (id == NULL) + return 0; + diag_set(ClientError, ER_CONSTRAINT_EXISTS, + constraint_type_strs[id->type], name, space_name(space)); + return -1; +} + +/** + * Put a new constraint name into the space's namespace of + * constraints, with duplicate check. + */ +static int +space_insert_constraint_id(struct space *space, enum constraint_type type, + const char *name) +{ + if (space_check_constraint_name_is_free(space, name) != 0) + return -1; + struct constraint_id *id = constraint_id_new(type, name); + if (id == NULL) + return -1; + if (space_add_constraint_id(space, id) != 0) { + constraint_id_delete(id); + return -1; + } + return 0; +} + +static inline void +space_delete_constraint_id(struct space *space, const char *name) +{ + constraint_id_delete(space_pop_constraint_id(space, name)); +} + +/** CreateConstraintID - add a new constraint id to a space. */ +class CreateConstraintID: public AlterSpaceOp +{ + struct constraint_id *new_id; +public: + CreateConstraintID(struct alter_space *alter, enum constraint_type type, + const char *name) + :AlterSpaceOp(alter), new_id(NULL) + { + new_id = constraint_id_new(type, name); + if (new_id == NULL) + diag_raise(); + } + virtual void prepare(struct alter_space *alter); + virtual void alter(struct alter_space *alter); + virtual void rollback(struct alter_space *alter); + virtual void commit(struct alter_space *alter, int64_t signature); + virtual ~CreateConstraintID(); +}; + +void +CreateConstraintID::prepare(struct alter_space *alter) +{ + if (space_check_constraint_name_is_free(alter->old_space, + new_id->name) != 0) + diag_raise(); +} + +void +CreateConstraintID::alter(struct alter_space *alter) +{ + /* Alter() can't fail, so can't just throw an error. */ + if (space_add_constraint_id(alter->old_space, new_id) != 0) + panic("Can't add a new constraint id, out of memory"); +} + +void +CreateConstraintID::rollback(struct alter_space *alter) +{ + space_delete_constraint_id(alter->new_space, new_id->name); + new_id = NULL; +} + +void +CreateConstraintID::commit(struct alter_space *alter, int64_t signature) +{ + (void) alter; + (void) signature; + /* + * Constraint id is added to the space, and should not be + * deleted from now on. + */ + new_id = NULL; +} + +CreateConstraintID::~CreateConstraintID() +{ + if (new_id != NULL) + constraint_id_delete(new_id); +} + +/** DropConstraintID - drop a constraint id from the space. */ +class DropConstraintID: public AlterSpaceOp +{ + struct constraint_id *old_id; + const char *name; +public: + DropConstraintID(struct alter_space *alter, const char *name) + :AlterSpaceOp(alter), old_id(NULL), name(name) + {} + virtual void alter(struct alter_space *alter); + virtual void commit(struct alter_space *alter , int64_t signature); + virtual void rollback(struct alter_space *alter); +}; + +void +DropConstraintID::alter(struct alter_space *alter) +{ + old_id = space_pop_constraint_id(alter->old_space, name); +} + +void +DropConstraintID::commit(struct alter_space *alter, int64_t signature) +{ + (void) alter; + (void) signature; + constraint_id_delete(old_id); +} + +void +DropConstraintID::rollback(struct alter_space *alter) +{ + if (space_add_constraint_id(alter->new_space, old_id) != 0) { + panic("Can't recover after constraint drop rollback (out of " + "memory)"); + } +} + /* }}} */ /** @@ -2428,6 +2578,7 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) old_space->def->uid, SC_SPACE, priv_type) != 0) return -1; struct index *old_index = space_index(old_space, iid); + struct index_def *old_def = old_index != NULL ? old_index->def : NULL; /* * Deal with various cases of dropping of the primary key. @@ -2501,6 +2652,10 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) if (alter_space_move_indexes(alter, 0, iid) != 0) return -1; try { + if (old_index->def->opts.is_unique) { + (void) new DropConstraintID(alter, + old_def->name); + } (void) new DropIndex(alter, old_index); } catch (Exception *e) { return -1; @@ -2510,18 +2665,22 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) if (old_index == NULL && new_tuple != NULL) { if (alter_space_move_indexes(alter, 0, iid)) return -1; - CreateIndex *create_index; + struct index_def *def = + index_def_new_from_tuple(new_tuple, old_space); + if (def == NULL) + return -1; + index_def_update_optionality(def, alter->new_min_field_count); try { - create_index = new CreateIndex(alter); + if (def->opts.is_unique) { + (void) new CreateConstraintID( + alter, iid == 0 ? CONSTRAINT_TYPE_PK : + CONSTRAINT_TYPE_UNIQUE, def->name); + } + (void) new CreateIndex(alter, def); } catch (Exception *e) { + index_def_delete(def); return -1; } - create_index->new_index_def = - index_def_new_from_tuple(new_tuple, old_space); - if (create_index->new_index_def == NULL) - return -1; - index_def_update_optionality(create_index->new_index_def, - alter->new_min_field_count); } /* Case 3 and 4: check if we need to rebuild index data. */ if (old_index != NULL && new_tuple != NULL) { @@ -2531,6 +2690,34 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) return -1; auto index_def_guard = make_scoped_guard([=] { index_def_delete(index_def); }); + /* + * We put a new name when either an index is + * becoming unique (i.e. constraint), or when a + * unique index's name is under change. + */ + bool do_new_constraint_id = + !old_def->opts.is_unique && index_def->opts.is_unique; + bool do_drop_constraint_id = + old_def->opts.is_unique && !index_def->opts.is_unique; + + if (old_def->opts.is_unique && index_def->opts.is_unique && + strcmp(index_def->name, old_def->name) != 0) { + do_new_constraint_id = true; + do_drop_constraint_id = true; + } + try { + if (do_new_constraint_id) { + (void) new CreateConstraintID( + alter, CONSTRAINT_TYPE_UNIQUE, + index_def->name); + } + if (do_drop_constraint_id) { + (void) new DropConstraintID(alter, + old_def->name); + } + } catch (Exception *e) { + return -1; + } /* * To detect which key parts are optional, * min_field_count is required. But @@ -4996,8 +5183,11 @@ on_create_fk_constraint_rollback(struct trigger *trigger, void *event) struct fk_constraint *fk = (struct fk_constraint *)trigger->data; rlist_del_entry(fk, in_parent_space); rlist_del_entry(fk, in_child_space); + struct space *child = space_by_id(fk->def->child_id); + assert(child != NULL); + space_delete_constraint_id(child, fk->def->name); space_reset_fk_constraint_mask(space_by_id(fk->def->parent_id)); - space_reset_fk_constraint_mask(space_by_id(fk->def->child_id)); + space_reset_fk_constraint_mask(child); fk_constraint_delete(fk); return 0; } @@ -5029,6 +5219,11 @@ on_drop_fk_constraint_rollback(struct trigger *trigger, void *event) struct fk_constraint *old_fk = (struct fk_constraint *)trigger->data; struct space *parent = space_by_id(old_fk->def->parent_id); struct space *child = space_by_id(old_fk->def->child_id); + if (space_insert_constraint_id(child, CONSTRAINT_TYPE_FK, + old_fk->def->name) != 0) { + panic("Can't recover after FK constraint drop rollback (out of " + "memory)"); + } rlist_add_entry(&child->child_fk_constraint, old_fk, in_child_space); rlist_add_entry(&parent->parent_fk_constraint, old_fk, in_parent_space); fk_constraint_set_mask(old_fk, &child->fk_constraint_mask, @@ -5210,15 +5405,19 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event) fk->def = fk_def; fk->index_id = fk_index->def->iid; if (old_tuple == NULL) { - rlist_add_entry(&child_space->child_fk_constraint, - fk, in_child_space); - rlist_add_entry(&parent_space->parent_fk_constraint, - fk, in_parent_space); struct trigger *on_rollback = txn_alter_trigger_new(on_create_fk_constraint_rollback, fk); if (on_rollback == NULL) return -1; + if (space_insert_constraint_id(child_space, + CONSTRAINT_TYPE_FK, + fk_def->name) != 0) + return -1; + rlist_add_entry(&child_space->child_fk_constraint, + fk, in_child_space); + rlist_add_entry(&parent_space->parent_fk_constraint, + fk, in_parent_space); txn_stmt_on_rollback(stmt, on_rollback); fk_constraint_set_mask(fk, &parent_space->fk_constraint_mask, @@ -5279,6 +5478,7 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event) old_fk); if (on_rollback == NULL) return -1; + space_delete_constraint_id(child_space, fk_def->name); txn_stmt_on_rollback(stmt, on_rollback); space_reset_fk_constraint_mask(child_space); space_reset_fk_constraint_mask(parent_space); @@ -5344,9 +5544,10 @@ on_create_ck_constraint_rollback(struct trigger *trigger, void * /* event */) assert(ck != NULL); struct space *space = space_by_id(ck->def->space_id); assert(space != NULL); - assert(space_ck_constraint_by_name(space, ck->def->name, - strlen(ck->def->name)) != NULL); + const char *name = ck->def->name; + assert(space_ck_constraint_by_name(space, name, strlen(name)) != NULL); space_remove_ck_constraint(space, ck); + space_delete_constraint_id(space, name); ck_constraint_delete(ck); if (trigger_run(&on_alter_space, space) != 0) return -1; @@ -5371,9 +5572,10 @@ on_drop_ck_constraint_rollback(struct trigger *trigger, void * /* event */) assert(ck != NULL); struct space *space = space_by_id(ck->def->space_id); assert(space != NULL); - assert(space_ck_constraint_by_name(space, ck->def->name, - strlen(ck->def->name)) == NULL); - if (space_add_ck_constraint(space, ck) != 0) + const char *name = ck->def->name; + assert(space_ck_constraint_by_name(space, name, strlen(name)) == NULL); + if (space_add_ck_constraint(space, ck) != 0 || + space_insert_constraint_id(space, CONSTRAINT_TYPE_CK, name) != 0) panic("Can't recover after CK constraint drop rollback"); if (trigger_run(&on_alter_space, space) != 0) return -1; @@ -5459,7 +5661,8 @@ on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event) const char *name = ck_def->name; struct ck_constraint *old_ck_constraint = space_ck_constraint_by_name(space, name, strlen(name)); - if (old_ck_constraint != NULL) { + bool is_insert = old_ck_constraint == NULL; + if (!is_insert) { struct ck_constraint_def *old_def = old_ck_constraint->def; assert(old_def->space_id == ck_def->space_id); @@ -5491,18 +5694,24 @@ on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event) auto ck_guard = make_scoped_guard([=] { ck_constraint_delete(new_ck_constraint); }); - if (old_ck_constraint != NULL) - rlist_del_entry(old_ck_constraint, link); if (space_add_ck_constraint(space, new_ck_constraint) != 0) return -1; - ck_guard.is_active = false; - if (old_tuple != NULL) { + if (!is_insert) { + rlist_del_entry(old_ck_constraint, link); on_rollback->data = old_ck_constraint; on_rollback->run = on_replace_ck_constraint_rollback; } else { + if (space_insert_constraint_id(space, + CONSTRAINT_TYPE_CK, + name) != 0) { + space_remove_ck_constraint(space, + new_ck_constraint); + return -1; + } on_rollback->data = new_ck_constraint; on_rollback->run = on_create_ck_constraint_rollback; } + ck_guard.is_active = false; on_commit->data = old_ck_constraint; on_commit->run = on_replace_ck_constraint_commit; } else { @@ -5516,6 +5725,7 @@ on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event) struct ck_constraint *old_ck_constraint = space_ck_constraint_by_name(space, name, name_len); assert(old_ck_constraint != NULL); + space_delete_constraint_id(space, old_ck_constraint->def->name); space_remove_ck_constraint(space, old_ck_constraint); on_commit->data = old_ck_constraint; on_commit->run = on_drop_ck_constraint_commit; diff --git a/src/box/constraint_id.c b/src/box/constraint_id.c new file mode 100755 index 000000000..ba6ed859c --- /dev/null +++ b/src/box/constraint_id.c @@ -0,0 +1,63 @@ +/* + * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS + * file. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * 1. Redistributions of source code must retain the above + * copyright notice, this list of conditions and the + * following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials + * provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL + * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ +#include "constraint_id.h" +#include "assoc.h" +#include "errcode.h" +#include "diag.h" + +const char *constraint_type_strs[] = { + [CONSTRAINT_TYPE_PK] = "PRIMARY KEY", + [CONSTRAINT_TYPE_UNIQUE] = "UNIQUE", + [CONSTRAINT_TYPE_FK] = "FOREIGN KEY", + [CONSTRAINT_TYPE_CK] = "CHECK", +}; + +struct constraint_id * +constraint_id_new(enum constraint_type type, const char *name) +{ + uint32_t len = strlen(name); + uint32_t size = sizeof(struct constraint_id) + len + 1; + struct constraint_id *ret = malloc(size); + if (ret == NULL) { + diag_set(OutOfMemory, size, "malloc", "ret"); + return NULL; + } + ret->type = type; + memcpy(ret->name, name, len + 1); + return ret; +} + +void +constraint_id_delete(struct constraint_id *id) +{ + free(id); +} diff --git a/src/box/constraint_id.h b/src/box/constraint_id.h new file mode 100755 index 000000000..21f067cdc --- /dev/null +++ b/src/box/constraint_id.h @@ -0,0 +1,64 @@ +#pragma once + +/* + * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS + * file. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * 1. Redistributions of source code must retain the above + * copyright notice, this list of conditions and the + * following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials + * provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL + * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ +#if defined(__cplusplus) +extern "C" { +#endif + +enum constraint_type { + CONSTRAINT_TYPE_PK = 0, + CONSTRAINT_TYPE_UNIQUE, + CONSTRAINT_TYPE_FK, + CONSTRAINT_TYPE_CK, +}; + +extern const char *constraint_type_strs[]; + +struct constraint_id { + /** Constraint type. */ + enum constraint_type type; + /** Zero-terminated string with name. */ + char name[0]; +}; + +/** Allocate memory and construct constraint id. */ +struct constraint_id * +constraint_id_new(enum constraint_type type, const char *name); + +/** Free memory of constraint id. */ +void +constraint_id_delete(struct constraint_id *id); + +#if defined(__cplusplus) +} +#endif diff --git a/src/box/errcode.h b/src/box/errcode.h index c660b1c70..c940299b1 100644 --- a/src/box/errcode.h +++ b/src/box/errcode.h @@ -222,7 +222,7 @@ struct errcode_record { /*167 */_(ER_CREATE_FK_CONSTRAINT, "Failed to create foreign key constraint '%s': %s") \ /*168 */_(ER_DROP_FK_CONSTRAINT, "Failed to drop foreign key constraint '%s': %s") \ /*169 */_(ER_NO_SUCH_CONSTRAINT, "Constraint %s does not exist") \ - /*170 */_(ER_CONSTRAINT_EXISTS, "Constraint %s already exists") \ + /*170 */_(ER_CONSTRAINT_EXISTS, "%s constraint '%s' already exists in space '%s'") \ /*171 */_(ER_SQL_TYPE_MISMATCH, "Type mismatch: can not convert %s to %s") \ /*172 */_(ER_ROWID_OVERFLOW, "Rowid is overflowed: too many entries in ephemeral space") \ /*173 */_(ER_DROP_COLLATION, "Can't drop collation %s : %s") \ diff --git a/src/box/space.c b/src/box/space.c index 94716a414..1c19d099b 100644 --- a/src/box/space.c +++ b/src/box/space.c @@ -45,6 +45,8 @@ #include "iproto_constants.h" #include "schema.h" #include "ck_constraint.h" +#include "assoc.h" +#include "constraint_id.h" int access_check_space(struct space *space, user_access_t access) @@ -202,6 +204,12 @@ space_create(struct space *space, struct engine *engine, } } } + space->constraint_ids = mh_strnptr_new(); + if (space->constraint_ids == NULL) { + diag_set(OutOfMemory, sizeof(*space->constraint_ids), "malloc", + "constraint_ids"); + goto fail; + } return 0; fail_free_indexes: @@ -258,9 +266,12 @@ space_delete(struct space *space) trigger_destroy(&space->on_replace); space_def_delete(space->def); /* - * SQL Triggers should be deleted with - * on_replace_dd_trigger on deletion from _trigger. + * SQL triggers and constraints should be deleted with + * on_replace_dd_ triggers on deletion from corresponding + * system space. */ + assert(mh_size(space->constraint_ids) == 0); + mh_strnptr_delete(space->constraint_ids); assert(space->sql_triggers == NULL); assert(rlist_empty(&space->parent_fk_constraint)); assert(rlist_empty(&space->child_fk_constraint)); @@ -617,6 +628,45 @@ space_remove_ck_constraint(struct space *space, struct ck_constraint *ck) } } +struct constraint_id * +space_find_constraint_id(struct space *space, const char *name) +{ + struct mh_strnptr_t *ids = space->constraint_ids; + uint32_t len = strlen(name); + mh_int_t pos = mh_strnptr_find_inp(ids, name, len); + if (pos == mh_end(ids)) + return NULL; + return (struct constraint_id *) mh_strnptr_node(ids, pos)->val; +} + +int +space_add_constraint_id(struct space *space, struct constraint_id *id) +{ + assert(space_find_constraint_id(space, id->name) == NULL); + struct mh_strnptr_t *ids = space->constraint_ids; + uint32_t len = strlen(id->name); + uint32_t hash = mh_strn_hash(id->name, len); + const struct mh_strnptr_node_t name_node = {id->name, len, hash, id}; + if (mh_strnptr_put(ids, &name_node, NULL, NULL) == mh_end(ids)) { + diag_set(OutOfMemory, sizeof(name_node), "malloc", "node"); + return -1; + } + return 0; +} + +struct constraint_id * +space_pop_constraint_id(struct space *space, const char *name) +{ + struct mh_strnptr_t *ids = space->constraint_ids; + uint32_t len = strlen(name); + mh_int_t pos = mh_strnptr_find_inp(ids, name, len); + assert(pos != mh_end(ids)); + struct constraint_id *id = (struct constraint_id *) + mh_strnptr_node(ids, pos)->val; + mh_strnptr_del(ids, pos, NULL); + return id; +} + /* {{{ Virtual method stubs */ size_t diff --git a/src/box/space.h b/src/box/space.h index 7926aa65e..9aea4e5be 100644 --- a/src/box/space.h +++ b/src/box/space.h @@ -52,6 +52,7 @@ struct port; struct tuple; struct tuple_format; struct ck_constraint; +struct constraint_id; struct space_vtab { /** Free a space instance. */ @@ -234,6 +235,10 @@ struct space { * of parent constraints as well as child ones. */ uint64_t fk_constraint_mask; + /** + * Hash table with constraint identifiers hashed by name. + */ + struct mh_strnptr_t *constraint_ids; }; /** Initialize a base space instance. */ @@ -516,6 +521,25 @@ space_add_ck_constraint(struct space *space, struct ck_constraint *ck); void space_remove_ck_constraint(struct space *space, struct ck_constraint *ck); +/** Find a constraint identifier by name. */ +struct constraint_id * +space_find_constraint_id(struct space *space, const char *name); + +/** + * Add a new constraint id to the space's hash table of all + * constraints. That is used to prevent existence of constraints + * with equal names. + */ +int +space_add_constraint_id(struct space *space, struct constraint_id *id); + +/** + * Remove a given name from the hash of all constraint + * identifiers of the given space. + */ +struct constraint_id * +space_pop_constraint_id(struct space *space, const char *name); + /* * Virtual method stubs. */ diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 51cd7ce63..9470ef4c3 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -56,6 +56,7 @@ #include "box/schema.h" #include "box/tuple_format.h" #include "box/coll_id_cache.h" +#include "box/constraint_id.h" void sql_finish_coding(struct Parse *parse_context) @@ -584,7 +585,7 @@ trim_space_snprintf(char *wptr, const char *str, uint32_t str_len) static void vdbe_emit_ck_constraint_create(struct Parse *parser, const struct ck_constraint_def *ck_def, - uint32_t reg_space_id); + uint32_t reg_space_id, const char *space_name); void sql_create_check_contraint(struct Parse *parser) @@ -664,7 +665,8 @@ sql_create_check_contraint(struct Parse *parser) struct Vdbe *v = sqlGetVdbe(parser); sqlVdbeAddOp2(v, OP_Integer, space->def->id, space_id_reg); - vdbe_emit_ck_constraint_create(parser, ck_def, space_id_reg); + vdbe_emit_ck_constraint_create(parser, ck_def, space_id_reg, + space->def->name); assert(sqlVdbeGetOp(v, v->nOp - 1)->opcode == OP_SInsert); sqlVdbeCountChanges(v); sqlVdbeChangeP5(v, OPFLAG_NCHANGE); @@ -969,11 +971,13 @@ emitNewSysSpaceSequenceRecord(Parse *pParse, int reg_space_id, int reg_seq_id) * @param parser Parsing context. * @param ck_def Check constraint definition to be serialized. * @param reg_space_id The VDBE register containing space id. -*/ + * @param space_name Name of the space owning the CHECK. For error + * message. + */ static void vdbe_emit_ck_constraint_create(struct Parse *parser, const struct ck_constraint_def *ck_def, - uint32_t reg_space_id) + uint32_t reg_space_id, const char *space_name) { struct sql *db = parser->db; struct Vdbe *v = sqlGetVdbe(parser); @@ -996,7 +1000,8 @@ vdbe_emit_ck_constraint_create(struct Parse *parser, ck_constraint_reg + 6); const char *error_msg = tt_sprintf(tnt_errcode_desc(ER_CONSTRAINT_EXISTS), - ck_def->name); + constraint_type_strs[CONSTRAINT_TYPE_CK], + ck_def->name, space_name); if (vdbe_emit_halt_with_presence_test(parser, BOX_CK_CONSTRAINT_ID, 0, ck_constraint_reg, 2, ER_CONSTRAINT_EXISTS, error_msg, @@ -1014,10 +1019,13 @@ vdbe_emit_ck_constraint_create(struct Parse *parser, * * @param parse_context Parsing context. * @param fk Foreign key to be created. + * @param space_name Name of the space owning the FOREIGN KEY. For + * error message. */ static void vdbe_emit_fk_constraint_create(struct Parse *parse_context, - const struct fk_constraint_def *fk) + const struct fk_constraint_def *fk, + const char *space_name) { assert(parse_context != NULL); assert(fk != NULL); @@ -1058,7 +1066,9 @@ vdbe_emit_fk_constraint_create(struct Parse *parse_context, * been created before. */ const char *error_msg = - tt_sprintf(tnt_errcode_desc(ER_CONSTRAINT_EXISTS), name_copy); + tt_sprintf(tnt_errcode_desc(ER_CONSTRAINT_EXISTS), + constraint_type_strs[CONSTRAINT_TYPE_FK], name_copy, + space_name); if (vdbe_emit_halt_with_presence_test(parse_context, BOX_FK_CONSTRAINT_ID, 0, constr_tuple_reg, 2, @@ -1272,13 +1282,13 @@ sqlEndTable(struct Parse *pParse) fk_def->parent_id = reg_space_id; } fk_def->child_id = reg_space_id; - vdbe_emit_fk_constraint_create(pParse, fk_def); + vdbe_emit_fk_constraint_create(pParse, fk_def, space_name_copy); } struct ck_constraint_parse *ck_parse; rlist_foreach_entry(ck_parse, &pParse->create_table_def.new_check, link) { vdbe_emit_ck_constraint_create(pParse, ck_parse->ck_def, - reg_space_id); + reg_space_id, space_name_copy); } } @@ -1959,7 +1969,8 @@ sql_create_foreign_key(struct Parse *parse_context) struct fk_constraint_parse, link); fk_parse->fk_def = fk_def; } else { - vdbe_emit_fk_constraint_create(parse_context, fk_def); + vdbe_emit_fk_constraint_create(parse_context, fk_def, + child_space->def->name); } exit_create_fk: diff --git a/test/sql-tap/alter2.test.lua b/test/sql-tap/alter2.test.lua index e0bd60727..f6d421424 100755 --- a/test/sql-tap/alter2.test.lua +++ b/test/sql-tap/alter2.test.lua @@ -246,7 +246,7 @@ test:do_catchsql_test( ALTER TABLE child ADD CONSTRAINT fk FOREIGN KEY (a) REFERENCES child; ]], { -- <alter2-5.1> - 1, "Constraint FK already exists" + 1, "FOREIGN KEY constraint 'FK' already exists in space 'CHILD'" -- </alter2-5.1> }) @@ -278,7 +278,7 @@ test:do_catchsql_test( "alter2-6.2", [[ ALTER TABLE t1 ADD CONSTRAINT ck CHECK(id > 0); - ]], { 1, "Constraint CK already exists" }) + ]], { 1, "CHECK constraint 'CK' already exists in space 'T1'" }) -- Make sure that CHECK constraint can be created only on empty space. -- diff --git a/test/sql/checks.result b/test/sql/checks.result index a952b2b70..ebdf8803a 100644 --- a/test/sql/checks.result +++ b/test/sql/checks.result @@ -260,7 +260,7 @@ s:drop() box.execute("CREATE TABLE T2(ID INT PRIMARY KEY, CONSTRAINT CK1 CHECK(ID > 0), CONSTRAINT CK1 CHECK(ID < 0))") --- - null -- Constraint CK1 already exists +- CHECK constraint 'CK1' already exists in space 'T2' ... box.space.T2 --- diff --git a/test/sql/clear.result b/test/sql/clear.result index afa6520e8..e7c498595 100644 --- a/test/sql/clear.result +++ b/test/sql/clear.result @@ -177,7 +177,7 @@ box.execute("DROP TABLE zoobar") box.execute("CREATE TABLE t1(id INT PRIMARY KEY, CONSTRAINT ck1 CHECK(id > 0), CONSTRAINT ck1 CHECK(id < 0));") --- - null -- Constraint CK1 already exists +- CHECK constraint 'CK1' already exists in space 'T1' ... box.space.t1 --- @@ -190,7 +190,7 @@ box.space._ck_constraint:select() box.execute("CREATE TABLE t2(id INT PRIMARY KEY, CONSTRAINT fk1 FOREIGN KEY(id) REFERENCES t2, CONSTRAINT fk1 FOREIGN KEY(id) REFERENCES t2);") --- - null -- Constraint FK1 already exists +- FOREIGN KEY constraint 'FK1' already exists in space 'T2' ... box.space.t2 --- @@ -207,7 +207,7 @@ box.space._fk_constraint:select() box.execute("CREATE TABLE t3(id INT PRIMARY KEY, CONSTRAINT ck1 CHECK(id > 0), CONSTRAINT ck1 FOREIGN KEY(id) REFERENCES t3, CONSTRAINT fk1 FOREIGN KEY(id) REFERENCES t3, CONSTRAINT ck1 CHECK(id < 0));") --- - null -- Constraint CK1 already exists +- FOREIGN KEY constraint 'CK1' already exists in space 'T3' ... box.space.t1 --- diff --git a/test/sql/constraint.result b/test/sql/constraint.result new file mode 100644 index 000000000..1063d6d9b --- /dev/null +++ b/test/sql/constraint.result @@ -0,0 +1,278 @@ +-- test-run result file version 2 +test_run = require('test_run').new() + | --- + | ... +engine = test_run:get_cfg('engine') + | --- + | ... +box.execute('pragma sql_default_engine=\''..engine..'\'') + | --- + | - row_count: 0 + | ... + +-- +-- Check a constraint name for duplicate within a single +-- <CREATE TABLE> statement. +-- +box.execute('CREATE TABLE t1 (i INT PRIMARY KEY);') + | --- + | - row_count: 1 + | ... +test_run:cmd("setopt delimiter ';'"); + | --- + | - true + | ... +box.execute([[CREATE TABLE t2 (i INT, CONSTRAINT c CHECK (i > 0), + CONSTRAINT c PRIMARY KEY (i));]]); + | --- + | - null + | - PRIMARY KEY constraint 'C' already exists in space 'T2' + | ... +box.execute([[CREATE TABLE t2 (i INT, + CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i), + CONSTRAINT c PRIMARY KEY (i));]]); + | --- + | - null + | - PRIMARY KEY constraint 'C' already exists in space 'T2' + | ... +box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY, + CONSTRAINT c CHECK (i > 0), + CONSTRAINT c UNIQUE (i));]]); + | --- + | - null + | - UNIQUE constraint 'C' already exists in space 'T2' + | ... +box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY, + CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i), + CONSTRAINT c UNIQUE (i));]]); + | --- + | - null + | - UNIQUE constraint 'C' already exists in space 'T2' + | ... +box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY, + CONSTRAINT c CHECK (i > 0), + CONSTRAINT c CHECK (i < 0));]]); + | --- + | - null + | - CHECK constraint 'C' already exists in space 'T2' + | ... +box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY, + CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i), + CONSTRAINT c CHECK (i > 0));]]); + | --- + | - null + | - FOREIGN KEY constraint 'C' already exists in space 'T2' + | ... +box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY CONSTRAINT c REFERENCES t1(i), + CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i))]]); + | --- + | - null + | - FOREIGN KEY constraint 'C' already exists in space 'T2' + | ... +test_run:cmd("setopt delimiter ''"); + | --- + | - true + | ... + +-- +-- Check a constraint name for duplicate using <ALTER TABLE> +-- statement. +-- +box.execute('CREATE TABLE t2 (i INT CONSTRAINT c PRIMARY KEY);') + | --- + | - row_count: 1 + | ... +box.execute('ALTER TABLE t2 ADD CONSTRAINT c CHECK(i > 0);') + | --- + | - null + | - PRIMARY KEY constraint 'C' already exists in space 'T2' + | ... +box.execute('ALTER TABLE t2 ADD CONSTRAINT c UNIQUE(i);') + | --- + | - null + | - Index 'C' already exists in space 'T2' + | ... +box.execute('ALTER TABLE t2 ADD CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i);') + | --- + | - null + | - PRIMARY KEY constraint 'C' already exists in space 'T2' + | ... + +-- +-- Make sure that a constraint's name isn't kept after the +-- constraint drop. +-- +box.execute('CREATE UNIQUE INDEX d ON t2(i);') + | --- + | - row_count: 1 + | ... +box.execute('DROP INDEX d ON t2;') + | --- + | - row_count: 1 + | ... +box.execute('ALTER TABLE t2 ADD CONSTRAINT d CHECK(i > 0);') + | --- + | - row_count: 1 + | ... +box.space.T2.ck_constraint.D:drop() + | --- + | ... +box.execute('ALTER TABLE t2 ADD CONSTRAINT d FOREIGN KEY(i) REFERENCES t1(i);') + | --- + | - row_count: 1 + | ... +box.execute('ALTER TABLE t2 DROP CONSTRAINT d;') + | --- + | - row_count: 1 + | ... + +-- +-- The same inside a transaction. +-- +box.begin() \ +box.execute('CREATE UNIQUE INDEX d ON t2(i);') \ +box.execute('DROP INDEX d ON t2;') \ +box.execute('ALTER TABLE t2 ADD CONSTRAINT d CHECK(i > 0);') \ +box.space.T2.ck_constraint.D:drop() \ +box.execute('ALTER TABLE t2 ADD CONSTRAINT d FOREIGN KEY(i) REFERENCES t1(i);') \ +box.execute('ALTER TABLE t2 DROP CONSTRAINT d;') \ +box.commit() + | --- + | ... + +-- +-- Circle renaming in a transaction. +-- +box.begin() \ +box.execute('CREATE UNIQUE INDEX d ON t2(i);') \ +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK (i > 0);') \ +-- D and E constraints exchange their names. Note, CHECK can't be \ +-- renamed, because CHECK name is a primary key of _ck_constraint. \ +-- It can be only dropped and created again with a new name. \ +box.space.T2.index.D:rename('F') \ +box.space.T2.ck_constraint.E:drop() \ +box.space.T2.index.F:rename('E') \ +box.execute('ALTER TABLE t2 ADD CONSTRAINT D CHECK (i > 0);') \ +-- Now an attempt to occupy the names will report, that the names \ +-- are really swapped. \ +_, err1 = box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK (i > 0);') \ +_, err2 = box.execute('CREATE UNIQUE INDEX d ON t2(i);') \ +box.rollback() + | --- + | ... +err1, err2 + | --- + | - UNIQUE constraint 'E' already exists in space 'T2' + | - CHECK constraint 'D' already exists in space 'T2' + | ... +-- After rollback ensure, that the names are free again. +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK (i > 0);') + | --- + | - row_count: 1 + | ... +box.execute('CREATE UNIQUE INDEX d ON t2(i);') + | --- + | - row_count: 1 + | ... +box.execute('DROP INDEX d ON t2;') + | --- + | - row_count: 1 + | ... +box.space.T2.ck_constraint.E:drop() + | --- + | ... + +-- +-- Make sure, that altering of an index name affects its record +-- in the space's constraint hash table. +-- +box.execute('CREATE UNIQUE INDEX d ON t2(i);') + | --- + | - row_count: 1 + | ... +box.space.T2.index.D:rename('E') + | --- + | ... +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);') + | --- + | - null + | - UNIQUE constraint 'E' already exists in space 'T2' + | ... + +-- +-- Try rename to an already occupied name. +-- +box.execute('ALTER TABLE t2 ADD CONSTRAINT f CHECK(i > 0);') + | --- + | - row_count: 1 + | ... +box.execute('CREATE UNIQUE INDEX g ON t2(i);') + | --- + | - row_count: 1 + | ... +box.space.T2.index.G:rename('F') + | --- + | - error: CHECK constraint 'F' already exists in space 'T2' + | ... +box.execute('DROP INDEX g ON t2;') + | --- + | - row_count: 1 + | ... +box.space.T2.ck_constraint.F:drop() + | --- + | ... + +-- +-- Make sure, that altering of an index uniqueness puts/drops +-- its name to/from the space's constraint hash table. +-- +box.space.T2.index.E:alter({unique = false}) + | --- + | ... +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);') + | --- + | - row_count: 1 + | ... +box.space.T2.index.E:alter({unique = true}) + | --- + | - error: CHECK constraint 'E' already exists in space 'T2' + | ... +box.space.T2.ck_constraint.E:drop() + | --- + | ... +box.space.T2.index.E:alter({unique = true}) + | --- + | ... +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);') + | --- + | - null + | - UNIQUE constraint 'E' already exists in space 'T2' + | ... + +-- Alter name and uniqueness of an unique index simultaneously. +box.space.T2.index.E:alter({name = 'D', unique = false}) + | --- + | ... +box.execute('CREATE UNIQUE INDEX e ON t2(i);') + | --- + | - row_count: 1 + | ... +-- The existing D index is not unique, but regardless of +-- uniqueness, index names should be unique in a space. +box.execute('CREATE UNIQUE INDEX d ON t2(i);') + | --- + | - null + | - Index 'D' already exists in space 'T2' + | ... + +-- +-- Cleanup. +-- +box.execute('DROP TABLE t2;') + | --- + | - row_count: 1 + | ... +box.execute('DROP TABLE t1;') + | --- + | - row_count: 1 + | ... diff --git a/test/sql/constraint.test.lua b/test/sql/constraint.test.lua new file mode 100755 index 000000000..ddaf09795 --- /dev/null +++ b/test/sql/constraint.test.lua @@ -0,0 +1,128 @@ +test_run = require('test_run').new() +engine = test_run:get_cfg('engine') +box.execute('pragma sql_default_engine=\''..engine..'\'') + +-- +-- Check a constraint name for duplicate within a single +-- <CREATE TABLE> statement. +-- +box.execute('CREATE TABLE t1 (i INT PRIMARY KEY);') +test_run:cmd("setopt delimiter ';'"); +box.execute([[CREATE TABLE t2 (i INT, CONSTRAINT c CHECK (i > 0), + CONSTRAINT c PRIMARY KEY (i));]]); +box.execute([[CREATE TABLE t2 (i INT, + CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i), + CONSTRAINT c PRIMARY KEY (i));]]); +box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY, + CONSTRAINT c CHECK (i > 0), + CONSTRAINT c UNIQUE (i));]]); +box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY, + CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i), + CONSTRAINT c UNIQUE (i));]]); +box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY, + CONSTRAINT c CHECK (i > 0), + CONSTRAINT c CHECK (i < 0));]]); +box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY, + CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i), + CONSTRAINT c CHECK (i > 0));]]); +box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY CONSTRAINT c REFERENCES t1(i), + CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i))]]); +test_run:cmd("setopt delimiter ''"); + +-- +-- Check a constraint name for duplicate using <ALTER TABLE> +-- statement. +-- +box.execute('CREATE TABLE t2 (i INT CONSTRAINT c PRIMARY KEY);') +box.execute('ALTER TABLE t2 ADD CONSTRAINT c CHECK(i > 0);') +box.execute('ALTER TABLE t2 ADD CONSTRAINT c UNIQUE(i);') +box.execute('ALTER TABLE t2 ADD CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i);') + +-- +-- Make sure that a constraint's name isn't kept after the +-- constraint drop. +-- +box.execute('CREATE UNIQUE INDEX d ON t2(i);') +box.execute('DROP INDEX d ON t2;') +box.execute('ALTER TABLE t2 ADD CONSTRAINT d CHECK(i > 0);') +box.space.T2.ck_constraint.D:drop() +box.execute('ALTER TABLE t2 ADD CONSTRAINT d FOREIGN KEY(i) REFERENCES t1(i);') +box.execute('ALTER TABLE t2 DROP CONSTRAINT d;') + +-- +-- The same inside a transaction. +-- +box.begin() \ +box.execute('CREATE UNIQUE INDEX d ON t2(i);') \ +box.execute('DROP INDEX d ON t2;') \ +box.execute('ALTER TABLE t2 ADD CONSTRAINT d CHECK(i > 0);') \ +box.space.T2.ck_constraint.D:drop() \ +box.execute('ALTER TABLE t2 ADD CONSTRAINT d FOREIGN KEY(i) REFERENCES t1(i);') \ +box.execute('ALTER TABLE t2 DROP CONSTRAINT d;') \ +box.commit() + +-- +-- Circle renaming in a transaction. +-- +box.begin() \ +box.execute('CREATE UNIQUE INDEX d ON t2(i);') \ +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK (i > 0);') \ +-- D and E constraints exchange their names. Note, CHECK can't be \ +-- renamed, because CHECK name is a primary key of _ck_constraint. \ +-- It can be only dropped and created again with a new name. \ +box.space.T2.index.D:rename('F') \ +box.space.T2.ck_constraint.E:drop() \ +box.space.T2.index.F:rename('E') \ +box.execute('ALTER TABLE t2 ADD CONSTRAINT D CHECK (i > 0);') \ +-- Now an attempt to occupy the names will report, that the names \ +-- are really swapped. \ +_, err1 = box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK (i > 0);') \ +_, err2 = box.execute('CREATE UNIQUE INDEX d ON t2(i);') \ +box.rollback() +err1, err2 +-- After rollback ensure, that the names are free again. +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK (i > 0);') +box.execute('CREATE UNIQUE INDEX d ON t2(i);') +box.execute('DROP INDEX d ON t2;') +box.space.T2.ck_constraint.E:drop() + +-- +-- Make sure, that altering of an index name affects its record +-- in the space's constraint hash table. +-- +box.execute('CREATE UNIQUE INDEX d ON t2(i);') +box.space.T2.index.D:rename('E') +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);') + +-- +-- Try rename to an already occupied name. +-- +box.execute('ALTER TABLE t2 ADD CONSTRAINT f CHECK(i > 0);') +box.execute('CREATE UNIQUE INDEX g ON t2(i);') +box.space.T2.index.G:rename('F') +box.execute('DROP INDEX g ON t2;') +box.space.T2.ck_constraint.F:drop() + +-- +-- Make sure, that altering of an index uniqueness puts/drops +-- its name to/from the space's constraint hash table. +-- +box.space.T2.index.E:alter({unique = false}) +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);') +box.space.T2.index.E:alter({unique = true}) +box.space.T2.ck_constraint.E:drop() +box.space.T2.index.E:alter({unique = true}) +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);') + +-- Alter name and uniqueness of an unique index simultaneously. +box.space.T2.index.E:alter({name = 'D', unique = false}) +box.execute('CREATE UNIQUE INDEX e ON t2(i);') +-- The existing D index is not unique, but regardless of +-- uniqueness, index names should be unique in a space. +box.execute('CREATE UNIQUE INDEX d ON t2(i);') + +-- +-- Cleanup. +-- +box.execute('DROP TABLE t2;') +box.execute('DROP TABLE t1;')
next prev parent reply other threads:[~2019-12-28 11:07 UTC|newest] Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-11-28 18:34 [Tarantool-patches] [PATCH v2 0/2] Add constraint names hash table to space Roman Khabibov 2019-11-28 18:34 ` [Tarantool-patches] [PATCH v2 1/2] box: introduce constraint names hash table Roman Khabibov 2019-11-30 1:03 ` Vladislav Shpilevoy 2019-12-04 16:23 ` [Tarantool-patches] [PATCH v2 1/3] " Roman Khabibov 2019-12-07 16:34 ` Vladislav Shpilevoy 2019-12-10 12:48 ` Roman Khabibov 2019-11-28 18:34 ` [Tarantool-patches] [PATCH v2 2/2] sql: make constraint operations transactional Roman Khabibov 2019-11-29 7:38 ` Roman Khabibov 2019-11-30 1:03 ` Vladislav Shpilevoy 2019-12-04 16:23 ` [Tarantool-patches] [PATCH v2 2/3] " Roman Khabibov 2019-12-05 18:43 ` Roman Khabibov 2019-12-07 16:35 ` Vladislav Shpilevoy 2019-12-10 12:49 ` [Tarantool-patches] [PATCH v2 2/3] box: " Roman Khabibov 2019-12-15 22:26 ` Vladislav Shpilevoy 2019-12-17 15:03 ` Roman Khabibov 2019-12-28 0:18 ` Nikita Pettik 2019-12-28 11:07 ` Vladislav Shpilevoy [this message] 2019-12-29 0:07 ` Nikita Pettik 2019-12-29 15:51 ` Vladislav Shpilevoy 2019-12-29 22:28 ` Nikita Pettik 2019-12-29 22:35 ` Vladislav Shpilevoy 2019-12-30 11:12 ` Sergey Ostanevich 2019-12-30 12:05 ` Nikita Pettik 2019-12-21 20:54 ` Sergey Ostanevich 2019-12-22 14:59 ` Vladislav Shpilevoy 2019-12-24 12:06 ` Roman Khabibov 2019-11-30 1:03 ` [Tarantool-patches] [PATCH v2 0/2] Add constraint names hash table to space Vladislav Shpilevoy 2019-12-04 16:23 ` [Tarantool-patches] [PATCH v2 0/3] " Roman Khabibov 2019-12-07 16:34 ` Vladislav Shpilevoy
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=b8edb143-c126-fe9b-893e-97ce6e8d8df3@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=korablev@tarantool.org \ --cc=roman.habibov@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v2 2/3] box: make constraint operations transactional' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox