[Commits] bzr commit into Mariadb 5.2, with Maria 2.0:maria/5.2 branch (igor:2823) Bug#604503

Igor Babaev igor at askmonty.org
Sat Jul 17 21:16:25 EEST 2010


#At lp:maria/5.2 based on revid:igor at askmonty.org-20100713174523-mjvsvvp6ow8dc81x

 2823 Igor Babaev	2010-07-17
      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. 
      modified:
        mysql-test/suite/vcol/r/vcol_misc.result
        mysql-test/suite/vcol/t/vcol_misc.test
        sql/field.h
        sql/filesort.cc
        sql/item_cmpfunc.cc
        sql/mysql_priv.h
        sql/records.cc
        sql/sql_base.cc
        sql/sql_class.cc
        sql/sql_class.h
        sql/sql_delete.cc
        sql/sql_handler.cc
        sql/sql_select.cc
        sql/sql_table.cc
        sql/sql_update.cc
        sql/table.cc
        sql/table.h

=== modified file 'mysql-test/suite/vcol/r/vcol_misc.result'
--- a/mysql-test/suite/vcol/r/vcol_misc.result	2010-07-13 17:45:23 +0000
+++ b/mysql-test/suite/vcol/r/vcol_misc.result	2010-07-17 18:16:16 +0000
@@ -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;

=== modified file 'mysql-test/suite/vcol/t/vcol_misc.test'
--- a/mysql-test/suite/vcol/t/vcol_misc.test	2010-07-13 17:45:23 +0000
+++ b/mysql-test/suite/vcol/t/vcol_misc.test	2010-07-17 18:16:16 +0000
@@ -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;

=== modified file 'sql/field.h'
--- a/sql/field.h	2010-06-01 19:52:20 +0000
+++ b/sql/field.h	2010-07-17 18:16:16 +0000
@@ -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;

=== modified file 'sql/filesort.cc'
--- a/sql/filesort.cc	2010-07-13 17:45:23 +0000
+++ b/sql/filesort.cc	2010-07-17 18:16:16 +0000
@@ -568,7 +568,7 @@ static ha_rows find_all_keys(SORTPARAM *
       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 *
       {
 	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

=== modified file 'sql/item_cmpfunc.cc'
--- a/sql/item_cmpfunc.cc	2010-04-28 12:52:24 +0000
+++ b/sql/item_cmpfunc.cc	2010-07-17 18:16:16 +0000
@@ -825,7 +825,6 @@ Arg_comparator::can_compare_as_dates(Ite
   return cmp_type;
 }
 
-
 /*
   Retrieves correct TIME value from the given item.
 
@@ -876,7 +875,12 @@ get_time_value(THD *thd, Item ***item_ar
   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_re
         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 ***ite
   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);

=== modified file 'sql/mysql_priv.h'
--- a/sql/mysql_priv.h	2010-06-16 17:01:22 +0000
+++ b/sql/mysql_priv.h	2010-07-17 18:16:16 +0000
@@ -1365,7 +1365,7 @@ find_field_in_table(THD *thd, TABLE *tab
                     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 */
 

=== modified file 'sql/records.cc'
--- a/sql/records.cc	2010-06-01 19:52:20 +0000
+++ b/sql/records.cc	2010-07-17 18:16:16 +0000
@@ -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;
 }
 

=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc	2010-06-01 19:52:20 +0000
+++ b/sql/sql_base.cc	2010-07-17 18:16:16 +0000
@@ -8252,7 +8252,7 @@ fill_record(THD * thd, List<Item> &field
         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
       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<
         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
   {
     TABLE *table= (*ptr)->table;
     if (table->vfield)
-      result= update_virtual_fields(table, TRUE);
+      result= update_virtual_fields(thd, table, TRUE);
   }
   return result;
 

=== modified file 'sql/sql_class.cc'
--- a/sql/sql_class.cc	2010-06-01 19:52:20 +0000
+++ b/sql/sql_class.cc	2010-07-17 18:16:16 +0000
@@ -802,6 +802,8 @@ THD::THD()
   thr_lock_owner_init(&main_lock_id, &lock_info);
 
   m_internal_handler= NULL;
+
+  arena_for_cached_items= 0;
 }
 
 

=== modified file 'sql/sql_class.h'
--- a/sql/sql_class.h	2010-06-16 17:01:22 +0000
+++ b/sql/sql_class.h	2010-07-17 18:16:16 +0000
@@ -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;

=== modified file 'sql/sql_delete.cc'
--- a/sql/sql_delete.cc	2010-07-03 03:24:39 +0000
+++ b/sql/sql_delete.cc	2010-07-17 18:16:16 +0000
@@ -304,7 +304,7 @@ bool mysql_delete(THD *thd, TABLE_LIST *
   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)
     {

=== modified file 'sql/sql_handler.cc'
--- a/sql/sql_handler.cc	2009-12-08 21:47:54 +0000
+++ b/sql/sql_handler.cc	2010-07-17 18:16:16 +0000
@@ -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)

=== modified file 'sql/sql_select.cc'
--- a/sql/sql_select.cc	2010-06-05 14:53:36 +0000
+++ b/sql/sql_select.cc	2010-07-17 18:16:16 +0000
@@ -11628,7 +11628,7 @@ evaluate_join_record(JOIN *join, JOIN_TA
   }
   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
     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

=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc	2010-06-05 14:53:36 +0000
+++ b/sql/sql_table.cc	2010-07-17 18:16:16 +0000
@@ -7891,7 +7891,7 @@ copy_data_between_tables(TABLE *from,TAB
       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,TAB
       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;

=== modified file 'sql/sql_update.cc'
--- a/sql/sql_update.cc	2010-07-03 03:24:39 +0000
+++ b/sql/sql_update.cc	2010-07-17 18:16:16 +0000
@@ -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())

=== modified file 'sql/table.cc'
--- a/sql/table.cc	2010-07-13 14:34:14 +0000
+++ b/sql/table.cc	2010-07-17 18:16:16 +0000
@@ -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
   }
   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, cons
 /*
   @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, cons
     >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, 
   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, 
       DBUG_PRINT("info", ("field '%s' - skipped", vfield->field_name));
     }
   }
+  thd->reset_arena_for_cached_items(0);
   DBUG_RETURN(0);
 }
 

=== modified file 'sql/table.h'
--- a/sql/table.h	2010-06-03 09:28:54 +0000
+++ b/sql/table.h	2010-07-17 18:16:16 +0000
@@ -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 */



More information about the commits mailing list