[Tarantool-patches] [PATCH v2 2/3] box: make constraint operations transactional
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Mon Dec 16 01:26:24 MSK 2019
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.
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index bef25b605..d5e836ae7 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -1033,10 +1034,13 @@ 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);
> + SWAP(alter->new_space->constraints,
> + alter->old_space->constraints);
1. For the sake of similarity I wrapped it into a function
space_swap_constraint_ids().
> space_cache_replace(alter->new_space, alter->old_space);
> alter_space_delete(alter);
> return 0;
> @@ -1764,6 +1771,85 @@ MoveCkConstraints::rollback(struct alter_space *alter)
> space_swap_ck_constraint(alter->new_space, alter->old_space);
> }
>
> +/**
> + * CreateConstraintDef - add a new constraint def to the space.
> + */
> +class CreateConstraintDef: public AlterSpaceOp
> +{
> +private:
> + struct constraint_def *new_constraint_def;
> +public:
> + CreateConstraintDef(struct alter_space *alter,
> + struct constraint_def *def)
> + :AlterSpaceOp(alter), new_constraint_def(def)
> + {}
> + virtual void alter(struct alter_space *alter);
> + virtual void rollback(struct alter_space *alter);
> +};
> +
2. new_constraint_def leaks when CreateIndex::prepare() fails.
Because in that case rollback triggers are not called.
alter_space_do() sets rollback triggers only after all prepare()
and alter() have been finished successfully.
Fixed in my commit.
> +void
> +CreateConstraintDef::alter(struct alter_space *alter)
> +{
> + assert(new_constraint_def != NULL);
> + if (space_put_constraint(alter->old_space, new_constraint_def) != 0)
> + diag_raise();
3. Well, I was wrong. alter_space_do() forbids exceptions after
prepare() calls have been finished. It is said explicitly in the
alter_space_do() comments. It means, that you can't raise here.
On the other hand you can't move it to prepare(), because if your
prepare() is finished, and then CreateIndex::prepare() fails,
then the new constraint stays in the space. Prepare() can't change
any state.
I changed it to panic().
> +}
> +
> +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;
4. Why (void) alter? It is used here. Even in the release mode.
I dropped it.
> + constraint_def_delete(new_constraint_def);
> +}
> +
> +/** DropConstraintDef - drop a constraint def from the space. */
> +class DropConstraintDef: public AlterSpaceOp
> +{
> +private:
> + struct constraint_def *old_constraint_def;
> + const char *name;
> +public:
> + DropConstraintDef(struct alter_space *alter, const char *name)
> + :AlterSpaceOp(alter), old_constraint_def(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
> +DropConstraintDef::alter(struct alter_space *alter)
> +{
> + assert(name != NULL);
> + old_constraint_def =
> + space_constraint_def_by_name(alter->old_space, name);
> + assert(old_constraint_def != NULL);
> + space_pop_constraint(alter->old_space, name);
5. This is exactly why I asked you to add pop() - to avoid
space_constraint_def_by_name(). I removed the latter. In some
other places too.
> +}
> +
> +void
> +DropConstraintDef::commit(struct alter_space *alter, int64_t signature)
> +{
> + (void) alter;
> + (void) signature;
> + assert(old_constraint_def != NULL);
> @@ -2363,6 +2449,76 @@ index_is_used_by_fk_constraint(struct rlist *fk_list, uint32_t iid)
> return false;
> }
>
> +/**
> + * Check if constraint with @a name exists within @a space. Call
> + * diag_set() if it is so.
> + *
> + * @param space Space to find constraint within.
> + * @param name Constraint name.
> + *
> + * @retval -1 Constraint exists.
> + * @retval 0 Doesn't exist.
> + */
> +static inline int
> +check_existence(struct space *space, const char *name)
> +{
6. Check existence of what? You do realize that in the
code I don't see comments or function's body, and it looks
like 'check_existence(...)'? From that line it is totally
unclear existence of what you are trying to check.
Changed to space_check_constraint_existence(), and used in
CreateConstraintID::prepare().
> + struct constraint_def *def = space_constraint_def_by_name(space, name);
> + if (def != NULL) {
> + diag_set(ClientError, ER_CONSTRAINT_EXISTS, name);
> + return -1;
> + }
> + return 0;
> +}
> +
> +/**
> + * Put the node of an unique index to the constraint hash table of
> + * @a space.
> + *
> + * This function is needed to wrap the duplicated piece of code
> + * inside on_replace_dd_index().
> + */
> +static int
> +create_index_as_constraint(struct alter_space *alter,
> + const struct index_def *def)
> +{
> + assert(def->opts.is_unique);
> + struct space *space = alter->old_space;
> + if (check_existence(space, def->name) != 0)
> + return -1;
> + struct constraint_def *constr_def =
> + constraint_def_new(space->def->id, def->iid == 0 ?
> + CONSTRAINT_TYPE_PK : CONSTRAINT_TYPE_UNIQUE,
> + def->name);
> + if (constr_def == NULL)
> + return -1;
> + try {
> + (void) new CreateConstraintDef(alter, constr_def);
> + } catch (Exception *e) {
> + constraint_def_delete(constr_def);
> + return -1;
> + }
> + return 0;
> +}
> +
> +/**
> + * Drop the node of an unique index from the constraint hash table
> + * of @a space.
> + *
> + * This function is needed to wrap the duplicated piece of code
> + * inside on_replace_dd_index().
> + */
> +static int
> +drop_index_as_constraint(struct alter_space *alter, const struct index_def *def)
> +{
> + assert(def->opts.is_unique);
> + try {
> + (void) new DropConstraintDef(alter, def->name);
> + } catch (Exception *e) {
> + return -1;
> + }
> + return 0;
> +}
7. I moved construction of constraint_id to CreateConstraintID()
constructor, and removed these two functions.
> +
> /**
> * Just like with _space, 3 major cases:
> *
> @@ -4984,6 +5166,48 @@ space_reset_fk_constraint_mask(struct space *space)
> }
> }
>
> +/**
> + * Put the node of a ck/fk constraint to the constraint hash table
> + * of @a space.
> + *
> + * This function is needed to wrap the duplicated piece of code
> + * inside the trigger functions below.
> + */
> +static int
> +create_fk_ck_as_constraint(struct space *space, enum constraint_type type,
> + const char *name)
> +{
> + assert(type == CONSTRAINT_TYPE_CK || type == CONSTRAINT_TYPE_FK);
> + struct constraint_def *constraint_def =
> + constraint_def_new(space->def->id, type, name);
> + if (constraint_def == NULL)
> + return -1;
> + if (space_put_constraint(space, constraint_def) != 0) {
> + constraint_def_delete(constraint_def);
> + return -1;
> + }
> + return 0;
> +}
> +
> +/**
> + * Drop the node of a ck/fk constraint from the constraint hash
> + * table of @a space.
> + *
> + * This function is needed to wrap the duplicated piece of code
> + * inside the trigger functions below.
> + */
> +static void
> +drop_fk_ck_as_constraint(struct space *space, const char *name)
> +{
> + struct constraint_def *constraint_def =
> + space_constraint_def_by_name(space, name);
> + assert(constraint_def != NULL);
> + assert(constraint_def->type == CONSTRAINT_TYPE_CK ||
> + constraint_def->type == CONSTRAINT_TYPE_FK);
> + space_pop_constraint(space, name);
> + constraint_def_delete(constraint_def);
> +}
8. FK and CK are constraints, so this is strange to
add 'as constraint' suffix. Also there is no reason
why they should be called 'fk_ck', because in fact
don't use anything specific to FK and CK.
I changed them to space_insert_constraint_id() and
space_delete_constraint_id().
> +
> /**
> * On rollback of creation we remove FK constraint from DD, i.e.
> * from parent's and child's lists of constraints and
> @@ -5029,8 +5256,14 @@ 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);
> + assert(child != NULL && parent != NULL);
9. Why did you add this assertion? You don't even need parent
space here (in your diff).
Dropped.
> rlist_add_entry(&child->child_fk_constraint, old_fk, in_child_space);
> rlist_add_entry(&parent->parent_fk_constraint, old_fk, in_parent_space);
> + if (create_fk_ck_as_constraint(child, CONSTRAINT_TYPE_FK,
> + old_fk->def->name) != 0) {
> + panic("Can't recover after FK constraint drop rollback (out of "
> + "memory)");
> + }
> fk_constraint_set_mask(old_fk, &child->fk_constraint_mask,
> FIELD_LINK_CHILD);
> fk_constraint_set_mask(old_fk, &parent->fk_constraint_mask,> @@ -5491,10 +5734,15 @@ 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)
> + if (is_insert && check_existence(space, name) != 0)
> + return -1;
> + if (space_add_ck_constraint(space, new_ck_constraint) != 0 ||
> + (is_insert &&
> + create_fk_ck_as_constraint(space, CONSTRAINT_TYPE_CK,
> + name) != 0))
10. Now if space_add_ck_constraint() succeeds and create_fk_ck_as_constraint()
does not, the added ck is not removed.
Fixed in my commit. See it and some explanations below.
================================================================================
commit f45592dd6994c03f6cc7f348ce4c524289c6a8e6
Author: Vladislav Shpilevoy <v.shpilevoy at tarantool.org>
Date: Sun Dec 15 20:07:29 2019 +0100
Review fixes
diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index bf3895262..19ce3d481 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -102,7 +102,7 @@ add_library(box STATIC
sequence.c
ck_constraint.c
fk_constraint.c
- constraint_def.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 b1ef7ec4f..300b63712 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -57,7 +57,7 @@
#include "version.h"
#include "sequence.h"
#include "sql.h"
-#include "constraint_def.h"
+#include "constraint_id.h"
/* {{{ Auxiliary functions and methods. */
@@ -704,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.
@@ -1039,8 +1045,7 @@ alter_space_rollback(struct trigger *trigger, void * /* event */)
*/
space_swap_triggers(alter->new_space, alter->old_space);
space_swap_fk_constraints(alter->new_space, alter->old_space);
- SWAP(alter->new_space->constraints,
- alter->old_space->constraints);
+ 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;
@@ -1152,8 +1157,7 @@ alter_space_do(struct txn_stmt *stmt, struct alter_space *alter)
*/
space_swap_triggers(alter->new_space, alter->old_space);
space_swap_fk_constraints(alter->new_space, alter->old_space);
- SWAP(alter->new_space->constraints,
- alter->old_space->constraints);
+ space_swap_constraint_ids(alter->new_space, alter->old_space);
/*
* The new space is ready. Time to update the space
* cache with it.
@@ -1415,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);
@@ -1772,49 +1776,115 @@ MoveCkConstraints::rollback(struct alter_space *alter)
}
/**
- * CreateConstraintDef - add a new constraint def to the space.
+ * Check if constraint with @a name exists within @a space. Call
+ * diag_set() if it is so.
*/
-class CreateConstraintDef: public AlterSpaceOp
+static inline int
+space_check_constraint_existence(struct space *space, const char *name)
{
-private:
- struct constraint_def *new_constraint_def;
+ 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:
- CreateConstraintDef(struct alter_space *alter,
- struct constraint_def *def)
- :AlterSpaceOp(alter), new_constraint_def(def)
- {}
+ 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
-CreateConstraintDef::alter(struct alter_space *alter)
+CreateConstraintID::prepare(struct alter_space *alter)
{
- assert(new_constraint_def != NULL);
- if (space_put_constraint(alter->old_space, new_constraint_def) != 0)
+ if (space_check_constraint_existence(alter->old_space,
+ new_id->name) != 0)
diag_raise();
}
void
-CreateConstraintDef::rollback(struct alter_space *alter)
+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)
{
- 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);
+ (void) signature;
+ /*
+ * Constraint id is added to the space, and should not be
+ * deleted from now on.
+ */
+ new_id = NULL;
}
-/** DropConstraintDef - drop a constraint def from the space. */
-class DropConstraintDef: public AlterSpaceOp
+CreateConstraintID::~CreateConstraintID()
{
-private:
- struct constraint_def *old_constraint_def;
+ 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:
- DropConstraintDef(struct alter_space *alter, const char *name)
- :AlterSpaceOp(alter), old_constraint_def(NULL), name(name)
+ 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);
@@ -1822,29 +1892,23 @@ public:
};
void
-DropConstraintDef::alter(struct alter_space *alter)
+DropConstraintID::alter(struct alter_space *alter)
{
- assert(name != NULL);
- old_constraint_def =
- space_constraint_def_by_name(alter->old_space, name);
- assert(old_constraint_def != NULL);
- space_pop_constraint(alter->old_space, name);
+ old_id = space_pop_constraint_id(alter->old_space, name);
}
void
-DropConstraintDef::commit(struct alter_space *alter, int64_t signature)
+DropConstraintID::commit(struct alter_space *alter, int64_t signature)
{
(void) alter;
(void) signature;
- assert(old_constraint_def != NULL);
- constraint_def_delete(old_constraint_def);
+ constraint_id_delete(old_id);
}
void
-DropConstraintDef::rollback(struct alter_space *alter)
+DropConstraintID::rollback(struct alter_space *alter)
{
- assert(old_constraint_def != NULL);
- if (space_put_constraint(alter->new_space, old_constraint_def) != 0) {
+ if (space_add_constraint_id(alter->new_space, old_id) != 0) {
panic("Can't recover after constraint drop rollback (out of "
"memory)");
}
@@ -2449,95 +2513,6 @@ index_is_used_by_fk_constraint(struct rlist *fk_list, uint32_t iid)
return false;
}
-/**
- * Just return string with constraint type to print it in an error
- * message.
- */
-static inline const char *
-constraint_type_str(struct constraint_def *def)
-{
- static const char *type_str[CONSTRAINT_TYPE_MAX] = {
- [CONSTRAINT_TYPE_PK] = "PRIMARY KEY",
- [CONSTRAINT_TYPE_UNIQUE] = "UNIQUE",
- [CONSTRAINT_TYPE_FK] = "FOREIGN KEY",
- [CONSTRAINT_TYPE_CK] = "CHECK",
- };
-
- return def->type <= CONSTRAINT_TYPE_MAX ? type_str[def->type] :
- "UNKNOWN";
-}
-
-/**
- * Check if constraint with @a name exists within @a space. Call
- * diag_set() if it is so.
- *
- * @param space Space to find constraint within.
- * @param name Constraint name.
- *
- * @retval -1 Constraint exists.
- * @retval 0 Doesn't exist.
- */
-static inline int
-check_existence(struct space *space, const char *name)
-{
- struct constraint_def *def = space_constraint_def_by_name(space, name);
- if (def != NULL) {
- diag_set(ClientError, ER_CONSTRAINT_EXISTS,
- constraint_type_str(def), name, space_name(space));
- return -1;
- }
- return 0;
-}
-
-/**
- * Put the node of an unique index to the constraint hash table of
- * @a space.
- *
- * This function is needed to wrap the duplicated piece of code
- * inside on_replace_dd_index().
- */
-static int
-create_index_as_constraint(struct alter_space *alter,
- const struct index_def *def)
-{
- assert(def->opts.is_unique);
- struct space *space = alter->old_space;
- if (check_existence(space, def->name) != 0)
- return -1;
- struct constraint_def *constr_def =
- constraint_def_new(space->def->id, def->iid == 0 ?
- CONSTRAINT_TYPE_PK : CONSTRAINT_TYPE_UNIQUE,
- def->name);
- if (constr_def == NULL)
- return -1;
- try {
- (void) new CreateConstraintDef(alter, constr_def);
- } catch (Exception *e) {
- constraint_def_delete(constr_def);
- return -1;
- }
- return 0;
-}
-
-/**
- * Drop the node of an unique index from the constraint hash table
- * of @a space.
- *
- * This function is needed to wrap the duplicated piece of code
- * inside on_replace_dd_index().
- */
-static int
-drop_index_as_constraint(struct alter_space *alter, const struct index_def *def)
-{
- assert(def->opts.is_unique);
- try {
- (void) new DropConstraintDef(alter, def->name);
- } catch (Exception *e) {
- return -1;
- }
- return 0;
-}
-
/**
* Just like with _space, 3 major cases:
*
@@ -2603,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.
@@ -2675,10 +2651,11 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)
}
if (alter_space_move_indexes(alter, 0, iid) != 0)
return -1;
- if (old_index->def->opts.is_unique &&
- drop_index_as_constraint(alter, old_index->def) != 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;
@@ -2692,20 +2669,18 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)
index_def_new_from_tuple(new_tuple, old_space);
if (def == NULL)
return -1;
- if (def->opts.is_unique &&
- create_index_as_constraint(alter, def) != 0) {
- index_def_delete(def);
- return -1;
- }
- CreateIndex *create_index;
+ 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 = def;
- index_def_update_optionality(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) {
@@ -2715,23 +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); });
- struct index_def *old_def = old_index->def;
/*
* We put a new name when either an index is
- * becoming unique (i.e. constraint), or when an
+ * becoming unique (i.e. constraint), or when a
================================================================================
'a unique', not 'an unique'. Because 'u' is ЙУ. So the first
sound is consonant.
================================================================================
* unique index's name is under change.
*/
- if (!old_def->opts.is_unique && index_def->opts.is_unique &&
- create_index_as_constraint(alter, index_def) != 0)
- return -1;
+ 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 &&
- (create_index_as_constraint(alter, index_def) != 0 ||
- drop_index_as_constraint(alter, old_def) != 0))
- return -1;
- if (old_def->opts.is_unique && !index_def->opts.is_unique &&
- drop_index_as_constraint(alter, old_def) != 0)
+ 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
@@ -5185,48 +5171,6 @@ space_reset_fk_constraint_mask(struct space *space)
}
}
-/**
- * Put the node of a ck/fk constraint to the constraint hash table
- * of @a space.
- *
- * This function is needed to wrap the duplicated piece of code
- * inside the trigger functions below.
- */
-static int
-create_fk_ck_as_constraint(struct space *space, enum constraint_type type,
- const char *name)
-{
- assert(type == CONSTRAINT_TYPE_CK || type == CONSTRAINT_TYPE_FK);
- struct constraint_def *constraint_def =
- constraint_def_new(space->def->id, type, name);
- if (constraint_def == NULL)
- return -1;
- if (space_put_constraint(space, constraint_def) != 0) {
- constraint_def_delete(constraint_def);
- return -1;
- }
- return 0;
-}
-
-/**
- * Drop the node of a ck/fk constraint from the constraint hash
- * table of @a space.
- *
- * This function is needed to wrap the duplicated piece of code
- * inside the trigger functions below.
- */
-static void
-drop_fk_ck_as_constraint(struct space *space, const char *name)
-{
- struct constraint_def *constraint_def =
- space_constraint_def_by_name(space, name);
- assert(constraint_def != NULL);
- assert(constraint_def->type == CONSTRAINT_TYPE_CK ||
- constraint_def->type == CONSTRAINT_TYPE_FK);
- space_pop_constraint(space, name);
- constraint_def_delete(constraint_def);
-}
-
/**
* On rollback of creation we remove FK constraint from DD, i.e.
* from parent's and child's lists of constraints and
@@ -5241,7 +5185,7 @@ on_create_fk_constraint_rollback(struct trigger *trigger, void *event)
rlist_del_entry(fk, in_child_space);
struct space *child = space_by_id(fk->def->child_id);
assert(child != NULL);
- drop_fk_ck_as_constraint(child, fk->def->name);
+ 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(child);
fk_constraint_delete(fk);
@@ -5275,14 +5219,13 @@ 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);
- assert(child != NULL && parent != NULL);
- rlist_add_entry(&child->child_fk_constraint, old_fk, in_child_space);
- rlist_add_entry(&parent->parent_fk_constraint, old_fk, in_parent_space);
- if (create_fk_ck_as_constraint(child, CONSTRAINT_TYPE_FK,
+ 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);
================================================================================
I moved rlists down, because you added create_fk_ck_as_constraint() right in the
middle of FK creation. It is better not to scramble code.
================================================================================
fk_constraint_set_mask(old_fk, &child->fk_constraint_mask,
FIELD_LINK_CHILD);
fk_constraint_set_mask(old_fk, &parent->fk_constraint_mask,
@@ -5462,14 +5405,12 @@ 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) {
- if (check_existence(child_space, fk_def->name) != 0)
- return -1;
================================================================================
I moved the check to space_insert_constraint_id(), because it was also
repeated.
================================================================================
struct trigger *on_rollback =
txn_alter_trigger_new(on_create_fk_constraint_rollback,
fk);
if (on_rollback == NULL)
return -1;
- if (create_fk_ck_as_constraint(child_space,
+ if (space_insert_constraint_id(child_space,
CONSTRAINT_TYPE_FK,
fk_def->name) != 0)
return -1;
@@ -5537,7 +5478,7 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event)
old_fk);
if (on_rollback == NULL)
return -1;
- drop_fk_ck_as_constraint(child_space, fk_def->name);
+ 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);
@@ -5606,8 +5547,8 @@ on_create_ck_constraint_rollback(struct trigger *trigger, void * /* event */)
const char *name = ck->def->name;
assert(space_ck_constraint_by_name(space, name, strlen(name)) != NULL);
space_remove_ck_constraint(space, ck);
- drop_fk_ck_as_constraint(space, name);
ck_constraint_delete(ck);
+ space_delete_constraint_id(space, name);
if (trigger_run(&on_alter_space, space) != 0)
return -1;
return 0;
@@ -5634,7 +5575,7 @@ on_drop_ck_constraint_rollback(struct trigger *trigger, void * /* event */)
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 ||
- create_fk_ck_as_constraint(space, CONSTRAINT_TYPE_CK, name) != 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;
@@ -5753,23 +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 (is_insert && check_existence(space, name) != 0)
- return -1;
- if (space_add_ck_constraint(space, new_ck_constraint) != 0 ||
- (is_insert &&
- create_fk_ck_as_constraint(space, CONSTRAINT_TYPE_CK,
- name) != 0))
+ if (space_add_ck_constraint(space, new_ck_constraint) != 0)
return -1;
- if (!is_insert)
+ if (!is_insert) {
rlist_del_entry(old_ck_constraint, link);
- ck_guard.is_active = false;
- if (old_tuple != NULL) {
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 {
@@ -5783,7 +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);
- drop_fk_ck_as_constraint(space, old_ck_constraint->def->name);
+ 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_def.c b/src/box/constraint_id.c
similarity index 77%
rename from src/box/constraint_def.c
rename to src/box/constraint_id.c
index 1d4367532..ba6ed859c 100755
--- a/src/box/constraint_def.c
+++ b/src/box/constraint_id.c
@@ -29,33 +29,35 @@
* THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*/
-
-#include "constraint_def.h"
+#include "constraint_id.h"
#include "assoc.h"
#include "errcode.h"
#include "diag.h"
-#include <string.h>
-#include <stdlib.h>
-struct constraint_def *
-constraint_def_new(uint32_t space_id, enum constraint_type type,
- const char *name)
+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_def) + len + 1;
- struct constraint_def *ret = malloc(size);
+ 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->space_id = space_id;
ret->type = type;
memcpy(ret->name, name, len + 1);
return ret;
}
void
-constraint_def_delete(struct constraint_def *def)
+constraint_id_delete(struct constraint_id *id)
{
- free(def);
+ free(id);
}
diff --git a/src/box/constraint_def.h b/src/box/constraint_id.h
similarity index 74%
rename from src/box/constraint_def.h
rename to src/box/constraint_id.h
index d1b1a725a..21f067cdc 100755
--- a/src/box/constraint_def.h
+++ b/src/box/constraint_id.h
@@ -31,9 +31,6 @@
* THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*/
-
-#include <stdint.h>
-
#if defined(__cplusplus)
extern "C" {
#endif
@@ -43,40 +40,24 @@ enum constraint_type {
CONSTRAINT_TYPE_UNIQUE,
CONSTRAINT_TYPE_FK,
CONSTRAINT_TYPE_CK,
-
- CONSTRAINT_TYPE_MAX,
};
-struct constraint_def {
- /** Space id. */
- uint32_t space_id;
+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 def object.
- *
- * @param space_id Space id.
- * @param type Constraint type.
- * @param name Constraint name.
- *
- * @retval ptr Constraint def object.
- * @retval NULL Memory allocation error.
- */
-struct constraint_def *
-constraint_def_new(uint32_t space_id, enum constraint_type type,
- const char *name);
+/** Allocate memory and construct constraint id. */
+struct constraint_id *
+constraint_id_new(enum constraint_type type, const char *name);
-/**
- * Free memory of constraint def object.
- *
- * @param def Constraint def.
- */
+/** Free memory of constraint id. */
void
-constraint_def_delete(struct constraint_def *def);
+constraint_id_delete(struct constraint_id *id);
#if defined(__cplusplus)
}
diff --git a/src/box/errcode.h b/src/box/errcode.h
index 658f64e9b..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, "%s constraint '%s' already exists within space '%s'") \
+ /*170 */_(ER_CONSTRAINT_EXISTS, "Constraint %s '%s' already exists in space '%s'") \
================================================================================
Error message is changed to conform with other similar messages:
<object type> <name> already exists in space <name>.
================================================================================
/*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") \
@@ -258,8 +258,6 @@ struct errcode_record {
/*203 */_(ER_BOOTSTRAP_READONLY, "Trying to bootstrap a local read-only instance as master") \
/*204 */_(ER_SQL_FUNC_WRONG_RET_COUNT, "SQL expects exactly one argument returned from %s, got %d")\
/*205 */_(ER_FUNC_INVALID_RETURN_TYPE, "Function '%s' returned value of invalid type: expected %s got %s") \
- /*206 */_(ER_CK_CONSTRAINT_EXISTS, "CHECK constraint '%s' already exists within space '%s'") \
- /*207 */_(ER_FK_CONSTRAINT_EXISTS, "FOREIGN KEY constraint '%s' already exists within space '%s'") \
/*
* !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/space.c b/src/box/space.c
index cfae93cf1..1c19d099b 100644
--- a/src/box/space.c
+++ b/src/box/space.c
@@ -46,7 +46,7 @@
#include "schema.h"
#include "ck_constraint.h"
#include "assoc.h"
-#include "constraint_def.h"
+#include "constraint_id.h"
int
access_check_space(struct space *space, user_access_t access)
@@ -204,11 +204,10 @@ space_create(struct space *space, struct engine *engine,
}
}
}
-
- space->constraints = mh_strnptr_new();
- if (space->constraints == NULL) {
- diag_set(OutOfMemory, sizeof(*space->constraints), "malloc",
- "constraints");
+ 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;
@@ -271,8 +270,8 @@ space_delete(struct space *space)
* on_replace_dd_ triggers on deletion from corresponding
* system space.
*/
- assert(mh_size(space->constraints) == 0);
- mh_strnptr_delete(space->constraints);
+ 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));
@@ -629,43 +628,43 @@ space_remove_ck_constraint(struct space *space, struct ck_constraint *ck)
}
}
-struct constraint_def *
-space_constraint_def_by_name(struct space *space, const char *name)
+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(space->constraints, name, len);
- if (pos == mh_end(space->constraints))
+ mh_int_t pos = mh_strnptr_find_inp(ids, name, len);
+ if (pos == mh_end(ids))
return NULL;
- return (struct constraint_def *)
- mh_strnptr_node(space->constraints, pos)->val;
+ return (struct constraint_id *) mh_strnptr_node(ids, pos)->val;
}
int
-space_put_constraint(struct space *space, struct constraint_def *def)
-{
- uint32_t len = strlen(def->name);
- uint32_t hash = mh_strn_hash(def->name, len);
- const struct mh_strnptr_node_t name_node = { def->name, len, hash, def};
- if (mh_strnptr_put(space->constraints, &name_node, NULL, NULL) ==
- mh_end(space->constraints)) {
- diag_set(OutOfMemory, sizeof(name_node), "malloc",
- "constraints");
+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_def *
-space_pop_constraint(struct space *space, const char *name)
+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(space->constraints, name, len);
- if (pos == mh_end(space->constraints))
- return NULL;
================================================================================
Pop() is never called not knowing in advance that the constraint exists.
================================================================================
- struct constraint_def *def = (struct constraint_def *)
- mh_strnptr_node(space->constraints, pos)->val;
- mh_strnptr_del(space->constraints, pos, NULL);
- return def;
+ 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 */
diff --git a/src/box/space.h b/src/box/space.h
index 08b5b4777..9aea4e5be 100644
--- a/src/box/space.h
+++ b/src/box/space.h
@@ -52,7 +52,7 @@ struct port;
struct tuple;
struct tuple_format;
struct ck_constraint;
-struct constraint_def;
+struct constraint_id;
struct space_vtab {
/** Free a space instance. */
@@ -235,8 +235,10 @@ struct space {
* of parent constraints as well as child ones.
*/
uint64_t fk_constraint_mask;
- /** Hash table with constraint def objects by name. */
- struct mh_strnptr_t *constraints;
+ /**
+ * Hash table with constraint identifiers hashed by name.
+ */
+ struct mh_strnptr_t *constraint_ids;
================================================================================
This is not constraints themselves. The constraint objects are
stored in other members. This is why I renamed it to ids.
================================================================================
};
/** Initialize a base space instance. */
@@ -519,44 +521,24 @@ space_add_ck_constraint(struct space *space, struct ck_constraint *ck);
void
space_remove_ck_constraint(struct space *space, struct ck_constraint *ck);
-/**
- * Find node with @a name in the constraint hash table of @a
- * space.
- *
- * @param space Space.
- * @param name Constraint name.
- *
- * @retval constraint_def object.
- * @retval NULL Constraint doesn't exist.
- */
-struct constraint_def *
-space_constraint_def_by_name(struct space *space, const char *name);
+/** Find a constraint identifier by name. */
+struct constraint_id *
+space_find_constraint_id(struct space *space, const char *name);
/**
- * Put node with @a def to the constraint hash table of @a space.
- *
- * @param space Space.
- * @param def Constraint def.
- *
- * @retval 0 Success.
- * @retval -1 Memory allocation error.
+ * 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_put_constraint(struct space *space, struct constraint_def *def);
+space_add_constraint_id(struct space *space, struct constraint_id *id);
/**
- * Remove node with @a name from the constraint hash table of @a
- * space. But don't destroy the constraint def object binded to
- * this @a name.
- *
- * @param space Space.
- * @param name Constraint name.
- *
- * @retval constraint_def object.
- * @retval NULL Constraint doesn't exist.
+ * Remove a given name from the hash of all constraint
+ * identifiers of the given space.
*/
-struct constraint_def *
-space_pop_constraint(struct space *space, const char *name);
+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 556df7786..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)
@@ -970,7 +971,9 @@ 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,
@@ -996,13 +999,13 @@ vdbe_emit_ck_constraint_create(struct Parse *parser,
sqlVdbeAddOp3(v, OP_MakeRecord, ck_constraint_reg, 6,
ck_constraint_reg + 6);
const char *error_msg =
- tt_sprintf(tnt_errcode_desc(ER_CK_CONSTRAINT_EXISTS),
- ck_def->name, space_name);
+ tt_sprintf(tnt_errcode_desc(ER_CONSTRAINT_EXISTS),
+ 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_CK_CONSTRAINT_EXISTS,
- error_msg, false,
- OP_NoConflict) != 0)
+ ER_CONSTRAINT_EXISTS, error_msg,
+ false, OP_NoConflict) != 0)
================================================================================
This being squashed with previous commits will result into 0 diff.
================================================================================
return;
sqlVdbeAddOp2(v, OP_SInsert, BOX_CK_CONSTRAINT_ID,
ck_constraint_reg + 6);
@@ -1016,6 +1019,8 @@ 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,
@@ -1061,14 +1066,14 @@ vdbe_emit_fk_constraint_create(struct Parse *parse_context,
* been created before.
*/
const char *error_msg =
- tt_sprintf(tnt_errcode_desc(ER_FK_CONSTRAINT_EXISTS),
- name_copy, space_name);
+ 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,
- ER_FK_CONSTRAINT_EXISTS,
- error_msg, false,
- OP_NoConflict) != 0)
+ ER_CONSTRAINT_EXISTS, error_msg,
+ false, OP_NoConflict) != 0)
return;
sqlVdbeAddOp2(vdbe, OP_Bool, fk->is_deferred, constr_tuple_reg + 3);
sqlVdbeAddOp4(vdbe, OP_String8, 0, constr_tuple_reg + 4, 0,
diff --git a/test/box/misc.result b/test/box/misc.result
index c343726da..d2a20307a 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -554,8 +554,6 @@ t;
203: box.error.BOOTSTRAP_READONLY
204: box.error.SQL_FUNC_WRONG_RET_COUNT
205: box.error.FUNC_INVALID_RETURN_TYPE
- 206: box.error.CK_CONSTRAINT_EXISTS
- 207: box.error.FK_CONSTRAINT_EXISTS
...
test_run:cmd("setopt delimiter ''");
---
diff --git a/test/sql-tap/alter2.test.lua b/test/sql-tap/alter2.test.lua
index f1b4ff660..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, "FOREIGN KEY constraint 'FK' already exists within space 'CHILD'"
+ 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, "CHECK constraint 'CK' already exists within space 'T1'" })
+ ]], { 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 afd7c3d25..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
-- CHECK constraint 'CK1' already exists within space 'T2'
+- Constraint CHECK 'CK1' already exists in space 'T2'
...
box.space.T2
---
diff --git a/test/sql/clear.result b/test/sql/clear.result
index 90070811a..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
-- CHECK constraint 'CK1' already exists within space 'T1'
+- 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
-- FOREIGN KEY constraint 'FK1' already exists within space 'T2'
+- 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
-- FOREIGN KEY constraint 'CK1' already exists within space 'T3'
+- Constraint FOREIGN KEY 'CK1' already exists in space 'T3'
...
box.space.t1
---
diff --git a/test/sql/constraint.result b/test/sql/constraint.result
index 7383acdac..e452e5305 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
- | - PRIMARY KEY constraint 'C' already exists within space 'T2'
+ | - 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
- | - PRIMARY KEY constraint 'C' already exists within space 'T2'
+ | - 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
- | - UNIQUE constraint 'C' already exists within space 'T2'
+ | - 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
- | - UNIQUE constraint 'C' already exists within space 'T2'
+ | - 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
- | - CHECK constraint 'C' already exists within space 'T2'
+ | - 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
- | - FOREIGN KEY constraint 'C' already exists within space 'T2'
+ | - 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
- | - FOREIGN KEY constraint 'C' already exists within space 'T2'
+ | - Constraint FOREIGN KEY '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
- | - PRIMARY KEY constraint 'C' already exists within space 'T2'
+ | - Constraint PRIMARY KEY '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
- | - PRIMARY KEY constraint 'C' already exists within space 'T2'
+ | - Constraint PRIMARY KEY 'C' already exists in space 'T2'
| ...
--
@@ -149,7 +149,7 @@ test_run:cmd("setopt delimiter ''");
| ...
--
--- Make sure, that altering of an index name affect to its record
+-- 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);')
@@ -162,7 +162,7 @@ box.space.T2.index.D:alter({name = 'E'})
box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);')
| ---
| - null
- | - UNIQUE constraint 'E' already exists within space 'T2'
+ | - Constraint UNIQUE '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: CHECK constraint 'E' already exists within space 'T2'
+ | - error: Constraint CHECK '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
- | - UNIQUE constraint 'E' already exists within space 'T2'
+ | - Constraint UNIQUE 'E' already exists in space 'T2'
| ...
-- Alter name and uniqueness of an unique index simultaneously.
diff --git a/test/sql/constraint.test.lua b/test/sql/constraint.test.lua
index e321b87aa..b41e16dc2 100755
--- a/test/sql/constraint.test.lua
+++ b/test/sql/constraint.test.lua
@@ -64,7 +64,7 @@ box.commit();
test_run:cmd("setopt delimiter ''");
--
--- Make sure, that altering of an index name affect to its record
+-- 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);')
More information about the Tarantool-patches
mailing list