Addressing review-1

sql/item_sum.cc

@@ -866,6 +866,9 @@ bool Aggregator_distinct::setup(THD *thd)
       {
         compare_key= (qsort_cmp2) key_cmp;
         cmp_arg= (void*)this;
+        encoder= item_sum->get_encoder_for_variable_size_keys(non_const_items);
+        if (!encoder)
+          return TRUE; // OOM
       }
       DBUG_ASSERT(tree == 0);
 
@@ -881,7 +884,8 @@ bool Aggregator_distinct::setup(THD *thd)
       if (!tree ||
           (tree->get_descriptor()->setup_for_item(thd, item_sum,
                                                   non_const_items,
-                                                  item_sum->get_arg_count())))
+                                                  item_sum->get_arg_count()))||
+          (encoder && encoder->init(tree_key_length)))
         return TRUE;
     }
     return FALSE;
@@ -984,7 +988,7 @@ int Aggregator_distinct::insert_record_to_unique()
   {
     uchar *rec_ptr;
     Descriptor *descriptor= tree->get_descriptor();
-    if ((rec_ptr= descriptor->make_record(true)) == NULL)
+    if ((rec_ptr= encoder->make_encoded_record(descriptor->get_keys(), true)) == NULL)
       return -1; // NULL value
     DBUG_ASSERT(descriptor->get_length_of_key(rec_ptr) <= tree->get_size());
     return tree->unique_add(rec_ptr);
@@ -1845,6 +1849,7 @@ Aggregator_distinct::~Aggregator_distinct()
     delete tmp_table_param;
     tmp_table_param= NULL;
   }
+  delete encoder;
 }
 
 
@@ -4039,7 +4044,7 @@ Item_func_group_concat(THD *thd, Name_resolution_context *context_arg,
    warning_for_row(FALSE),
    force_copy_fields(0), row_limit(NULL),
    offset_limit(NULL), limit_clause(limit_clause),
-   copy_offset_limit(0), copy_row_limit(0), original(0)
+   copy_offset_limit(0), copy_row_limit(0), original(0), encoder(NULL)
 {
   Item *item_select;
   Item **arg_ptr;
@@ -4108,7 +4113,8 @@ Item_func_group_concat::Item_func_group_concat(THD *thd,
   force_copy_fields(item->force_copy_fields),
   row_limit(item->row_limit), offset_limit(item->offset_limit),
   limit_clause(item->limit_clause),copy_offset_limit(item->copy_offset_limit),
-  copy_row_limit(item->copy_row_limit), original(item)
+  copy_row_limit(item->copy_row_limit), original(item),
+  encoder(item->encoder)
 {
   quick_group= item->quick_group;
   result.set_charset(collation.collation);
@@ -4566,6 +4572,8 @@ bool Item_func_group_concat::setup(THD *thd)
 
   if (distinct)
   {
+    if (allow_packing)
+      encoder= get_encoder_for_variable_size_keys(non_const_items);
     unique_filter= get_unique(get_comparator_function_for_distinct(allow_packing),
                              (void*)this,
                              tree_key_length + get_null_bytes(),
@@ -4575,7 +4583,8 @@ bool Item_func_group_concat::setup(THD *thd)
     if (!unique_filter ||
         (unique_filter->get_descriptor()->setup_for_item(thd, this,
                                                          non_const_items,
-                                                         arg_count_field)))
+                                                         arg_count_field)) ||
+        (encoder && encoder->init(tree_key_length + get_null_bytes())))
       DBUG_RETURN(TRUE);
   }
   if ((row_limit && row_limit->cmp_type() != INT_RESULT) ||
@@ -4890,6 +4899,8 @@ Item_func_group_concat::~Item_func_group_concat()
 {
   if (!original && unique_filter)
     delete unique_filter;
+  if (!original && encoder)
+    delete encoder;
 }
 
 
@@ -4944,7 +4955,7 @@ int Item_func_group_concat::insert_packed_record_to_unique()
 {
   Descriptor *descriptor= unique_filter->get_descriptor();
   uchar *rec_ptr;
-  if ((rec_ptr= descriptor->make_record(skip_nulls())) == NULL)
+  if (!(rec_ptr= encoder->make_encoded_record(descriptor->get_keys(),skip_nulls())))
     return -1; // NULL value
   DBUG_ASSERT(descriptor->get_length_of_key(rec_ptr)
                         <= unique_filter->get_size());
@@ -4992,3 +5003,19 @@ Item_func_group_concat::get_descriptor_for_variable_size_keys(uint args_count,
   else
     return new Variable_size_composite_key_desc_for_gconcat(size_arg);
 }
+
+
+Encode_key* Item_sum::get_encoder_for_variable_size_keys(uint args_count)
+{
+  return new Encode_variable_size_key();
+}
+
+
+Encode_key*
+Item_func_group_concat::get_encoder_for_variable_size_keys(uint args_count)
+{
+  if (args_count == 1)
+    return new Encode_variable_size_key();
+  else
+    return new Encode_key_for_group_concat();
+}

sql/item_sum.h

@@ -313,6 +313,7 @@ class Window_spec;
   but still return false from Item_sum::const_item().
 */
 class Unique_impl;
+class Encode_key;
 
 class Item_sum :public Item_func_or_sum
 {
@@ -602,6 +603,7 @@ class Item_sum :public Item_func_or_sum
                                                          uint size_arg);
   virtual Descriptor *get_descriptor_for_variable_size_keys(uint args_count,
                                                             uint size_arg);
+  virtual Encode_key* get_encoder_for_variable_size_keys(uint args_count);
 };
 
 
@@ -687,11 +689,12 @@ class Aggregator_distinct : public Aggregator
     instead of calling the relevant val_..() method.
   */
   bool use_distinct_values;
+  Encode_key *encoder;
 
 public:
   Aggregator_distinct (Item_sum *sum) :
     Aggregator(sum), table(NULL), tmp_table_param(NULL), tree(NULL),
-    always_null(false), use_distinct_values(false) {}
+    always_null(false), use_distinct_values(false) , encoder(NULL) {}
   virtual ~Aggregator_distinct ();
   Aggregator_type Aggrtype() { return DISTINCT_AGGREGATOR; }
 
@@ -1916,6 +1919,8 @@ class Item_func_group_concat : public Item_sum
   */
   Item_func_group_concat *original;
 
+  Encode_key *encoder;
+
   /*
     Used by Item_func_group_concat and Item_func_json_arrayagg. The latter
     needs null values but the former doesn't.
@@ -2043,6 +2048,7 @@ class Item_func_group_concat : public Item_sum
                                                  uint size_arg) override;
   Descriptor *get_descriptor_for_variable_size_keys(uint args_count,
                                                     uint size_arg) override;
+  Encode_key* get_encoder_for_variable_size_keys(uint args_count) override;
 };
 
 #endif /* ITEM_SUM_INCLUDED */

sql/sql_statistics.cc

@@ -1670,6 +1670,7 @@ class Count_distinct_field: public Sql_alloc
 
   ulonglong distincts;
   ulonglong distincts_single_occurence;
+  Encode_key *encoder;
 
 public:
   
@@ -1695,12 +1696,13 @@ class Count_distinct_field: public Sql_alloc
     table_field= field;
     tree_key_length= 0;
     tree= NULL;
+    encoder= NULL;
   }
 
   virtual ~Count_distinct_field()
   {
     delete tree;
-    tree= NULL;
+    delete encoder;
   }
 
   /* 
@@ -1751,6 +1753,9 @@ class Count_distinct_field: public Sql_alloc
     if (table_field->is_packable())
     {
       tree_key_length= compute_packable_length(table_field);
+      encoder= new Encode_variable_size_key();
+      if (!encoder || encoder->init(tree_key_length))
+        return TRUE; // OOM
       desc= new Variable_size_keys_simple(tree_key_length);
     }
     else
@@ -1780,7 +1785,7 @@ class Count_distinct_field: public Sql_alloc
     if (tree->is_variable_sized())
     {
       Descriptor *descriptor= tree->get_descriptor();
-      uchar *rec_ptr=  descriptor->make_record(true);
+      uchar *rec_ptr=  encoder->make_encoded_record(descriptor->get_keys(), true);
       DBUG_ASSERT(descriptor->get_length_of_key(rec_ptr) <= tree->get_size());
       return tree->unique_add(rec_ptr);
     }

sql/uniques.cc

@@ -1027,24 +1027,6 @@ int Variable_size_keys_simple::compare_keys(uchar *a, uchar *b)
 }
 
 
-uchar* Variable_size_composite_key_desc::make_record(bool exclude_nulls)
-{
-  return make_encoded_record(sort_keys, exclude_nulls);
-}
-
-uchar*
-Variable_size_composite_key_desc_for_gconcat::make_record(bool exclude_nulls)
-{
-  return make_encoded_record(sort_keys, exclude_nulls);
-}
-
-
-uchar* Variable_size_keys_simple::make_record(bool exclude_nulls)
-{
-  return make_encoded_record(sort_keys, exclude_nulls);
-}
-
-
 /*
   @brief
     Create the sortorder and Sort keys structures for a descriptor
@@ -1072,27 +1054,6 @@ bool Descriptor::init(THD *thd, uint count)
 }
 
 
-bool Variable_size_composite_key_desc::init(THD *thd, uint count)
-{
-  return Descriptor::init(thd, count) ||
-         Encode_variable_size_key::init(max_length);
-}
-
-
-bool Variable_size_composite_key_desc_for_gconcat::init(THD *thd, uint count)
-{
-  return Descriptor::init(thd, count) ||
-         Encode_key_for_group_concat::init(max_length);
-}
-
-
-bool Variable_size_keys_simple::init(THD *thd, uint count)
-{
-  return Descriptor::init(thd, count) ||
-         Encode_variable_size_key::init(max_length);
-}
-
-
 bool
 Variable_size_composite_key_desc_for_gconcat::setup_for_item(THD *thd,
                                                           Item_sum *item,

sql/uniques.h

@@ -120,10 +120,9 @@ class Descriptor : public Sql_alloc
   { return false; }
   virtual bool setup_for_field(THD *thd, Field *field) { return false; }
 
-  virtual Sort_keys *get_keys() { return sort_keys; }
-  SORT_FIELD *get_sortorder() { return sortorder; }
+  inline Sort_keys *get_keys() { return sort_keys; }
+  inline SORT_FIELD *get_sortorder() { return sortorder; }
 
-  virtual uchar* make_record(bool exclude_nulls) { return NULL; }
   virtual bool is_single_arg() = 0;
   virtual bool init(THD *thd, uint count);
 };
@@ -264,34 +263,27 @@ class Variable_size_keys_descriptor : public Descriptor
   that the number of arguments with DISTINCT is 1.
 */
 
-class Variable_size_keys_simple : public Variable_size_keys_descriptor,
-                                  public Encode_variable_size_key
+class Variable_size_keys_simple : public Variable_size_keys_descriptor
 {
 public:
   Variable_size_keys_simple(uint length)
-    :Variable_size_keys_descriptor(length), Encode_variable_size_key() {}
+    :Variable_size_keys_descriptor(length){}
   ~Variable_size_keys_simple() {}
   int compare_keys(uchar *a, uchar *b) override;
-  uchar* make_record(bool exclude_nulls) override;
-  uchar* get_rec_ptr() { return rec_ptr; }
   bool is_single_arg() override { return true; }
-  bool init(THD *thd, uint count) override;
 };
 
 
 /*
   Descriptor for variable sized keys with multiple key parts
 */
-class Variable_size_composite_key_desc : public Variable_size_keys_descriptor,
-                                         public Encode_variable_size_key
+class Variable_size_composite_key_desc : public Variable_size_keys_descriptor
 {
 public:
   Variable_size_composite_key_desc(uint length)
-    : Variable_size_keys_descriptor(length), Encode_variable_size_key() {}
+    : Variable_size_keys_descriptor(length){}
   ~Variable_size_composite_key_desc() {}
   int compare_keys(uchar *a, uchar *b) override;
-  uchar* make_record(bool exclude_nulls) override;
-  bool init(THD *thd, uint count) override;
 };
 
 
@@ -301,18 +293,15 @@ class Variable_size_composite_key_desc : public Variable_size_keys_descriptor,
 */
 
 class Variable_size_composite_key_desc_for_gconcat :
-                                         public Variable_size_keys_descriptor,
-                                         public Encode_key_for_group_concat
+                                         public Variable_size_keys_descriptor
 {
 public:
   Variable_size_composite_key_desc_for_gconcat(uint length)
-    : Variable_size_keys_descriptor(length), Encode_key_for_group_concat() {}
+    : Variable_size_keys_descriptor(length) {}
   ~Variable_size_composite_key_desc_for_gconcat() {}
   int compare_keys(uchar *a, uchar *b) override;
-  uchar* make_record(bool exclude_nulls) override;
   bool setup_for_item(THD *thd, Item_sum *item,
                       uint non_const_args, uint arg_count) override;
-  bool init(THD *thd, uint count) override;
 };