[Tarantool-patches] [PATCH v2 2/3] box: make constraint operations transactional
Nikita Pettik
korablev at tarantool.org
Sat Dec 28 03:18:33 MSK 2019
On 17 Dec 18:03, Roman Khabibov wrote:
> >>
> >> @TarantoolBot document
> >> Title: Table constraints in SQL
> >>
> >> SQL:
> >> According to ANSI SQL, table constraint is one of the following
> >> entities: PRIMARY KEY, UNIQUE, FOREIGN KEY, CHECK. Every
> >> constraint have its own name passed by user or automatically
have -> has
> >> generated. And these names must be unique within one table/space.
> >> Naming in SQL is case-insensitive, so "CONSTRAINT c" and
False. Naming in SQL is case-sensitive. It's all about uppercasing
unquoted identifiers:
CREATE TABLE t2 (i INT, CONSTRAINT "c" CHECK (i > 0), CONSTRAINT c PRIMARY KEY (i));
---
- row_count: 1
...
> >> "CONSTRAINT C" are the same. For example, you will get error, if
> >> you try to:
> >>
> >> CREATE TABLE t2 (i INT, CONSTRAINT c CHECK (i > 0),
> >> CONSTRAINT c PRIMARY KEY (i));
> >>
> >> or
> >>
> >> CREATE TABLE t2 (i INT CONSTRAINT c PRIMARY KEY);
> >> ALTER TABLE t2 ADD CONSTRAINT c CHECK(i > 0);');
> >> The same is for any other constraint types.
> >>
>
> +/**
> + * Check if constraint with @a name exists within @a space. Call
> + * diag_set() if it is so.
> + */
> +static inline int
> +space_check_constraint_existence(struct space *space, const char *name)
IMHO this function looks a bit strange (or I'd say - contradictory).
Let's simply inline it, only 2 usages tho.
> +{
> + struct constraint_id *id = space_find_constraint_id(space, name);
> + if (id == NULL)
> + return 0;
> + diag_set(ClientError, ER_CONSTRAINT_EXISTS,
> + constraint_type_strs[id->type], name, space_name(space));
> + return -1;
> +}
> +
> +
> +static inline void
> +space_delete_constraint_id(struct space *space, const char *name)
Why do we need this wrapper?
> +{
> + constraint_id_delete(space_pop_constraint_id(space, name));
> +}
> +
> diff --git a/src/box/errcode.h b/src/box/errcode.h
> index c660b1c70..094a63ee1 100644
> --- a/src/box/errcode.h
> +++ b/src/box/errcode.h
> @@ -222,7 +222,7 @@ struct errcode_record {
> /*167 */_(ER_CREATE_FK_CONSTRAINT, "Failed to create foreign key constraint '%s': %s") \
> /*168 */_(ER_DROP_FK_CONSTRAINT, "Failed to drop foreign key constraint '%s': %s") \
> /*169 */_(ER_NO_SUCH_CONSTRAINT, "Constraint %s does not exist") \
> - /*170 */_(ER_CONSTRAINT_EXISTS, "Constraint %s already exists") \
> + /*170 */_(ER_CONSTRAINT_EXISTS, "Constraint %s '%s' already exists in space '%s'") \
I'd better place type of constraint at the beggining of sentence:
Check constraint xxx already exists
Foreign key constraint ...
Unique constraint ...
Sounds way much better, doesn't it?
> new file mode 100755
> index 000000000..b41e16dc2
> --- /dev/null
> +++ b/test/sql/constraint.test.lua
> @@ -0,0 +1,93 @@
> +test_run = require('test_run').new()
> +engine = test_run:get_cfg('engine')
> +box.execute('pragma sql_default_engine=\''..engine..'\'')
> +
> +-- Make sure, that altering of an index name affects its record
> +-- in the space's constraint hash table.
> +--
> +box.execute('CREATE UNIQUE INDEX d ON t2(i);')
> +box.space.T2.index.D:alter({name = 'E'})
> +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);')
As far as I see, you missed case of altering index (or other constraint)
name to already occupied by another constraint.
> +--
> +-- Make sure, that altering of an index uniqueness puts/drops
> +-- its name to/from the space's constraint hash table.
> +--
> +box.space.T2.index.E:alter({unique = false})
> +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);')
> +box.space.T2.index.E:alter({unique = true})
> +box.space.T2.ck_constraint.E:drop()
> +box.space.T2.index.E:alter({unique = true})
> +box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);')
> +
> +-- Alter name and uniqueness of an unique index simultaneously.
> +box.space.T2.index.E:alter({name = 'D', unique = false})
> +box.execute('CREATE UNIQUE INDEX e ON t2(i);')
And make sure that CREATE UNIQUE INDEX d ON t2(i); fails
What about circle renaming of consraint inside transaction which
is rolled back later?
More information about the Tarantool-patches
mailing list