From: Roman Khabibov <roman.habibov@tarantool.org> To: Sergey Ostanevich <sergos@tarantool.org> Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>, tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH v2 2/3] box: make constraint operations transactional Date: Tue, 17 Dec 2019 18:03:07 +0300 [thread overview] Message-ID: <88F2A14C-2D86-4C17-90F1-4B0388CF71C9@tarantool.org> (raw) In-Reply-To: <20007117-f560-e06d-16a5-67e811c8f9f8@tarantool.org> Hi! Vlad, thanks for the review. Sergos or Nikita, can you, please, do a second one? https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3503-constr-names_v3 > On Dec 16, 2019, at 01:26, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote: > > Hi! Thanks for the fixes! > >>> 2. This will fail in Release build mode, because >>> assert arguments are not calculated in Release. So you >>> delete the object, but it is still in the hash table. >> +void >> +CreateConstraintDef::rollback(struct alter_space *alter) >> +{ >> + struct constraint_def *constraint_def = >> + space_pop_constraint(alter->new_space, >> + new_constraint_def->name); >> + assert(constraint_def == new_constraint_def); >> + (void) alter; >> + constraint_def_delete(new_constraint_def); >> +} >> > > And it still fails in the release mode, on build stage. > https://travis-ci.org/tarantool/tarantool/builds/623163899?utm_source=github_status&utm_medium=notification > >>>> if (on_rollback == NULL) >>>> return -1; >>>> + struct constraint_def *constr_def = >>>> + constraint_def_new(child_space->def->id, >>>> + CONSTRAINT_TYPE_FK, >>>> + fk_def->name); >>>> + if (constr_def == NULL) >>>> + return -1; >>>> + if (space_put_constraint(child_space, constr_def) != 0) { >>>> + constraint_def_delete(constr_def); >>>> + return -1; >>>> + }> box: make constraint operations transactional >> >> Put constraint names into the space's hash table and drop them on >> replace in corresponding system spaces (_index, _fk_constraint, >> _ck_constraint). >> >> Closes #3503 >> >> @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 >> generated. And these names must be unique within one table/space. >> Naming in SQL is case-insensitive, so "CONSTRAINT c" and >> "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. >> >> Lua/box: >> >> You also can create/drop table constraints from box. See >> space_object:create_check_constraint() and space_object:create_index() >> (if an index is unique). Naming in box is case-sensitive, so 'c' and >> 'C' are not the same (see naming policy). For example, an unique >> index is a constraint, but a non-unique index is not. So, a non-unique >> index can have the same name with a check/foreign key constraint >> within one space: >> >> box.execute('CREATE TABLE t2 (i INT PRIMARY KEY);'); >> box.execute('CREATE INDEX e ON t2(i);'); >> box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);'); >> >> But, if you try to: >> >> box.space.T2.index.E:alter({unique = true}); >> >> You will get error, because index 'e' is becoming unique. >> > > The patch is getting better too slow, and 2.3.1 release is coming, > so I've finished it. I've pushed my review fixes on top of the > branch. Please, review my fixes. If you are ok with them, squash it > and all your 3 commits into 1 commit, and keep this commit message. > Then send to Nikita for a second review. > > General changes I made: > > - Renamed constraint_def -> constraint_id. Def is definition. Type > and name is not a definition actually. It is identifier consisting > of two parts. Definition includes CHECK expression, FOREIGN KEY > column list, etc. This is not a definition. By definition you can > create the object. By constraint_def it was not possible. > > - Removed space_id from constraint_id, because it is the same in all > constraint identifiers of a space. And it was not used anyway. > > - I removed ER_CK_CONSTRAINT_EXISTS and ER_FK_CONSTRAINT_EXISTS, > because appeared, that ER_CONSTRAINT_EXISTS works. Even with > the new format. I don't know what was a problem with it. > > - String names of constraint types are moved to constraint_id.c. > Because this is where enum is defined. And it is logical to put > the strings somewhere near. And it is consistent with all the > other 'strs' arrays. > > Other changes can be found in the 10 comments below. > > My review fixes are appended to the end of the email. I’m ok with it. I also fixed one use-after-free bug appeared after me. commit cb846d4ce625c2095f6607cfb126158e88ff7cf4 Author: Roman Khabibov <roman.habibov@tarantool.org> Date: Tue Nov 5 18:06:59 2019 +0300 box: make constraint operations transactional Put constraint names into the space's hash table and drop them on replace in corresponding system spaces (_index, _fk_constraint, _ck_constraint). Closes #3503 @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 generated. And these names must be unique within one table/space. Naming in SQL is case-insensitive, so "CONSTRAINT c" and "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. Lua/box: You also can create/drop table constraints from box. See space_object:create_check_constraint() and space_object:create_index() (if an index is unique). Naming in box is case-sensitive, so 'c' and 'C' are not the same (see naming policy). For example, an unique index is a constraint, but a non-unique index is not. So, a non-unique index can have the same name with a check/foreign key constraint within one space: box.execute('CREATE TABLE t2 (i INT PRIMARY KEY);'); box.execute('CREATE INDEX e ON t2(i);'); box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);'); But, if you try to: box.space.T2.index.E:alter({unique = true}); You will get error, because index 'e' is becoming unique. 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..2e4fa3c41 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 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) +{ + 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_existence(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_existence(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..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'") \ /*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..ac3ed000e 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, "Constraint FOREIGN KEY '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, "Constraint CHECK '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..488659e84 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 +- Constraint CHECK 'CK1' already exists in space 'T2' ... box.space.T2 --- diff --git a/test/sql/clear.result b/test/sql/clear.result index afa6520e8..988a972e7 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 +- Constraint CHECK '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 +- Constraint FOREIGN KEY '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 +- Constraint FOREIGN KEY '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..e452e5305 --- /dev/null +++ b/test/sql/constraint.result @@ -0,0 +1,214 @@ +-- 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 + | - Constraint PRIMARY KEY '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' + | ... +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' + | ... +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' + | ... +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' + | ... +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' + | ... +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' + | ... +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 + | - Constraint PRIMARY KEY '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 + | - Constraint PRIMARY KEY '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. +-- +test_run:cmd("setopt delimiter ';'"); + | --- + | - true + | ... +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(); + | --- + | ... +test_run:cmd("setopt delimiter ''"); + | --- + | - true + | ... + +-- +-- 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:alter({name = 'E'}) + | --- + | ... +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);') + | --- + | - null + | - Constraint UNIQUE 'E' already exists in space 'T2' + | ... + +-- +-- 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: Constraint CHECK '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 + | - Constraint UNIQUE '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 + | ... + +-- +-- 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..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..'\'') + +-- +-- 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. +-- +test_run:cmd("setopt delimiter ';'"); +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(); +test_run:cmd("setopt delimiter ''"); + +-- +-- 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);') + +-- +-- 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);') + +-- +-- Cleanup. +-- +box.execute('DROP TABLE t2;') +box.execute('DROP TABLE t1;')
next prev parent reply other threads:[~2019-12-17 15:03 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 [this message] 2019-12-28 0:18 ` Nikita Pettik 2019-12-28 11:07 ` Vladislav Shpilevoy 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=88F2A14C-2D86-4C17-90F1-4B0388CF71C9@tarantool.org \ --to=roman.habibov@tarantool.org \ --cc=sergos@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@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