MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:eugene Date:March 29 2006 7:30pm
Subject:bk commit into 4.1 tree (evgen:1.2473) BUG#15560
View as plain text  
Below is the list of changes that have just been committed into a local
4.1 repository of evgen. When evgen does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet
  1.2473 06/03/29 23:30:34 evgen@stripped +8 -0
  Fixed bug#15560: GROUP_CONCAT wasn't ready for WITH ROLLUP queries
  
  The GROUP_CONCAT uses its own temporary table. When ROLLUP is present
  it creates the second copy of Item_func_group_concat. This copy receives the
  same list of arguments that original group_concat does. When the copy is
  set up the result_fields of functions from the argument list are reset to the
  temporary table of this copy.
  As a result of this action data from functions flow directly to the ROLLUP copy
  and the original group_concat functions shows wrong result.
  Since queries with COUNT(DISTINCT ...) use temporary tables to store
  the results the COUNT function they are also affected by this bug.
  
  The idea of the fix is to copy content of the result_field for the function
  under GROUP_CONCAT/COUNT from  the first temporary table to the second one,
  rather than setting result_field to point to the second temporary table.
  To achieve this goal force_copy_fields flag is added to Item_func_group_concat
  and Item_sum_count_distinct classes. This flag is initialized to 0 and set to 1
  into the make_unique() member function of both classes.
  To the TMP_TABLE_PARAM structure is modified to include the similar flag as
  well.
  The create_tmp_table() function passes that flag to create_tmp_field().
  When the flag is set the create_tmp_field() function will set result_field
  as a source field and will not reset that result field to newly created 
  field for Item_func_result_field and its descendants. Due to this there
  will be created copy func to copy data from old result_field to newly 
  created field.

  sql/item_sum.h
    1.82 06/03/29 23:28:47 evgen@stripped +6 -3
    Fixed bug#15560: GROUP_CONCAT wasn't ready for WITH ROLLUP queries
    Added the flag 'force_copy_fields' to the Item_func_group_concat and Item_sum_count_distinct classes.

  sql/item_sum.cc
    1.147 06/03/29 23:25:19 evgen@stripped +6 -0
    Fixed bug#15560: GROUP_CONCAT wasn't ready for WITH ROLLUP queries
    Added initialization of the force_copy_fields flag and passing it to create_tmp_table() through TMP_TBLE_PARAM in the Item_func_group_concat and Item_sum_count_distinct member functions.

  sql/mysql_priv.h
    1.374 06/03/29 23:20:58 evgen@stripped +3 -2
    Fixed bug#15560: GROUP_CONCAT wasn't ready for WITH ROLLUP queries
    Added the bool parameter 'make_copy_field' to create_tmp_field().

  sql/sql_class.h
    1.284 06/03/29 23:18:44 evgen@stripped +3 -2
    Fixed bug#15560: GROUP_CONCAT wasn't ready for WITH ROLLUP queries
    Added the flag 'force_copy_fields' to the structure TMP_TABLE_PARAM in order to make create_tmp_field() force the creation of 'copy_field' objects.

  sql/sql_select.cc
    1.450 06/03/29 23:17:40 evgen@stripped +15 -6
    Fixed bug#15560: GROUP_CONCAT wasn't ready for WITH ROLLUP queries
    
    Added the flag 'make_copy_field' to create_tmp_field(), so that for Item_result_field descendants create_tmp_field() sets the item's result field as a source field and deny resetting that result field to a new value.

  sql/sql_table.cc
    1.305 06/03/29 23:15:39 evgen@stripped +1 -1
    Fixed bug#15560: GROUP_CONCAT wasn't ready for WITH ROLLUP queries
    Added 0 as a last parameter to create_tmp_field()  to force old behaviour.

  mysql-test/r/func_gconcat.result
    1.43 06/03/29 23:15:07 evgen@stripped +15 -0
    Added test for bug#15560: GROUP_CONCAT wasn't ready for WITH ROLLUP queries

  mysql-test/t/func_gconcat.test
    1.34 06/03/29 23:14:58 evgen@stripped +8 -0
    Added test for bug#15560: GROUP_CONCAT wasn't ready for WITH ROLLUP queries

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	evgen
# Host:	moonbone.local
# Root:	/work/15560-bug-4.1-mysql

--- 1.146/sql/item_sum.cc	2005-09-21 10:49:16 +04:00
+++ 1.147/sql/item_sum.cc	2006-03-29 23:25:19 +04:00
@@ -1185,6 +1185,7 @@
   original= 0;
   use_tree= 0; // to prevent delete_tree call on uninitialized tree
   tree= &tree_base;
+  force_copy_fields= 1;
 }
 
 
@@ -1219,6 +1220,7 @@
     free_tmp_table(thd, table);
     tmp_table_param->cleanup();
   }
+  tmp_table_param->force_copy_fields= force_copy_fields;
   if (!(table= create_tmp_table(thd, tmp_table_param, list, (ORDER*) 0, 1,
 				0,
 				select_lex->options | thd->options,
@@ -1724,6 +1726,7 @@
 					       String *is_separator)
   :Item_sum(), tmp_table_param(0), max_elements_in_tree(0), warning(0),
    key_length(0), tree_mode(0), distinct(is_distinct), warning_for_row(0),
+   force_copy_fields(0),
    separator(is_separator), tree(&tree_base), table(0),
    order(0), tables_list(0),
    arg_count_order(0), arg_count_field(0),
@@ -1785,6 +1788,7 @@
   tree_mode(item->tree_mode),
   distinct(item->distinct),
   warning_for_row(item->warning_for_row),
+  force_copy_fields(item->force_copy_fields),
   separator(item->separator),
   tree(item->tree),
   table(item->table),
@@ -2004,6 +2008,7 @@
     free_tmp_table(thd, table);
     tmp_table_param->cleanup();
   }
+  tmp_table_param->force_copy_fields= force_copy_fields;
   /*
     We have to create a temporary table to get descriptions of fields 
     (types, sizes and so on).
@@ -2079,6 +2084,7 @@
   original= 0;
   tree_mode= 0; // to prevent delete_tree call on uninitialized tree
   tree= &tree_base;
+  force_copy_fields= 1;
 }
 
 

--- 1.81/sql/item_sum.h	2005-09-07 09:03:08 +04:00
+++ 1.82/sql/item_sum.h	2006-03-29 23:28:47 +04:00
@@ -185,6 +185,7 @@
   TMP_TABLE_PARAM *tmp_table_param;
   TREE tree_base;
   TREE *tree;
+  bool force_copy_fields;
   /*
     Following is 0 normal object and pointer to original one for copy 
     (to correctly free resources)
@@ -226,15 +227,16 @@
   public:
   Item_sum_count_distinct(List<Item> &list)
     :Item_sum_int(list), table(0), used_table_cache(~(table_map) 0),
-     tmp_table_param(0), tree(&tree_base), original(0), use_tree(0),
-     always_null(0)
+     tmp_table_param(0), tree(&tree_base), force_copy_fields(0), original(0),
+     use_tree(0), always_null(0)
   { quick_group= 0; }
   Item_sum_count_distinct(THD *thd, Item_sum_count_distinct *item)
     :Item_sum_int(thd, item), table(item->table),
      used_table_cache(item->used_table_cache),
      field_lengths(item->field_lengths),
      tmp_table_param(item->tmp_table_param),
-     tree(item->tree), original(item), key_length(item->key_length),
+     tree(item->tree), force_copy_fields(item->force_copy_fields),
+     original(item), key_length(item->key_length),
      max_elements_in_tree(item->max_elements_in_tree),
      rec_offset(item->rec_offset), use_tree(item->use_tree),
      always_null(item->always_null)
@@ -685,6 +687,7 @@
   bool distinct;
   bool warning_for_row;
   bool always_null;
+  bool force_copy_fields;
 
   friend int group_concat_key_cmp_with_distinct(void* arg, byte* key1,
 					      byte* key2);

--- 1.373/sql/mysql_priv.h	2005-12-07 21:54:07 +03:00
+++ 1.374/sql/mysql_priv.h	2006-03-29 23:20:58 +04:00
@@ -537,8 +537,9 @@
 		SELECT_LEX_UNIT *unit);
 int mysql_handle_derived(LEX *lex);
 Field *create_tmp_field(THD *thd, TABLE *table,Item *item, Item::Type type,
-			Item ***copy_func, Field **from_field,
-			bool group, bool modify_item, uint convert_blob_length);
+                        Item ***copy_func, Field **from_field,
+                        bool group, bool modify_item, uint convert_blob_length,
+                        bool make_copy_field);
 int mysql_prepare_table(THD *thd, HA_CREATE_INFO *create_info,
 		       List<create_field> &fields,
 		       List<Key> &keys, uint &db_options, 

--- 1.283/sql/sql_class.h	2005-09-21 02:18:26 +04:00
+++ 1.284/sql/sql_class.h	2006-03-29 23:18:44 +04:00
@@ -1334,10 +1334,11 @@
   bool  using_indirect_summary_function;
   /* If >0 convert all blob fields to varchar(convert_blob_length) */
   uint  convert_blob_length; 
-
+  bool force_copy_fields;
   TMP_TABLE_PARAM()
     :copy_field(0), group_parts(0),
-    group_length(0), group_null_parts(0), convert_blob_length(0)
+    group_length(0), group_null_parts(0), convert_blob_length(0),
+    force_copy_fields(0)
   {}
   ~TMP_TABLE_PARAM()
   {

--- 1.449/sql/sql_select.cc	2006-01-26 17:00:46 +03:00
+++ 1.450/sql/sql_select.cc	2006-03-29 23:17:40 +04:00
@@ -4984,7 +4984,8 @@
 
 Field *create_tmp_field(THD *thd, TABLE *table,Item *item, Item::Type type,
                         Item ***copy_func, Field **from_field,
-                        bool group, bool modify_item, uint convert_blob_length)
+                        bool group, bool modify_item, uint convert_blob_length,
+                        bool make_copy_field)
 {
   switch (type) {
   case Item::SUM_FUNC_ITEM:
@@ -5071,7 +5072,13 @@
   case Item::REF_ITEM:
   case Item::NULL_ITEM:
   case Item::VARBIN_ITEM:
-    return create_tmp_field_from_item(thd, item, table, copy_func, modify_item,
+    if (make_copy_field)
+    {
+      DBUG_ASSERT(((Item_result_field*)item)->result_field);
+      *from_field= ((Item_result_field*)item)->result_field;
+    }
+    return create_tmp_field_from_item(thd, item, table, (make_copy_field ? 0 :
+                                      copy_func), modify_item,
                                       convert_blob_length);
   case Item::TYPE_HOLDER:
     return ((Item_type_holder *)item)->make_field_by_type(table);
@@ -5110,7 +5117,7 @@
   Item **copy_func;
   MI_COLUMNDEF *recinfo;
   uint temp_pool_slot=MY_BIT_NONE;
-
+  bool force_copy_fields= param->force_copy_fields;
   DBUG_ENTER("create_tmp_table");
   DBUG_PRINT("enter",("distinct: %d  save_sum_fields: %d  rows_limit: %lu  group: %d",
 		      (int) distinct, (int) save_sum_fields,
@@ -5241,7 +5248,7 @@
 	  Field *new_field=
             create_tmp_field(thd, table, arg, arg->type(), &copy_func,
                              tmp_from_field, group != 0,not_all_columns,
-                             param->convert_blob_length);
+                             param->convert_blob_length, 0);
 	  if (!new_field)
 	    goto err;					// Should be OOM
 	  tmp_from_field++;
@@ -5279,8 +5286,10 @@
       */
       Field *new_field= create_tmp_field(thd, table, item, type, &copy_func,
                                          tmp_from_field, group != 0,
-                                         not_all_columns || group !=0,
-                                         param->convert_blob_length);
+                                         !force_copy_fields &&
+                                           (not_all_columns || group !=0),
+                                         param->convert_blob_length,
+                                         force_copy_fields);
       if (!new_field)
       {
 	if (thd->is_fatal_error)

--- 1.304/sql/sql_table.cc	2006-01-16 18:21:39 +03:00
+++ 1.305/sql/sql_table.cc	2006-03-29 23:15:39 +04:00
@@ -1562,7 +1562,7 @@
       field=item->tmp_table_field(&tmp_table);
     else
       field=create_tmp_field(thd, &tmp_table, item, item->type(),
-				  (Item ***) 0, &tmp_field, 0, 0, 0);
+				  (Item ***) 0, &tmp_field, 0, 0, 0, 0);
     if (!field ||
 	!(cr_field=new create_field(field,(item->type() == Item::FIELD_ITEM ?
 					   ((Item_field *)item)->field :

--- 1.42/mysql-test/r/func_gconcat.result	2005-10-15 18:46:09 +04:00
+++ 1.43/mysql-test/r/func_gconcat.result	2006-03-29 23:15:07 +04:00
@@ -589,3 +589,18 @@
 ,x
 ,z
 DROP TABLE t1;
+create table t1(f1 int);
+insert into t1 values(1),(2),(3);
+select f1, group_concat(f1+1) from t1 group by f1 with rollup;
+f1	group_concat(f1+1)
+1	2
+2	3
+3	4
+NULL	2,3,4
+select count(distinct (f1+1)) from t1 group by f1 with rollup;
+count(distinct (f1+1))
+1
+1
+1
+3
+drop table t1;

--- 1.33/mysql-test/t/func_gconcat.test	2005-09-08 03:33:06 +04:00
+++ 1.34/mysql-test/t/func_gconcat.test	2006-03-29 23:14:58 +04:00
@@ -382,4 +382,12 @@
 
 DROP TABLE t1;
 
+#
+# Bug #15560: GROUP_CONCAT wasn't ready for WITH ROLLUP queries
+#
+create table t1(f1 int);
+insert into t1 values(1),(2),(3);
+select f1, group_concat(f1+1) from t1 group by f1 with rollup;
+select count(distinct (f1+1)) from t1 group by f1 with rollup;
+drop table t1;
 # End of 4.1 tests
Thread
bk commit into 4.1 tree (evgen:1.2473) BUG#15560eugene29 Mar