From 452860dfdead7f47b0312598db95ea09201e9d4e Mon Sep 17 00:00:00 2001 From: Igor Babaev Date: Sat, 17 Jul 2010 11:16:16 -0700 Subject: [PATCH] Fixed bug #604503. If the expression for a virtual column of table contained datetime comparison then the execution of the second query that used this virtual column caused a crash. It happened because the execution of the first query that used this virtual column inserted a cached item into the expression tree. The cached tree was allocated in the statement memory while the expression tree was allocated in the table memory. Now the cached items that are inserted into expressions for virtual columns with datetime comparisons are always allocated in the same mem_root as the expressions for virtual columns. So now the inserted cached items are valid for any queries that use these virtual columns. --- mysql-test/suite/vcol/r/vcol_misc.result | 25 +++++++++++++++++ mysql-test/suite/vcol/t/vcol_misc.test | 22 +++++++++++++++ sql/field.h | 11 +------- sql/filesort.cc | 4 +-- sql/item_cmpfunc.cc | 16 ++++++++++- sql/mysql_priv.h | 2 +- sql/records.cc | 4 +-- sql/sql_base.cc | 8 +++--- sql/sql_class.cc | 2 ++ sql/sql_class.h | 21 +++++++++++++++ sql/sql_delete.cc | 2 +- sql/sql_handler.cc | 2 +- sql/sql_select.cc | 8 +++--- sql/sql_table.cc | 4 +-- sql/sql_update.cc | 4 +-- sql/table.cc | 34 +++++++++++++++--------- sql/table.h | 9 +++++++ 17 files changed, 136 insertions(+), 42 deletions(-) diff --git a/mysql-test/suite/vcol/r/vcol_misc.result b/mysql-test/suite/vcol/r/vcol_misc.result index fe41e7e65f3..3c461a2e308 100644 --- a/mysql-test/suite/vcol/r/vcol_misc.result +++ b/mysql-test/suite/vcol/r/vcol_misc.result @@ -45,3 +45,28 @@ C 1 1 DROP TABLE t1; +CREATE TABLE t1 ( +a datetime NOT NULL DEFAULT '2000-01-01', +v boolean AS (a < '2001-01-01') +); +INSERT INTO t1(a) VALUES ('2002-02-15'); +INSERT INTO t1(a) VALUES ('2000-10-15'); +SELECT a, v FROM t1; +a v +2002-02-15 00:00:00 0 +2000-10-15 00:00:00 1 +SELECT a, v FROM t1; +a v +2002-02-15 00:00:00 0 +2000-10-15 00:00:00 1 +CREATE TABLE t2 ( +a datetime NOT NULL DEFAULT '2000-01-01', +v boolean AS (a < '2001-01-01') PERSISTENT +); +INSERT INTO t2(a) VALUES ('2002-02-15'); +INSERT INTO t2(a) VALUES ('2000-10-15'); +SELECT * FROM t2; +a v +2002-02-15 00:00:00 0 +2000-10-15 00:00:00 1 +DROP TABLE t1, t2; diff --git a/mysql-test/suite/vcol/t/vcol_misc.test b/mysql-test/suite/vcol/t/vcol_misc.test index 2ed08a69fa2..20ae15dcd9c 100644 --- a/mysql-test/suite/vcol/t/vcol_misc.test +++ b/mysql-test/suite/vcol/t/vcol_misc.test @@ -43,5 +43,27 @@ SELECT 1 AS C FROM t1 ORDER BY v; DROP TABLE t1; +# +# Bug#604503: Virtual column expression with datetime comparison +# +CREATE TABLE t1 ( + a datetime NOT NULL DEFAULT '2000-01-01', + v boolean AS (a < '2001-01-01') +); +INSERT INTO t1(a) VALUES ('2002-02-15'); +INSERT INTO t1(a) VALUES ('2000-10-15'); +SELECT a, v FROM t1; +SELECT a, v FROM t1; + +CREATE TABLE t2 ( + a datetime NOT NULL DEFAULT '2000-01-01', + v boolean AS (a < '2001-01-01') PERSISTENT +); +INSERT INTO t2(a) VALUES ('2002-02-15'); +INSERT INTO t2(a) VALUES ('2000-10-15'); + +SELECT * FROM t2; + +DROP TABLE t1, t2; diff --git a/sql/field.h b/sql/field.h index 9b31cbc7b70..53cfb982e32 100644 --- a/sql/field.h +++ b/sql/field.h @@ -73,20 +73,11 @@ public: Item *expr_item; /* Text representation of the defining expression */ LEX_STRING expr_str; - /* - The list of items created when the defining expression for the virtual - column is being parsed and validated. These items are freed in the closefrm - function when the table containing this virtual column is removed from - the TABLE cache. - TODO. Items for all different virtual columns of a table should be put into - one list attached to the TABLE structure. - */ - Item *item_free_list; Virtual_column_info() : field_type((enum enum_field_types)MYSQL_TYPE_VIRTUAL), stored_in_db(FALSE), in_partitioning_expr(FALSE), - expr_item(NULL), item_free_list(NULL) + expr_item(NULL) { expr_str.str= NULL; expr_str.length= 0; diff --git a/sql/filesort.cc b/sql/filesort.cc index 270f0f1ac37..0997eeca4d7 100644 --- a/sql/filesort.cc +++ b/sql/filesort.cc @@ -568,7 +568,7 @@ static ha_rows find_all_keys(SORTPARAM *param, SQL_SELECT *select, if ((error= select->quick->get_next())) break; if (!error) - update_virtual_fields(sort_form); + update_virtual_fields(thd, sort_form); file->position(sort_form->record[0]); DBUG_EXECUTE_IF("debug_filesort", dbug_print_record(sort_form, TRUE);); } @@ -587,7 +587,7 @@ static ha_rows find_all_keys(SORTPARAM *param, SQL_SELECT *select, { error=file->ha_rnd_next(sort_form->record[0]); if (!error) - update_virtual_fields(sort_form); + update_virtual_fields(thd, sort_form); if (!flag) { my_store_ptr(ref_pos,ref_length,record); // Position to row diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 01eb1ca506a..43958b31212 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -825,7 +825,6 @@ Arg_comparator::can_compare_as_dates(Item *a, Item *b, ulonglong *const_value) return cmp_type; } - /* Retrieves correct TIME value from the given item. @@ -876,7 +875,12 @@ get_time_value(THD *thd, Item ***item_arg, Item **cache_arg, if (item->const_item() && cache_arg && (item->type() != Item::FUNC_ITEM || ((Item_func*)item)->functype() != Item_func::GUSERVAR_FUNC)) { + Query_arena backup; + Query_arena *save_arena= thd->switch_to_arena_for_cached_items(&backup); Item_cache_int *cache= new Item_cache_int(); + if (save_arena) + thd->set_query_arena(save_arena); + /* Mark the cache as non-const to prevent re-caching. */ cache->set_used_tables(1); cache->store_longlong(item, value); @@ -912,7 +916,12 @@ int Arg_comparator::set_cmp_func(Item_result_field *owner_arg, cache_converted_constant can't be used here because it can't correctly convert a DATETIME value from string to int representation. */ + Query_arena backup; + Query_arena *save_arena= thd->switch_to_arena_for_cached_items(&backup); Item_cache_int *cache= new Item_cache_int(); + if (save_arena) + thd->set_query_arena(save_arena); + /* Mark the cache as non-const to prevent re-caching. */ cache->set_used_tables(1); if (!(*a)->is_datetime()) @@ -1142,7 +1151,12 @@ get_datetime_value(THD *thd, Item ***item_arg, Item **cache_arg, if (item->const_item() && cache_arg && (item->type() != Item::FUNC_ITEM || ((Item_func*)item)->functype() != Item_func::GUSERVAR_FUNC)) { + Query_arena backup; + Query_arena *save_arena= thd->switch_to_arena_for_cached_items(&backup); Item_cache_int *cache= new Item_cache_int(MYSQL_TYPE_DATETIME); + if (save_arena) + thd->set_query_arena(save_arena); + /* Mark the cache as non-const to prevent re-caching. */ cache->set_used_tables(1); cache->store_longlong(item, value); diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index 299222e4c95..4eb7f651ec6 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -1365,7 +1365,7 @@ find_field_in_table(THD *thd, TABLE *table, const char *name, uint length, bool allow_rowid, uint *cached_field_index_ptr); Field * find_field_in_table_sef(TABLE *table, const char *name); -int update_virtual_fields(TABLE *table, bool ignore_stored= FALSE); +int update_virtual_fields(THD *thd, TABLE *table, bool ignore_stored= FALSE); #endif /* MYSQL_SERVER */ diff --git a/sql/records.cc b/sql/records.cc index e2a1ea9b4af..b4552a3c450 100644 --- a/sql/records.cc +++ b/sql/records.cc @@ -327,7 +327,7 @@ static int rr_quick(READ_RECORD *info) break; } } - update_virtual_fields(info->table); + update_virtual_fields(info->thd, info->table); return tmp; } @@ -396,7 +396,7 @@ int rr_sequential(READ_RECORD *info) } } if (!tmp) - update_virtual_fields(info->table); + update_virtual_fields(info->thd, info->table); return tmp; } diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 8baff521c02..e4c7df6131e 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -8252,7 +8252,7 @@ fill_record(THD * thd, List &fields, List &values, prev_table= table; if (table->vfield) { - if (update_virtual_fields(table, TRUE)) + if (update_virtual_fields(thd, table, TRUE)) { goto err; } @@ -8320,7 +8320,7 @@ fill_record_n_invoke_before_triggers(THD *thd, List &fields, if (item_field && item_field->field && (table= item_field->field->table) && table->vfield) - result= update_virtual_fields(table, TRUE); + result= update_virtual_fields(thd, table, TRUE); } } return result; @@ -8411,7 +8411,7 @@ fill_record(THD *thd, Field **ptr, List &values, bool ignore_errors) prev_table= table; if (table->vfield) { - if (update_virtual_fields(table, TRUE)) + if (update_virtual_fields(thd, table, TRUE)) { goto err; } @@ -8471,7 +8471,7 @@ fill_record_n_invoke_before_triggers(THD *thd, Field **ptr, { TABLE *table= (*ptr)->table; if (table->vfield) - result= update_virtual_fields(table, TRUE); + result= update_virtual_fields(thd, table, TRUE); } return result; diff --git a/sql/sql_class.cc b/sql/sql_class.cc index bae42ad85bd..a6d783bf6f4 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -802,6 +802,8 @@ THD::THD() thr_lock_owner_init(&main_lock_id, &lock_info); m_internal_handler= NULL; + + arena_for_cached_items= 0; } diff --git a/sql/sql_class.h b/sql/sql_class.h index e9129983240..3680e28246e 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -2379,6 +2379,27 @@ public: Protected with LOCK_thd_data mutex. */ void set_query(char *query_arg, uint32 query_length_arg); + +private: + /* + This reference points to the table arena when the expression + for a virtual column is being evaluated + */ + Query_arena *arena_for_cached_items; + +public: + void reset_arena_for_cached_items(Query_arena *new_arena) + { + arena_for_cached_items= new_arena; + } + Query_arena *switch_to_arena_for_cached_items(Query_arena *backup) + { + if (!arena_for_cached_items) + return 0; + set_n_backup_active_arena(arena_for_cached_items, backup); + return backup; + } + private: /** The current internal error handler for this thread, or NULL. */ Internal_error_handler *m_internal_handler; diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc index eb05077f17a..8a016295eaa 100644 --- a/sql/sql_delete.cc +++ b/sql/sql_delete.cc @@ -304,7 +304,7 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, COND *conds, while (!(error=info.read_record(&info)) && !thd->killed && ! thd->is_error()) { - update_virtual_fields(table); + update_virtual_fields(thd, table); // thd->is_error() is tested to disallow delete row on error if (!select || select->skip_record(thd) > 0) { diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc index 2f3ce99ab9c..61f29886da8 100644 --- a/sql/sql_handler.cc +++ b/sql/sql_handler.cc @@ -664,7 +664,7 @@ retry: goto ok; } /* Generate values for virtual fields */ - update_virtual_fields(table); + update_virtual_fields(thd, table); if (cond && !cond->val_int()) continue; if (num_rows >= offset_limit_cnt) diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 8df5e4f697f..9912fa9a4e2 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -11628,7 +11628,7 @@ evaluate_join_record(JOIN *join, JOIN_TAB *join_tab, } DBUG_PRINT("info", ("select cond 0x%lx", (ulong)select_cond)); - update_virtual_fields(join_tab->table); + update_virtual_fields(join->thd, join_tab->table); if (select_cond) { @@ -11850,7 +11850,7 @@ flush_cached_records(JOIN *join,JOIN_TAB *join_tab,bool skip_last) int err= 0; SQL_SELECT *select=join_tab->select; if (rc == NESTED_LOOP_OK) - update_virtual_fields(join_tab->table); + update_virtual_fields(join->thd, join_tab->table); if (rc == NESTED_LOOP_OK && (!join_tab->cache.select || (err= join_tab->cache.select->skip_record(join->thd)) != 0 )) @@ -12038,7 +12038,7 @@ join_read_system(JOIN_TAB *tab) empty_record(table); // Make empty record return -1; } - update_virtual_fields(table); + update_virtual_fields(tab->join->thd, table); store_record(table,record[1]); } else if (!table->status) // Only happens with left join @@ -12087,7 +12087,7 @@ join_read_const(JOIN_TAB *tab) return report_error(table, error); return -1; } - update_virtual_fields(table); + update_virtual_fields(tab->join->thd, table); store_record(table,record[1]); } else if (!(table->status & ~STATUS_NULL_ROW)) // Only happens with left join diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 624aab93e33..1d1233786ed 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -7891,7 +7891,7 @@ copy_data_between_tables(TABLE *from,TABLE *to, error= 1; break; } - update_virtual_fields(from); + update_virtual_fields(thd, from); thd->row_count++; /* Return error if source table isn't empty. */ if (error_if_not_empty) @@ -7912,7 +7912,7 @@ copy_data_between_tables(TABLE *from,TABLE *to, copy_ptr->do_copy(copy_ptr); } prev_insert_id= to->file->next_insert_id; - update_virtual_fields(to, TRUE); + update_virtual_fields(thd, to, TRUE); if (thd->is_error()) { error= 1; diff --git a/sql/sql_update.cc b/sql/sql_update.cc index e260310206a..4e2b1411f32 100644 --- a/sql/sql_update.cc +++ b/sql/sql_update.cc @@ -469,7 +469,7 @@ int mysql_update(THD *thd, while (!(error=info.read_record(&info)) && !thd->killed && !thd->is_error()) { - update_virtual_fields(table); + update_virtual_fields(thd, table); if (!select || select->skip_record(thd) > 0) { if (table->file->was_semi_consistent_read()) @@ -576,7 +576,7 @@ int mysql_update(THD *thd, while (!(error=info.read_record(&info)) && !thd->killed) { - update_virtual_fields(table); + update_virtual_fields(thd, table); if (!select || select->skip_record(thd) > 0) { if (table->file->was_semi_consistent_read()) diff --git a/sql/table.cc b/sql/table.cc index 451515c1262..334596e28b2 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -1928,10 +1928,8 @@ end: parses it, building an item object for it. The pointer to this item is placed into in field->vcol_info.expr_item. After this the function performs semantic analysis of the item by calling the the function fix_vcol_expr. - Since the defining expression is part of the table definition the item - for it is created in table->memroot within a separate Query_arena. - The free_list of this arena is saved in field->vcol_info.item_free_list - to be freed when the table defition is removed from the TABLE_SHARE cache. + Since the defining expression is part of the table definition the item for + it is created in table->memroot within the special arena TABLE::expr_arena. @note Before passing 'vcol_expr" to the parser the function embraces it in @@ -1988,9 +1986,18 @@ bool unpack_vcol_info_from_frm(THD *thd, */ Query_arena *backup_stmt_arena_ptr= thd->stmt_arena; Query_arena backup_arena; - Query_arena vcol_arena(&table->mem_root, Query_arena::INITIALIZED); - thd->set_n_backup_active_arena(&vcol_arena, &backup_arena); - thd->stmt_arena= &vcol_arena; + Query_arena *vcol_arena= table->expr_arena; + if (!vcol_arena) + { + Query_arena expr_arena(&table->mem_root, Query_arena::INITIALIZED); + if (!(vcol_arena= (Query_arena *) alloc_root(&table->mem_root, + sizeof(Query_arena)))) + goto err; + *vcol_arena= expr_arena; + table->expr_arena= vcol_arena; + } + thd->set_n_backup_active_arena(vcol_arena, &backup_arena); + thd->stmt_arena= vcol_arena; thd->lex->parse_vcol_expr= TRUE; old_character_set_client= thd->variables.character_set_client; @@ -2012,7 +2019,6 @@ bool unpack_vcol_info_from_frm(THD *thd, field->vcol_info= 0; goto err; } - field->vcol_info->item_free_list= thd->free_list; goto end; err: @@ -2021,7 +2027,8 @@ err: thd->free_items(); end: thd->stmt_arena= backup_stmt_arena_ptr; - thd->restore_active_arena(&vcol_arena, &backup_arena); + if (vcol_arena) + thd->restore_active_arena(vcol_arena, &backup_arena); thd->variables.character_set_client= old_character_set_client; DBUG_RETURN(rc); @@ -2444,12 +2451,12 @@ int closefrm(register TABLE *table, bool free_share) } my_free((char*) table->alias, MYF(MY_ALLOW_ZERO_PTR)); table->alias= 0; + if (table->expr_arena) + table->expr_arena->free_items(); if (table->field) { for (Field **ptr=table->field ; *ptr ; ptr++) { - if ((*ptr)->vcol_info) - free_items((*ptr)->vcol_info->item_free_list); delete *ptr; } table->field= 0; @@ -5416,6 +5423,7 @@ size_t max_row_length(TABLE *table, const uchar *data) /* @brief Compute values for virtual columns used in query + @param thd Thread handle @param table The TABLE object @param for_write Requests to compute only fields needed for write @@ -5432,7 +5440,7 @@ size_t max_row_length(TABLE *table, const uchar *data) >0 Error occurred when storing a virtual field value */ -int update_virtual_fields(TABLE *table, bool for_write) +int update_virtual_fields(THD *thd, TABLE *table, bool for_write) { DBUG_ENTER("update_virtual_fields"); Field **vfield_ptr, *vfield; @@ -5440,6 +5448,7 @@ int update_virtual_fields(TABLE *table, bool for_write) if (!table || !table->vfield) DBUG_RETURN(0); + thd->reset_arena_for_cached_items(table->expr_arena); /* Iterate over virtual fields in the table */ for (vfield_ptr= table->vfield; *vfield_ptr; vfield_ptr++) { @@ -5458,6 +5467,7 @@ int update_virtual_fields(TABLE *table, bool for_write) DBUG_PRINT("info", ("field '%s' - skipped", vfield->field_name)); } } + thd->reset_arena_for_cached_items(0); DBUG_RETURN(0); } diff --git a/sql/table.h b/sql/table.h index 1424414ade8..f968a3ee173 100644 --- a/sql/table.h +++ b/sql/table.h @@ -27,6 +27,7 @@ class st_select_lex; class partition_info; class COND_EQUAL; class Security_context; +class Query_arena; /*************************************************************************/ @@ -869,6 +870,14 @@ struct st_table { MEM_ROOT mem_root; GRANT_INFO grant; FILESORT_INFO sort; + /* + The arena which the items for expressions from the table definition + are associated with. + Currently only the items of the expressions for virtual columns are + associated with this arena. + TODO: To attach the partitioning expressions to this arena. + */ + Query_arena *expr_arena; #ifdef WITH_PARTITION_STORAGE_ENGINE partition_info *part_info; /* Partition related information */ bool no_partitions_used; /* If true, all partitions have been pruned away */