MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:eugene Date:March 15 2006 9:15am
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/15 12:14:58 evgen@stripped +11 -0
  Fixed bug#15560: GROUP_CONCAT wasn't ready for WITH ROLLUP queries
  
  The GROUP_CONCAT is using it's internal temporary table. When ROLLUP is present,
  it creates second copy of Item_func_group_concat. That copy receives same list
  of arguments that original group_concat. When the copy sets up it resets
  result_fields of functions, from it's argument list, to it's temporary table.
  As a result of this a data from functions flows directly to the ROLLUP copy,
  and the orginal group_concat functions shows wrong result.
  Due to COUNT(DISTINCT ...) also uses an internal temporary table, it is also
  affected by this bug.
  
  The solution is to copy content of the function result field from first
  temporary table to the second rather than setting result field to field in
  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 initialized to 0 and set to 1
  into the make_unique() member function of both classes.
  The create_tmp_table() function is modified to pass that flag to
  create_tmp_field(). When the flag is set, the create_tmp_field() function
  for Item_func_result_field and it's descendants will set result_field as a
  source field and will not reset that result field to newly created field.
  Due to this there will be created copy func to copy data from old result_field
  to newly created field.

  sql/sql_table.cc
    1.305 06/03/15 12:13:17 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.

  sql/sql_derived.cc
    1.80 06/03/15 12:12:44 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_table() to force old behaviour.

  sql/sql_union.cc
    1.145 06/03/15 12:12:19 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_table() to force old behaviour.

  sql/sql_update.cc
    1.154 06/03/15 12:11:06 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_table() to force old behaviour.

  sql/item_sum.cc
    1.147 06/03/15 12:08:32 evgen@stripped +6 -2
    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() in Item_func_group_concat and Item_sum_count_distinct member functions.

  sql/item_sum.h
    1.82 06/03/15 12:06:29 evgen@stripped +6 -3
    Fixed bug#15560: GROUP_CONCAT wasn't ready for WITH ROLLUP queries
    To the Item_func_group_concat and Item_sum_count_distinct classes the force_copy_fields flag is added.

  sql/mysql_priv.h
    1.374 06/03/15 12:02:37 evgen@stripped +3 -2
    Fixed bug#15560: GROUP_CONCAT wasn't ready for WITH ROLLUP queries
    To the creat_tmp_field() added the bool make_copy_field parameter.

  sql/sql_select.cc
    1.450 06/03/15 11:42:21 evgen@stripped +17 -8
    Fixed bug#15560: GROUP_CONCAT wasn't ready for WITH ROLLUP queries
    To the create_tmp_field() added the make_copy_field flag which for Item_result_field descendants will set item's result field as a source field and deny resetting that result field to new value.

  sql/sql_select.h
    1.80 06/03/15 11:39:02 evgen@stripped +1 -1
    Fixed bug#15560: GROUP_CONCAT wasn't ready for WITH ROLLUP queries
    Added bool force_copy_field parameter to the create_tmp_table() function.

  mysql-test/r/func_gconcat.result
    1.43 06/03/15 11:37:48 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/15 11:36:09 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:	sunlight.local
# Root:	/work_local/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-15 12:08:32 +03:00
@@ -1185,6 +1185,7 @@ void Item_sum_count_distinct::make_uniqu
   original= 0;
   use_tree= 0; // to prevent delete_tree call on uninitialized tree
   tree= &tree_base;
+  force_copy_fields= 1;
 }
 
 
@@ -1222,7 +1223,7 @@ bool Item_sum_count_distinct::setup(THD 
   if (!(table= create_tmp_table(thd, tmp_table_param, list, (ORDER*) 0, 1,
 				0,
 				select_lex->options | thd->options,
-				HA_POS_ERROR, (char*)"")))
+				HA_POS_ERROR, (char*)"", force_copy_fields)))
     return 1;
   table->file->extra(HA_EXTRA_NO_ROWS);		// Don't update rows
   table->no_rows=1;
@@ -1724,6 +1725,7 @@ Item_func_group_concat::Item_func_group_
 					       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 +1787,7 @@ Item_func_group_concat::Item_func_group_
   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),
@@ -2017,7 +2020,7 @@ bool Item_func_group_concat::setup(THD *
   if (!(table=create_tmp_table(thd, tmp_table_param, all_fields, 
 			       (ORDER*) 0, 0, TRUE,
                                select_lex->options | thd->options,
-			       HA_POS_ERROR,(char *) "")))
+			       HA_POS_ERROR,(char *) "", force_copy_fields)))
     DBUG_RETURN(1);
   table->file->extra(HA_EXTRA_NO_ROWS);
   table->no_rows= 1;
@@ -2079,6 +2082,7 @@ void Item_func_group_concat::make_unique
   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-15 12:06:29 +03:00
@@ -185,6 +185,7 @@ class Item_sum_count_distinct :public It
   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 @@ class Item_sum_count_distinct :public It
   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 @@ class Item_func_group_concat : public It
   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-15 12:02:37 +03:00
@@ -537,8 +537,9 @@ int mysql_union(THD *thd, LEX *lex, sele
 		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.449/sql/sql_select.cc	2006-01-26 17:00:46 +03:00
+++ 1.450/sql/sql_select.cc	2006-03-15 11:42:21 +03:00
@@ -878,7 +878,7 @@ JOIN::optimize()
 			   select_options,
 			   (order == 0 || skip_sort_order) ? select_limit :
 			   HA_POS_ERROR,
-			   (char *) "")))
+			   (char *) "", 0)))
       DBUG_RETURN(1);
 
     /*
@@ -1269,7 +1269,7 @@ JOIN::exec()
 						!curr_join->group_list,
 						1, curr_join->select_options,
 						HA_POS_ERROR,
-						(char *) "")))
+						(char *) "", 0)))
 	  DBUG_VOID_RETURN;
 	curr_join->exec_tmp_table2= exec_tmp_table2;
       }
@@ -4984,7 +4984,8 @@ static Field* create_tmp_field_from_item
 
 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 @@ Field *create_tmp_field(THD *thd, TABLE 
   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);
@@ -5092,7 +5099,7 @@ TABLE *
 create_tmp_table(THD *thd,TMP_TABLE_PARAM *param,List<Item> &fields,
 		 ORDER *group, bool distinct, bool save_sum_fields,
 		 ulong select_options, ha_rows rows_limit,
-		 char *table_alias)
+		 char *table_alias, bool force_copy_field)
 {
   TABLE *table;
   uint	i,field_count,reclength,null_count,null_pack_length,
@@ -5241,7 +5248,7 @@ create_tmp_table(THD *thd,TMP_TABLE_PARA
 	  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 @@ create_tmp_table(THD *thd,TMP_TABLE_PARA
       */
       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_field &&
+                                           (not_all_columns || group !=0),
+                                         param->convert_blob_length,
+                                         force_copy_field);
       if (!new_field)
       {
 	if (thd->is_fatal_error)

--- 1.79/sql/sql_select.h	2006-01-18 14:48:54 +03:00
+++ 1.80/sql/sql_select.h	2006-03-15 11:39:02 +03:00
@@ -339,7 +339,7 @@ bool store_val_in_field(Field *field,Ite
 TABLE *create_tmp_table(THD *thd,TMP_TABLE_PARAM *param,List<Item> &fields,
 			ORDER *group, bool distinct, bool save_sum_fields,
 			ulong select_options, ha_rows rows_limit,
-			char* alias);
+			char* alias, bool force_copy_field);
 void free_tmp_table(THD *thd, TABLE *entry);
 void count_field_types(TMP_TABLE_PARAM *param, List<Item> &fields,
 		       bool reset_with_sum_func);

--- 1.304/sql/sql_table.cc	2006-01-16 18:21:39 +03:00
+++ 1.305/sql/sql_table.cc	2006-03-15 12:13:17 +03:00
@@ -1562,7 +1562,7 @@ TABLE *create_table_from_items(THD *thd,
       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.153/sql/sql_update.cc	2006-01-26 00:09:01 +03:00
+++ 1.154/sql/sql_update.cc	2006-03-15 12:11:06 +03:00
@@ -942,7 +942,7 @@ multi_update::initialize_tables(JOIN *jo
 					   (ORDER*) &group, 0, 0,
 					   TMP_TABLE_ALL_COLUMNS,
 					   HA_POS_ERROR,
-					   (char *) "")))
+					   (char *) "", 0)))
       DBUG_RETURN(1);
     tmp_tables[cnt]->file->extra(HA_EXTRA_WRITE_CACHE);
   }

--- 1.79/sql/sql_derived.cc	2005-03-23 09:36:42 +03:00
+++ 1.80/sql/sql_derived.cc	2006-03-15 12:12:44 +03:00
@@ -145,7 +145,7 @@ static int mysql_derived(THD *thd, LEX *
 				(first_select->options | thd->options |
 				 TMP_TABLE_ALL_COLUMNS),
 				HA_POS_ERROR,
-				org_table_list->alias)))
+				org_table_list->alias, 0)))
   {
     res= -1;
     goto exit;

--- 1.144/sql/sql_union.cc	2005-08-09 01:13:46 +04:00
+++ 1.145/sql/sql_union.cc	2006-03-15 12:12:19 +03:00
@@ -313,7 +313,7 @@ int st_select_lex_unit::prepare(THD *thd
 				  &union_result->tmp_table_param, types,
 				  (ORDER*) 0, (bool) union_distinct, 1, 
                                   create_options, HA_POS_ERROR, 
-                                  (char *) tmp_table_alias)))
+                                  (char *) tmp_table_alias, 0)))
       goto err;
     table->file->extra(HA_EXTRA_WRITE_CACHE);
     table->file->extra(HA_EXTRA_IGNORE_DUP_KEY);

--- 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-15 11:37:48 +03:00
@@ -589,3 +589,18 @@ GROUP_CONCAT(a ORDER BY a)
 ,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-15 11:36:09 +03:00
@@ -382,4 +382,12 @@ SELECT GROUP_CONCAT(a ORDER BY a) FROM t
 
 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#15560eugene15 Mar