MDEV-34940: Fix Item_func_collect inheritance

Creates new parent Item_sum_str for Item_func_group_concat and
Item_func_collect, migrating shared methods from each of those
latter classes to the former.  Simultaneously, tighten up the
method scopes.

sql/item_jsonfunc.h

@@ -708,7 +708,7 @@ class Item_func_json_format: public Item_json_func
 
 class Item_func_json_arrayagg : public Item_func_group_concat
 {
-protected:
+private:
   /*
     Overrides Item_func_group_concat::skip_nulls()
     NULL-s should be added to the result as JSON null value.

sql/item_sum.cc

@@ -3553,6 +3553,57 @@ String *Item_sum_udf_str::val_str(String *str)
 #endif /* HAVE_DLOPEN */
 
 
+Item_sum_str::Item_sum_str(THD *thd)
+  : Item_sum(thd)
+{
+}
+
+Item_sum_str::Item_sum_str(THD *thd, Item_sum_str *item)
+  : Item_sum(thd, item)
+{
+}
+
+Item_sum_str::Item_sum_str(THD *thd, Item *item_par)
+  : Item_sum(thd, item_par)
+{
+}
+
+
+bool
+Item_sum_str::fix_fields(THD *thd, Item **ref)
+{
+  DBUG_ASSERT(fixed() == 0);
+
+  if (init_sum_func_check(thd))
+    return TRUE;
+
+  set_maybe_null();
+
+  /*
+    Fix fields for select list and ORDER clause
+  */
+
+  for (uint i=0 ; i < arg_count ; i++)
+  {
+    if (args[i]->fix_fields_if_needed_for_scalar(thd, &args[i]))
+      return TRUE;
+    /* We should ignore FIELD's in arguments to sum functions */
+    with_flags|= (args[i]->with_flags & ~item_with_t::FIELD);
+  }
+
+  bool ffi_result= fix_fields_impl(thd, ref);
+
+  if (ffi_result)
+      return ffi_result;
+
+  if (check_sum_func(thd, ref))
+    return TRUE;
+
+  base_flags|= item_base_t::FIXED;
+  return FALSE;
+}
+
+
 /*****************************************************************************
  GROUP_CONCAT function
 
@@ -3933,7 +3984,7 @@ Item_func_group_concat(THD *thd, Name_resolution_context *context_arg,
                        const SQL_I_List<ORDER> &order_list,
                        String *separator_arg, bool limit_clause,
                        Item *row_limit_arg, Item *offset_limit_arg)
-  :Item_sum(thd), tmp_table_param(0), separator(separator_arg), tree(0),
+  :Item_sum_str(thd), tmp_table_param(0), separator(separator_arg), tree(0),
    unique_filter(NULL), table(0),
    order(0), context(context_arg),
    arg_count_order(order_list.elements),
@@ -3996,7 +4047,7 @@ Item_func_group_concat(THD *thd, Name_resolution_context *context_arg,
 
 Item_func_group_concat::Item_func_group_concat(THD *thd,
                                                Item_func_group_concat *item)
-  :Item_sum(thd, item),
+  :Item_sum_str(thd, item),
   tmp_table_param(item->tmp_table_param),
   separator(item->separator),
   tree(item->tree),
@@ -4265,28 +4316,8 @@ bool Item_func_group_concat::add(bool exclude_nulls)
 
 
 bool
-Item_func_group_concat::fix_fields(THD *thd, Item **ref)
+Item_func_group_concat::fix_fields_impl(THD *thd, Item **ref)
 {
-  uint i;                       /* for loop variable */
-  DBUG_ASSERT(fixed() == 0);
-
-  if (init_sum_func_check(thd))
-    return TRUE;
-
-  set_maybe_null();
-
-  /*
-    Fix fields for select list and ORDER clause
-  */
-
-  for (i=0 ; i < arg_count ; i++)
-  {
-    if (args[i]->fix_fields_if_needed_for_scalar(thd, &args[i]))
-      return TRUE;
-    /* We should ignore FIELD's in arguments to sum functions */
-    with_flags|= (args[i]->with_flags & ~item_with_t::FIELD);
-  }
-
   /* skip charset aggregation for order columns */
   if (agg_arg_charsets_for_string_result(collation,
                                          args, arg_count - arg_count_order))
@@ -4326,10 +4357,6 @@ Item_func_group_concat::fix_fields(THD *thd, Item **ref)
     separator= new_separator;
   }
 
-  if (check_sum_func(thd, ref))
-    return TRUE;
-
-  base_flags|= item_base_t::FIXED;
   return FALSE;
 }
 
@@ -4637,7 +4664,7 @@ void Item_func_collect::clear() {
 void Item_func_collect::cleanup() {
   List_iterator<String> geometries_iterator(geometries);
   geometries.delete_elements();
-  Item_sum_int::cleanup();
+  Item_sum::cleanup();
 }
 
 
@@ -4707,7 +4734,7 @@ bool Item_func_collect::list_contains_element(String *wkb) {
 
 
 Item_func_collect::Item_func_collect(THD *thd, bool is_distinct, Item *item_par) :
-  Item_sum_int(thd, item_par),
+  Item_sum_str(thd, item_par),
   mem_root(thd->mem_root),
   is_distinct(is_distinct),
   group_collect_max_len(thd->variables.group_concat_max_len)
@@ -4716,7 +4743,7 @@ Item_func_collect::Item_func_collect(THD *thd, bool is_distinct, Item *item_par)
 
 
 Item_func_collect::Item_func_collect(THD *thd, bool is_distinct, Item_func_collect *item) :
-  Item_sum_int(thd, item),
+  Item_sum_str(thd, item),
   mem_root(thd->mem_root),
   is_distinct(is_distinct),
   group_collect_max_len(thd->variables.group_concat_max_len)

sql/item_sum.h

@@ -1953,9 +1953,56 @@ int dump_leaf_key(void* key_arg,
                   void* item_arg);
 C_MODE_END
 
-class Item_func_group_concat : public Item_sum
+class Item_sum_str : public Item_sum
 {
+public:
+  Item_sum_str(THD *thd);
+  Item_sum_str(THD *thd, Item_sum_str *item);
+  Item_sum_str(THD *thd, Item *item_par);
+  bool fix_fields(THD *, Item **) override;
+
+  longlong val_int() override
+  {
+    String *res;
+    char *end_ptr;
+    int error;
+    if (!(res= val_str(&str_value)))
+      return (longlong) 0;
+    end_ptr= (char*) res->ptr()+ res->length();
+    return my_strtoll10(res->ptr(), &end_ptr, &error);
+  }
+
+  double val_real() override
+  {
+    int error;
+    const char *end;
+    String *res;
+    if (!(res= val_str(&str_value)))
+      return 0.0;
+    end= res->ptr() + res->length();
+    return (my_strtod(res->ptr(), (char**) &end, &error));
+  }
+
+  my_decimal *val_decimal(my_decimal *decimal_value) override
+  {
+    return val_decimal_from_string(decimal_value);
+  }
+
+  bool get_date(THD *thd, MYSQL_TIME *ltime, date_mode_t fuzzydate) override
+  {
+    return get_date_from_string(thd, ltime, fuzzydate);
+  }
+
+  void no_rows_in_result() override {}
+  void reset_field() override { DBUG_ASSERT(0); }        // not used
+  void update_field() override { DBUG_ASSERT(0); }       // not used
+
 protected:
+  virtual bool fix_fields_impl(THD *, Item **) = 0;
+};
+
+class Item_func_group_concat : public Item_sum_str
+{
   TMP_TABLE_PARAM *tmp_table_param;
   String result;
   String *separator;
@@ -2034,27 +2081,8 @@ class Item_func_group_concat : public Item_sum
   virtual String *get_str_from_field(Item *i, Field *f, String *tmp,
                                      const uchar *key, size_t offset)
     { return f->val_str(tmp, key + offset); }
-  virtual void cut_max_length(String *result,
-                              uint old_length, uint max_length) const;
   bool uses_non_standard_aggregator_for_distinct() const override
     { return distinct; }
-
-public:
-  // Methods used by ColumnStore
-  bool get_distinct() const { return distinct; }
-  uint get_count_field() const { return arg_count_field; }
-  uint get_order_field() const { return arg_count_order; }
-  const String* get_separator() const { return separator; }
-  ORDER** get_order() const { return order; }
-
-public:
-  Item_func_group_concat(THD *thd, Name_resolution_context *context_arg,
-                         bool is_distinct, List<Item> *is_select,
-                         const SQL_I_List<ORDER> &is_order, String *is_separator,
-                         bool limit_clause, Item *row_limit, Item *offset_limit);
-
-  Item_func_group_concat(THD *thd, Item_func_group_concat *item);
-  ~Item_func_group_concat();
   void cleanup() override;
 
   enum Sumfunctype sum_func () const override {return GROUP_CONCAT_FUNC;}
@@ -2074,56 +2102,44 @@ class Item_func_group_concat : public Item_sum
   {
     return add(skip_nulls());
   }
-  void reset_field() override { DBUG_ASSERT(0); }        // not used
-  void update_field() override { DBUG_ASSERT(0); }       // not used
-  bool fix_fields(THD *,Item **) override;
+  bool fix_fields_impl(THD *,Item **) override;
   bool setup(THD *thd) override;
   void make_unique() override;
-  double val_real() override
-  {
-    int error;
-    const char *end;
-    String *res;
-    if (!(res= val_str(&str_value)))
-      return 0.0;
-    end= res->ptr() + res->length();
-    return (my_strtod(res->ptr(), (char**) &end, &error));
-  }
-  longlong val_int() override
-  {
-    String *res;
-    char *end_ptr;
-    int error;
-    if (!(res= val_str(&str_value)))
-      return (longlong) 0;
-    end_ptr= (char*) res->ptr()+ res->length();
-    return my_strtoll10(res->ptr(), &end_ptr, &error);
-  }
-  my_decimal *val_decimal(my_decimal *decimal_value) override
-  {
-    return val_decimal_from_string(decimal_value);
-  }
-  bool get_date(THD *thd, MYSQL_TIME *ltime, date_mode_t fuzzydate) override
-  {
-    return get_date_from_string(thd, ltime, fuzzydate);
-  }
-  String *val_str(String *str) override;
   Item *copy_or_same(THD* thd) override;
-  void no_rows_in_result() override {}
   void print(String *str, enum_query_type query_type) override;
   bool change_context_processor(void *cntx) override
     { context= (Name_resolution_context *)cntx; return FALSE; }
   Item *do_get_copy(THD *thd) const override
   { return get_item_copy<Item_func_group_concat>(thd, this); }
+
+protected:
+  virtual void cut_max_length(String *result,
+                              uint old_length, uint max_length) const;
+  String *val_str(String *str) override;
+
+public:
+  // Methods used by ColumnStore
+  bool get_distinct() const { return distinct; }
+  uint get_count_field() const { return arg_count_field; }
+  uint get_order_field() const { return arg_count_order; }
+  const String* get_separator() const { return separator; }
+  ORDER** get_order() const { return order; }
+
+  Item_func_group_concat(THD *thd, Name_resolution_context *context_arg,
+                         bool is_distinct, List<Item> *is_select,
+                         const SQL_I_List<ORDER> &is_order, String *is_separator,
+                         bool limit_clause, Item *row_limit, Item *offset_limit);
+
+  Item_func_group_concat(THD *thd, Item_func_group_concat *item);
+  ~Item_func_group_concat();
   qsort_cmp2 get_comparator_function_for_distinct();
   qsort_cmp2 get_comparator_function_for_order_by();
   uchar* get_record_pointer();
   uint get_null_bytes();
-
 };
 
 
-class Item_func_collect :public Item_sum_int // XXX why *int* ???
+class Item_func_collect : public Item_sum_str
 {
   uint32 srid;
   bool has_cached_result;
@@ -2140,27 +2156,13 @@ class Item_func_collect :public Item_sum_int // XXX why *int* ???
   void remove() override;
   bool list_contains_element(String* wkb);
 
-public:
-  Item_func_collect(THD *thd, bool is_distinct, Item *item_par);
-  Item_func_collect(THD *thd, bool is_distinct, Item_func_collect *item);
-
-  bool fix_length_and_dec(THD *thd) override
-  {
-    Item_sum_int::fix_length_and_dec(thd);
-    base_flags|= item_base_t::MAYBE_NULL;
-    return false;
-  }
   enum Sumfunctype sum_func () const override
   {
     return GEOMETRY_COLLECT_FUNC;
   }
-  void no_rows_in_result() override {; }
   const Type_handler *type_handler() const override
   { return &type_handler_string; }
-  longlong val_int() override { return 0; }
   String *val_str(String*str) override;
-  void reset_field() override {DBUG_ASSERT(0);}
-  void update_field() override {DBUG_ASSERT(0);}
   LEX_CSTRING func_name_cstring() const override
   {
     return { STRING_WITH_LEN("st_collect(") };
@@ -2173,5 +2175,14 @@ class Item_func_collect :public Item_sum_int // XXX why *int* ???
   {
     return true;
   }
+
+  bool fix_fields_impl(THD *,Item **) override
+  {
+    return FALSE;
+  }
+
+public:
+  Item_func_collect(THD *thd, bool is_distinct, Item *item_par);
+  Item_func_collect(THD *thd, bool is_distinct, Item_func_collect *item);
 };
 #endif /* ITEM_SUM_INCLUDED */