#At file:///work/bzr/5.0-5.1.29-rc-38691/
2689 Gleb Shchepa 2008-10-07
Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while
``FLUSH TABLES WITH READ LOCK''
Concurrent execution of 1) multitable update with a
NATURAL/USING join and 2) a such query as "FLUSH TABLES
WITH READ LOCK" or "ALTER TABLE" of updating table led
to a server crash.
The mysql_multi_update_prepare() function call is optimized
to lock updating tables only, so it postpones locking to
the last, and if locking fails, it does cleanup of modified
syntax structures and repeats a query analysis. However,
that cleanup procedure was incomplete for NATURAL/USING join
syntax data: 1) some Field_item items pointed into freed
table structures, and 2) the TABLE_LIST::join_columns fields
was not reset.
Major change:
short-living Field *Natural_join_column::table_field has
been replaced with long-living Item*.
modified:
mysql-test/r/lock_multi.result
mysql-test/t/lock_multi.test
sql/item.cc
sql/sql_base.cc
sql/sql_class.cc
sql/sql_class.h
sql/sql_update.cc
sql/table.cc
sql/table.h
per-file messages:
mysql-test/r/lock_multi.result
Added test case for bug #38691.
mysql-test/t/lock_multi.test
Added test case for bug #38691.
sql/item.cc
Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while
``FLUSH TABLES WITH READ LOCK''
The Item_field constructor has been modified to allocate
and copy original database/table/field names always (not
during PS preparation/1st execution only), because
an initialization of Item_field items with a pointer to
short-living Field structures is a common practice.
sql/sql_base.cc
Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while
``FLUSH TABLES WITH READ LOCK''
1) Type adjustment for Natural_join_column::table_field
(Field to Item_field);
2) The setup_natural_join_row_types function has been
updated to take into account new thd->multiupdate_reopen
flag to skip unnecessary reinitialization of
Natural_join_column::join_columns during table
reopening after lock_tables() failure (like the
'first_execution' flag for PS).
sql/sql_class.cc
Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while
``FLUSH TABLES WITH READ LOCK''
Initialization of new THD::multiupdate_reopen has
been added.
sql/sql_class.h
Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while
``FLUSH TABLES WITH READ LOCK''
The THD::multiupdate_reopen flag has been added to
mark table reopening state for multitable update.
sql/sql_update.cc
Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while
``FLUSH TABLES WITH READ LOCK''
1) Extra cleanup calls have been added to reset
Natural_join_column::table_field items.
2) Setting/resetting on the THD::multiupdate_reopen
flag has been added.
sql/table.cc
Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while
``FLUSH TABLES WITH READ LOCK''
Type adjustment for Natural_join_column::table_field
(Field to Item_field);
sql/table.h
Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while
``FLUSH TABLES WITH READ LOCK''
Type of the Natural_join_column::table_field field has
been changed from Field that points into short-living
TABLE memory to long-living Item_field that can be
linked to (fixed) reopened table.
=== modified file 'mysql-test/r/lock_multi.result'
--- a/mysql-test/r/lock_multi.result 2007-11-28 12:18:01 +0000
+++ b/mysql-test/r/lock_multi.result 2008-10-07 11:42:39 +0000
@@ -99,3 +99,13 @@ kill query
ERROR 70100: Query execution was interrupted
unlock tables;
drop table t1;
+CREATE TABLE t1 (
+a int(11) unsigned default NULL,
+b varchar(255) default NULL,
+UNIQUE KEY a (a),
+KEY b (b)
+);
+INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 3);
+CREATE TABLE t2 SELECT * FROM t1;
+CREATE TABLE t3 SELECT * FROM t1;
+DROP TABLE t1, t2, t3;
=== modified file 'mysql-test/t/lock_multi.test'
--- a/mysql-test/t/lock_multi.test 2007-11-28 12:18:01 +0000
+++ b/mysql-test/t/lock_multi.test 2008-10-07 11:42:39 +0000
@@ -281,4 +281,123 @@ unlock tables;
connection default;
drop table t1;
+#
+# Bug #38691: segfault/abort in ``UPDATE ...JOIN'' while
+# ``FLUSH TABLES WITH READ LOCK''
+#
+
+--connection default
+CREATE TABLE t1 (
+ a int(11) unsigned default NULL,
+ b varchar(255) default NULL,
+ UNIQUE KEY a (a),
+ KEY b (b)
+);
+
+--echo # test altering of columns that multiupdate doesn't use
+
+--echo # normal mode
+
+INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 3);
+CREATE TABLE t2 SELECT * FROM t1;
+CREATE TABLE t3 SELECT * FROM t1;
+
+--disable_query_log
+let $i = 100;
+while ($i) {
+--dec $i
+
+--connection writer
+ send UPDATE t2 INNER JOIN (t1 JOIN t3 USING(a)) USING(a)
+ SET a = NULL WHERE t1.b <> t2.b;
+
+--connection locker
+ ALTER TABLE t2 ADD COLUMN (c INT);
+ ALTER TABLE t2 DROP COLUMN c;
+
+--connection writer
+--reap
+}
+
+--echo # PS mode
+
+--connection writer
+PREPARE stmt FROM 'UPDATE t2 INNER JOIN (t1 JOIN t3 USING(a)) USING(a)
+ SET a = NULL WHERE t1.b <> t2.b';
+
+let $i = 100;
+while ($i) {
+--dec $i
+
+--connection writer
+--send EXECUTE stmt
+
+--connection locker
+ ALTER TABLE t2 ADD COLUMN (c INT);
+ ALTER TABLE t2 DROP COLUMN c;
+
+--connection writer
+--reap
+}
+--enable_query_log
+
+
+--echo # test altering of columns that multiupdate uses
+
+--echo # normal mode
+
+--connection default
+
+--disable_query_log
+let $i = 100;
+while ($i) {
+ dec $i;
+
+--connection locker
+--error 0,1060
+ ALTER TABLE t2 ADD COLUMN a int(11) unsigned default NULL;
+ UPDATE t2 SET a=b;
+
+--connection writer
+--send UPDATE t2 INNER JOIN (t1 JOIN t3 USING(a)) USING(a) SET a = NULL WHERE t1.b <> t2.b
+
+--connection locker
+--error 0,1091
+ ALTER TABLE t2 DROP COLUMN a;
+
+--connection writer
+--error 0,1054
+--reap
+}
+--enable_query_log
+
+--echo # PS mode
+
+--disable_query_log
+let $i = 100;
+while ($i) {
+ dec $i;
+
+--connection locker
+--error 0,1060
+ ALTER TABLE t2 ADD COLUMN a int(11) unsigned default NULL;
+ UPDATE t2 SET a=b;
+
+--connection writer
+ PREPARE stmt FROM 'UPDATE t2 INNER JOIN (t1 JOIN t3 USING(a)) USING(a) SET a = NULL WHERE t1.b <> t2.b';
+--send EXECUTE stmt
+
+--connection locker
+--error 0,1091
+ ALTER TABLE t2 DROP COLUMN a;
+
+--connection writer
+--error 0,1054
+--reap
+
+}
+--enable_query_log
+--connection default
+DROP TABLE t1, t2, t3;
+
# End of 5.0 tests
=== modified file 'sql/item.cc'
--- a/sql/item.cc 2008-10-01 09:48:47 +0000
+++ b/sql/item.cc 2008-10-07 11:42:39 +0000
@@ -1758,14 +1758,16 @@ Item_field::Item_field(THD *thd, Name_re
We need to copy db_name, table_name and field_name because they must
be allocated in the statement memory, not in table memory (the table
structure can go away and pop up again between subsequent executions
- of a prepared statement).
+ of a prepared statement or after the close_tables_for_reopen() call
+ in mysql_multi_update_prepare()).
*/
- if (thd->stmt_arena->is_stmt_prepare_or_first_sp_execute())
{
if (db_name)
orig_db_name= thd->strdup(db_name);
- orig_table_name= thd->strdup(table_name);
- orig_field_name= thd->strdup(field_name);
+ if (table_name)
+ orig_table_name= thd->strdup(table_name);
+ if (field_name)
+ orig_field_name= thd->strdup(field_name);
/*
We don't restore 'name' in cleanup because it's not changed
during execution. Still we need it to point to persistent
=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc 2008-03-21 15:23:17 +0000
+++ b/sql/sql_base.cc 2008-10-07 11:42:39 +0000
@@ -3617,8 +3617,21 @@ find_field_in_natural_join(THD *thd, TAB
{
/* This is a base table. */
DBUG_ASSERT(nj_col->view_field == NULL);
- DBUG_ASSERT(nj_col->table_ref->table == nj_col->table_field->table);
- found_field= nj_col->table_field;
+ /*
+ This fix_fields is not necessary (initially this item is fixed by
+ the Item_field constructor; after reopen_tables the Item_func_eq
+ calls fix_fields on that item), it's just a check during table
+ reopening for columns that was dropped by the concurrent connection.
+ */
+ if (!nj_col->table_field->fixed &&
+ nj_col->table_field->fix_fields(thd, (Item **)&nj_col->table_field))
+ {
+ DBUG_PRINT("info", ("column '%s' was dropped by the concurrent connection",
+ nj_col->table_field->name));
+ DBUG_RETURN(NULL);
+ }
+ DBUG_ASSERT(nj_col->table_ref->table == nj_col->table_field->field->table);
+ found_field= nj_col->table_field->field;
update_field_dependencies(thd, found_field, nj_col->table_ref->table);
}
@@ -4450,7 +4463,7 @@ mark_common_columns(THD *thd, TABLE_LIST
const char *field_name_1;
/* true if field_name_1 is a member of using_fields */
bool is_using_column_1;
- if (!(nj_col_1= it_1.get_or_create_column_ref(leaf_1)))
+ if (!(nj_col_1= it_1.get_or_create_column_ref(thd, leaf_1)))
goto err;
field_name_1= nj_col_1->name();
is_using_column_1= using_fields &&
@@ -4471,7 +4484,7 @@ mark_common_columns(THD *thd, TABLE_LIST
{
Natural_join_column *cur_nj_col_2;
const char *cur_field_name_2;
- if (!(cur_nj_col_2= it_2.get_or_create_column_ref(leaf_2)))
+ if (!(cur_nj_col_2= it_2.get_or_create_column_ref(thd, leaf_2)))
goto err;
cur_field_name_2= cur_nj_col_2->name();
DBUG_PRINT ("info", ("cur_field_name_2=%s.%s",
@@ -4963,8 +4976,12 @@ static bool setup_natural_join_row_types
{
table_ref= left_neighbor;
left_neighbor= table_ref_it++;
- /* For stored procedures do not redo work if already done. */
- if (context->select_lex->first_execution)
+ /*
+ Do not redo work if already done:
+ 1) for stored procedures,
+ 2) for multitable update after lock failure and table reopening.
+ */
+ if (context->select_lex->first_execution && !thd->multiupdate_reopen)
{
if (store_top_level_join_columns(thd, table_ref,
left_neighbor, right_neighbor))
=== modified file 'sql/sql_class.cc'
--- a/sql/sql_class.cc 2008-09-17 06:34:00 +0000
+++ b/sql/sql_class.cc 2008-10-07 11:42:39 +0000
@@ -178,6 +178,7 @@ THD::THD()
last_insert_id_used(0), last_insert_id_used_bin_log(0), insert_id_used(0),
clear_next_insert_id(0), in_lock_tables(0), bootstrap(0),
derived_tables_processing(FALSE), spcont(NULL),
+ multiupdate_reopen(0),
m_parser_state(NULL)
{
ulong tmp;
=== modified file 'sql/sql_class.h'
--- a/sql/sql_class.h 2008-09-17 06:34:00 +0000
+++ b/sql/sql_class.h 2008-10-07 11:42:39 +0000
@@ -1574,6 +1574,12 @@ public:
query_id_t first_query_id;
} binlog_evt_union;
+ /*
+ TRUE if mysql_multi_update_prepare is reopening tables after lock_table
+ failure. For use in setup_natural_join_row_types only.
+ */
+ bool multiupdate_reopen;
+
/**
Internal parser state.
Note that since the parser is not re-entrant, we keep only one parser
=== modified file 'sql/sql_update.cc'
--- a/sql/sql_update.cc 2008-07-15 14:13:21 +0000
+++ b/sql/sql_update.cc 2008-10-07 11:42:39 +0000
@@ -744,7 +744,7 @@ reopen_tables:
if (((original_multiupdate || need_reopen) &&
open_tables(thd, &table_list, &table_count, 0)) ||
mysql_handle_derived(lex, &mysql_derived_prepare))
- DBUG_RETURN(TRUE);
+ goto err;
/*
setup_tables() need for VIEWs. JOIN::prepare() will call setup_tables()
second time, but this call will do nothing (there are check for second
@@ -756,10 +756,10 @@ reopen_tables:
table_list, &lex->select_lex.where,
&lex->select_lex.leaf_tables, FALSE,
UPDATE_ACL, SELECT_ACL))
- DBUG_RETURN(TRUE);
+ goto err;
if (setup_fields_with_no_wrap(thd, 0, *fields, 1, 0, 0))
- DBUG_RETURN(TRUE);
+ goto err;
for (tl= table_list; tl ; tl= tl->next_local)
{
@@ -772,7 +772,7 @@ reopen_tables:
if (update_view && check_fields(thd, *fields))
{
- DBUG_RETURN(TRUE);
+ goto err;
}
tables_for_update= get_table_map(fields);
@@ -795,7 +795,7 @@ reopen_tables:
if (!tl->updatable || check_key_in_view(thd, tl))
{
my_error(ER_NON_UPDATABLE_TABLE, MYF(0), tl->alias, "UPDATE");
- DBUG_RETURN(TRUE);
+ goto err;
}
if (table->triggers)
@@ -832,7 +832,7 @@ reopen_tables:
tl->db, &tl->grant.privilege, 0, 0,
test(tl->schema_table)) ||
(grant_option && check_grant(thd, want_privilege, tl, 0, 1, 0)))
- DBUG_RETURN(TRUE);
+ goto err;
}
}
@@ -846,7 +846,7 @@ reopen_tables:
{
my_error(ER_VIEW_MULTIUPDATE, MYF(0),
tl->view_db.str, tl->view_name.str);
- DBUG_RETURN(-1);
+ goto err;
}
}
}
@@ -855,7 +855,10 @@ reopen_tables:
if (lock_tables(thd, table_list, table_count, &need_reopen))
{
if (!need_reopen)
- DBUG_RETURN(TRUE);
+ goto err;
+
+ DBUG_PRINT("info", ("lock_tables failed, reopening"));
+ thd->multiupdate_reopen= TRUE;
/*
We have to reopen tables since some of them were altered or dropped
@@ -872,10 +875,20 @@ reopen_tables:
for (TABLE_LIST *tbl= table_list; tbl; tbl= tbl->next_global)
tbl->cleanup_items();
+ /*
+ Also we need to cleanup Natural_join_column::table_field items.
+ To not to traverse a join tree we will cleanup whole
+ thd->free_list (in PS execution mode that list may not contain
+ items from 'fields' list, so the cleanup above is necessary to.
+ */
+ cleanup_items(thd->free_list);
+
close_tables_for_reopen(thd, &table_list);
goto reopen_tables;
}
+ thd->multiupdate_reopen= FALSE;
+
/*
Check that we are not using table that we are updating, but we should
skip all tables of UPDATE SELECT itself
@@ -901,7 +914,7 @@ reopen_tables:
if ((duplicate= unique_table(thd, tl, table_list, 0)))
{
update_non_unique_table_error(table_list, "UPDATE", duplicate);
- DBUG_RETURN(TRUE);
+ goto err;
}
}
}
@@ -913,9 +926,13 @@ reopen_tables:
if (thd->fill_derived_tables() &&
mysql_handle_derived(lex, &mysql_derived_filling))
- DBUG_RETURN(TRUE);
+ goto err;
DBUG_RETURN (FALSE);
+
+err:
+ thd->multiupdate_reopen= FALSE;
+ DBUG_RETURN(TRUE);
}
=== modified file 'sql/table.cc'
--- a/sql/table.cc 2007-10-10 13:26:02 +0000
+++ b/sql/table.cc 2008-10-07 11:42:39 +0000
@@ -2191,7 +2191,7 @@ TABLE_LIST *TABLE_LIST::find_underlying_
}
/*
- cleunup items belonged to view fields translation table
+ cleanup items belonged to view fields translation table
SYNOPSIS
TABLE_LIST::cleanup_items()
@@ -2637,10 +2637,10 @@ Natural_join_column::Natural_join_column
}
-Natural_join_column::Natural_join_column(Field *field_param,
+Natural_join_column::Natural_join_column(Item_field *field_param,
TABLE_LIST *tab)
{
- DBUG_ASSERT(tab->table == field_param->table);
+ DBUG_ASSERT(tab->table == field_param->field->table);
table_field= field_param;
view_field= NULL;
table_ref= tab;
@@ -2668,7 +2668,7 @@ Item *Natural_join_column::create_item(T
return create_view_field(thd, table_ref, &view_field->item,
view_field->name);
}
- return new Item_field(thd, &thd->lex->current_select->context, table_field);
+ return table_field;
}
@@ -2679,7 +2679,7 @@ Field *Natural_join_column::field()
DBUG_ASSERT(table_field == NULL);
return NULL;
}
- return table_field;
+ return table_field->field;
}
@@ -2811,7 +2811,7 @@ void Field_iterator_natural_join::next()
cur_column_ref= column_ref_it++;
DBUG_ASSERT(!cur_column_ref || ! cur_column_ref->table_field ||
cur_column_ref->table_ref->table ==
- cur_column_ref->table_field->table);
+ cur_column_ref->table_field->field->table);
}
@@ -2975,7 +2975,7 @@ GRANT_INFO *Field_iterator_table_ref::gr
*/
Natural_join_column *
-Field_iterator_table_ref::get_or_create_column_ref(TABLE_LIST *parent_table_ref)
+Field_iterator_table_ref::get_or_create_column_ref(THD *thd, TABLE_LIST *parent_table_ref)
{
Natural_join_column *nj_col;
bool is_created= TRUE;
@@ -2988,7 +2988,11 @@ Field_iterator_table_ref::get_or_create_
{
/* The field belongs to a stored table. */
Field *tmp_field= table_field_it.field();
- nj_col= new Natural_join_column(tmp_field, table_ref);
+ Item_field *tmp_item=
+ new Item_field(thd, &thd->lex->current_select->context, tmp_field);
+ if (!tmp_item)
+ return NULL;
+ nj_col= new Natural_join_column(tmp_item, table_ref);
field_count= table_ref->table->s->fields;
}
else if (field_it == &view_field_it)
@@ -3012,7 +3016,7 @@ Field_iterator_table_ref::get_or_create_
DBUG_ASSERT(nj_col);
}
DBUG_ASSERT(!nj_col->table_field ||
- nj_col->table_ref->table == nj_col->table_field->table);
+ nj_col->table_ref->table == nj_col->table_field->field->table);
/*
If the natural join column was just created add it to the list of
@@ -3077,7 +3081,7 @@ Field_iterator_table_ref::get_natural_co
nj_col= natural_join_it.column_ref();
DBUG_ASSERT(nj_col &&
(!nj_col->table_field ||
- nj_col->table_ref->table == nj_col->table_field->table));
+ nj_col->table_ref->table == nj_col->table_field->field->table));
return nj_col;
}
=== modified file 'sql/table.h'
--- a/sql/table.h 2007-09-24 12:34:10 +0000
+++ b/sql/table.h 2008-10-07 11:42:39 +0000
@@ -18,6 +18,7 @@
class Item; /* Needed by ORDER */
class Item_subselect;
+class Item_field;
class GRANT_TABLE;
class st_select_lex_unit;
class st_select_lex;
@@ -469,7 +470,7 @@ class Natural_join_column: public Sql_al
{
public:
Field_translator *view_field; /* Column reference of merge view. */
- Field *table_field; /* Column reference of table or temp view. */
+ Item_field *table_field; /* Column reference of table or temp view. */
TABLE_LIST *table_ref; /* Original base table/view reference. */
/*
True if a common join column of two NATURAL/USING join operands. Notice
@@ -481,7 +482,7 @@ public:
bool is_common;
public:
Natural_join_column(Field_translator *field_param, TABLE_LIST *tab);
- Natural_join_column(Field *field_param, TABLE_LIST *tab);
+ Natural_join_column(Item_field *field_param, TABLE_LIST *tab);
const char *name();
Item *create_item(THD *thd);
Field *field();
@@ -899,7 +900,7 @@ public:
GRANT_INFO *grant();
Item *create_item(THD *thd) { return field_it->create_item(thd); }
Field *field() { return field_it->field(); }
- Natural_join_column *get_or_create_column_ref(TABLE_LIST *parent_table_ref);
+ Natural_join_column *get_or_create_column_ref(THD *thd, TABLE_LIST *parent_table_ref);
Natural_join_column *get_natural_column_ref();
};
