Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Garbage in case of failed fk constraint creation #4183

Closed
kshcherbatov opened this issue Apr 26, 2019 · 1 comment
Closed

Garbage in case of failed fk constraint creation #4183

kshcherbatov opened this issue Apr 26, 2019 · 1 comment
Assignees
Labels
bug Something isn't working sql
Milestone

Comments

@kshcherbatov
Copy link
Contributor

Tarantool doesn't rollback CREATE TABLE in case of incomplete creation.
Mainly, this problem was solved in #3592, but to print a pretty error message, we use vdbe_emit_halt_with_presence_test that brakes this machinery.

tarantool> box.execute("create table t2(id int primary key, constraint fk1 foreign key(id) references t2, constraint fk1 foreign key(id) references t2)")
---
- error: Constraint FK1 already exists
...

tarantool> box.space._fk_constraint:select()
---
- - ['FK1', 512, 512, false, 'simple', 'no_action', 'no_action', [0], [0]]
...
tarantool> box.space.T2
---
- engine: memtx
  before_replace: 'function: 0x4014a830'
  on_replace: 'function: 0x4014a808'
  field_count: 1
  ...

We may 1) just remove vdbe_emit_halt_with_presence_test from vdbe_emit_fk_constraint_create and vdbe_emit_fk_constraint_drop or 2) extend vdbe_emit_halt_with_presence_test to solve this problem.

In case 1) of removal vdbe_emit_halt_with_presence_test, the error message is still correct and informative enough:

tarantool> box.execute("create table t2(id int primary key, constraint fk1 foreign key(id) references t2, constraint fk1 foreign key(id) references t2)")
---
- error: Duplicate key exists in unique index 'primary' in space '_fk_constraint'
...
@kshcherbatov kshcherbatov added sql bug Something isn't working labels Apr 26, 2019
@kshcherbatov
Copy link
Contributor Author

The behavior and error message between FK and CK constraints must match.
See #3691

@kyukhin kyukhin added this to the 2.2.0 milestone May 28, 2019
ImeevMA added a commit that referenced this issue Jun 25, 2019
This opcode is required to set an error using diag_set() without
halting VDBE. This will allow us to run destructors between error
setting and VDBE halting.

Needed for #4183
ImeevMA added a commit that referenced this issue Jun 25, 2019
This patch makes VDBE run destructors before halting in case
constraint creation failed.

Closes #4183
ImeevMA added a commit that referenced this issue Jun 27, 2019
This opcode is required to set an error using diag_set() without
halting VDBE. This will allow us to run destructors between error
setting and VDBE halting.

Needed for #4183
ImeevMA added a commit that referenced this issue Jun 27, 2019
This patch makes VDBE run destructors before halting in case
constraint creation failed.

Part of #4183
ImeevMA added a commit that referenced this issue Jun 27, 2019
Prior to this patch, the data needed to create the constraints in
VDBE was stored in the temporary registers of the parser. Since
this data is necessary for creating destructors, it was
transferred to standard registers.

Closes #4183
ImeevMA added a commit that referenced this issue Jun 28, 2019
This patch makes VDBE run destructors before halting in case
constraint creation failed.

Part of #4183
ImeevMA added a commit that referenced this issue Jun 28, 2019
Prior to this patch, the data needed to create the constraints in
VDBE was stored in the temporary registers of the parser. Since
this data is necessary for creating destructors, it was
transferred to standard registers.

Closes #4183
ImeevMA added a commit that referenced this issue Jul 3, 2019
To separate the error setting and execution halting, a new opcode
OP_SetDiag was created. The only functionality of the opcode is
the execution of diag_set(). It is important to note that
OP_SetDiag does not set is_aborted to true, so we can continue
working with other opcodes, if necessary. This function allows us
to perform cleanup in some special cases, for example, when
creating a constraint failed because of the creation of two or
more constraints with the same name in the same CREATE TABLE
statement.

Since now diag_set() is executed in OP_SetDiag, this functionality
has been removed from OP_Halt.

Needed for #4183
ImeevMA added a commit that referenced this issue Jul 3, 2019
This patch makes VDBE to perform a clean-up if the creation of a
constraint fails because of the creation of two or more
constraints with the same name in the same CREATE TABLE statement.
For example:
CREATE TABLE t1(
	id INT PRIMARY KEY,
	CONSTRAINT ck1 CHECK(id > 1),
	CONSTRAINT ck1 CHECK(id < 10)
);

Part of #4183
ImeevMA added a commit that referenced this issue Jul 3, 2019
Prior to this patch, data needed to form tuple to be inserted to
_fk_constraint and _ck_constraint system spaces (to create
corresponding constraints) was stored in the range of temporary
register. After insertion, temporary registers are released. On
the other hand, this data is required for providing clean-up in
case of creation fail (i.e. removing already created constraints
within one CREATE TABLE statement). Hence, instead of using
temporary registers let's use ordinary ones.

Closes #4183
ImeevMA added a commit that referenced this issue Jul 4, 2019
To separate the error setting and execution halting, a new opcode
OP_SetDiag was created. The only functionality of the opcode is
the execution of diag_set(). It is important to note that
OP_SetDiag does not set is_aborted to true, so we can continue
working with other opcodes, if necessary. This function allows us
to perform cleanup in some special cases, for example, when
creating a constraint failed because of the creation of two or
more constraints with the same name in the same CREATE TABLE
statement.

Since now diag_set() is executed in OP_SetDiag, this functionality
has been removed from OP_Halt.

Needed for #4183
ImeevMA added a commit that referenced this issue Jul 4, 2019
This patch makes VDBE to perform a clean-up if the creation of a
constraint fails because of the creation of two or more
constraints with the same name in the same CREATE TABLE statement.
For example:
CREATE TABLE t1(
	id INT PRIMARY KEY,
	CONSTRAINT ck1 CHECK(id > 1),
	CONSTRAINT ck1 CHECK(id < 10)
);

Part of #4183
ImeevMA added a commit that referenced this issue Jul 4, 2019
Prior to this patch, data needed to form tuple to be inserted to
_fk_constraint and _ck_constraint system spaces (to create
corresponding constraints) was stored in the range of temporary
register. After insertion, temporary registers are released. On
the other hand, this data is required for providing clean-up in
case of creation fail (i.e. removing already created constraints
within one CREATE TABLE statement). Hence, instead of using
temporary registers let's use ordinary ones.

Closes #4183
ImeevMA added a commit that referenced this issue Jul 4, 2019
To separate the error setting and execution halting, a new opcode
OP_SetDiag was created. The only functionality of the opcode is
the execution of diag_set(). It is important to note that
OP_SetDiag does not set is_aborted to true, so we can continue
working with other opcodes, if necessary. This function allows us
to perform cleanup in some special cases, for example, when
creating a constraint failed because of the creation of two or
more constraints with the same name in the same CREATE TABLE
statement.

Since now diag_set() is executed in OP_SetDiag, this functionality
has been removed from OP_Halt.

Needed for #4183
ImeevMA added a commit that referenced this issue Jul 4, 2019
This patch makes VDBE to perform a clean-up if the creation of a
constraint fails because of the creation of two or more
constraints with the same name in the same CREATE TABLE statement.
For example:
CREATE TABLE t1(
	id INT PRIMARY KEY,
	CONSTRAINT ck1 CHECK(id > 1),
	CONSTRAINT ck1 CHECK(id < 10)
);

Part of #4183
ImeevMA added a commit that referenced this issue Jul 4, 2019
Prior to this patch, data needed to form tuple to be inserted to
_fk_constraint and _ck_constraint system spaces (to create
corresponding constraints) was stored in the range of temporary
register. After insertion, temporary registers are released. On
the other hand, this data is required for providing clean-up in
case of creation fail (i.e. removing already created constraints
within one CREATE TABLE statement). Hence, instead of using
temporary registers let's use ordinary ones.

Closes #4183
ImeevMA added a commit that referenced this issue Jul 4, 2019
To separate the error setting and execution halting, a new opcode
OP_SetDiag was created. The only functionality of the opcode is
the execution of diag_set(). It is important to note that
OP_SetDiag does not set is_aborted to true, so we can continue
working with other opcodes, if necessary. This function allows us
to perform cleanup in some special cases, for example, when
creating a constraint failed because of the creation of two or
more constraints with the same name in the same CREATE TABLE
statement.

Since now diag_set() is executed in OP_SetDiag, this functionality
has been removed from OP_Halt.

Needed for #4183
ImeevMA added a commit that referenced this issue Jul 4, 2019
This patch makes VDBE to perform a clean-up if the creation of a
constraint fails because of the creation of two or more
constraints of the same type with the same name and in the same
CREATE TABLE statement.

For example:
CREATE TABLE t1(
	id INT PRIMARY KEY,
	CONSTRAINT ck1 CHECK(id > 1),
	CONSTRAINT ck1 CHECK(id < 10)
);

Part of #4183
ImeevMA added a commit that referenced this issue Jul 4, 2019
Prior to this patch, data needed to form tuple to be inserted to
_fk_constraint and _ck_constraint system spaces (to create
corresponding constraints) was stored in the range of temporary
register. After insertion, temporary registers are released. On
the other hand, this data is required for providing clean-up in
case of creation fail (i.e. removing already created constraints
within one CREATE TABLE statement). Hence, instead of using
temporary registers let's use ordinary ones.

Closes #4183
ImeevMA added a commit that referenced this issue Jul 15, 2019
To separate the error setting and execution halting, a new opcode
OP_SetDiag was created. The only functionality of the opcode is
the execution of diag_set(). It is important to note that
OP_SetDiag does not set is_aborted to true, so we can continue
working with other opcodes, if necessary. This function allows us
to perform cleanup in some special cases, for example, when
creating a constraint failed because of the creation of two or
more constraints with the same name in the same CREATE TABLE
statement.

Since now diag_set() is executed in OP_SetDiag, this functionality
has been removed from OP_Halt.

Needed for #4183
ImeevMA added a commit that referenced this issue Jul 15, 2019
This patch makes VDBE to perform a clean-up if the creation of a
constraint fails because of the creation of two or more
constraints of the same type with the same name and in the same
CREATE TABLE statement.

For example:
CREATE TABLE t1(
	id INT PRIMARY KEY,
	CONSTRAINT ck1 CHECK(id > 1),
	CONSTRAINT ck1 CHECK(id < 10)
);

Part of #4183
ImeevMA added a commit that referenced this issue Jul 15, 2019
Prior to this patch, data needed to form tuple to be inserted to
_fk_constraint and _ck_constraint system spaces (to create
corresponding constraints) was stored in the range of temporary
register. After insertion, temporary registers are released. On
the other hand, this data is required for providing clean-up in
case of creation fail (i.e. removing already created constraints
within one CREATE TABLE statement). Hence, instead of using
temporary registers let's use ordinary ones.

Closes #4183
kyukhin pushed a commit that referenced this issue Jul 18, 2019
To separate the error setting and execution halting, a new opcode
OP_SetDiag was created. The only functionality of the opcode is
the execution of diag_set(). It is important to note that
OP_SetDiag does not set is_aborted to true, so we can continue
working with other opcodes, if necessary. This function allows us
to perform cleanup in some special cases, for example, when
creating a constraint failed because of the creation of two or
more constraints with the same name in the same CREATE TABLE
statement.

Since now diag_set() is executed in OP_SetDiag, this functionality
has been removed from OP_Halt.

Needed for #4183
kyukhin pushed a commit that referenced this issue Jul 18, 2019
This patch makes VDBE to perform a clean-up if the creation of a
constraint fails because of the creation of two or more
constraints of the same type with the same name and in the same
CREATE TABLE statement.

For example:
CREATE TABLE t1(
	id INT PRIMARY KEY,
	CONSTRAINT ck1 CHECK(id > 1),
	CONSTRAINT ck1 CHECK(id < 10)
);

Part of #4183
@kyukhin kyukhin added the tmp label Aug 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working sql
Projects
None yet
Development

No branches or pull requests

4 participants