#At file:///data0/martin/bzr/bug43580-push/5.1bt-gca/ based on revid:alfranio.correia@stripped
2864 Martin Hansson 2009-05-05
Bug#43580: Issue with Innodb on multi-table update
Certain multi-updates gave different results on InnoDB from
to MyISAM, due to on-the-fly updates being used on the former and
the update order matters.
Fixed by turning off on-the-fly updates when update order
dependencies are present.
@ mysql-test/r/innodb_mysql.result
Bug#43580: Test result.
@ mysql-test/suite/rpl/r/rpl_slave_skip.result
Bug#43580: Changed test result. The InnoDB result is now what it would have been on MyISAM.
@ mysql-test/t/innodb_mysql.test
Bug#43580: Test case.
@ sql/sql_base.cc
Bug#43580: Added a word of caution about using tmp_set here.
@ sql/sql_update.cc
Bug#43580: Fix.
Calls to TABLE::mark_columns_needed_for_update() are moved
from mysql_multi_update_prepare() and right before the decison
to do on-the-fly updates to the place where we do (or don't do)
on-the-fly updates.
modified:
mysql-test/r/innodb_mysql.result
mysql-test/suite/rpl/r/rpl_slave_skip.result
mysql-test/t/innodb_mysql.test
sql/sql_base.cc
sql/sql_update.cc
=== modified file 'mysql-test/r/innodb_mysql.result'
--- a/mysql-test/r/innodb_mysql.result 2009-03-30 08:44:17 +0000
+++ b/mysql-test/r/innodb_mysql.result 2009-05-05 09:38:19 +0000
@@ -2021,4 +2021,31 @@ DROP TABLE t4;
DROP TABLE t1;
DROP TABLE t2;
DROP TABLE t3;
+CREATE TABLE t1 (a INT, b INT, KEY (a)) ENGINE = INNODB;
+CREATE TABLE t2 (a INT KEY, b INT, KEY (b)) ENGINE = INNODB;
+CREATE TABLE t3 (a INT, b INT KEY, KEY (a)) ENGINE = INNODB;
+CREATE TABLE t4 (a INT KEY, b INT, KEY (b)) ENGINE = INNODB;
+INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 3), (4, 4), (5, 5), (6, 6);
+INSERT INTO t2 VALUES (1, 1), (2, 2), (3, 3), (4, 4), (5, 5);
+INSERT INTO t3 VALUES (1, 101), (2, 102), (3, 103), (4, 104), (5, 105), (6, 106);
+INSERT INTO t4 VALUES (1, 1), (2, 2), (3, 3), (4, 4), (5, 5);
+UPDATE t1, t2 SET t1.a = t1.a + 100, t2.b = t1.a + 10
+WHERE t1.a BETWEEN 2 AND 4 AND t2.a = t1.b;
+SELECT * FROM t2;
+a b
+1 1
+2 12
+3 13
+4 14
+5 5
+UPDATE t3, t4 SET t3.a = t3.a + 100, t4.b = t3.a + 10
+WHERE t3.a BETWEEN 2 AND 4 AND t4.a = t3.b - 100;
+SELECT * FROM t4;
+a b
+1 1
+2 12
+3 13
+4 14
+5 5
+DROP TABLE t1, t2, t3, t4;
End of 5.1 tests
=== modified file 'mysql-test/suite/rpl/r/rpl_slave_skip.result'
--- a/mysql-test/suite/rpl/r/rpl_slave_skip.result 2009-01-23 12:22:05 +0000
+++ b/mysql-test/suite/rpl/r/rpl_slave_skip.result 2009-05-05 09:38:19 +0000
@@ -39,8 +39,8 @@ a b
SELECT * FROM t2;
c d
1 2
-2 16
-3 54
+2 8
+3 18
**** On Slave ****
START SLAVE UNTIL MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=762;
SHOW SLAVE STATUS;
@@ -50,7 +50,7 @@ Master_User root
Master_Port MASTER_PORT
Connect_Retry 1
Master_Log_File master-bin.000001
-Read_Master_Log_Pos 1133
+Read_Master_Log_Pos 1115
Relay_Log_File #
Relay_Log_Pos #
Relay_Master_Log_File master-bin.000001
=== modified file 'mysql-test/t/innodb_mysql.test'
--- a/mysql-test/t/innodb_mysql.test 2009-03-27 16:08:14 +0000
+++ b/mysql-test/t/innodb_mysql.test 2009-05-05 09:38:19 +0000
@@ -332,4 +332,31 @@ DROP TABLE t1;
DROP TABLE t2;
DROP TABLE t3;
+#
+# Bug#43580: Issue with Innodb on multi-table update
+#
+CREATE TABLE t1 (a INT, b INT, KEY (a)) ENGINE = INNODB;
+CREATE TABLE t2 (a INT KEY, b INT, KEY (b)) ENGINE = INNODB;
+
+CREATE TABLE t3 (a INT, b INT KEY, KEY (a)) ENGINE = INNODB;
+CREATE TABLE t4 (a INT KEY, b INT, KEY (b)) ENGINE = INNODB;
+
+INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 3), (4, 4), (5, 5), (6, 6);
+INSERT INTO t2 VALUES (1, 1), (2, 2), (3, 3), (4, 4), (5, 5);
+
+INSERT INTO t3 VALUES (1, 101), (2, 102), (3, 103), (4, 104), (5, 105), (6, 106);
+INSERT INTO t4 VALUES (1, 1), (2, 2), (3, 3), (4, 4), (5, 5);
+
+UPDATE t1, t2 SET t1.a = t1.a + 100, t2.b = t1.a + 10
+WHERE t1.a BETWEEN 2 AND 4 AND t2.a = t1.b;
+--sorted_result
+SELECT * FROM t2;
+
+UPDATE t3, t4 SET t3.a = t3.a + 100, t4.b = t3.a + 10
+WHERE t3.a BETWEEN 2 AND 4 AND t4.a = t3.b - 100;
+--sorted_result
+SELECT * FROM t4;
+
+DROP TABLE t1, t2, t3, t4;
+
--echo End of 5.1 tests
=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc 2009-04-17 05:51:51 +0000
+++ b/sql/sql_base.cc 2009-05-05 09:38:19 +0000
@@ -5585,6 +5585,13 @@ static void update_field_dependencies(TH
other_bitmap= table->read_set;
}
+ /*
+ The test-and-set mechanism in the bitmap is not reliable during
+ multi-UPDATE statements under MARK_COLUMNS_READ mode
+ (thd->mark_used_columns == MARK_COLUMNS_READ), as this bitmap contains
+ only those columns that are used in the SET clause. I.e they are being
+ set here. See multi_update::prepare()
+ */
if (bitmap_fast_test_and_set(current_bitmap, field->field_index))
{
if (thd->mark_used_columns == MARK_COLUMNS_WRITE)
=== modified file 'sql/sql_update.cc'
--- a/sql/sql_update.cc 2009-04-08 23:42:51 +0000
+++ b/sql/sql_update.cc 2009-05-05 09:38:19 +0000
@@ -1031,7 +1031,6 @@ reopen_tables:
DBUG_RETURN(TRUE);
}
- table->mark_columns_needed_for_update();
DBUG_PRINT("info",("setting table `%s` for update", tl->alias));
/*
If table will be updated we should not downgrade lock for it and
@@ -1275,12 +1274,40 @@ int multi_update::prepare(List<Item> &no
}
/*
+ We gather the set of columns read during evaluation of SET expression in
+ TABLE::tmp_set by pointing TABLE::read_set to it and then restore it after
+ setup_fields().
+ */
+ for (table_ref= leaves; table_ref; table_ref= table_ref->next_leaf)
+ {
+ TABLE *table= table_ref->table;
+ if (tables_to_update & table->map)
+ {
+ DBUG_ASSERT(table->read_set == &table->def_read_set);
+ table->read_set= &table->tmp_set;
+ bitmap_clear_all(table->read_set);
+ }
+ }
+
+ /*
We have to check values after setup_tables to get covering_keys right in
reference tables
*/
- if (setup_fields(thd, 0, *values, MARK_COLUMNS_READ, 0, 0))
- DBUG_RETURN(1);
+ int error= setup_fields(thd, 0, *values, MARK_COLUMNS_READ, 0, 0);
+
+ for (table_ref= leaves; table_ref; table_ref= table_ref->next_leaf)
+ {
+ TABLE *table= table_ref->table;
+ if (tables_to_update & table->map)
+ {
+ table->read_set= &table->def_read_set;
+ bitmap_union(table->read_set, &table->tmp_set);
+ }
+ }
+
+ if (error)
+ DBUG_RETURN(1);
/*
Save tables beeing updated in update_tables
@@ -1375,6 +1402,8 @@ int multi_update::prepare(List<Item> &no
a row in this table will never be read twice. This is true under
the following conditions:
+ - No column is both written to and read in SET expressions.
+
- We are doing a table scan and the data is in a separate file (MyISAM) or
if we don't update a clustered key.
@@ -1389,6 +1418,9 @@ int multi_update::prepare(List<Item> &no
WARNING
This code is a bit dependent of how make_join_readinfo() works.
+ The field table->tmp_set is used for keeping track of which fields are
+ read during evaluation of the SET expression. See multi_update::prepare.
+
RETURN
0 Not safe to update
1 Safe to update
@@ -1409,6 +1441,8 @@ static bool safe_update_on_fly(THD *thd,
case JT_REF_OR_NULL:
return !is_key_used(table, join_tab->ref.key, table->write_set);
case JT_ALL:
+ if (bitmap_is_overlapping(&table->tmp_set, table->write_set))
+ return FALSE;
/* If range search on index */
if (join_tab->quick)
return !join_tab->quick->is_keys_used(table->write_set);
@@ -1464,17 +1498,18 @@ multi_update::initialize_tables(JOIN *jo
ORDER group;
TMP_TABLE_PARAM *tmp_param;
- table->mark_columns_needed_for_update();
if (ignore)
table->file->extra(HA_EXTRA_IGNORE_DUP_KEY);
if (table == main_table) // First table in join
{
if (safe_update_on_fly(thd, join->join_tab, table_ref, all_tables))
{
- table_to_update= main_table; // Update table on the fly
+ table->mark_columns_needed_for_update();
+ table_to_update= table; // Update table on the fly
continue;
}
}
+ table->mark_columns_needed_for_update();
table->prepare_for_position();
/*
Attachment: [text/bzr-bundle] bzr/mhansson@mysql.com-20090505093819-9fr164ha7czeuq4c.bundle
