From 5ad9035605d0ae5294441a91d7f1269a73133b0c Mon Sep 17 00:00:00 2001 From: "svoj@mysql.com/april.(none)" <> Date: Wed, 20 Dec 2006 19:05:35 +0400 Subject: [PATCH] BUG#21310 - Trees in SQL causing a "crashed" table with MyISAM storage engine An update that used a join of a table to itself and modified the table on one side of the join reported the table as crashed or updated wrong rows. Fixed by creating temporary table for self-joined multi update statement. --- mysql-test/r/myisam.result | 16 +++++ mysql-test/t/myisam.test | 19 +++++ sql/lock.cc | 2 +- sql/sql_update.cc | 139 +++++++++++++++++-------------------- 4 files changed, 99 insertions(+), 77 deletions(-) diff --git a/mysql-test/r/myisam.result b/mysql-test/r/myisam.result index b34c127595f..73cee4835bf 100644 --- a/mysql-test/r/myisam.result +++ b/mysql-test/r/myisam.result @@ -944,3 +944,19 @@ select * from t1; a 42 drop table t1; +CREATE TABLE t1(a VARCHAR(16)); +INSERT INTO t1 VALUES('aaaaaaaa'),(NULL); +UPDATE t1 AS ta1, t1 AS ta2 SET ta1.a='aaaaaaaaaaaaaaaa'; +SELECT * FROM t1; +a +aaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaa +DROP TABLE t1; +CREATE TABLE t1(a INT); +INSERT INTO t1 VALUES(1),(2); +UPDATE t1,t1 AS t2 SET t1.a=t1.a+2 WHERE t1.a=t2.a-1; +SELECT * FROM t1 ORDER BY a; +a +2 +3 +DROP TABLE t1; diff --git a/mysql-test/t/myisam.test b/mysql-test/t/myisam.test index 745e3a2e377..7868e7bad4e 100644 --- a/mysql-test/t/myisam.test +++ b/mysql-test/t/myisam.test @@ -890,4 +890,23 @@ connection default; select * from t1; drop table t1; +# +# BUG#21310 - Trees in SQL causing a "crashed" table with MyISAM storage +# engine +# + +# A simplified test case that reflect crashed table issue. +CREATE TABLE t1(a VARCHAR(16)); +INSERT INTO t1 VALUES('aaaaaaaa'),(NULL); +UPDATE t1 AS ta1, t1 AS ta2 SET ta1.a='aaaaaaaaaaaaaaaa'; +SELECT * FROM t1; +DROP TABLE t1; + +# A test case that reflect wrong result set. +CREATE TABLE t1(a INT); +INSERT INTO t1 VALUES(1),(2); +UPDATE t1,t1 AS t2 SET t1.a=t1.a+2 WHERE t1.a=t2.a-1; +SELECT * FROM t1 ORDER BY a; +DROP TABLE t1; + # End of 4.1 tests diff --git a/sql/lock.cc b/sql/lock.cc index ab4a81034ba..3b2b2857f65 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -528,7 +528,7 @@ int mysql_lock_have_duplicate(THD *thd, TABLE *table, TABLE_LIST *tables) for (; tables; tables= tables->next) { table2= tables->table; - if (table2->tmp_table == TMP_TABLE) + if (table2->tmp_table == TMP_TABLE || table == table2) continue; /* All tables in list must be in lock. */ diff --git a/sql/sql_update.cc b/sql/sql_update.cc index 6742fbad5c4..9a326535adc 100644 --- a/sql/sql_update.cc +++ b/sql/sql_update.cc @@ -23,8 +23,6 @@ #include "mysql_priv.h" #include "sql_select.h" -static bool safe_update_on_fly(JOIN_TAB *join_tab, List *fields); - /* Return 0 if row hasn't changed */ static bool compare_record(TABLE *table, ulong query_id) @@ -846,30 +844,72 @@ int multi_update::prepare(List ¬_used_values, for (i=0 ; i < table_count ; i++) set_if_bigger(max_fields, fields_for_table[i]->elements); copy_field= new Copy_field[max_fields]; - - /* - Mark all copies of tables that are updates to ensure that - init_read_record() will not try to enable a cache on them - - The problem is that for queries like - - UPDATE t1, t1 AS t2 SET t1.b=t2.c WHERE t1.a=t2.a; - - the row buffer may contain things that doesn't match what is on disk - which will cause an error when reading a row. - (This issue is mostly relevent for MyISAM tables) - */ - for (table_ref= all_tables; table_ref; table_ref=table_ref->next) - { - TABLE *table=table_ref->table; - if ((tables_to_update & table->map) && - mysql_lock_have_duplicate(thd, table, update_tables)) - table->no_cache= 1; // Disable row cache - } DBUG_RETURN(thd->is_fatal_error != 0); } +/* + Check if table is safe to update on fly + + SYNOPSIS + safe_update_on_fly() + thd Thread handler + join_tab How table is used in join + all_tables List of tables + fields Fields that are updated + + NOTES + We can update the first table in join on the fly if we know that + a row in this table will never be read twice. This is true under + the following conditions: + + - We are doing a table scan and the data is in a separate file (MyISAM) or + if we don't update a clustered key. + + - We are doing a range scan and we don't update the scan key or + the primary key for a clustered table handler. + + - Table is not joined to itself. + + WARNING + This code is a bit dependent of how make_join_readinfo() works. + + RETURN + 0 Not safe to update + 1 Safe to update +*/ + +static bool safe_update_on_fly(THD *thd, JOIN_TAB *join_tab, + TABLE_LIST *all_tables, List *fields) +{ + TABLE *table= join_tab->table; + /* First check if a table is not joined to itself. */ + if (mysql_lock_have_duplicate(thd, table, all_tables)) + return 0; + switch (join_tab->type) { + case JT_SYSTEM: + case JT_CONST: + case JT_EQ_REF: + return 1; // At most one matching row + case JT_REF: + return !check_if_key_used(table, join_tab->ref.key, *fields); + case JT_ALL: + /* If range search on index */ + if (join_tab->quick) + return !check_if_key_used(table, join_tab->quick->index, + *fields); + /* If scanning in clustered key */ + if ((table->file->table_flags() & HA_PRIMARY_KEY_IN_READ_INDEX) && + table->primary_key < MAX_KEY) + return !check_if_key_used(table, table->primary_key, *fields); + return 1; + default: + break; // Avoid compler warning + } + return 0; +} + + /* Initialize table for multi table @@ -905,7 +945,7 @@ multi_update::initialize_tables(JOIN *join) table->file->extra(HA_EXTRA_IGNORE_DUP_KEY); if (table == main_table) // First table in join { - if (safe_update_on_fly(join->join_tab, &temp_fields)) + if (safe_update_on_fly(thd, join->join_tab, all_tables, &temp_fields)) { table_to_update= main_table; // Update table on the fly continue; @@ -951,59 +991,6 @@ multi_update::initialize_tables(JOIN *join) DBUG_RETURN(0); } -/* - Check if table is safe to update on fly - - SYNOPSIS - safe_update_on_fly - join_tab How table is used in join - fields Fields that are updated - - NOTES - We can update the first table in join on the fly if we know that - a row in this tabel will never be read twice. This is true under - the folloing conditions: - - - We are doing a table scan and the data is in a separate file (MyISAM) or - if we don't update a clustered key. - - - We are doing a range scan and we don't update the scan key or - the primary key for a clustered table handler. - - WARNING - This code is a bit dependent of how make_join_readinfo() works. - - RETURN - 0 Not safe to update - 1 Safe to update -*/ - -static bool safe_update_on_fly(JOIN_TAB *join_tab, List *fields) -{ - TABLE *table= join_tab->table; - switch (join_tab->type) { - case JT_SYSTEM: - case JT_CONST: - case JT_EQ_REF: - return 1; // At most one matching row - case JT_REF: - return !check_if_key_used(table, join_tab->ref.key, *fields); - case JT_ALL: - /* If range search on index */ - if (join_tab->quick) - return !check_if_key_used(table, join_tab->quick->index, - *fields); - /* If scanning in clustered key */ - if ((table->file->table_flags() & HA_PRIMARY_KEY_IN_READ_INDEX) && - table->primary_key < MAX_KEY) - return !check_if_key_used(table, table->primary_key, *fields); - return 1; - default: - break; // Avoid compler warning - } - return 0; -} - multi_update::~multi_update() {