[Tarantool-patches] [PATCH] sql: fix update of a self-referenced table
Chris Sosnin
k.sosnin at tarantool.org
Tue Feb 18 20:55:11 MSK 2020
An update of a self-referenced table is not handled correctly:
if a new row doesn't reference itself, it is considered to violate
the constraint. This patch makes vdbe to inspect the table on updates
too with a little change: the row we find should be different from the
one we are updating.
Closes #4661
---
I've tried to refactor a patch a little, shortened the comments under
the 66 characters line, rewrote some of them completely. I agree with
Vlad that FK code looks quite complicated (that's why it took me so
long to write the first version of the patch), so if it's needed, I
can try to give a verbal explanation.
branch:https://github.com/tarantool/tarantool/tree/ksosnin/gh-4661-fk-self-ref-table
issue:https://github.com/tarantool/tarantool/issues/4661
src/box/sql/fk_constraint.c | 51 ++++++++++++++++++++++++++++------
test/sql/foreign-keys.result | 44 +++++++++++++++++++++++++++++
test/sql/foreign-keys.test.lua | 12 ++++++++
3 files changed, 99 insertions(+), 8 deletions(-)
diff --git a/src/box/sql/fk_constraint.c b/src/box/sql/fk_constraint.c
index 482220a95..2e1d8d053 100644
--- a/src/box/sql/fk_constraint.c
+++ b/src/box/sql/fk_constraint.c
@@ -225,7 +225,8 @@ fk_constraint_lookup_parent(struct Parse *parse_context, struct space *parent,
* we are about to increment the constraint-counter (i.e.
* this is an INSERT operation), then check if the row
* being inserted matches itself. If so, do not increment
- * the constraint-counter.
+ * the constraint-counter. Otherwise, we must inspect the
+ * table.
*
* If any of the parent-key values are NULL, then the row
* cannot match itself. So set JUMPIFNULL to make sure we
@@ -244,14 +245,14 @@ fk_constraint_lookup_parent(struct Parse *parse_context, struct space *parent,
}
sqlVdbeGoto(v, ok_label);
}
- /**
+ /*
* Inspect a parent table with OP_Found.
- * We mustn't make it for a self-referenced table since
- * it's tuple will be modified by the update operation.
- * And since the foreign key has already detected a
- * conflict, fk counter must be increased.
+ * We mustn't make it for a self-referenced
+ * table on deletion during an update operation
+ * since it's tuple will be modified later.
*/
- if (!(fk_constraint_is_self_referenced(fk_def) && is_update)) {
+ if (!(fk_constraint_is_self_referenced(fk_def) && is_update &&
+ incr_count == -1)) {
int temp_regs = sqlGetTempRange(parse_context, field_count);
int rec_reg = sqlGetTempReg(parse_context);
vdbe_emit_open_cursor(parse_context, cursor, referenced_idx,
@@ -269,7 +270,41 @@ fk_constraint_lookup_parent(struct Parse *parse_context, struct space *parent,
(char *) sql_index_type_str(parse_context->db,
idx->def),
P4_DYNAMIC);
- sqlVdbeAddOp4Int(v, OP_Found, cursor, ok_label, rec_reg, 0);
+
+ /*
+ * In this particular case, we should check, that
+ * the row we found and the row being updated
+ * are different, otherwise the update operation
+ * will violate the constraint. This check is
+ * correct, since we've already verified that
+ * the row doesn't match itself.
+ */
+ bool check_not_equal = incr_count == 1 && is_update &&
+ fk_constraint_is_self_referenced(fk_def);
+
+ int jump = sqlVdbeCurrentAddr(v) + 1;
+ sqlVdbeAddOp4Int(v, OP_Found, cursor,
+ check_not_equal ? jump + 1 : ok_label,
+ rec_reg, 0);
+ if (check_not_equal) {
+ /*
+ * We've found nothing, there's no need
+ * to compare rows.
+ */
+ sqlVdbeGoto(v, jump + 4);
+ /*
+ * Otherwise, cursor is pointing to the
+ * found entry, compare its index to the
+ * index of the tuple being updated.
+ */
+ int idx_reg = sqlGetTempReg(parse_context);
+ sqlVdbeAddOp3(v, OP_Column, cursor, referenced_idx,
+ idx_reg);
+ sqlVdbeAddOp3(v, OP_Eq, reg_data + referenced_idx + 1,
+ jump + 4, idx_reg);
+ sqlReleaseTempReg(parse_context, idx_reg);
+ sqlVdbeGoto(v, ok_label);
+ }
sqlReleaseTempReg(parse_context, rec_reg);
sqlReleaseTempRange(parse_context, temp_regs, field_count);
}
diff --git a/test/sql/foreign-keys.result b/test/sql/foreign-keys.result
index f1d973443..a9b10ee38 100644
--- a/test/sql/foreign-keys.result
+++ b/test/sql/foreign-keys.result
@@ -429,6 +429,50 @@ box.execute("CREATE TABLE t4 (id INT PRIMARY KEY REFERENCES t4);")
box.space.T4:drop()
---
...
+-- gh-4661: UPDATE must work correctly on self-referenced spaces.
+--
+box.execute('CREATE TABLE t5(a INT UNIQUE, b INT REFERENCES t5(a), id INT PRIMARY KEY AUTOINCREMENT)')
+---
+- row_count: 1
+...
+box.execute('INSERT INTO t5(a,b) VALUES(1, null)')
+---
+- autoincrement_ids:
+ - 1
+ row_count: 1
+...
+box.execute('INSERT INTO t5(a,b) VALUES(2, null)')
+---
+- autoincrement_ids:
+ - 2
+ row_count: 1
+...
+box.execute('UPDATE t5 SET b = 2 WHERE a = 1')
+---
+- row_count: 1
+...
+box.execute('UPDATE t5 SET b = 1 WHERE a = 2')
+---
+- row_count: 1
+...
+box.execute('INSERT INTO t5(a,b) VALUES(3, 2)')
+---
+- autoincrement_ids:
+ - 3
+ row_count: 1
+...
+box.execute('UPDATE t5 SET a = 4 WHERE a = 3')
+---
+- row_count: 1
+...
+box.execute('UPDATE t5 SET b = 4 WHERE a = 1')
+---
+- row_count: 1
+...
+box.execute('DROP TABLE t5')
+---
+- row_count: 1
+...
-- Make sure that child space can feature no PK.
--
t1 = box.schema.create_space("T1", {format = {'ID'}})
diff --git a/test/sql/foreign-keys.test.lua b/test/sql/foreign-keys.test.lua
index d2dd88d28..4fb651787 100644
--- a/test/sql/foreign-keys.test.lua
+++ b/test/sql/foreign-keys.test.lua
@@ -179,6 +179,18 @@ box.space.T1:drop()
box.execute("CREATE TABLE t4 (id INT PRIMARY KEY REFERENCES t4);")
box.space.T4:drop()
+-- gh-4661: UPDATE must work correctly on self-referenced spaces.
+--
+box.execute('CREATE TABLE t5(a INT UNIQUE, b INT REFERENCES t5(a), id INT PRIMARY KEY AUTOINCREMENT)')
+box.execute('INSERT INTO t5(a,b) VALUES(1, null)')
+box.execute('INSERT INTO t5(a,b) VALUES(2, null)')
+box.execute('UPDATE t5 SET b = 2 WHERE a = 1')
+box.execute('UPDATE t5 SET b = 1 WHERE a = 2')
+box.execute('INSERT INTO t5(a,b) VALUES(3, 2)')
+box.execute('UPDATE t5 SET a = 4 WHERE a = 3')
+box.execute('UPDATE t5 SET b = 4 WHERE a = 1')
+box.execute('DROP TABLE t5')
+
-- Make sure that child space can feature no PK.
--
t1 = box.schema.create_space("T1", {format = {'ID'}})
--
2.21.1 (Apple Git-122.3)
More information about the Tarantool-patches
mailing list