Post review fixes, WIP

include/my_dir.h

@@ -16,6 +16,7 @@
 #ifndef MY_DIR_H
 #define MY_DIR_H
 
+#include <my_global.h>
 #include <sys/stat.h>
 
 #ifdef	__cplusplus

sql/field.h

@@ -1171,7 +1171,7 @@ class Field: public Value_source
     returns the sort_length for a field without the suffix length bytes
     for field with binary charset.
   */
-  virtual uint32 sort_length_without_suffix() const { return pack_length(); }
+  virtual uint32 max_storage_size_without_length_storage() const { return pack_length(); }
 
   /*
     sort_suffix_length() return the length bytes needed to store the length
@@ -4250,7 +4250,7 @@ class Field_varstring :public Field_longstr {
   {
     return (uint32) field_length + sort_suffix_length();
   }
-  uint32 sort_length_without_suffix() const override
+  uint32 max_storage_size_without_length_storage() const override
   {
     return (uint32) field_length;
   }
@@ -4600,7 +4600,7 @@ class Field_blob :public Field_longstr {
   { return (uint32) (packlength); }
   uint row_pack_length() const override { return pack_length_no_ptr(); }
   uint32 sort_length() const override;
-  uint32 sort_length_without_suffix() const override
+  uint32 max_storage_size_without_length_storage() const override
   {
     return (uint32)field_length;
   }

sql/filesort.cc

@@ -2767,6 +2767,12 @@ void SORT_FIELD::setup_key_part(Field *fld, bool is_mem_comparable_arg)
   item= NULL;
   reverse= false;
   SORT_FIELD_ATTR::setup_key_part(fld, is_mem_comparable_arg);
+  if (is_variable_sized())
+    key_compare_fun= SORT_FIELD::compare_packed_varstrings;
+  else if (is_mem_comparable)
+    key_compare_fun= SORT_FIELD::compare_packed_fixed_size_vals;
+  else
+    key_compare_fun= SORT_FIELD::compare_fixed_size_vals;
 }
 
 
@@ -2782,12 +2788,10 @@ void SORT_FIELD::setup_key_part(Field *fld, bool is_mem_comparable_arg)
 */
 void SORT_FIELD_ATTR::setup_key_part(Field *field, bool is_mem_comparable_arg)
 {
-  original_length= length= field->sort_length_without_suffix();
+  original_length= length= field->max_storage_size_without_length_storage();
   cs= field->sort_charset();
   suffix_length= 0;
-  type= field->is_packable() ?
-        SORT_FIELD_ATTR::VARIABLE_SIZE :
-        SORT_FIELD_ATTR::FIXED_SIZE;
+  type= field->is_packable() ? VARIABLE_SIZE : FIXED_SIZE;
   maybe_null= field->maybe_null();
   is_mem_comparable= is_mem_comparable_arg;
   length_bytes= is_variable_sized() ? number_storage_requirement(length) : 0;
@@ -2809,25 +2813,6 @@ void SORT_FIELD_ATTR::setup_key_part(Field *field, bool is_mem_comparable_arg)
     0      either key a and key b are both NULL or both are NOT NULL
 */
 
-enum null_comparison_result {
-  BOTH_NULL,
-  A_NULL,
-  B_NULL,
-  BOTH_NOT_NULL
-};
-
-inline enum null_comparison_result compare_null_flag(bool a_null,
-                                                     bool b_null)
-{
-  if (a_null && b_null)
-    return BOTH_NULL;
-  if (a_null)
-    return A_NULL;
-  if (b_null)
-    return B_NULL;
-  return BOTH_NOT_NULL;
-}
-
 /*
   Compare two keys.
 
@@ -2849,33 +2834,29 @@ int SORT_FIELD::compare_keys(const uchar *a, size_t *a_len,
                              const uchar *b, size_t *b_len) const
 {
   int result;
+  bool a_null = !*a;
+  bool b_null = !*b;
+
   if (maybe_null)
   {
-    /* In sort keys (or packed keys) null_byte is 0 if the key is null. */
-    switch (compare_null_flag(!*a, !*b)) {
-    case BOTH_NULL:
+    if (a_null)
     {
-      // Null byte is always stored.
-      *a_len= *b_len= 1;
-      return 0;
-    }
-    case A_NULL:
+      if (b_null)
+      {
+
+        // Null byte is always stored.
+        *a_len= *b_len= 1;
+        return 0;
+      }
       return -1;
-    case B_NULL:
-      return 1;
-    default:
-      break;
     }
+    if (b_null)
+      return 1;
     a++;
     b++;
   }
 
-  if (is_variable_sized())
-    result= compare_packed_varstrings(a, a_len, b, b_len);
-  else if (is_mem_comparable)
-      result= compare_packed_fixed_size_vals(a, a_len, b, b_len);
-  else
-    result= compare_fixed_size_vals(a, a_len, b, b_len);
+  result= key_compare_fun(this, a, a_len, b, b_len);
 
   // Null byte is always stored.
   if (result == 0 && maybe_null)
@@ -2887,25 +2868,28 @@ int SORT_FIELD::compare_keys(const uchar *a, size_t *a_len,
 }
 
 
-int SORT_FIELD::compare_packed_varstrings(const uchar *a, size_t *a_len,
-                                          const uchar *b, size_t *b_len) const
+int SORT_FIELD::compare_packed_varstrings(const SORT_FIELD *sort_field,
+                                          const uchar *a, size_t *a_len,
+                                          const uchar *b, size_t *b_len)
 {
   int retval;
   size_t a_length, b_length;
+  uint length_bytes= sort_field->length_bytes;
+  uint suffix_length= sort_field->suffix_length;
   a_length= read_key_part_length(a, length_bytes);
   b_length= read_key_part_length(b, length_bytes);
 
   *a_len= length_bytes + a_length;
   *b_len= length_bytes + b_length;
 
-  retval= cs->strnncollsp(a + length_bytes,
-                          a_length - suffix_length,
-                          b + length_bytes,
-                          b_length - suffix_length);
+  retval= sort_field->cs->strnncollsp(a + length_bytes,
+                                      a_length - suffix_length,
+                                      b + length_bytes,
+                                      b_length - suffix_length);
 
   if (!retval && suffix_length)
   {
-    DBUG_ASSERT(cs == &my_charset_bin);
+    DBUG_ASSERT(sort_field->cs == &my_charset_bin);
     // comparing the length stored in suffix bytes for binary strings
     a= a + length_bytes + a_length - suffix_length;
     b= b + length_bytes + b_length - suffix_length;
@@ -2916,20 +2900,22 @@ int SORT_FIELD::compare_packed_varstrings(const uchar *a, size_t *a_len,
 }
 
 
-int SORT_FIELD::compare_packed_fixed_size_vals(const uchar *a, size_t *a_len,
-                                               const uchar *b, size_t *b_len) const
+int SORT_FIELD::compare_packed_fixed_size_vals(const SORT_FIELD *sort_field,
+                                               const uchar *a, size_t *a_len,
+                                               const uchar *b, size_t *b_len)
 {
-  *a_len= length;
-  *b_len= length;
-  return memcmp(a, b, length);
+  *a_len= sort_field->length;
+  *b_len= sort_field->length;
+  return memcmp(a, b, sort_field->length);
 }
 
-int SORT_FIELD::compare_fixed_size_vals(const uchar *a, size_t *a_len,
-                                        const uchar *b, size_t *b_len) const
+int SORT_FIELD::compare_fixed_size_vals(const SORT_FIELD *sort_field,
+                                        const uchar *a, size_t *a_len,
+                                        const uchar *b, size_t *b_len)
 {
-  *a_len= length;
-  *b_len= length;
-  return field->cmp(a, b);
+  *a_len= sort_field->length;
+  *b_len= sort_field->length;
+  return sort_field->field->cmp(a, b);
 }
 
 

sql/item_sum.cc

@@ -4191,7 +4191,7 @@ bool Item_func_group_concat::add(bool exclude_nulls)
     row to the output buffer here. That will be done in val_str.
   */
   if (!warning_for_row && (!tree && !distinct))
-    dump_leaf_key(get_record_pointer(), 1, this);
+    this->dump_leaf_key_impl(get_record_pointer(), false);
 
   return 0;
 }
@@ -4535,11 +4535,11 @@ bool Item_sum::is_packing_allowed(uint *total_length) const
       // 1 byte for nullability;
       tot_length+= MY_TEST(field->maybe_null());
 
-      tot_length+= field->sort_length_without_suffix();
+      tot_length+= field->max_storage_size_without_length_storage();
       if (field->is_packable())
       {
-        size_of_packable_fields+=
-              number_storage_requirement(field->sort_length_without_suffix());
+        size_of_packable_fields+= number_storage_requirement(
+            field->max_storage_size_without_length_storage());
       }
     }
   }

sql/sql_class.h

@@ -7381,12 +7381,19 @@ struct SORT_FIELD: public SORT_FIELD_ATTR
                    const uchar *b, size_t *b_len) const;
 
 private:
-  int compare_fixed_size_vals(const uchar *a, size_t *a_len,
-                              const uchar *b, size_t *b_len) const;
-  int compare_packed_fixed_size_vals(const uchar *a, size_t *a_len,
-                                     const uchar *b, size_t *b_len) const;
-  int compare_packed_varstrings(const uchar *a, size_t *a_len,
-                                const uchar *b, size_t *b_len) const;
+  typedef int (*compare_fun_type) (const SORT_FIELD *,
+                                   const uchar *, size_t *,
+                                   const uchar *, size_t *);
+  compare_fun_type key_compare_fun = nullptr;
+  static int compare_fixed_size_vals(const SORT_FIELD *sort_field,
+                                     const uchar *a, size_t *a_len,
+                                     const uchar *b, size_t *b_len);
+  static int compare_packed_fixed_size_vals(const SORT_FIELD *sort_field,
+                                            const uchar *a, size_t *a_len,
+                                            const uchar *b, size_t *b_len);
+  static int compare_packed_varstrings(const SORT_FIELD *sort_field,
+                                       const uchar *a, size_t *a_len,
+                                       const uchar *b, size_t *b_len);
 };
 
 

sql/uniques.cc

@@ -53,19 +53,21 @@ int Unique::unique_write_to_file_with_count(uchar* key, element_count count,
          my_b_write(&unique->file, (uchar*)&count, sizeof(element_count)) ? 1 : 0;
 }
 
-int Unique::unique_write_to_ptrs(uchar* key, element_count count, Unique *unique)
+int Unique::unique_write_to_ptrs(uchar* key, element_count count,
+                                 Unique *unique)
 {
   memcpy(unique->sort.record_pointers, key, unique->size);
   unique->sort.record_pointers+=unique->size;
   return 0;
 }
 
-int Unique::unique_intersect_write_to_ptrs(uchar* key, element_count count, Unique *unique)
+int Unique::unique_intersect_write_to_ptrs(uchar* key, element_count count,
+                                           Unique *unique)
 {
   if (count >= unique->min_dupl_count)
   {
     memcpy(unique->sort.record_pointers, key, unique->size);
-    unique->sort.record_pointers+=unique->size;
+    unique->sort.record_pointers+= unique->size;
   }
   else
     unique->filtered_out_elems++;
@@ -76,11 +78,11 @@ int Unique::unique_intersect_write_to_ptrs(uchar* key, element_count count, Uniq
 Unique::Unique(Keys_descriptor *desc,
                size_t max_in_memory_size_arg,
                uint min_dupl_count_arg)
-  :max_in_memory_size(max_in_memory_size_arg),
-   size(desc->get_max_key_length()),
+  :size(desc->get_max_key_length()),
+   max_in_memory_size(max_in_memory_size_arg),
+   max_elements(max_in_memory_size / ALIGN_SIZE(sizeof(TREE_ELEMENT)+size)),
    full_size(min_dupl_count_arg ? size + sizeof(element_count) : size),
    min_dupl_count(min_dupl_count_arg),
-   with_counters(MY_TEST(min_dupl_count_arg)),
    memory_used(0),
    elements(0),
    keys_descriptor(desc)
@@ -88,14 +90,9 @@ Unique::Unique(Keys_descriptor *desc,
   my_b_clear(&file);
   init_tree(&tree, (max_in_memory_size / 16), 0, 0, unique_compare_keys,
             NULL, desc, MYF(MY_THREAD_SPECIFIC));
-  /* If the following fail's the next add will also fail */
+  /* If the following fails the next add will also fail */
   my_init_dynamic_array(PSI_INSTRUMENT_ME, &file_ptrs, sizeof(Merge_chunk), 16,
                         16, MYF(MY_THREAD_SPECIFIC));
-  /*
-    If you change the following, change it in get_max_elements function, too.
-  */
-  max_elements= (ulong) (max_in_memory_size /
-                         ALIGN_SIZE(sizeof(TREE_ELEMENT)+size));
   if (!max_elements)
   {
     max_elements= 1;
@@ -382,7 +379,7 @@ Unique::~Unique()
 }
 
 
-    /* Write tree to disk; clear tree */
+/* Write tree to disk; clear tree */
 bool Unique::flush()
 {
   Merge_chunk file_ptr;
@@ -391,10 +388,9 @@ bool Unique::flush()
   file_ptr.set_file_position(my_b_tell(&file));
 
   tree_walk_action action= min_dupl_count ?
-		           (tree_walk_action) unique_write_to_file_with_count :
-		           (tree_walk_action) unique_write_to_file;
-  if (tree_walk(&tree, action,
-		(void*) this, left_root_right) ||
+    (tree_walk_action) unique_write_to_file_with_count :
+    (tree_walk_action) unique_write_to_file;
+  if (tree_walk(&tree, action, (void*) this, left_root_right) ||
       insert_dynamic(&file_ptrs, (uchar*) &file_ptr))
     return 1;
   /**
@@ -699,7 +695,7 @@ bool Unique::walk(TABLE *table, tree_walk_action action, void *walk_action_arg)
                     (Merge_chunk *) file_ptrs.buffer,
                     (Merge_chunk *) file_ptrs.buffer + file_ptrs.elements,
                     action, walk_action_arg,
-                    tree.compare, tree.custom_arg, &file, with_counters,
+                    tree.compare, tree.custom_arg, &file, is_count_stored(),
                     min_dupl_count, is_variable_sized());
   }
   my_free(merge_buffer);

sql/uniques.h

@@ -237,14 +237,14 @@ class Unique : public Sql_alloc
 {
   DYNAMIC_ARRAY file_ptrs;
   /* Total number of elements that will be stored in-memory */
-  ulong max_elements;
+  uint size;
   size_t max_in_memory_size;
+  size_t max_elements;
   IO_CACHE file;
   TREE tree;
  /* Number of elements filtered out due to min_dupl_count when storing results
     to table. See Unique::get */
   ulong filtered_out_elems;
-  uint size;
 
   const uint full_size;   /* Size of element + space needed to store the number of
                              duplicates found for the element. */
@@ -252,7 +252,6 @@ class Unique : public Sql_alloc
                                 required for it to be written to
                                 record_pointers.
                                 always 0 for unions, > 0 for intersections */
-  const bool with_counters;
 
   // size in bytes used for storing keys in the Unique tree
   size_t memory_used;
@@ -297,7 +296,7 @@ class Unique : public Sql_alloc
   bool unique_add(void *ptr, uint key_size)
   {
     DBUG_ENTER("unique_add");
-    DBUG_PRINT("info", ("tree %u - %lu", tree.elements_in_tree, max_elements));
+    DBUG_PRINT("info", ("tree %u - %zu", tree.elements_in_tree, max_elements));
     TREE_ELEMENT *res;
     size_t rec_size= key_size + sizeof(TREE_ELEMENT) + tree.size_of_element;
 
@@ -378,7 +377,7 @@ class Unique : public Sql_alloc
 
   uint get_size() const { return size; }
   uint get_full_size() const { return full_size; }
-  bool is_count_stored() { return with_counters; }
+  bool is_count_stored() { return min_dupl_count; }
   int write_record_to_file(uchar *key);
   size_t get_max_in_memory_size() const { return max_in_memory_size; }