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

@@ -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;

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;

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;

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

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);

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 */
 

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;
 }
 

sql/sql_base.cc

@@ -8252,7 +8252,7 @@ fill_record(THD * thd, List<Item> &fields, List<Item> &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<Item> &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<Item> &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;
 

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;
 }
 
 

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;

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)
     {

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)

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

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;

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())

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
+    Since the defining expression is part of the table definition the item for
-    for it is created in table->memroot within a separate Query_arena.
+    it is created in table->memroot within the special arena TABLE::expr_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.
 
   @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);
+  Query_arena *vcol_arena= table->expr_arena;
-  thd->set_n_backup_active_arena(&vcol_arena, &backup_arena);
+  if (!vcol_arena)
-  thd->stmt_arena= &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);
 }
 

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 */