Cleanup10.5-mdev21829

9 files changed, 104 insertions, 322 deletions

diff --git a/sql/field.h b/sql/field.h
index 1bde82d2ba2..efd4a74251d 100644
--- a/sql/field.h
+++ b/sql/field.h
@@ -1111,7 +1111,7 @@ public:
   virtual uint32 sort_length() const { return pack_length(); }
   /*
     returns the sort_length for a field without the suffix length bytes
-    for fields with binary charset.
+    for field with binary charset.
   */
   virtual uint32 sort_length_without_suffix() const { return pack_length(); }
 
diff --git a/sql/filesort.cc b/sql/filesort.cc
index c03d4a1e31d..26a1ff10e27 100644
--- a/sql/filesort.cc
+++ b/sql/filesort.cc
@@ -3077,6 +3077,15 @@ int Sort_keys::compare_keys(uchar *a, uchar *b)
 }
 
 
+/*
+  @brief
+    Compare two packed sort keys with a single keypart
+
+  @retval
+    >0   key a greater than b
+    =0   key a equal to b
+    <0   key a less than b
+*/
 int Sort_keys::compare_keys_for_single_arg(uchar *a, uchar *b)
 {
   SORT_FIELD *sort_field= begin();
@@ -3360,8 +3369,20 @@ int Variable_sized_keys::compare_packed_keys(uchar *a_ptr, uchar *b_ptr)
 }
 
 
+int Variable_sized_keys::compare_keys_for_single_arg(uchar *a, uchar *b)
+{
+  return sort_keys->compare_keys_for_single_arg(a + size_of_length_field,
+                                                b + size_of_length_field);
+}
+
+
 /*
-  TODO varun:  add description
+  @brief
+    Make a record with packed values for a key
+
+  @retval
+    0         NULL value
+    >0        length of the packed record
 */
 uint Variable_sized_keys::make_packed_record(bool exclude_nulls)
 {
diff --git a/sql/item_sum.cc b/sql/item_sum.cc
index c8811fa334c..d1023ba65db 100644
--- a/sql/item_sum.cc
+++ b/sql/item_sum.cc
@@ -728,6 +728,29 @@ Aggregator_distinct::composite_packed_key_cmp(void* arg,
 }
 
 
+/**
+  Correctly compare composite keys.
+
+  Used by the Unique class to compare packed keys which have a single argument
+
+  @param arg     Pointer to the relevant Aggregator_distinct instance
+  @param key1    left key image
+  @param key2    right key image
+
+  @return        comparison result
+    @retval <0       if key1 < key2
+    @retval =0       if key1 = key2
+    @retval >0       if key1 > key2
+*/
+int
+Aggregator_distinct::packed_key_cmp_single_arg(void *arg, uchar *key1, uchar *key2)
+{
+  Aggregator_distinct *aggr= (Aggregator_distinct *) arg;
+  DBUG_ASSERT(aggr->variable_sized_keys);
+  return aggr->variable_sized_keys->compare_keys_for_single_arg(key1, key2);
+}
+
+
 /***************************************************************************/
 
 C_MODE_START
@@ -899,8 +922,9 @@ bool Aggregator_distinct::setup(THD *thd)
 
       if (allow_packing)
       {
-        compare_key= (qsort_cmp2) composite_packed_key_cmp;
+        compare_key= get_compare_func_for_packed_keys();
         cmp_arg= (void*)this;
+
         variable_sized_keys= new Variable_sized_keys(tree_key_length);
         if (variable_sized_keys == NULL)
           return true;
@@ -968,9 +992,9 @@ bool Aggregator_distinct::setup(THD *thd)
       simple_raw_key_cmp because the table contains numbers only; decimals
       are converted to binary representation as well.
     */
-    tree= new Unique(simple_raw_key_cmp, &tree_key_length, tree_key_length,
-                     item_sum->ram_limitation(thd));
-
+    tree= item_sum->get_unique(simple_raw_key_cmp, &tree_key_length, tree_key_length,
+                               item_sum->ram_limitation(thd), 0,
+                               false);
     DBUG_RETURN(tree == 0);
   }
 }
@@ -1047,6 +1071,21 @@ int Aggregator_distinct::insert_record_to_unique()
 }
 
 
+/*
+  @brief
+    Get compare function for packed keys
+
+    @retval
+      comparision function
+*/
+qsort_cmp2 Aggregator_distinct::get_compare_func_for_packed_keys()
+{
+  return item_sum->get_arg_count() == 1 ?
+         (qsort_cmp2) packed_key_cmp_single_arg:
+         (qsort_cmp2) composite_packed_key_cmp;
+}
+
+
 /**
   Process incoming row. 
   
@@ -4808,6 +4847,8 @@ uint Item_func_group_concat::get_null_bytes()
 
 bool Item_func_group_concat::is_distinct_packed()
 {
+  DBUG_ASSERT((variable_sized_keys != NULL) ==
+              (unique_filter && unique_filter->is_packed()));
   return unique_filter && unique_filter->is_packed();
 }
 
@@ -4896,34 +4937,16 @@ bool Item_sum::is_packing_allowed(TABLE *table, uint* total_length)
 }
 
 
+/*
+  @brief
+    Get unique instance to filter out duplicate for AGG_FUNC(DISTINCT col....)
+*/
 Unique* Item_sum::get_unique(qsort_cmp2 comp_func, void *comp_func_fixed_arg,
                              uint size_arg, size_t max_in_memory_size_arg,
                              uint min_dupl_count_arg, bool allow_packing)
 {
-
-  if (allow_packing)
-  {
-    if (get_arg_count() == 1)
-      return new Unique_packed_single_arg(comp_func, comp_func_fixed_arg,
-                                          size_arg, max_in_memory_size_arg,
-                                          min_dupl_count_arg);
-
-    return new Unique_packed(comp_func, comp_func_fixed_arg, size_arg,
-                             max_in_memory_size_arg, min_dupl_count_arg);
-  }
-  return new Unique(comp_func, comp_func_fixed_arg, size_arg,
-                    max_in_memory_size_arg, min_dupl_count_arg);
-
-}
-
-
-Unique* Item_func_group_concat::get_unique(qsort_cmp2 comp_func, void *comp_func_fixed_arg,
-                                           uint size_arg, size_t max_in_memory_size_arg,
-                                           uint min_dupl_count_arg, bool allow_packing)
-{
-
   if (allow_packing)
-    return new Unique_packed(comp_func, comp_func_fixed_arg, size_arg,
+   return new Unique_packed(comp_func, comp_func_fixed_arg, size_arg,
                              max_in_memory_size_arg, min_dupl_count_arg);
   return new Unique(comp_func, comp_func_fixed_arg, size_arg,
                     max_in_memory_size_arg, min_dupl_count_arg);
@@ -4931,7 +4954,6 @@ Unique* Item_func_group_concat::get_unique(qsort_cmp2 comp_func, void *comp_func
 }
 
 
-
 void Item_func_group_concat::print(String *str, enum_query_type query_type)
 {
   str->append(func_name());
@@ -4986,6 +5008,15 @@ Item_func_group_concat::~Item_func_group_concat()
 }
 
 
+/*
+  @brief
+    Insert a record inside the Unique tree
+
+  @retval
+    -1       NULL value, record rejected
+     0       record succesfully inserted into the tree
+     1       error
+*/
 int Item_func_group_concat::insert_record_to_unique()
 {
   if (unique_filter->is_packed())
diff --git a/sql/item_sum.h b/sql/item_sum.h
index ae271efbaed..e530495e976 100644
--- a/sql/item_sum.h
+++ b/sql/item_sum.h
@@ -682,6 +682,10 @@ class Aggregator_distinct : public Aggregator
   */
   bool use_distinct_values;
 
+  /*
+    Used to store information about variable sized keys
+    @see sql_sort.h for the class definition
+  */
   Variable_sized_keys *variable_sized_keys;
 
 public:
@@ -702,8 +706,10 @@ public:
   bool unique_walk_function(void *element);
   bool unique_walk_function_for_count(void *element);
   int insert_record_to_unique();
+  qsort_cmp2 get_compare_func_for_packed_keys();
   static int composite_key_cmp(void* arg, uchar* key1, uchar* key2);
   static int composite_packed_key_cmp(void* arg, uchar* key1, uchar* key2);
+  static int packed_key_cmp_single_arg(void *arg, uchar *key1, uchar *key2);
 };
 
 
@@ -2030,9 +2036,6 @@ public:
   uint get_null_bytes();
   bool is_distinct_packed();
   bool is_packing_allowed(uint* total_length);
-  Unique *get_unique(qsort_cmp2 comp_func, void *comp_func_fixed_arg,
-                     uint size_arg, size_t max_in_memory_size_arg,
-                     uint min_dupl_count_arg, bool allow_packing);
   static int dump_leaf_key(void* key_arg,
                            element_count count __attribute__((unused)),
                            void* item_arg);
diff --git a/sql/sql_sort.h b/sql/sql_sort.h
index 4a4300c4c13..342add1445b 100644
--- a/sql/sql_sort.h
+++ b/sql/sql_sort.h
@@ -753,6 +753,7 @@ public:
   bool setup(THD *thd, Field *field);
   uint make_packed_record(bool exclude_nulls);
   int compare_packed_keys(uchar *a, uchar *b);
+  int compare_keys_for_single_arg(uchar *a, uchar *b);
   static void store_packed_length(uchar *p, uint sz)
   {
     int4store(p, sz - size_of_length_field);
diff --git a/sql/sql_statistics.cc b/sql/sql_statistics.cc
index 74f3f442cd6..60e5b17fa81 100644
--- a/sql/sql_statistics.cc
+++ b/sql/sql_statistics.cc
@@ -1723,20 +1723,19 @@ public:
       if (!variable_size_keys)
         return true; // OOM
 
-      tree= new Unique_packed_single_arg((qsort_cmp2) simple_packed_str_key_cmp,
-                                         (void*) this, tree_key_length,
-                                         max_heap_table_size, 1);
+      tree= new Unique_packed((qsort_cmp2) simple_packed_str_key_cmp,
+                              (void*) this, tree_key_length,
+                              max_heap_table_size, 1);
+      if (!tree)
+        return true; // OOM
       return variable_size_keys->setup(thd, table_field);
-
     }
 
     tree_key_length= table_field->pack_length();
     tree= new Unique((qsort_cmp2) simple_str_key_cmp, (void*) table_field,
                      tree_key_length, max_heap_table_size, 1);
 
-    if (!tree)
-      return true;  // OOM
-    return false;
+    return tree == NULL;
   }
 
 
@@ -1835,7 +1834,7 @@ int Count_distinct_field::simple_packed_str_key_cmp(void* arg,
 {
   Count_distinct_field *compare_arg= (Count_distinct_field*)arg;
   DBUG_ASSERT(compare_arg->variable_size_keys);
-  return compare_arg->variable_size_keys->compare_packed_keys(key1, key2);
+  return compare_arg->variable_size_keys->compare_keys_for_single_arg(key1, key2);
 }
 
 
@@ -1853,7 +1852,7 @@ public:
 
   bool add()
   {
-    longlong val= table_field->val_int();   
+    longlong val= table_field->val_int();
     return tree->unique_add(&val, tree->get_size());
   }
   bool setup(THD *thd, size_t max_heap_table_size)
@@ -1862,10 +1861,7 @@ public:
     tree= new Unique((qsort_cmp2) simple_ulonglong_key_cmp,
                      (void*) &tree_key_length,
                      tree_key_length, max_heap_table_size, 1);
-    if (!tree)
-      return true;  // OOM
-
-    return tree->setup(thd, table_field);
+    return tree == NULL;
   }
   static int simple_ulonglong_key_cmp(void* arg, uchar* key1, uchar* key2);
 };
diff --git a/sql/sql_type.h b/sql/sql_type.h
index 920735af42e..117391d99cb 100644
--- a/sql/sql_type.h
+++ b/sql/sql_type.h
@@ -3817,6 +3817,11 @@ public:
 
   /*
     create a compact size key part for a sort key
+
+    @param to                   buffer to store value of keypart
+    @param item                 item corresponding to the keypart
+    @param sort_field           sort field structure
+    @param tmp_buffer           temporary buffer to store the packed value if needed
   */
   virtual uint make_packed_sort_key_part(uchar *to, Item *item,
                                          const SORT_FIELD_ATTR *sort_field,
diff --git a/sql/uniques.cc b/sql/uniques.cc
index 377c8c3cc29..4ec032276fe 100644
--- a/sql/uniques.cc
+++ b/sql/uniques.cc
@@ -627,7 +627,6 @@ static bool merge_walk(uchar *merge_buffer, size_t merge_buffer_size,
            get_counter_from_merged_element(top->current_key(), cnt_ofs) : 1;
       if (walk_action(top->current_key(), cnt, walk_action_arg))
         goto end;
-
       top->advance_current_key(key_length);
     }
     while (top->decrement_mem_count());
@@ -862,135 +861,8 @@ Unique_packed::Unique_packed(qsort_cmp2 comp_func, void *comp_func_fixed_arg,
                             uint size_arg, size_t max_in_memory_size_arg,
                             uint min_dupl_count_arg):
   Unique(comp_func, comp_func_fixed_arg, size_arg,
-         max_in_memory_size_arg, min_dupl_count_arg),
-  packed_rec_ptr(NULL),
-  sortorder(NULL), sort_keys(NULL)
+         max_in_memory_size_arg, min_dupl_count_arg)
 {
-  packed_rec_ptr= (uchar *)my_malloc(PSI_INSTRUMENT_ME,
-                                     size_arg,
-                                     MYF(MY_WME | MY_THREAD_SPECIFIC));
-
-  tmp_buffer.alloc(size_arg);
-}
-
-
-Unique_packed::~Unique_packed()
-{
-  my_free(packed_rec_ptr);
-}
-
-
-/*
-  @brief
-    Setup the structures that are used when Unique stores packed values
-
-  @param thd                   thread structure
-  @param item                  item of aggregate function
-  @param non_const_args        number of non constant arguments
-  @param arg_count             total number of arguments
-
-  @note
-    This implementation is used by GROUP_CONCAT and COUNT_DISTINCT
-    as it can have more than one arguments in the argument list.
-
-  @retval
-    TRUE  error
-    FALSE setup successful
-*/
-
-bool
-Unique_packed::setup(THD *thd, Item_sum *item,
-                     uint non_const_args, uint arg_count)
-{
-  SORT_FIELD *sort,*pos;
-  if (sortorder)
-    return false;
-  DBUG_ASSERT(sort_keys == NULL);
-  sortorder= (SORT_FIELD*) thd->alloc(sizeof(SORT_FIELD) * non_const_args);
-  pos= sort= sortorder;
-  if (!pos)
-    return true;
-  sort_keys= new Sort_keys(sortorder, non_const_args);
-  if (!sort_keys)
-    return true;
-  sort=pos= sortorder;
-  for (uint i= 0; i < arg_count; i++)
-  {
-    Item *arg= item->get_arg(i);
-    if (arg->const_item())
-      continue;
-
-    if (arg->type() == Item::FIELD_ITEM)
-    {
-      Field *field= ((Item_field*)arg)->field;
-      pos->setup(field, false);
-    }
-    else
-      pos->setup(arg, false);
-    pos++;
-  }
-  return false;
-}
-
-
-/*
-  @brief
-    Setup the structures that are used when Unique stores packed values
-
-  @param thd                   thread structure
-  @param field                 field structure
-
-  @retval
-    TRUE  error
-    FALSE setup successful
-*/
-
-bool Unique_packed::setup(THD *thd, Field *field)
-{
-  SORT_FIELD *sort,*pos;
-  if (sortorder)
-    return false;
-
-  DBUG_ASSERT(sort_keys == NULL);
-  sortorder= (SORT_FIELD*) thd->alloc(sizeof(SORT_FIELD));
-  pos= sort= sortorder;
-  if (!pos)
-    return true;
-  sort_keys= new Sort_keys(sortorder, 1);
-  if (!sort_keys)
-    return true;
-  sort=pos= sortorder;
-  pos->setup(field, false);  // Nulls are always excluded
-
-  return false;
-}
-
-
-/*
-  @brief
-    Compare two packed keys inside the Unique tree
-
-  @param a_ptr             packed sort key
-  @param b_ptr             packed sort key
-
-  @retval
-    >0   key a_ptr greater than b_ptr
-    =0   key a_ptr equal to b_ptr
-    <0   key a_ptr less than b_ptr
-
-*/
-
-int Unique_packed::compare_packed_keys(uchar *a_ptr, uchar *b_ptr)
-{
-  return sort_keys->compare_keys(a_ptr + size_of_length_field,
-                                 b_ptr + size_of_length_field);
-}
-
-
-int Unique_packed_single_arg::compare_packed_keys(uchar *a_ptr, uchar *b_ptr)
-{
-  return sort_keys->compare_keys_for_single_arg(a_ptr + size_of_length_field,
-                                                b_ptr + size_of_length_field);
 }
 
 
@@ -1015,48 +887,3 @@ int Unique_packed::write_record_to_file(uchar *key)
 {
   return my_b_write(get_file(), key, read_packed_length(key));
 }
-
-
-/*
-  TODO varun:  add description
-*/
-uint Unique_packed::make_packed_record(bool exclude_nulls)
-{
-  Field *field;
-  SORT_FIELD *sort_field;
-  uint length;
-  uchar *orig_to, *to;
-  orig_to= to= packed_rec_ptr;
-  to+= size_of_length_field;
-
-  for (sort_field=sort_keys->begin() ;
-       sort_field != sort_keys->end() ;
-       sort_field++)
-  {
-    bool maybe_null=0;
-    if ((field=sort_field->field))
-    {
-      // Field
-      length= field->make_packed_sort_key_part(to, sort_field);
-    }
-    else
-    {           // Item
-      Item *item= sort_field->item;
-      length= item->type_handler()->make_packed_sort_key_part(to, item,
-                                                              sort_field,
-                                                              &tmp_buffer);
-    }
-
-    if ((maybe_null= sort_field->maybe_null))
-    {
-      if (exclude_nulls && length == 0)
-        return 0;   // NULL value
-      to++;
-    }
-    to+= length;
-  }
-
-  length= static_cast<int>(to - orig_to);
-  store_packed_length(orig_to, length);
-  return length;
-}
diff --git a/sql/uniques.h b/sql/uniques.h
index 8d02364a93b..3d90039b597 100644
--- a/sql/uniques.h
+++ b/sql/uniques.h
@@ -140,51 +140,10 @@ public:
   size_t get_max_in_memory_size() const { return max_in_memory_size; }
   bool is_count_stored() { return with_counters; }
   IO_CACHE *get_file ()  { return &file; }
-  virtual uchar *get_packed_rec_ptr()
-  {
-    DBUG_ASSERT(0);
-    return NULL;
-  }
-  virtual Sort_keys *get_keys()
-  {
-    DBUG_ASSERT(0);
-    return NULL;
-  }
-  virtual SORT_FIELD *get_sortorder()
-  {
-    DBUG_ASSERT(0);
-    return NULL;
-  }
-  virtual bool setup(THD *thd, Item_sum *item, uint non_const_args,
-                     uint arg_count)
-  {
-    return false;
-  }
-
-  virtual bool setup(THD *thd, Field *field)
-  {
-    return false;
-  }
-
-  virtual int compare_packed_keys(uchar *a, uchar *b)
-  {
-    DBUG_ASSERT(0);
-    return 0;
-  }
-
-  virtual uint get_length(uchar *ptr, bool exclude_nulls)
-  {
-    return size;
-  }
   virtual int write_record_to_file(uchar *key);
 
   // returns TRUE if the unique tree stores packed values
   virtual bool is_packed() { return false; }
-  virtual uint make_packed_record(bool exclude_nulls)
-  {
-    DBUG_ASSERT(0);
-    return 0;
-  }
 
   friend int unique_write_to_file(uchar* key, element_count count, Unique *unique);
   friend int unique_write_to_ptrs(uchar* key, element_count count, Unique *unique);
@@ -201,64 +160,18 @@ public:
   Unique_packed class: derived from Unique class, used to store
   records in packed format to efficiently utilize the space provided
   inside the tree.
-
-  The format is as follows:
-
-    <sort_key_length><packed_value_1><packed_value2> ....... <packed_valueN>
-
-    format for a n-part key
-
-  <sort_key_length> is the length of the whole key.
-  Each packed value is encoded as follows:
-
-    <null_byte=0>  // This is a an SQL NULL
-    [<null_byte=1>] <packed_value>  // this a non-NULL value
-  null_byte is present if the field/item is NULLable.
-  SQL NULL is encoded as just one NULL-indicator byte.
 */
 
 class Unique_packed : public Unique
 {
 protected:
-  /*
-    Packed record ptr for a record of the table, the packed value in this
-    record is added to the unique tree
-  */
-  uchar* packed_rec_ptr;
-
-  String tmp_buffer;
-
-  /*
-    Array of SORT_FIELD structure storing the information about the key parts
-    in the sort key of the Unique tree
-    @see Unique::setup()
-  */
-  SORT_FIELD *sortorder;
-
-  /*
-    Structure storing information about usage of keys
-  */
-  Sort_keys *sort_keys;
-
   public:
   Unique_packed(qsort_cmp2 comp_func, void *comp_func_fixed_arg,
                 uint size_arg, size_t max_in_memory_size_arg,
                 uint min_dupl_count_arg);
 
-  virtual ~Unique_packed();
   bool is_packed() { return true; }
-  uchar *get_packed_rec_ptr() { return packed_rec_ptr; }
-  Sort_keys *get_keys() { return sort_keys; }
-  SORT_FIELD *get_sortorder() { return sortorder; }
-  bool setup(THD *thd, Item_sum *item, uint non_const_args, uint arg_count);
-  bool setup(THD *thd, Field *field);
-  virtual int compare_packed_keys(uchar *a, uchar *b);
   int write_record_to_file(uchar *key);
-  uint make_packed_record(bool exclude_nulls);
-  static void store_packed_length(uchar *p, uint sz)
-  {
-    int4store(p, sz - size_of_length_field);
-  }
 
   // returns the length of the key along with the length bytes for the key
   static uint read_packed_length(uchar *p)
@@ -269,19 +182,4 @@ protected:
   static const uint size_of_length_field= 4;
 };
 
-
-class Unique_packed_single_arg : public Unique_packed
-{
-  public:
-    Unique_packed_single_arg(qsort_cmp2 comp_func, void *comp_func_fixed_arg,
-                             uint size_arg, size_t max_in_memory_size_arg,
-                             uint min_dupl_count_arg):
-      Unique_packed(comp_func, comp_func_fixed_arg, size_arg,
-                max_in_memory_size_arg, min_dupl_count_arg)
-  {}
-
-  int compare_packed_keys(uchar *a, uchar *b);
-
-};
-
 #endif /* UNIQUE_INCLUDED */