Blob support for the group_concat() function.

(Bug #4315: GROUP_CONCAT with ORDER BY returns strange results for TEXT fields
 Bug #5564: Strange behaviour with group_concat and distinct
 Bug #5970: group_concat doesn't print warnings)

mysql-test/r/query_cache.result

@@ -901,6 +901,8 @@ set group_concat_max_len=10;
 select group_concat(a) FROM t1 group by b;
 group_concat(a)
 1234567890
+Warnings:
+Warning	1260	1 line(s) were cut by GROUP_CONCAT()
 set group_concat_max_len=1024;
 select group_concat(a) FROM t1 group by b;
 group_concat(a)

mysql-test/t/func_gconcat.test

@@ -240,3 +240,40 @@ SELECT t1.A_ID, GROUP_CONCAT(t2.B_DESC) AS B_DESC FROM t1 LEFT JOIN t2 ON t1.A_I
 DROP TABLE t1;
 DROP TABLE t2;
 
+#
+# blobs
+#
+
+create table t1 (a int, b text);
+insert into t1 values (1, 'bb'), (1, 'ccc'), (1, 'a'), (1, 'bb'), (1, 'ccc');
+insert into t1 values (2, 'BB'), (2, 'CCC'), (2, 'A'), (2, 'BB'), (2, 'CCC');
+select group_concat(b) from t1 group by a;
+select group_concat(distinct b) from t1 group by a;
+select group_concat(b order by b) from t1 group by a;
+select group_concat(distinct b order by b) from t1 group by a;
+set local group_concat_max_len=4;
+select group_concat(b) from t1 group by a;
+select group_concat(distinct b) from t1 group by a;
+select group_concat(b order by b) from t1 group by a;
+select group_concat(distinct b order by b) from t1 group by a;
+
+#
+# long blobs
+#
+
+insert into t1 values (1, concat(repeat('1', 300), '2')), 
+(1, concat(repeat('1', 300), '2')), (1, concat(repeat('0', 300), '1')), 
+(2, concat(repeat('1', 300), '2')), (2, concat(repeat('1', 300), '2')), 
+(2, concat(repeat('0', 300), '1'));
+set local group_concat_max_len=1024;
+select group_concat(b) from t1 group by a;
+select group_concat(distinct b) from t1 group by a;
+select group_concat(b order by b) from t1 group by a;
+select group_concat(distinct b order by b) from t1 group by a;
+set local group_concat_max_len=400;
+select group_concat(b) from t1 group by a;
+select group_concat(distinct b) from t1 group by a;
+select group_concat(b order by b) from t1 group by a;
+select group_concat(distinct b order by b) from t1 group by a;
+
+drop table t1;

sql/item_sum.cc

@@ -1921,16 +1921,20 @@ bool Item_func_group_concat::add()
   }
 
   null_value= FALSE;
+
+  TREE_ELEMENT *el= 0;                          // Only for safety
   if (tree_mode)
-  {
-    if (!tree_insert(tree, table->record[0], 0, tree->custom_arg))
-      return 1;
-  }
-  else
-  {
-    if (result.length() <= group_concat_max_len && !warning_for_row)
-      dump_leaf_key(table->record[0], 1, this);
-  }
+    el= tree_insert(tree, table->record[0], 0, tree->custom_arg);
+  /*
+    If the row is not a duplicate (el->count == 1)
+    we can dump the row here in case of GROUP_CONCAT(DISTINCT...)
+    instead of doing tree traverse later.
+  */
+  if (result.length() <= group_concat_max_len && 
+      !warning_for_row &&
+      (!tree_mode || (el->count == 1 && distinct && !arg_count_order)))
+    dump_leaf_key(table->record[0], 1, this);
+
   return 0;
 }
 
@@ -1979,6 +1983,8 @@ Item_func_group_concat::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)
   thd->allow_sum_func= 1;			
   if (!(tmp_table_param= new TMP_TABLE_PARAM))
     return 1;
+  /* We'll convert all blobs to varchar fields in the temporary table */
+  tmp_table_param->convert_blob_length= group_concat_max_len;
   tables_list= tables;
   fixed= 1;
   return 0;
@@ -2076,9 +2082,7 @@ bool Item_func_group_concat::setup(THD *thd)
     }
     else
     {
-       compare_key= NULL;
-      if (distinct)
-        compare_key= (qsort_cmp2) group_concat_key_cmp_with_distinct;
+      compare_key= (qsort_cmp2) group_concat_key_cmp_with_distinct;
     }
     /*
       Create a tree of sort. Tree is used for a sort and a remove double 
@@ -2121,6 +2125,12 @@ String* Item_func_group_concat::val_str(String* str)
   DBUG_ASSERT(fixed == 1);
   if (null_value)
     return 0;
+  if (count_cut_values && !warning_available)
+  {
+    warning_available= TRUE;
+    warning= push_warning(item_thd, MYSQL_ERROR::WARN_LEVEL_WARN,
+                          ER_CUT_VALUE_GROUP_CONCAT, NULL);
+  }
   if (result.length())
     return &result;
   if (tree_mode)
@@ -2128,12 +2138,6 @@ String* Item_func_group_concat::val_str(String* str)
     tree_walk(tree, (tree_walk_action)&dump_leaf_key, (void*)this,
               left_root_right);
   }
-  if (count_cut_values && !warning_available)
-  {
-    warning_available= TRUE;
-    warning= push_warning(item_thd, MYSQL_ERROR::WARN_LEVEL_WARN,
-                           ER_CUT_VALUE_GROUP_CONCAT, NULL);
-  }
   return &result;
 }
 

sql/mysql_priv.h

@@ -521,7 +521,7 @@ int mysql_union(THD *thd, LEX *lex, select_result *result,
 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);
+			bool group, bool modify_item, uint convert_blob_length);
 int mysql_prepare_table(THD *thd, HA_CREATE_INFO *create_info,
 		       List<create_field> &fields,
 		       List<Key> &keys, uint &db_options, 

sql/sql_class.h

@@ -1218,10 +1218,12 @@ class TMP_TABLE_PARAM :public Sql_alloc
   uint	group_parts,group_length,group_null_parts;
   uint	quick_group;
   bool  using_indirect_summary_function;
+  /* If >0 convert all blob fields to varchar(convert_blob_length) */
+  uint  convert_blob_length; 
 
   TMP_TABLE_PARAM()
     :copy_funcs_it(copy_funcs), copy_field(0), group_parts(0),
-    group_length(0), group_null_parts(0)
+    group_length(0), group_null_parts(0), convert_blob_length(0)
   {}
   ~TMP_TABLE_PARAM()
   {

sql/sql_select.cc

@@ -4583,21 +4583,28 @@ const_expression_in_where(COND *cond, Item *comp_item, Item **const_item)
 			the record in the original table.
 			If modify_item is 0 then fill_record() will update
 			the temporary table
+    convert_blob_length If >0 create a varstring(convert_blob_length) field 
+                        instead of blob.
 
   RETURN
     0			on error
     new_created field
 */
-static Field* create_tmp_field_from_field(THD *thd,
-					  Field* org_field,
-					  Item *item,
-					  TABLE *table,
-					  bool modify_item)
+
+static Field* create_tmp_field_from_field(THD *thd, Field* org_field,
+                                          Item *item, TABLE *table,
+                                          bool modify_item,
+                                          uint convert_blob_length)
 {
   Field *new_field;
 
-  // The following should always be true
-  if ((new_field= org_field->new_field(&thd->mem_root,table)))
+  if (convert_blob_length && org_field->flags & BLOB_FLAG)
+    new_field= new Field_varstring(convert_blob_length, org_field->maybe_null(),
+                                   org_field->field_name, table,
+                                   org_field->charset());
+  else
+    new_field= org_field->new_field(&thd->mem_root, table);
+  if (new_field)
   {
     if (modify_item)
       ((Item_field *)item)->result_field= new_field;
@@ -4628,16 +4635,16 @@ static Field* create_tmp_field_from_field(THD *thd,
 			the record in the original table.
 			If modify_item is 0 then fill_record() will update
 			the temporary table
+    convert_blob_length If >0 create a varstring(convert_blob_length) field 
+                        instead of blob.
 
   RETURN
     0			on error
     new_created field
 */
-static Field* create_tmp_field_from_item(THD *thd,
-					 Item *item,
-					 TABLE *table,
-					 Item ***copy_func,
-					 bool modify_item)
+static Field* create_tmp_field_from_item(THD *thd, Item *item, TABLE *table,
+                                         Item ***copy_func, bool modify_item,
+                                         uint convert_blob_length)
 {
   bool maybe_null=item->maybe_null;
   Field *new_field;
@@ -4654,13 +4661,18 @@ static Field* create_tmp_field_from_item(THD *thd,
     break;
   case STRING_RESULT:
     if (item->max_length > 255)
-      new_field=  new Field_blob(item->max_length, maybe_null,
-				 item->name, table,
-				 item->collation.collation);
+    {
+      if (convert_blob_length)
+        new_field= new Field_varstring(convert_blob_length, maybe_null,
+                                       item->name, table,
+                                       item->collation.collation);
+      else
+        new_field= new Field_blob(item->max_length, maybe_null, item->name,
+                                  table, item->collation.collation);
+    }
     else
-      new_field= new Field_string(item->max_length, maybe_null,
-				  item->name, table,
-				  item->collation.collation);
+      new_field= new Field_string(item->max_length, maybe_null, item->name, 
+                                  table, item->collation.collation);
     break;
   case ROW_RESULT: 
   default: 
@@ -4697,6 +4709,8 @@ static Field* create_tmp_field_from_item(THD *thd,
 			the record in the original table.
 			If modify_item is 0 then fill_record() will update
 			the temporary table
+    convert_blob_length If >0 create a varstring(convert_blob_length) field 
+                        instead of blob.
 
   RETURN
     0			on error
@@ -4704,8 +4718,8 @@ static Field* create_tmp_field_from_item(THD *thd,
 */
 
 Field *create_tmp_field(THD *thd, TABLE *table,Item *item, Item::Type type,
-			Item ***copy_func, Field **from_field,
-			bool group, bool modify_item)
+                        Item ***copy_func, Field **from_field,
+                        bool group, bool modify_item, uint convert_blob_length)
 {
   switch (type) {
   case Item::SUM_FUNC_ITEM:
@@ -4740,8 +4754,15 @@ Field *create_tmp_field(THD *thd, TABLE *table,Item *item, Item::Type type,
 				  item->name,table,item->unsigned_flag);
       case STRING_RESULT:
 	if (item_sum->max_length > 255)
-	  return  new Field_blob(item_sum->max_length,maybe_null,
-				 item->name,table,item->collation.collation);
+        {
+          if (convert_blob_length)
+            return new Field_varstring(convert_blob_length, maybe_null,
+                                       item->name, table,
+                                       item->collation.collation);
+          else
+            return new Field_blob(item_sum->max_length, maybe_null, item->name,
+                                  table, item->collation.collation);
+        }
 	return	new Field_string(item_sum->max_length,maybe_null,
 				 item->name,table,item->collation.collation);
       case ROW_RESULT:
@@ -4758,8 +4779,8 @@ Field *create_tmp_field(THD *thd, TABLE *table,Item *item, Item::Type type,
   case Item::DEFAULT_VALUE_ITEM:
   {
     Item_field *field= (Item_field*) item;
-    return create_tmp_field_from_field(thd, (*from_field= field->field),
-				       item, table, modify_item);
+    return create_tmp_field_from_field(thd, (*from_field= field->field), item,
+                                       table, modify_item, convert_blob_length);
   }
   case Item::FUNC_ITEM:
   case Item::COND_ITEM:
@@ -4774,14 +4795,16 @@ Field *create_tmp_field(THD *thd, TABLE *table,Item *item, Item::Type type,
   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);
+    return create_tmp_field_from_item(thd, item, table, copy_func, modify_item,
+                                      convert_blob_length);
   case Item::TYPE_HOLDER:
   {
     Field *example= ((Item_type_holder *)item)->example();
     if (example)
-      return create_tmp_field_from_field(thd, example, item, table, 0);
-    return create_tmp_field_from_item(thd, item, table, copy_func, 0);
+      return create_tmp_field_from_field(thd, example, item, table, 0,
+                                         convert_blob_length);
+    return create_tmp_field_from_item(thd, item, table, copy_func, 0,
+                                      convert_blob_length);
   }
   default:					// Dosen't have to be stored
     return 0;
@@ -4944,8 +4967,9 @@ create_tmp_table(THD *thd,TMP_TABLE_PARAM *param,List<Item> &fields,
 	if (!arg->const_item())
 	{
 	  Field *new_field=
-	    create_tmp_field(thd, table,arg,arg->type(),&copy_func,
-			     tmp_from_field, group != 0,not_all_columns);
+            create_tmp_field(thd, table, arg, arg->type(), &copy_func,
+                             tmp_from_field, group != 0,not_all_columns,
+                             param->convert_blob_length);
 	  if (!new_field)
 	    goto err;					// Should be OOM
 	  tmp_from_field++;
@@ -4981,9 +5005,10 @@ create_tmp_table(THD *thd,TMP_TABLE_PARAM *param,List<Item> &fields,
 	We here distinguish between UNION and multi-table-updates by the fact
 	that in the later case group is set to the row pointer.
       */
-      Field *new_field=create_tmp_field(thd, table, item,type, &copy_func,
-					tmp_from_field, group != 0,
-					not_all_columns || group !=0);
+      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);
       if (!new_field)
       {
 	if (thd->is_fatal_error)

sql/sql_table.cc

@@ -1378,7 +1378,7 @@ TABLE *create_table_from_items(THD *thd, HA_CREATE_INFO *create_info,
       field=item->tmp_table_field(&tmp_table);
     else
       field=create_tmp_field(thd, &tmp_table, item, item->type(),
-				  (Item ***) 0, &tmp_field,0,0);
+				  (Item ***) 0, &tmp_field, 0, 0, 0);
     if (!field ||
 	!(cr_field=new create_field(field,(item->type() == Item::FIELD_ITEM ?
 					   ((Item_field *)item)->field :