1berry/MariaDB-server
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix for MDEV-5531: double call procedure in one session - hard shutdown the server

Browse Source 
Main fix was to not cache derivied tables as they may be temporary tables that are deleted before the next query.
This was a bit tricky as Item_field::fix_fields depended on cached_tables to be set to resolve some columns.



mysql-test/r/sp-bugs.result:
  Added test case
mysql-test/t/sp-bugs.test:
  Added test case
sql/item.cc:
  Fixed fix_outer_field to handle case where found field did not have in cached_table
  Idea is that if cached_table is not avaliable, use from_field->table->pos_in_table_list instead
sql/records.cc:
  Also accept INTERNAL_TMP_TABLE for memmap
sql/sql_base.cc:
  More DBUG_PRINT
  Fixed that setup_natural_join_row_types() is not run twice.
  Original code modified context->first_name_resolution_table also for second executions.
  This was wrong as this could give wrong results if some joins had been optimized away between calls.
sql/sql_derived.cc:
  Mark derived tables as internal temporary tables (INTERNAL_TMP_TABLE), not as NON_TRANSACTIONAL_TMP_TABLE.
  This is more correct as the tables are not visible by the end user.
sql/sql_insert.cc:
  Reset pos_in_table_list before calling fix_fields.
  One of the consequences of the change of not caching all generated tables in Item_ident is that
  pos_in_table_list needs to be correct in calls to fix_fields.
sql/sql_lex.cc:
  More DBUG_PRINT
sql/sql_parse.cc:
  Don't cache derivied tables as they may be temporary tables that are deleted before the next query
sql/sql_select.cc:
  Reset table_vector. This was required as some code checked the vector to see if temporary tables had already been created.
sql/table.cc:
  Mark tables with field translations as cacheable (as these will not disapper between stmt executions.
This commit is contained in:
Michael Widenius 2014-01-24 14:50:18 +02:00
parent 7335c6f2a4
commit d15b3386db
11 changed files with 213 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
mysql-test/r/sp-bugs.result
View File

@ -222,3 +222,49 @@ testf_bug11763507
DROP PROCEDURE testp_bug11763507;
DROP FUNCTION testf_bug11763507;
#END OF BUG#11763507 test.
#
# MDEV-5531 double call procedure in one session
#
CREATE TABLE `t1` (
`id` int(10) unsigned NOT NULL AUTO_INCREMENT,
`create_ts` int(10) unsigned DEFAULT '0',
PRIMARY KEY (`id`)
) ENGINE=MyISAM AUTO_INCREMENT=0 DEFAULT CHARSET=utf8;
CREATE PROCEDURE test_5531 (IN step TINYINT(1))
BEGIN
DECLARE counts INT DEFAULT 0;
DECLARE cur1 CURSOR FOR
SELECT ct.id
FROM (SELECT NULL) AS z
JOIN (
SELECT id
FROM `t1`
LIMIT 10
) AS ct
JOIN (SELECT NULL) AS x ON(
EXISTS(
SELECT 1
FROM `t1`
WHERE id=ct.id
LIMIT 1
)
);
IF step=1 THEN
TRUNCATE t1;
REPEAT
INSERT INTO `t1`
(create_ts) VALUES
(UNIX_TIMESTAMP());
SET counts=counts+1;
UNTIL counts>150 END REPEAT;
SET max_sp_recursion_depth=1;
CALL test_5531(2);
SET max_sp_recursion_depth=2;
CALL test_5531(2);
ELSEIF step=2 THEN
OPEN cur1; CLOSE cur1;
END IF;
END $$
CALL test_5531(1);
DROP PROCEDURE test_5531;
DROP TABLE t1;

57
mysql-test/t/sp-bugs.test
View File

@ -228,3 +228,60 @@ DROP PROCEDURE testp_bug11763507;
DROP FUNCTION testf_bug11763507;
--echo #END OF BUG#11763507 test.
--echo #
--echo # MDEV-5531 double call procedure in one session
--echo #
CREATE TABLE `t1` (
`id` int(10) unsigned NOT NULL AUTO_INCREMENT,
`create_ts` int(10) unsigned DEFAULT '0',
PRIMARY KEY (`id`)
) ENGINE=MyISAM AUTO_INCREMENT=0 DEFAULT CHARSET=utf8;
DELIMITER $$;
CREATE PROCEDURE test_5531 (IN step TINYINT(1))
BEGIN
DECLARE counts INT DEFAULT 0;
DECLARE cur1 CURSOR FOR
SELECT ct.id
FROM (SELECT NULL) AS z
JOIN (
SELECT id
FROM `t1`
LIMIT 10
) AS ct
JOIN (SELECT NULL) AS x ON(
EXISTS(
SELECT 1
FROM `t1`
WHERE id=ct.id
LIMIT 1
)
);
IF step=1 THEN
TRUNCATE t1;
REPEAT
INSERT INTO `t1`
(create_ts) VALUES
(UNIX_TIMESTAMP());
SET counts=counts+1;
UNTIL counts>150 END REPEAT;
SET max_sp_recursion_depth=1;
CALL test_5531(2);
SET max_sp_recursion_depth=2;
CALL test_5531(2);
ELSEIF step=2 THEN
OPEN cur1; CLOSE cur1;
END IF;
END $$
DELIMITER ;$$
CALL test_5531(1);
DROP PROCEDURE test_5531;
DROP TABLE t1;

54
sql/item.cc
View File

@ -4700,6 +4700,12 @@ bool is_outer_table(TABLE_LIST *table, SELECT_LEX *select)
/**
Resolve the name of an outer select column reference.
@param[in] thd current thread
@param[in,out] from_field found field reference or (Field*)not_found_field
@param[in,out] reference view column if this item was resolved to a
view column
@description
The method resolves the column reference represented by 'this' as a column
present in outer selects that contain current select.
@ -4709,10 +4715,16 @@ bool is_outer_table(TABLE_LIST *table, SELECT_LEX *select)
current select as dependent. The found reference of field should be
provided in 'from_field'.
@param[in] thd current thread
@param[in,out] from_field found field reference or (Field*)not_found_field
@param[in,out] reference view column if this item was resolved to a
view column
The cache is critical for prepared statements of type:
SELECT a FROM (SELECT a FROM test.t1) AS s1 NATURAL JOIN t2 AS s2;
This is internally converted to a join similar to
SELECT a FROM t1 AS s1,t2 AS s2 WHERE t2.a=t1.a;
Without the cache, we would on re-prepare not know if 'a' did match
s1.a or s2.a.
@note
This is the inner loop of Item_field::fix_fields:
@ -4742,7 +4754,12 @@ Item_field::fix_outer_field(THD *thd, Field **from_field, Item **reference)
enum_parsing_place place= NO_MATTER;
bool field_found= (*from_field != not_found_field);
bool upward_lookup= FALSE;
TABLE_LIST *table_list;
/* Calulate the TABLE_LIST for the table */
table_list= (cached_table ? cached_table :
field_found && (*from_field) != view_ref_found ?
(*from_field)->table->pos_in_table_list : 0);
/*
If there are outer contexts (outer selects, but current select is
not derived table or view) try to resolve this reference in the
@ -4761,6 +4778,15 @@ Item_field::fix_outer_field(THD *thd, Field **from_field, Item **reference)
if (current_sel->master_unit()->first_select()->linkage !=
DERIVED_TABLE_TYPE)
outer_context= context->outer_context;
/*
This assert is to ensure we have an outer contex when *from_field
is set.
If this would not be the case, we would assert in mark_as_dependent
as last_checked_countex == context
*/
DBUG_ASSERT(outer_context || !*from_field ||
*from_field == not_found_field);
for (;
outer_context;
outer_context= outer_context->outer_context)
@ -4777,7 +4803,7 @@ Item_field::fix_outer_field(THD *thd, Field **from_field, Item **reference)
to find_field_in_tables(). Only need to find appropriate context.
*/
if (field_found && outer_context->select_lex !=
cached_table->select_lex)
table_list->select_lex)
continue;
/*
In case of a view, find_field_in_tables() writes the pointer to
@ -4976,9 +5002,9 @@ Item_field::fix_outer_field(THD *thd, Field **from_field, Item **reference)
if (last_checked_context->select_lex->having_fix_field)
{
Item_ref *rf;
rf= new Item_ref(context,
(cached_table->db[0] ? cached_table->db : 0),
(char*) cached_table->alias, (char*) field_name);
rf= new Item_ref(context, (*from_field)->table->s->db.str,
(*from_field)->table->alias.c_ptr(),
(char*) field_name);
if (!rf)
return -1;
thd->change_item_tree(reference, rf);
@ -5049,6 +5075,7 @@ bool Item_field::fix_fields(THD *thd, Item **reference)
if (!field) // If field is not checked
{
TABLE_LIST *table_list;
/*
In case of view, find_field_in_tables() write pointer to view field
expression to 'reference', i.e. it substitute that expression instead
@ -5134,11 +5161,14 @@ bool Item_field::fix_fields(THD *thd, Item **reference)
else if (!from_field)
goto error;
if (!outer_fixed && cached_table && cached_table->select_lex &&
table_list= (cached_table ? cached_table :
from_field != view_ref_found ?
from_field->table->pos_in_table_list : 0);
if (!outer_fixed && table_list && table_list->select_lex &&
context->select_lex &&
cached_table->select_lex != context->select_lex &&
!context->select_lex->is_merged_child_of(cached_table->select_lex) &&
is_outer_table(cached_table, context->select_lex))
table_list->select_lex != context->select_lex &&
!context->select_lex->is_merged_child_of(table_list->select_lex) &&
is_outer_table(table_list, context->select_lex))
{
int ret;
if ((ret= fix_outer_field(thd, &from_field, reference)) < 0)

3
sql/records.cc
View File

@ -189,7 +189,8 @@ bool init_read_record(READ_RECORD *info,THD *thd, TABLE *table,
info->table=table;
info->forms= &info->table; /* Only one table */
if (table->s->tmp_table == NON_TRANSACTIONAL_TMP_TABLE &&
if ((table->s->tmp_table == INTERNAL_TMP_TABLE ||
table->s->tmp_table == NON_TRANSACTIONAL_TMP_TABLE) &&
!table->sort.addon_field)
(void) table->file->extra(HA_EXTRA_MMAP);

60
sql/sql_base.cc
View File

@ -235,6 +235,7 @@ static void check_unused(THD *thd)
uint count= 0, open_files= 0, idx= 0;
TABLE *cur_link, *start_link, *entry;
TABLE_SHARE *share;
DBUG_ENTER("check_unused");
if ((start_link=cur_link=unused_tables))
{
@ -243,7 +244,7 @@ static void check_unused(THD *thd)
if (cur_link != cur_link->next->prev || cur_link != cur_link->prev->next)
{
DBUG_PRINT("error",("Unused_links aren't linked properly")); /* purecov: inspected */
return; /* purecov: inspected */
DBUG_VOID_RETURN; /* purecov: inspected */
}
} while (count++ < table_cache_count &&
(cur_link=cur_link->next) != start_link);
@ -291,6 +292,7 @@ static void check_unused(THD *thd)
DBUG_PRINT("error",("Unused_links doesn't match open_cache: diff: %d", /* purecov: inspected */
count)); /* purecov: inspected */
}
DBUG_VOID_RETURN;
}
#else
#define check_unused(A)
@ -3103,6 +3105,7 @@ retry_share:
MDL_deadlock_handler mdl_deadlock_handler(ot_ctx);
bool wait_result;
DBUG_PRINT("info", ("old version of table share found"));
release_table_share(share);
mysql_mutex_unlock(&LOCK_open);
@ -3127,6 +3130,7 @@ retry_share:
and try to reopen them. Note: refresh_version is currently
changed only during FLUSH TABLES.
*/
DBUG_PRINT("info", ("share version differs between tables"));
release_table_share(share);
mysql_mutex_unlock(&LOCK_open);
(void)ot_ctx->request_backoff_action(Open_table_context::OT_REOPEN_TABLES,
@ -3139,12 +3143,17 @@ retry_share:
{
table= share->free_tables.front();
table_def_use_table(thd, table);
/* We need to release share as we have EXTRA reference to it in our hands. */
/*
We need to release share as we have EXTRA reference to it in our hands.
*/
DBUG_PRINT("info", ("release temporarily acquired table share"));
release_table_share(share);
}
else
{
/* We have too many TABLE instances around let us try to get rid of them. */
/*
We have too many TABLE instances around let us try to get rid of them.
*/
while (table_cache_count > table_cache_size && unused_tables)
free_cache_entry(unused_tables);
@ -3217,6 +3226,7 @@ err_unlock:
release_table_share(share);
mysql_mutex_unlock(&LOCK_open);
DBUG_PRINT("exit", ("failed"));
DBUG_RETURN(TRUE);
}
@ -6973,7 +6983,7 @@ find_field_in_tables(THD *thd, Item_ident *item,
*/
if (db)
return cur_field;
if (found)
{
if (report_error == REPORT_ALL_ERRORS ||
@ -6988,7 +6998,7 @@ find_field_in_tables(THD *thd, Item_ident *item,
if (found)
return found;
/*
If the field was qualified and there were no tables to search, issue
an error that an unknown table was given. The situation is detected
@ -7919,6 +7929,7 @@ err:
TRUE Error
FALSE OK
*/
static bool setup_natural_join_row_types(THD *thd,
List<TABLE_LIST> *from_clause,
Name_resolution_context *context)
@ -7928,6 +7939,18 @@ static bool setup_natural_join_row_types(THD *thd,
if (from_clause->elements == 0)
DBUG_RETURN(false); /* We come here in the case of UNIONs. */
/*
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_natural_join_processing)
{
DBUG_PRINT("info", ("using cached store_top_level_join_columns"));
DBUG_RETURN(false);
}
context->select_lex->first_natural_join_processing= false;
List_iterator_fast<TABLE_LIST> table_ref_it(*from_clause);
TABLE_LIST *table_ref; /* Current table reference. */
/* Table reference to the left of the current. */
@ -7944,22 +7967,15 @@ static bool setup_natural_join_row_types(THD *thd,
left_neighbor= table_ref_it++;
}
while (left_neighbor && left_neighbor->sj_subq_pred);
/*
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_natural_join_processing)
if (store_top_level_join_columns(thd, table_ref,
left_neighbor, right_neighbor))
DBUG_RETURN(true);
if (left_neighbor)
{
if (store_top_level_join_columns(thd, table_ref,
left_neighbor, right_neighbor))
DBUG_RETURN(true);
if (left_neighbor)
{
TABLE_LIST *first_leaf_on_the_right;
first_leaf_on_the_right= table_ref->first_leaf_for_name_resolution();
left_neighbor->next_name_resolution_table= first_leaf_on_the_right;
}
TABLE_LIST *first_leaf_on_the_right;
first_leaf_on_the_right= table_ref->first_leaf_for_name_resolution();
left_neighbor->next_name_resolution_table= first_leaf_on_the_right;
}
right_neighbor= table_ref;
}
@ -7973,8 +7989,6 @@ static bool setup_natural_join_row_types(THD *thd,
DBUG_ASSERT(right_neighbor);
context->first_name_resolution_table=
right_neighbor->first_leaf_for_name_resolution();
context->select_lex->first_natural_join_processing= false;
DBUG_RETURN (false);
}
@ -8576,7 +8590,7 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name,
if (!(item= field_iterator.create_item(thd)))
DBUG_RETURN(TRUE);
// DBUG_ASSERT(item->fixed);
/* cache the table for the Item_fields inserted by expanding stars */
if (item->type() == Item::FIELD_ITEM && tables->cacheable_table)
((Item_field *)item)->cached_table= tables;

2
sql/sql_derived.cc
View File

@ -716,7 +716,7 @@ exit:
{
TABLE *table= derived->table;
table->derived_select_number= first_select->select_number;
table->s->tmp_table= NON_TRANSACTIONAL_TMP_TABLE;
table->s->tmp_table= INTERNAL_TMP_TABLE;
#ifndef NO_EMBEDDED_ACCESS_CHECKS
if (derived->referencing_view)
table->grant= derived->grant;

3
sql/sql_insert.cc
View File

@ -2386,6 +2386,9 @@ TABLE *Delayed_insert::get_local_table(THD* client_thd)
bitmap= (uchar*) (field + share->fields + 1);
copy->record[0]= (bitmap + share->column_bitmap_size*3);
memcpy((char*) copy->record[0], (char*) table->record[0], share->reclength);
/* Ensure we don't use the table list of the original table */
copy->pos_in_table_list= 0;
/*
Make a copy of all fields.
The copied fields need to point into the copied record. This is done

10
sql/sql_lex.cc
View File

@ -2127,14 +2127,15 @@ void st_select_lex_unit::exclude_tree()
this to 'last' as dependent
SYNOPSIS
last - pointer to last st_select_lex struct, before wich all
last - pointer to last st_select_lex struct, before which all
st_select_lex have to be marked as dependent
NOTE
'last' should be reachable from this st_select_lex_node
*/
bool st_select_lex::mark_as_dependent(THD *thd, st_select_lex *last, Item *dependency)
bool st_select_lex::mark_as_dependent(THD *thd, st_select_lex *last,
Item *dependency)
{
DBUG_ASSERT(this != last);
@ -2325,7 +2326,10 @@ bool st_select_lex::setup_ref_array(THD *thd, uint order_group_num)
If we need a bigger array, we must allocate a new one.
*/
if (ref_pointer_array_size >= n_elems)
{
DBUG_PRINT("info", ("reusing old ref_array"));
return false;
}
}
ref_pointer_array= static_cast<Item**>(arena->alloc(sizeof(Item*) * n_elems));
if (ref_pointer_array != NULL)
@ -3328,6 +3332,7 @@ static void fix_prepare_info_in_table_list(THD *thd, TABLE_LIST *tbl)
void st_select_lex::fix_prepare_information(THD *thd, Item **conds,
Item **having_conds)
{
DBUG_ENTER("st_select_lex::fix_prepare_information");
if (!thd->stmt_arena->is_conventional() && first_execution)
{
first_execution= 0;
@ -3356,6 +3361,7 @@ void st_select_lex::fix_prepare_information(THD *thd, Item **conds,
}
fix_prepare_info_in_table_list(thd, table_list.first);
}
DBUG_VOID_RETURN;
}

7
sql/sql_parse.cc
View File

@ -1957,6 +1957,7 @@ mysql_execute_command(THD *thd)
bool have_table_map_for_update= FALSE;
#endif
DBUG_ENTER("mysql_execute_command");
#ifdef WITH_PARTITION_STORAGE_ENGINE
thd->work_part_info= 0;
#endif
@ -6128,7 +6129,11 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd,
ptr->schema_table= schema_table;
}
ptr->select_lex= lex->current_select;
ptr->cacheable_table= 1;
/*
We can't cache internal temporary tables between prepares as the
table may be deleted before next exection.
*/
ptr->cacheable_table= !table->is_derived_table();
ptr->index_hints= index_hints_arg;
ptr->option= option ? option->str : 0;
/* check that used name is unique */

8
sql/sql_select.cc
View File

@ -3180,7 +3180,7 @@ make_join_statistics(JOIN *join, List<TABLE_LIST> &tables_list,
stat_ref=(JOIN_TAB**) join->thd->alloc(sizeof(JOIN_TAB*)*
(MAX_TABLES + table_count + 1));
stat_vector= stat_ref + MAX_TABLES;
table_vector=(TABLE**) join->thd->alloc(sizeof(TABLE*)*(table_count*2));
table_vector=(TABLE**) join->thd->calloc(sizeof(TABLE*)*(table_count*2));
join->positions= new (join->thd->mem_root) POSITION[(table_count+1)];
/*
best_positions is ok to allocate with alloc() as we copy things to it with
@ -14802,7 +14802,8 @@ create_tmp_table(THD *thd, TMP_TABLE_PARAM *param, List<Item> &fields,
save_sum_fields|= param->precomputed_group_by;
DBUG_ENTER("create_tmp_table");
DBUG_PRINT("enter",
("distinct: %d save_sum_fields: %d rows_limit: %lu group: %d",
("table_alias: '%s' distinct: %d save_sum_fields: %d "
"rows_limit: %lu group: %d", table_alias,
(int) distinct, (int) save_sum_fields,
(ulong) rows_limit,test(group)));
@ -16258,7 +16259,8 @@ free_tmp_table(THD *thd, TABLE *entry)
MEM_ROOT own_root= entry->mem_root;
const char *save_proc_info;
DBUG_ENTER("free_tmp_table");
DBUG_PRINT("enter",("table: %s",entry->alias.c_ptr()));
DBUG_PRINT("enter",("table: %s alias: %s",entry->s->table_name.str,
entry->alias.c_ptr()));
save_proc_info=thd->proc_info;
thd_proc_info(thd, "removing tmp table");

9
sql/table.cc
View File

@ -4089,6 +4089,7 @@ bool TABLE_LIST::create_field_translation(THD *thd)
uint field_count= 0;
Query_arena *arena= thd->stmt_arena, backup;
bool res= FALSE;
DBUG_ENTER("TABLE_LIST::create_field_translation");
if (thd->stmt_arena->is_conventional() ||
thd->stmt_arena->is_stmt_prepare_or_first_sp_execute())
@ -4109,7 +4110,7 @@ bool TABLE_LIST::create_field_translation(THD *thd)
if (field_translation)
{
/*
Update items in the field translation aftet view have been prepared.
Update items in the field translation after view have been prepared.
It's needed because some items in the select list, like IN subselects,
might be substituted for optimized ones.
*/
@ -4122,7 +4123,7 @@ bool TABLE_LIST::create_field_translation(THD *thd)
field_translation_updated= TRUE;
}
return FALSE;
DBUG_RETURN(FALSE);
}
if (arena->is_conventional())
@ -4148,12 +4149,14 @@ bool TABLE_LIST::create_field_translation(THD *thd)
}
field_translation= transl;
field_translation_end= transl + field_count;
/* It's safe to cache this table for prepared statements */
cacheable_table= 1;
exit:
if (arena)
thd->restore_active_arena(arena, &backup);
return res;
DBUG_RETURN(res);
}