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.

Author: DaveGosselin-MariaDB

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

@@ -3521,6 +3521,42 @@ String *Item_sum_udf_str::val_str(String *str)
 #endif /* HAVE_DLOPEN */
 
 
+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);
+if (args[i]->check_type_can_return_str(
+ Item_func_group_concat::func_name_cstring()))
+ return true;
+ }
+
+ if (fix_fields_impl(thd, ref))
+ return TRUE;
+
+ if (check_sum_func(thd, ref))
+ return TRUE;
+
+ base_flags|= item_base_t::FIXED;
+ return FALSE;
+}
+
+
 /*****************************************************************************
  GROUP_CONCAT function
 
@@ -3900,7 +3936,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),
@@ -3963,7 +3999,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),
@@ -4232,31 +4268,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);
- if (args[i]->check_type_can_return_str(
- Item_func_group_concat::func_name_cstring()))
- return true;
- }
-
  /* skip charset aggregation for order columns */
  if (agg_arg_charsets_for_string_result(collation,
  args, arg_count - arg_count_order))
@@ -4296,10 +4309,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;
 }
 
@@ -4609,7 +4618,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_str::cleanup();
 }
 
 
@@ -4679,7 +4688,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)
@@ -4688,7 +4697,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

@@ -1948,9 +1948,65 @@ 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(thd)
+ {}
+
+ Item_sum_str(THD *thd, Item_sum_str *item)
+ : Item_sum(thd, item)
+ {}
+
+ Item_sum_str(THD *thd, Item *item_par)
+ : Item_sum(thd, 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;
@@ -2030,27 +2086,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;}
@@ -2070,56 +2107,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;
@@ -2136,27 +2161,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(") };
@@ -2169,5 +2180,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 */