addressing-review

sql/item_sum.cc

@@ -988,7 +988,7 @@ int Aggregator_distinct::insert_record_to_unique()
   {
     uchar *rec_ptr;
     Descriptor *descriptor= tree->get_descriptor();
-    if ((rec_ptr= encoder->make_encoded_record(descriptor->get_keys(), true)) == NULL)
+    if ((rec_ptr= encoder->make_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);
@@ -4955,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= encoder->make_encoded_record(descriptor->get_keys(),skip_nulls())))
+  if (!(rec_ptr= encoder->make_record(descriptor->get_keys(),skip_nulls())))
     return -1; // NULL value
   DBUG_ASSERT(descriptor->get_length_of_key(rec_ptr)
                         <= unique_filter->get_size());
@@ -5005,17 +5005,17 @@ Item_func_group_concat::get_descriptor_for_variable_size_keys(uint args_count,
 }
 
 
-Encode_key* Item_sum::get_encoder_for_variable_size_keys(uint args_count)
+Key_encoder* Item_sum::get_encoder_for_variable_size_keys(uint args_count)
 {
-  return new Encode_variable_size_key();
+  return new Key_encoder_for_variable_size_key();
 }
 
 
-Encode_key*
+Key_encoder*
 Item_func_group_concat::get_encoder_for_variable_size_keys(uint args_count)
 {
   if (args_count == 1)
-    return new Encode_variable_size_key();
+    return new Key_encoder_for_variable_size_key();
   else
-    return new Encode_key_for_group_concat();
+    return new Key_encoder_for_group_concat();
 }

sql/item_sum.h

@@ -313,7 +313,7 @@ class Window_spec;
   but still return false from Item_sum::const_item().
 */
 class Unique_impl;
-class Encode_key;
+class Key_encoder;
 
 class Item_sum :public Item_func_or_sum
 {
@@ -603,7 +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);
+  virtual Key_encoder* get_encoder_for_variable_size_keys(uint args_count);
 };
 
 
@@ -689,7 +689,7 @@ class Aggregator_distinct : public Aggregator
     instead of calling the relevant val_..() method.
   */
   bool use_distinct_values;
-  Encode_key *encoder;
+  Key_encoder *encoder;
 
 public:
   Aggregator_distinct (Item_sum *sum) :
@@ -1919,7 +1919,7 @@ class Item_func_group_concat : public Item_sum
   */
   Item_func_group_concat *original;
 
-  Encode_key *encoder;
+  Key_encoder *encoder;
 
   /*
     Used by Item_func_group_concat and Item_func_json_arrayagg. The latter
@@ -2048,7 +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;
+  Key_encoder* get_encoder_for_variable_size_keys(uint args_count) override;
 };
 
 #endif /* ITEM_SUM_INCLUDED */

sql/sql_statistics.cc

@@ -1670,7 +1670,7 @@ class Count_distinct_field: public Sql_alloc
 
   ulonglong distincts;
   ulonglong distincts_single_occurence;
-  Encode_key *encoder;
+  Key_encoder *encoder;
 
 public:
   
@@ -1753,7 +1753,7 @@ 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();
+      encoder= new Key_encoder_for_variable_size_key();
       if (!encoder || encoder->init(tree_key_length))
         return TRUE; // OOM
       desc= new Variable_size_keys_simple(tree_key_length);
@@ -1785,7 +1785,7 @@ class Count_distinct_field: public Sql_alloc
     if (tree->is_variable_sized())
     {
       Descriptor *descriptor= tree->get_descriptor();
-      uchar *rec_ptr=  encoder->make_encoded_record(descriptor->get_keys(), true);
+      uchar *rec_ptr=  encoder->make_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

@@ -1221,7 +1221,7 @@ int Fixed_size_keys_for_group_concat::compare_keys(uchar *key1, uchar *key2)
 }
 
 
-bool Encode_key::init(uint length)
+bool Key_encoder::init(uint length)
 {
   if (tmp_buffer.alloc(length))
     return true;
@@ -1232,7 +1232,7 @@ bool Encode_key::init(uint length)
 }
 
 
-Encode_key::~Encode_key()
+Key_encoder::~Key_encoder()
 {
   my_free(rec_ptr);
 }
@@ -1246,8 +1246,9 @@ Encode_key::~Encode_key()
     0         NULL value
     >0        length of the packed record
 */
-uchar* Encode_variable_size_key::make_encoded_record(Sort_keys *sort_keys,
-                                                     bool exclude_nulls)
+
+uchar* Key_encoder_for_variable_size_key::make_record(Sort_keys *sort_keys,
+                                                      bool exclude_nulls)
 {
   Field *field;
   SORT_FIELD *sort_field;
@@ -1290,8 +1291,7 @@ uchar* Encode_variable_size_key::make_encoded_record(Sort_keys *sort_keys,
 }
 
 
-uchar*
-Encode_key_for_group_concat::make_encoded_record(Sort_keys *sort_keys,
+uchar* Key_encoder_for_group_concat::make_record(Sort_keys *sort_keys,
                                                  bool exclude_nulls)
 {
   Field *field;

sql/uniques.h

@@ -27,7 +27,7 @@
     Currently this encoding is only done for variable size keys
 */
 
-class Encode_key
+class Key_encoder
 {
 protected:
   /*
@@ -38,36 +38,35 @@ class Encode_key
 
   String tmp_buffer;
 public:
-  virtual ~Encode_key();
-  virtual uchar* make_encoded_record(Sort_keys *keys, bool exclude_nulls) = 0;
+  virtual ~Key_encoder();
+  virtual uchar* make_record(Sort_keys *keys, bool exclude_nulls) = 0;
   bool init(uint length);
   uchar *get_rec_ptr() { return rec_ptr; }
 };
 
 
-class Encode_variable_size_key : public Encode_key
+class Key_encoder_for_variable_size_key : public Key_encoder
 {
 public:
-  Encode_variable_size_key()
+  Key_encoder_for_variable_size_key()
   {
     rec_ptr= NULL;
   }
-  virtual ~Encode_variable_size_key() {}
-  uchar* make_encoded_record(Sort_keys *keys, bool exclude_nulls) override;
+  virtual ~Key_encoder_for_variable_size_key() {}
+  uchar* make_record(Sort_keys *keys, bool exclude_nulls) override;
 };
 
 
-class Encode_key_for_group_concat : public Encode_variable_size_key
+class Key_encoder_for_group_concat : public Key_encoder_for_variable_size_key
 {
 public:
-  Encode_key_for_group_concat() : Encode_variable_size_key(){}
-  ~Encode_key_for_group_concat() {}
-  uchar* make_encoded_record(Sort_keys *keys, bool exclude_nulls) override;
+  Key_encoder_for_group_concat() : Key_encoder_for_variable_size_key(){}
+  ~Key_encoder_for_group_concat() {}
+  uchar* make_record(Sort_keys *keys, bool exclude_nulls) override;
 };
 
 
 /*
-
   Descriptor class storing information about the keys that would be
   inserted in the Unique tree. This is an abstract class which is
   extended by other class to support descriptors for keys with fixed and
@@ -141,7 +140,7 @@ class Fixed_size_keys_descriptor : public Descriptor
   bool setup_for_field(THD *thd, Field *field);
   bool setup_for_item(THD *thd, Item_sum *item,
                       uint non_const_args, uint arg_count);
-  virtual int compare_keys(uchar *a, uchar *b) override;
+  int compare_keys(uchar *a, uchar *b) override;
   virtual bool is_single_arg() override { return true; }
 };
 
@@ -161,6 +160,8 @@ class Fixed_size_keys_mem_comparable: public Fixed_size_keys_descriptor
 
 /*
   Descriptor for fixed size keys for rowid comparison
+
+  Used by index merge and delete queries
 */
 class Fixed_size_keys_for_rowids: public Fixed_size_keys_descriptor
 {
@@ -234,8 +235,8 @@ class Variable_size_keys_descriptor : public Descriptor
   {
     return read_packed_length(ptr);
   }
-  virtual int compare_keys(uchar *a, uchar *b) override { return 0; }
-  virtual bool is_single_arg() override { return false; }
+  int compare_keys(uchar *a, uchar *b) override { return 0; }
+  bool is_single_arg() override { return false; }
 
   virtual bool setup_for_item(THD *thd, Item_sum *item,
                               uint non_const_args, uint arg_count) override;
@@ -261,6 +262,9 @@ class Variable_size_keys_descriptor : public Descriptor
   Used by EITS, JSON_ARRAYAGG.
   COUNT(DISTINCT col) AND GROUP_CONCAT(DISTINCT col) are also allowed
   that the number of arguments with DISTINCT is 1.
+
+  Comparisons happens only for packable fields with a collation specific
+  function
 */
 
 class Variable_size_keys_simple : public Variable_size_keys_descriptor
@@ -275,8 +279,15 @@ class Variable_size_keys_simple : public Variable_size_keys_descriptor
 
 
 /*
-  Descriptor for variable sized keys with multiple key parts
+  Descriptor for variable sized keys with multiple key parts for
+  COUNT(DISTINCT col1, col2 ....)
+
+  Comparison happens in the following way:
+  a) comparison for fixed fields happen with memcmp
+  b) comparison for packable fields happen with collation specific comparison
+     function
 */
+
 class Variable_size_composite_key_desc : public Variable_size_keys_descriptor
 {
 public:
@@ -289,7 +300,14 @@ class Variable_size_composite_key_desc : public Variable_size_keys_descriptor
 
 
 /*
-  Descriptor for variable sized keys with multiple key parts for GROUP_CONCAT
+  Descriptor for variable sized keys with multiple key parts
+  for GROUP_CONCAT
+
+  Comparison happens in the following way:
+  a) comparison for fixed fields happen with original values,
+     so Field::cmp is used
+  b) comparison for packable fields happen with collation specific comparison
+     function
 */
 
 class Variable_size_composite_key_desc_for_gconcat :