More improvements

sql/item_sum.cc

@@ -878,7 +878,8 @@ bool Aggregator_distinct::setup(THD *thd)
         but this has to be handled - otherwise someone can crash
         the server with a DoS attack
       */
-      if (!tree || (tree->get_descriptor()->setup(thd, item_sum,
+      if (!tree ||
+          (tree->get_descriptor()->setup_for_item(thd, item_sum,
                                                   non_const_items,
                                                   item_sum->get_arg_count())))
         return TRUE;
@@ -4576,9 +4577,9 @@ bool Item_func_group_concat::setup(THD *thd)
                              non_const_items);
 
     if (!unique_filter ||
-        (unique_filter->get_descriptor()->setup(thd, this,
-                                                non_const_items,
-                                                arg_count_field)))
+        (unique_filter->get_descriptor()->setup_for_item(thd, this,
+                                                         non_const_items,
+                                                         arg_count_field)))
       DBUG_RETURN(TRUE);
   }
   if ((row_limit && row_limit->cmp_type() != INT_RESULT) ||
@@ -4818,8 +4819,6 @@ Item_sum::get_unique(qsort_cmp2 comp_func, void *comp_func_fixed_arg,
       desc= new Variable_size_keys_simple(size_arg);
     else
       desc= new Variable_size_composite_key_desc(size_arg);
-    if (!desc || desc->init())
-      return NULL;
   }
   else
   {
@@ -4827,9 +4826,10 @@ Item_sum::get_unique(qsort_cmp2 comp_func, void *comp_func_fixed_arg,
       desc= new Fixed_size_keys_descriptor(size_arg);
     else
       desc= new Fixed_size_composite_keys_descriptor(size_arg);
-    if (!desc || desc->init())
-      return NULL;
   }
+
+  if (!desc)
+    return NULL;
   return new Unique_impl(comp_func, comp_func_fixed_arg, size_arg,
                          max_in_memory_size_arg, min_dupl_count_arg, desc);
 }
@@ -4851,9 +4851,7 @@ Item_func_group_concat::get_unique(qsort_cmp2 comp_func,
     if (number_of_args == 1)
       desc= new Variable_size_keys_simple(size_arg);
     else
-      desc= new Variable_size_composite_key_desc(size_arg);
-    if (!desc || desc->init())
-      return NULL;
+      desc= new Variable_size_composite_key_desc_for_gconcat(size_arg);
   }
   else
   {
@@ -4861,10 +4859,11 @@ Item_func_group_concat::get_unique(qsort_cmp2 comp_func,
       desc= new Fixed_size_keys_descriptor_with_nulls(size_arg);
     else
       desc= new Fixed_size_keys_for_group_concat(size_arg);
-    if (!desc)
-      return NULL;
   }
 
+  if (!desc)
+    return NULL;
+
   return new Unique_impl(comp_func, comp_func_fixed_arg, size_arg,
                          max_in_memory_size_arg, min_dupl_count_arg, desc);
 }
@@ -4934,7 +4933,7 @@ Item_func_group_concat::~Item_func_group_concat()
 */
 int Item_func_group_concat::insert_record_to_unique()
 {
-  if (unique_filter->is_variable_sized())
+  if (unique_filter->is_variable_sized() && unique_filter->is_single_arg())
   {
     uint packed_length;
     if ((packed_length= unique_filter->get_descriptor()->
@@ -4960,5 +4959,17 @@ int Item_func_group_concat::insert_record_to_unique()
     key_length used to initialize the tree didn't include space for them.
   */
 
+  if (unique_filter->is_variable_sized())
+  {
+    DBUG_ASSERT(!unique_filter->is_single_arg());
+    uint packed_length;
+    if ((packed_length= unique_filter->get_descriptor()->
+                        make_record(skip_nulls())) == 0)
+      return -1; // NULL value
+    DBUG_ASSERT(packed_length <= unique_filter->get_size());
+    return unique_filter->unique_add(unique_filter->get_descriptor()
+                                     ->get_rec_ptr());
+  }
+
   return unique_filter->unique_add(get_record_pointer());
 }

sql/sql_statistics.cc

@@ -1703,6 +1703,22 @@ class Count_distinct_field: public Sql_alloc
   }
 
 
+  /*
+    @brief
+      Calculate the max length to store the length of a packable field
+
+    @param
+      field               Field structure
+
+  */
+  uint compute_packable_length(Field *field)
+  {
+    return table_field->max_packed_col_length(table_field->pack_length()) +
+           Variable_size_keys_descriptor::size_of_length_field +
+           MY_TEST(table_field->maybe_null());
+  }
+
+
   /*
     @brief
       Create and setup the Unique object for the column
@@ -1716,33 +1732,22 @@ class Count_distinct_field: public Sql_alloc
     Descriptor *desc;
     if (table_field->is_packable())
     {
-      tree_key_length= table_field->
-                       max_packed_col_length(table_field->pack_length());
-
-      tree_key_length+= Variable_size_keys_descriptor::size_of_length_field;
-      tree_key_length+= MY_TEST(table_field->maybe_null());
-
+      tree_key_length= compute_packable_length(table_field);
       desc= new Variable_size_keys_simple(tree_key_length);
-      if (!desc || desc->init())
-        return true; // OOM
-      tree= new Unique_impl((qsort_cmp2) key_cmp,
-                            (void*) this, tree_key_length,
-                             max_heap_table_size, 1, desc);
-      if (!tree)
-        return true; // OOM
-      return tree->get_descriptor()->setup(thd, table_field);
     }
-
-    tree_key_length= table_field->pack_length();
-    desc= new Fixed_size_keys_descriptor(tree_key_length);
+    else
+    {
+      tree_key_length= table_field->pack_length();
+      desc= new Fixed_size_keys_descriptor(tree_key_length);
+    }
     if (!desc)
       return true;  // OOM
-    tree= new Unique_impl((qsort_cmp2) key_cmp, (void*) this,
-                          tree_key_length, max_heap_table_size, 1, desc);
+    tree= new Unique_impl((qsort_cmp2) key_cmp,
+                          (void*) this, tree_key_length,
+                           max_heap_table_size, 1, desc);
     if (!tree)
       return true; // OOM
-
-    return tree->get_descriptor()->setup(thd, table_field);
+    return tree->get_descriptor()->setup_for_field(thd, table_field);
   }
 
 

sql/uniques.cc

@@ -876,6 +876,9 @@ int Unique_impl::write_record_to_file(uchar *key)
 }
 
 
+/*                VARIABLE SIZE KEYS DESCRIPTOR                             */
+
+
 Variable_size_keys_descriptor::Variable_size_keys_descriptor(uint length)
 {
   max_length= length;
@@ -884,13 +887,16 @@ Variable_size_keys_descriptor::Variable_size_keys_descriptor(uint length)
   sortorder= NULL;
 }
 
-Variable_size_keys_descriptor::~Variable_size_keys_descriptor()
+
+Variable_size_composite_key_desc::Variable_size_composite_key_desc(uint length)
+   : Variable_size_keys_descriptor(length), Encode_record()
 {
 }
 
 
-Variable_size_composite_key_desc::Variable_size_composite_key_desc(uint length)
-   : Variable_size_keys_descriptor(length), Encode_record()
+Variable_size_composite_key_desc_for_gconcat::
+Variable_size_composite_key_desc_for_gconcat(uint length)
+   : Variable_size_keys_descriptor(length), Encode_record_for_group_concat()
 {
 }
 
@@ -920,12 +926,13 @@ Variable_size_keys_simple::Variable_size_keys_simple(uint length)
 */
 
 bool
-Variable_size_keys_descriptor::setup(THD *thd, Item_sum *item,
-                                     uint non_const_args, uint arg_count)
+Variable_size_keys_descriptor::setup_for_item(THD *thd, Item_sum *item,
+                                              uint non_const_args,
+                                              uint arg_count)
 {
   SORT_FIELD *sort,*pos;
-  if (sortorder)
-    return false;
+  if (init())
+    return true;
   DBUG_ASSERT(sort_keys == NULL);
   sortorder= (SORT_FIELD*) thd->alloc(sizeof(SORT_FIELD) * non_const_args);
   pos= sort= sortorder;
@@ -966,11 +973,11 @@ Variable_size_keys_descriptor::setup(THD *thd, Item_sum *item,
     FALSE setup successful
 */
 
-bool Variable_size_keys_descriptor::setup(THD *thd, Field *field)
+bool Variable_size_keys_descriptor::setup_for_field(THD *thd, Field *field)
 {
   SORT_FIELD *sort,*pos;
-  if (sortorder)
-    return false;
+  if (init())
+    return true;
 
   DBUG_ASSERT(sort_keys == NULL);
   sortorder= (SORT_FIELD*) thd->alloc(sizeof(SORT_FIELD));
@@ -1024,11 +1031,55 @@ int Variable_size_composite_key_desc::compare_keys(uchar *a_ptr,
 }
 
 
+int Variable_size_composite_key_desc_for_gconcat::compare_keys(uchar *a_ptr,
+                                                               uchar *b_ptr)
+{
+  uchar *a= a_ptr + Variable_size_keys_descriptor::size_of_length_field;
+  uchar *b= b_ptr + Variable_size_keys_descriptor::size_of_length_field;
+  int retval= 0;
+  size_t a_len, b_len;
+  for (SORT_FIELD *sort_field= sort_keys->begin();
+       sort_field != sort_keys->end(); sort_field++)
+  {
+    if (sort_field->is_variable_sized())
+    {
+      retval= sort_field->compare_packed_varstrings(a, &a_len, b, &b_len);
+      a+= a_len;
+      b+= b_len;
+    }
+    else
+    {
+      DBUG_ASSERT(sort_field->field);
+      retval= sort_field->field->cmp(a, b);
+      a+= sort_field->length;
+      b+= sort_field->length;
+    }
+
+    if (retval)
+      return sort_field->reverse ? -retval : retval;
+  }
+  return retval;
+}
+
+
+int Variable_size_keys_simple::compare_keys(uchar *a, uchar *b)
+{
+  return sort_keys->compare_keys_for_single_arg(a + size_of_length_field,
+                                                b + size_of_length_field);
+}
+
+
+
 uint Variable_size_composite_key_desc::make_record(bool exclude_nulls)
 {
   return make_encoded_record(sort_keys, exclude_nulls);
 }
 
+uint Variable_size_composite_key_desc_for_gconcat::make_record(bool exclude_nulls)
+{
+  return make_encoded_record(sort_keys, exclude_nulls);
+}
+
 
 uint Variable_size_keys_simple::make_record(bool exclude_nulls)
 {
@@ -1042,16 +1093,47 @@ bool Variable_size_composite_key_desc::init()
 }
 
 
+bool Variable_size_composite_key_desc_for_gconcat::init()
+{
+  return Encode_record::init(max_length);
+}
+
+
 bool Variable_size_keys_simple::init()
 {
   return Encode_record::init(max_length);
 }
 
 
-int Variable_size_keys_simple::compare_keys(uchar *a, uchar *b)
+bool
+Variable_size_composite_key_desc_for_gconcat::setup_for_item(THD *thd,
+                                                          Item_sum *item,
+                                                          uint non_const_args,
+                                                          uint arg_count)
 {
-  return sort_keys->compare_keys_for_single_arg(a + size_of_length_field,
-                                                b + size_of_length_field);
+  SORT_FIELD *sort,*pos;
+  if (init())
+    return true;
+  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;
+
+    Field *field= arg->get_tmp_table_field();
+    pos->setup(field, false);
+    pos++;
+  }
+  return false;
 }
 
 
@@ -1077,8 +1159,9 @@ int Fixed_size_keys_descriptor::compare_keys(uchar *a, uchar *b)
 
 
 bool
-Fixed_size_keys_descriptor::setup(THD *thd, Item_sum *item,
-                                  uint non_const_args, uint arg_count)
+Fixed_size_keys_descriptor::setup_for_item(THD *thd, Item_sum *item,
+                                           uint non_const_args,
+                                           uint arg_count)
 {
   SORT_FIELD *sort,*pos;
   if (sortorder)
@@ -1109,7 +1192,7 @@ Fixed_size_keys_descriptor::setup(THD *thd, Item_sum *item,
 
 
 bool
-Fixed_size_keys_descriptor::setup(THD *thd, Field *field)
+Fixed_size_keys_descriptor::setup_for_field(THD *thd, Field *field)
 {
   SORT_FIELD *sort,*pos;
   if (sortorder)
@@ -1282,16 +1365,43 @@ uint Encode_record::make_encoded_record(Sort_keys *sort_keys,
 
 
 uint
-Encode_record_for_count_distinct::make_encoded_record(Sort_keys *sort_keys,
-                                                      bool exclude_nulls)
+Encode_record_for_group_concat::make_encoded_record(Sort_keys *sort_keys,
+                                                    bool exclude_nulls)
 {
-  return 0;
+  Field *field;
+  SORT_FIELD *sort_field;
+  uint length;
+  uchar *orig_to, *to;
+
+  orig_to= to= rec_ptr;
+  to+= Variable_size_keys_descriptor::size_of_length_field;
+
+  for (sort_field=sort_keys->begin() ;
+       sort_field != sort_keys->end() ;
+       sort_field++)
+  {
+    bool maybe_null=0;
+    DBUG_ASSERT(sort_field->field);
+    field=sort_field->field;
+    length= field->make_packed_key_part(to, sort_field);
+
+    if ((maybe_null= sort_field->maybe_null))
+    {
+      if (exclude_nulls && length == 0)  // rejecting NULLS
+        return 0;
+      to++;
+    }
+    to+= length;
+  }
+
+  length= static_cast<uint>(to - orig_to);
+  Variable_size_keys_descriptor::store_packed_length(orig_to, length);
+  return length;
 }
 
 
-uint
-Encode_record_for_group_concat::make_encoded_record(Sort_keys *sort_keys,
-                                                    bool exclude_nulls)
+bool Descriptor::is_single_arg()
 {
-  return 0;
+  DBUG_ASSERT(sort_keys);
+  return !(sort_keys->size() > 1) ;
 }
\ No newline at end of file

sql/uniques.h

@@ -58,16 +58,17 @@ class Descriptor : public Sql_alloc
   virtual int compare_keys(uchar *a, uchar *b) = 0;
 
   // Fill structures like sort_keys, sortorder
-  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 bool setup_for_item(THD *thd, Item_sum *item,
+                              uint non_const_args, uint arg_count)
+  { 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; }
 
   /* need to be moved to a separate class */
   virtual uchar *get_rec_ptr() { return NULL; }
   virtual uint make_record(bool exclude_nulls) { return 0; }
-  virtual bool init() { return false; }
+  virtual bool is_single_arg();
 };
 
 
@@ -81,9 +82,9 @@ class Fixed_size_keys_descriptor : public Descriptor
   Fixed_size_keys_descriptor(uint length);
   virtual ~Fixed_size_keys_descriptor() {}
   uint get_length_of_key(uchar *ptr) override { return max_length; }
-  bool setup(THD *thd, Field *field);
-  bool setup(THD *thd, Item_sum *item,
-             uint non_const_args, uint arg_count);
+  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;
 };
 
@@ -162,7 +163,7 @@ class Fixed_size_composite_keys_descriptor : public Fixed_size_keys_descriptor
 };
 
 
-class Encode_record : public Sql_alloc
+class Encode_record
 {
 protected:
   /*
@@ -181,16 +182,6 @@ class Encode_record : public Sql_alloc
 };
 
 
-class Encode_record_for_count_distinct : public Encode_record
-{
-
-public:
-  Encode_record_for_count_distinct() : Encode_record() {}
-  ~Encode_record_for_count_distinct() {}
-  uint make_encoded_record(Sort_keys *keys, bool exclude_nulls) override;
-};
-
-
 class Encode_record_for_group_concat : public Encode_record
 {
 public:
@@ -200,21 +191,26 @@ class Encode_record_for_group_concat : public Encode_record
 };
 
 
+/*
+  Descriptor for variable size keys
+*/
 
 class Variable_size_keys_descriptor : public Descriptor
 {
 public:
   Variable_size_keys_descriptor(uint length);
-  virtual ~Variable_size_keys_descriptor();
+  virtual ~Variable_size_keys_descriptor() {}
   uint get_length_of_key(uchar *ptr) override
   {
     return read_packed_length(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_keys(uchar *a, uchar *b) override { return 0; }
+  virtual bool init() { return false; }
+  bool setup_for_item(THD *thd, Item_sum *item,
+                      uint non_const_args, uint arg_count) override;
+  bool setup_for_field(THD *thd, Field *field) override;
   // All need to be moved to some new class
   // returns the length of the key along with the length bytes for the key
   static uint read_packed_length(uchar *p)
@@ -230,33 +226,55 @@ class Variable_size_keys_descriptor : public Descriptor
 
 
 /*
-  Descriptor for variable size keys
+
+  Descriptor for variable size keys with only one component
+
+  Used by EITS, JSON_ARRAYAGG,
+  COUNT(DISTINCT col1) AND GROUP_CONCAT(DISTINCT col1) => only one item is allowed
+
+*/
+
+class Variable_size_keys_simple : public Variable_size_keys_descriptor,
+                                  public Encode_record
+{
+public:
+  Variable_size_keys_simple(uint length);
+  virtual ~Variable_size_keys_simple() {}
+  int compare_keys(uchar *a, uchar *b) override;
+  uint make_record(bool exclude_nulls) override;
+  uchar* get_rec_ptr() { return rec_ptr; }
+  bool init() override;
+};
+
+
+/*
+  Descriptor for variable sized keys with multiple key parts
 */
 class Variable_size_composite_key_desc : public Variable_size_keys_descriptor,
                                          public Encode_record
 {
 public:
   Variable_size_composite_key_desc(uint length);
-  ~Variable_size_composite_key_desc() {}
+  virtual ~Variable_size_composite_key_desc() {}
   int compare_keys(uchar *a, uchar *b) override;
   uint make_record(bool exclude_nulls) override;
-  bool init() override;
   uchar* get_rec_ptr() { return rec_ptr; }
+  bool init() override;
 };
 
 
-/* Descriptor for variable size keys with only one component */
-
-class Variable_size_keys_simple : public Variable_size_keys_descriptor,
-                                  public Encode_record
+class Variable_size_composite_key_desc_for_gconcat :
+  public Variable_size_keys_descriptor , public Encode_record_for_group_concat
 {
 public:
-  Variable_size_keys_simple(uint length);
-  ~Variable_size_keys_simple() {}
+  Variable_size_composite_key_desc_for_gconcat(uint length);
+  virtual ~Variable_size_composite_key_desc_for_gconcat() {}
   int compare_keys(uchar *a, uchar *b) override;
   uint make_record(bool exclude_nulls) override;
-  bool init() override;
   uchar* get_rec_ptr() { return rec_ptr; }
+  bool init() override;
+  bool setup_for_item(THD *thd, Item_sum *item,
+                      uint non_const_args, uint arg_count) override;
 };
 
 
@@ -438,6 +456,7 @@ class Unique_impl : public Unique {
 
   // returns TRUE if the unique tree stores packed values
   bool is_variable_sized() { return m_descriptor->is_variable_sized(); }
+  bool is_single_arg() { return m_descriptor->is_single_arg(); }
   Descriptor* get_descriptor() { return m_descriptor; }
 
   friend int unique_write_to_file(uchar* key, element_count count, Unique_impl *unique);