-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Bug#102586: multi-table DELETE with ON DELETE CASCADE breaks row-based replication #719
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
base: trunk
Are you sure you want to change the base?
Changes from 1 commit
05415aa
23e360a
baeaf45
6416dfc
6a37612
b1f9b04
da35310
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| include/rpl/init_source_replica.inc | ||
| Warnings: | ||
| Note #### Sending passwords in plain text without SSL/TLS is extremely insecure. | ||
| Note #### Storing MySQL user name or password information in the connection metadata repository is not secure and is therefore not recommended. Please consider using the USER and PASSWORD connection options for START REPLICA; see the 'START REPLICA Syntax' in the MySQL Manual for more information. | ||
| [connection master] | ||
| # | ||
| # ON DELETE CASCADE | ||
| # | ||
| CREATE TABLE t1 (id INT PRIMARY KEY) ENGINE=InnoDB; | ||
| CREATE TABLE t2 ( | ||
| id INT PRIMARY KEY, | ||
| parent_id INT, | ||
| FOREIGN KEY (parent_id) REFERENCES t1(id) ON DELETE CASCADE | ||
| ) ENGINE=InnoDB; | ||
| INSERT INTO t1 VALUES (1), (2); | ||
| INSERT INTO t2 VALUES (1, 1), (2, 1), (3, 2); | ||
| DELETE p, c FROM t1 p LEFT JOIN t2 c ON c.parent_id = p.id WHERE p.id = 1; | ||
| SELECT * FROM t1 ORDER BY id; | ||
| id | ||
| 2 | ||
| SELECT * FROM t2 ORDER BY id; | ||
| id parent_id | ||
| 3 2 | ||
| include/rpl/sync_to_replica.inc | ||
| include/diff_tables.inc [master:test.t1, slave:test.t1] | ||
| include/diff_tables.inc [master:test.t2, slave:test.t2] | ||
| [connection master] | ||
| DROP TABLE t2, t1; | ||
| # | ||
| # ON DELETE SET NULL, which is not deferred and not affected | ||
| # | ||
| CREATE TABLE t1 (id INT PRIMARY KEY) ENGINE=InnoDB; | ||
| CREATE TABLE t2 ( | ||
| id INT PRIMARY KEY, | ||
| parent_id INT, | ||
| FOREIGN KEY (parent_id) REFERENCES t1(id) ON DELETE SET NULL | ||
| ) ENGINE=InnoDB; | ||
| INSERT INTO t1 VALUES (1), (2); | ||
| INSERT INTO t2 VALUES (1, 1), (2, 1), (3, 2); | ||
| DELETE p, c FROM t1 p LEFT JOIN t2 c ON c.parent_id = p.id WHERE p.id = 1; | ||
| SELECT * FROM t1 ORDER BY id; | ||
| id | ||
| 2 | ||
| SELECT * FROM t2 ORDER BY id; | ||
| id parent_id | ||
| 2 NULL | ||
| 3 2 | ||
| include/rpl/sync_to_replica.inc | ||
| include/diff_tables.inc [master:test.t1, slave:test.t1] | ||
| include/diff_tables.inc [master:test.t2, slave:test.t2] | ||
| [connection master] | ||
| DROP TABLE t2, t1; | ||
| include/rpl/deinit.inc | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| # ==== Purpose ==== | ||
| # | ||
| # Check that a multi-table DELETE which names both a foreign key parent table | ||
| # and a child table with a cascading delete rule does not break row-based | ||
| # replication. | ||
| # | ||
| # ==== Implementation ==== | ||
| # | ||
| # 1. On the source, run a multi-table DELETE covering a parent table and its | ||
| # ON DELETE CASCADE child. | ||
| # 2. Synchronize the replica and compare both tables. Before this fix the | ||
| # applier stopped with ER_KEY_NOT_FOUND: the parent row was deleted while | ||
| # the join was still scanning, so the cascade removed the child rows on the | ||
| # replica before the logged child row events were applied. | ||
| # 3. Repeat with an ON DELETE SET NULL child, which replicates correctly and | ||
| # is covered here so the difference stays visible. | ||
| # | ||
| # ==== References ==== | ||
| # | ||
| # Bug#80821: Replication breaks if multi-table DELETE is used in conjunction | ||
| # with Foreign Key | ||
| # Bug#102586: Foreign Key ON DELETE CASCADE breaks with RBR and multiple-table | ||
| # DELETE | ||
| # | ||
| ############################################################################### | ||
| --source include/have_binlog_format_row.inc | ||
| --source include/rpl/init_source_replica.inc | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be good if the test case reproduced the bug without setting up a replication environment. The bug is in optimizer code, not replication code, so using replication to show the bug is probably an unnecessary complication. Here's one way to show the bug without relying on replication: Without your patch, t1 contains 6 rows after the DELETE statement is executed, and t2 contains 18 rows. With your patch applied, t1 is empty after the DELETE statement is executed, and t2 contains 2 rows. This is the correct result. |
||
|
|
||
| --echo # | ||
| --echo # ON DELETE CASCADE | ||
| --echo # | ||
|
|
||
| CREATE TABLE t1 (id INT PRIMARY KEY) ENGINE=InnoDB; | ||
| CREATE TABLE t2 ( | ||
| id INT PRIMARY KEY, | ||
| parent_id INT, | ||
| FOREIGN KEY (parent_id) REFERENCES t1(id) ON DELETE CASCADE | ||
| ) ENGINE=InnoDB; | ||
|
|
||
| INSERT INTO t1 VALUES (1), (2); | ||
| INSERT INTO t2 VALUES (1, 1), (2, 1), (3, 2); | ||
|
|
||
| DELETE p, c FROM t1 p LEFT JOIN t2 c ON c.parent_id = p.id WHERE p.id = 1; | ||
|
|
||
| SELECT * FROM t1 ORDER BY id; | ||
| SELECT * FROM t2 ORDER BY id; | ||
|
|
||
| --source include/rpl/sync_to_replica.inc | ||
|
|
||
| --let $diff_tables= master:test.t1, slave:test.t1 | ||
| --source include/diff_tables.inc | ||
| --let $diff_tables= master:test.t2, slave:test.t2 | ||
| --source include/diff_tables.inc | ||
|
|
||
| --let $rpl_connection_name= master | ||
| --source include/connection.inc | ||
| DROP TABLE t2, t1; | ||
|
|
||
| --echo # | ||
| --echo # ON DELETE SET NULL, which is not deferred and not affected | ||
| --echo # | ||
|
|
||
| CREATE TABLE t1 (id INT PRIMARY KEY) ENGINE=InnoDB; | ||
| CREATE TABLE t2 ( | ||
| id INT PRIMARY KEY, | ||
| parent_id INT, | ||
| FOREIGN KEY (parent_id) REFERENCES t1(id) ON DELETE SET NULL | ||
| ) ENGINE=InnoDB; | ||
|
|
||
| INSERT INTO t1 VALUES (1), (2); | ||
| INSERT INTO t2 VALUES (1, 1), (2, 1), (3, 2); | ||
|
|
||
| DELETE p, c FROM t1 p LEFT JOIN t2 c ON c.parent_id = p.id WHERE p.id = 1; | ||
|
|
||
| SELECT * FROM t1 ORDER BY id; | ||
| SELECT * FROM t2 ORDER BY id; | ||
|
|
||
| --source include/rpl/sync_to_replica.inc | ||
|
|
||
| --let $diff_tables= master:test.t1, slave:test.t1 | ||
| --source include/diff_tables.inc | ||
| --let $diff_tables= master:test.t2, slave:test.t2 | ||
| --source include/diff_tables.inc | ||
|
|
||
| --let $rpl_connection_name= master | ||
| --source include/connection.inc | ||
| DROP TABLE t2, t1; | ||
|
|
||
| --source include/rpl/deinit.inc | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2256,6 +2256,53 @@ Table_ref *unique_table(const Table_ref *table, Table_ref *table_list, | |
| return dup; | ||
| } | ||
|
|
||
| /** | ||
| Test whether deleting a row from the subject table of a multi-table DELETE | ||
| can cascade to another table which the same statement reads. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem isn't limited to multi-table DELETE. Multi-table UPDATE seems to have the same issue. For example: When updating the first row in t1, the update cascades to t2 and modifies it in a way so that the second row of t1 no longer has a match in t2. The statement therefore updates only the first row of t1, whereas it should have updated both rows. |
||
|
|
||
| Deleting from such a table while the join is still scanning is unsafe for | ||
| row-based replication: the cascade removes the child rows on the source and | ||
| logs row events for them, while the statement also logs the row events for | ||
| the child rows it deletes itself. On the replica the cascade has already | ||
| removed those rows by the time the logged child events are applied, which | ||
| breaks the applier with ER_KEY_NOT_FOUND. Deferring the delete until the | ||
| join has finished avoids the overlap. | ||
|
|
||
| Only ON DELETE CASCADE deletes child rows, so only that rule is considered. | ||
| ON DELETE SET NULL updates the child rows instead, which leaves them | ||
| findable for the logged events and replicates correctly. | ||
|
|
||
| @param table table to be checked (must be updatable base table) | ||
| @param leaf_tables leaf tables of the query block to check against | ||
|
|
||
| @retval true Deleting from @p table cascades to one of @p leaf_tables. | ||
| @retval false No cascading dependency within the query. | ||
| */ | ||
|
|
||
| bool delete_cascades_to_queried_table(const Table_ref *table, | ||
| const Table_ref *leaf_tables) { | ||
| assert(table->table != nullptr); | ||
|
|
||
| const TABLE_SHARE *share = table->table->s; | ||
| for (const TABLE_SHARE_FOREIGN_KEY_PARENT_INFO *fk_p = | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This only checks if deletes from Example: Expected result: All three tables are empty. Actual result: |
||
| share->foreign_key_parent; | ||
| fk_p < share->foreign_key_parent + share->foreign_key_parents; ++fk_p) { | ||
| if (fk_p->delete_rule != dd::Foreign_key::RULE_CASCADE) continue; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need to do this check for all actions except The expected result is that |
||
|
|
||
| for (const Table_ref *tl = leaf_tables; tl != nullptr; tl = tl->next_leaf) { | ||
| if (tl->table == nullptr) continue; // View or derived table. | ||
| const TABLE_SHARE *child_share = tl->table->s; | ||
| if (my_strcasecmp(table_alias_charset, child_share->db.str, | ||
| fk_p->referencing_table_db.str) == 0 && | ||
| my_strcasecmp(table_alias_charset, child_share->table_name.str, | ||
| fk_p->referencing_table_name.str) == 0) | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| Issue correct error message in case we found 2 duplicate tables which | ||
| prevent some update operation | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this result is incorrect. It should have contained only the
(3, 2)row. The(2, NULL)row was originally(2, 1)and should have qualified both in the ON clause and in the WHERE clause and get deleted. delete_cascades_to_queried_table() should also checkSET NULLactions to fix this.