Commit 6fc7c074 authored by unknown's avatar unknown
Browse files

Cleanup of Item_func_group_concat 


mysql-test/r/func_gconcat.result:
  Test result fixed.
sql/item_sum.cc:
  Cleanup of Item_func_group_concat:
  - last unobvious things commented
  - don't store NULLs in the tree.
  - remove unneeded variables.
  - use setup_order, not setup_group to setup group concat order list
sql/item_sum.h:
  Cleanup for Item_func_group_concat: remove unneded variables, methods,
  move a bunch of variables to private: section.
parent d1deb659

mysql-test/r/func_gconcat.result

+1 −1
Original line number Diff line number Diff line
@@ -166,7 +166,7 @@ set group_concat_max_len = 1024; 
select group_concat(sum(a)) from t1 group by grp;

ERROR HY000: Invalid use of group function

select grp,group_concat(c order by 2) from t1 group by grp;

ERROR 42S22: Unknown column '2' in 'group statement'

ERROR 42S22: Unknown column '2' in 'order clause'

drop table t1;

create table t1 ( URL_ID int(11), URL varchar(80));

create table t2 ( REQ_ID int(11), URL_ID int(11));

sql/item_sum.cc

+151 −183
Original line number Diff line number Diff line
@@ -2575,8 +2575,9 @@ int group_concat_key_cmp_with_distinct(void* arg, byte* key1, 
				       byte* key2)

{

  Item_func_group_concat* grp_item= (Item_func_group_concat*)arg;

  TABLE *table= grp_item->table;

  Item **field_item, **end;

  char *record= (char*) grp_item->table->record[0];

  char *record= (char*) table->record[0] + table->s->null_bytes;



  for (field_item= grp_item->args, end= field_item + grp_item->arg_count_field;

       field_item < end;
@@ -2609,7 +2610,8 @@ int group_concat_key_cmp_with_order(void* arg, byte* key1, byte* key2) 
{

  Item_func_group_concat* grp_item= (Item_func_group_concat*) arg;

  ORDER **order_item, **end;

  char *record= (char*) grp_item->table->record[0];

  TABLE *table= grp_item->table;

  char *record= (char*) table->record[0] + table->s->null_bytes;



  for (order_item= grp_item->order, end=order_item+ grp_item->arg_count_order;

       order_item < end;
@@ -2622,6 +2624,7 @@ int group_concat_key_cmp_with_order(void* arg, byte* key1, byte* key2) 
      the temporary table, not the original field

    */

    Field *field= item->get_tmp_table_field();

    /* If the item is a constant, there is no tmp table field */

    if (field)

    {

      int res;
@@ -2633,7 +2636,7 @@ int group_concat_key_cmp_with_order(void* arg, byte* key1, byte* key2) 
  /*

    We can't return 0 because in that case the tree class would remove this

    item as double value. This would cause problems for case-changes and

    if the the returned values are not the same we do the sort on.

    if the returned values are not the same we do the sort on.

  */

  return 1;

}
@@ -2665,48 +2668,48 @@ int group_concat_key_cmp_with_distinct_and_order(void* arg,byte* key1, 
int dump_leaf_key(byte* key, uint32 count __attribute__((unused)),

                  Item_func_group_concat *item)

{

  char buff[MAX_FIELD_WIDTH];

  String tmp((char *)&buff,sizeof(buff),default_charset_info), tmp2;

  char *record= (char*) item->table->record[0];

  TABLE *table= item->table;

  char *record= (char*) table->record[0] + table->s->null_bytes;

  String tmp(table->record[1], table->s->reclength, default_charset_info), tmp2;

  String &result= item->result;

  Item **arg= item->args, **arg_end= item->args + item->arg_count_field;



  if (item->result.length())

    item->result.append(*item->separator);

  if (result.length())

    result.append(*item->separator);



  tmp.length(0);



  for (uint i= 0; i < item->arg_count_field; i++)

  for (; arg < arg_end; arg++)

  {

    Item *show_item= item->args[i];

    if (!show_item->const_item())

    String *res;

    if (! (*arg)->const_item())

    {

      /*

	We have to use get_tmp_table_field() instead of

	real_item()->get_tmp_table_field() because we want the field in

	the temporary table, not the original field

        We also can't use table->field array to access the fields

        because it contains both order and arg list fields.

      */

      Field *field= show_item->get_tmp_table_field();

      String *res;

      Field *field= (*arg)->get_tmp_table_field();

      char *save_ptr= field->ptr;

      uint offset= (uint) (save_ptr - record);

      DBUG_ASSERT(offset < item->table->s->reclength);

      DBUG_ASSERT(offset < table->s->reclength);

      field->ptr= (char *) key + offset;

      res= field->val_str(&tmp,&tmp2);

      item->result.append(*res);

      field->ptr= save_ptr;

    }

    else

    {

      String *res= show_item->val_str(&tmp);

      res= (*arg)->val_str(&tmp);

    if (res)

        item->result.append(*res);

    }

      result.append(*res);

  }



  /* stop if length of result more than group_concat_max_len */  

  if (item->result.length() > item->group_concat_max_len)

  /* stop if length of result more than max_length */

  if (result.length() > item->max_length)

  {

    item->count_cut_values++;

    item->result.length(item->group_concat_max_len);

    result.length(item->max_length);

    item->warning_for_row= TRUE;

    return 1;

  }
@@ -2716,34 +2719,29 @@ int dump_leaf_key(byte* key, uint32 count __attribute__((unused)), 


/*

  Constructor of Item_func_group_concat

  is_distinct - distinct

  is_select - list of expression for show values

  is_order - list of sort columns 

  is_separator - string value of separator

  distinct_arg - distinct

  select_list - list of expression for show values

  order_list - list of sort columns

  separator_arg - string value of separator

*/



Item_func_group_concat::Item_func_group_concat(bool is_distinct,

					       List<Item> *is_select,

					       SQL_LIST *is_order,

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

   separator(is_separator), tree(&tree_base), table(0),

Item_func_group_concat::

Item_func_group_concat(bool distinct_arg, List<Item> *select_list,

                       SQL_LIST *order_list, String *separator_arg)

  :tmp_table_param(0), warning(0),

   separator(separator_arg), tree(0), table(0),

   order(0), tables_list(0),

   arg_count_order(0), arg_count_field(0),

   count_cut_values(0)

   arg_count_order(order_list ? order_list->elements : 0),

   arg_count_field(select_list->elements),

   count_cut_values(0),

   distinct(distinct_arg),

   warning_for_row(FALSE),

   original(0)

{

  Item *item_select;

  Item **arg_ptr;



  original= 0;

  quick_group= 0;

  mark_as_sum_func();

  order= 0;

  group_concat_max_len= current_thd->variables.group_concat_max_len;

    

  arg_count_field= is_select->elements;

  arg_count_order= is_order ? is_order->elements : 0;

  quick_group= FALSE;

  arg_count= arg_count_field + arg_count_order;



  /*
@@ -2759,7 +2757,7 @@ Item_func_group_concat::Item_func_group_concat(bool is_distinct, 
  order= (ORDER**)(args + arg_count);



  /* fill args items of show and sort */

  List_iterator_fast<Item> li(*is_select);

  List_iterator_fast<Item> li(*select_list);



  for (arg_ptr=args ; (item_select= li++) ; arg_ptr++)

    *arg_ptr= item_select;
@@ -2767,7 +2765,7 @@ Item_func_group_concat::Item_func_group_concat(bool is_distinct, 
  if (arg_count_order)

  {

    ORDER **order_ptr= order;

    for (ORDER *order_item= (ORDER*) is_order->first;

    for (ORDER *order_item= (ORDER*) order_list->first;

         order_item != NULL;

         order_item= order_item->next)

    {
@@ -2781,24 +2779,20 @@ Item_func_group_concat::Item_func_group_concat(bool is_distinct, 


Item_func_group_concat::Item_func_group_concat(THD *thd,

                                               Item_func_group_concat *item)

  :Item_sum(thd, item),item_thd(thd),

  :Item_sum(thd, item),

  tmp_table_param(item->tmp_table_param),

  max_elements_in_tree(item->max_elements_in_tree),

  warning(item->warning),

  key_length(item->key_length), 

  tree_mode(item->tree_mode),

  distinct(item->distinct),

  warning_for_row(item->warning_for_row),

  separator(item->separator),

  tree(item->tree),

  table(item->table),

  order(item->order),

  tables_list(item->tables_list),

  group_concat_max_len(item->group_concat_max_len),

  arg_count_order(item->arg_count_order),

  arg_count_field(item->arg_count_field),

  field_list_offset(item->field_list_offset),

  count_cut_values(item->count_cut_values),

  distinct(item->distinct),

  warning_for_row(item->warning_for_row),

  always_null(item->always_null),

  original(item)

{

  quick_group= item->quick_group;
@@ -2817,18 +2811,17 @@ void Item_func_group_concat::cleanup() 
  */

  if (!original)

  {

    THD *thd= current_thd;

    delete tmp_table_param;

    tmp_table_param= 0;

    if (table)

    {

      THD *thd= table->in_use;

      free_tmp_table(thd, table);

      table= 0;

    }

    delete tmp_table_param;

    tmp_table_param= 0;

    if (tree_mode)

      if (tree)

      {

      tree_mode= 0;

        delete_tree(tree);

        tree= 0;

      }

      if (warning)

      {
@@ -2838,12 +2831,10 @@ void Item_func_group_concat::cleanup() 
        warning= 0;

      }

    }

  DBUG_VOID_RETURN;

    DBUG_ASSERT(tree == 0);

    DBUG_ASSERT(warning == 0);

  }





Item_func_group_concat::~Item_func_group_concat()

{

  DBUG_VOID_RETURN;

}
@@ -2859,14 +2850,9 @@ void Item_func_group_concat::clear() 
  result.copy();

  null_value= TRUE;

  warning_for_row= FALSE;

  if (table)

  {

    table->file->extra(HA_EXTRA_NO_CACHE);

    table->file->delete_all_rows();

    table->file->extra(HA_EXTRA_WRITE_CACHE);

  }

  if (tree_mode)

  if (tree)

    reset_tree(tree);

  /* No need to reset the table as we never call write_row */

}
@@ -2895,29 +2881,23 @@ bool Item_func_group_concat::add() 
  null_value= FALSE;



  TREE_ELEMENT *el= 0;                          // Only for safety

  if (tree_mode)

    el= tree_insert(tree, table->record[0], 0, tree->custom_arg);

  if (tree)

    el= tree_insert(tree, table->record[0] + table->s->null_bytes, 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 && 

  if (result.length() <= max_length &&

      !warning_for_row &&

      (!tree_mode || (el->count == 1 && distinct && !arg_count_order)))

    dump_leaf_key(table->record[0], 1, this);

      (!tree || (el->count == 1 && distinct && !arg_count_order)))

    dump_leaf_key(table->record[0] + table->s->null_bytes, 1, this);



  return 0;

}





void Item_func_group_concat::reset_field()

{

  if (tree_mode)

    reset_tree(tree);

}





bool

Item_func_group_concat::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref)

{
@@ -2930,10 +2910,11 @@ Item_func_group_concat::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref) 
               MYF(0));

    return TRUE;

  }

  if (!args)                      /* allocation in constructor may fail */

    return TRUE;



  thd->allow_sum_func= 0;

  maybe_null= 0;

  item_thd= thd;



  /*

    Fix fields for select list and ORDER clause
@@ -2951,12 +2932,8 @@ Item_func_group_concat::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref) 


  result_field= 0;

  null_value= 1;

  max_length= group_concat_max_len;

  thd->allow_sum_func= 1;

  if (!(tmp_table_param= new TMP_TABLE_PARAM))

    return TRUE;

  /* We'll convert all blobs to varchar fields in the temporary table */

  tmp_table_param->convert_blob_length= group_concat_max_len;

  max_length= thd->variables.group_concat_max_len;

  tables_list= tables;

  fixed= 1;

  return FALSE;
@@ -2967,52 +2944,54 @@ bool Item_func_group_concat::setup(THD *thd) 
{

  List<Item> list;

  SELECT_LEX *select_lex= thd->lex->current_select;

  uint const_fields;

  qsort_cmp2 compare_key;

  DBUG_ENTER("Item_func_group_concat::setup");



  if (select_lex->linkage == GLOBAL_OPTIONS_TYPE)

    DBUG_RETURN(1);



  /*

    push all not constant fields to list and create temp table

    Currently setup() can be called twice. Please add

    assertion here when this is fixed.

  */

  const_fields= 0;

  if (table || tree)

    DBUG_RETURN(FALSE);



  if (!(tmp_table_param= new TMP_TABLE_PARAM))

    DBUG_RETURN(TRUE);



  /* We'll convert all blobs to varchar fields in the temporary table */

  tmp_table_param->convert_blob_length= max_length;

  /* Push all not constant fields to the list and create a temp table */

  always_null= 0;

  for (uint i= 0; i < arg_count_field; i++)

  {

    Item *item= args[i];

    if (list.push_back(item))

      DBUG_RETURN(1);

      DBUG_RETURN(TRUE);

    if (item->const_item())

    {

      const_fields++;

      (void) item->val_int();

      if (item->null_value)

      {

        always_null= 1;

        break;

      }

    }

  }

  if (always_null)

    DBUG_RETURN(0);

    DBUG_RETURN(FALSE);



  List<Item> all_fields(list);

  if (arg_count_order)

  {

    bool hidden_group_fields;

    setup_group(thd, args, tables_list, list, all_fields, *order,

                &hidden_group_fields);

  }



  count_field_types(tmp_table_param,all_fields,0);

  if (table)

  {

  /*

      We come here when we are getting the result from a temporary table,

      not the original tables used in the query

    Try to find every ORDER expression in the list of GROUP_CONCAT

    arguments. If an expression is not found, prepend it to

    "all_fields". The resulting field list is used as input to create

    tmp table columns.

  */

    free_tmp_table(thd, table);

    tmp_table_param->cleanup();

  }

  if (arg_count_order &&

      setup_order(thd, args, tables_list, list, all_fields, *order))

    DBUG_RETURN(TRUE);



  count_field_types(tmp_table_param,all_fields,0);

  DBUG_ASSERT(table == 0);

  /*

    We have to create a temporary table to get descriptions of fields

    (types, sizes and so on).
@@ -3027,22 +3006,21 @@ bool Item_func_group_concat::setup(THD *thd) 
                                (ORDER*) 0, 0, TRUE,

                                select_lex->options | thd->options,

                                HA_POS_ERROR, (char*) "")))

    DBUG_RETURN(1);

    DBUG_RETURN(TRUE);

  table->file->extra(HA_EXTRA_NO_ROWS);

  table->no_rows= 1;



  key_length= table->s->reclength;



  /* Offset to first result field in table */

  field_list_offset= table->s->fields - (list.elements - const_fields);



  if (tree_mode)

    delete_tree(tree);



  /* choose function of sort */  

  tree_mode= distinct || arg_count_order; 

  if (tree_mode)

  if (distinct || arg_count_order)

  {

    /*

      Need sorting: init tree and choose a function to sort.

      Don't reserve space for NULLs: if any of gconcat arguments is NULL,

      the row is not added to the result.

    */

    uint tree_key_length= table->s->reclength - table->s->null_bytes;



    tree= &tree_base;

    if (arg_count_order)

    {

      if (distinct)
@@ -3055,27 +3033,16 @@ bool Item_func_group_concat::setup(THD *thd) 
      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 

      values (according with syntax of the function). If function doesn't

      contain DISTINCT and ORDER BY clauses, we don't create this tree.

      Create a tree for sorting. The tree is used to sort and to remove

      duplicate values (according to the syntax of this function). If there

      is no DISTINCT or ORDER BY clauses, we don't create this tree.

    */

    init_tree(tree, min(thd->variables.max_heap_table_size,

                        thd->variables.sortbuff_size/16), 0,

              key_length, compare_key, 0, NULL, (void*) this);

    max_elements_in_tree= (key_length ? 

			   thd->variables.max_heap_table_size/key_length : 1);

  };



  /*

    Copy table and tree_mode if they belong to this item (if item have not 

    pointer to original item from which was made copy => it own its objects)

  */

  if (original)

  {

    original->table= table;

    original->tree_mode= tree_mode;

              tree_key_length, compare_key, 0, NULL, (void*) this);

  }

  DBUG_RETURN(0);



  DBUG_RETURN(FALSE);

}
@@ -3083,10 +3050,10 @@ bool Item_func_group_concat::setup(THD *thd) 


void Item_func_group_concat::make_unique()

{

  tmp_table_param= 0;

  table=0;

  original= 0;

  tree_mode= 0; // to prevent delete_tree call on uninitialized tree

  tree= &tree_base;

  tree= 0;

}
@@ -3096,16 +3063,17 @@ String* Item_func_group_concat::val_str(String* str) 
  if (null_value)

    return 0;

  if (count_cut_values && !warning)

    warning= push_warning(item_thd, MYSQL_ERROR::WARN_LEVEL_WARN,

  {

    DBUG_ASSERT(table);

    warning= push_warning(table->in_use, MYSQL_ERROR::WARN_LEVEL_WARN,

                          ER_CUT_VALUE_GROUP_CONCAT,

                          ER(ER_CUT_VALUE_GROUP_CONCAT));

  }

  if (result.length())

    return &result;

  if (tree_mode)

  {

  if (tree)

    tree_walk(tree, (tree_walk_action)&dump_leaf_key, (void*)this,

              left_root_right);

  }

  return &result;

}

sql/item_sum.h

+20 −27
Original line number Diff line number Diff line
@@ -834,15 +834,26 @@ class MYSQL_ERROR; 


class Item_func_group_concat : public Item_sum

{

  THD *item_thd;

  TMP_TABLE_PARAM *tmp_table_param;

  uint max_elements_in_tree;

  MYSQL_ERROR *warning;

  uint key_length;

  bool tree_mode;

  String result;

  String *separator;

  TREE tree_base;

  TREE *tree;

  TABLE *table;

  ORDER **order;

  TABLE_LIST *tables_list;

  uint arg_count_order;               // total count of ORDER BY items

  uint arg_count_field;               // count of arguments

  uint count_cut_values;

  bool distinct;

  bool warning_for_row;

  bool always_null;

  /*

    Following is 0 normal object and pointer to original one for copy

    (to correctly free resources)

  */

  Item_func_group_concat *original;



  friend int group_concat_key_cmp_with_distinct(void* arg, byte* key1,

					      byte* key2);
@@ -855,29 +866,11 @@ class Item_func_group_concat : public Item_sum 
			   Item_func_group_concat *group_concat_item);



public:

  String result;

  String *separator;

  TREE tree_base;

  TREE *tree;

  TABLE *table;

  ORDER **order;

  TABLE_LIST *tables_list;

  ulong group_concat_max_len;

  uint arg_count_order;

  uint arg_count_field;

  uint field_list_offset;

  uint count_cut_values;

  /*

    Following is 0 normal object and pointer to original one for copy 

    (to correctly free resources)

  */

  Item_func_group_concat *original;

  

  Item_func_group_concat(bool is_distinct,List<Item> *is_select,

                         SQL_LIST *is_order,String *is_separator);



  Item_func_group_concat(THD *thd, Item_func_group_concat *item);

  ~Item_func_group_concat();

  ~Item_func_group_concat() {}

  void cleanup();



  enum Sumfunctype sum_func () const {return GROUP_CONCAT_FUNC;}
@@ -885,11 +878,11 @@ class Item_func_group_concat : public Item_sum 
  virtual Item_result result_type () const { return STRING_RESULT; }

  void clear();

  bool add();

  void reset_field();

  void reset_field() {}                         // not used

  void update_field() {}                        // not used

  bool fix_fields(THD *, TABLE_LIST *, Item **);

  bool setup(THD *thd);

  void make_unique();

  virtual void update_field() {}

  double val_real()

  {

    String *res;  res=val_str(&str_value);