Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MDEV-34940: Fix Item_func_collect inheritance #3525

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion sql/item_jsonfunc.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
69 changes: 39 additions & 30 deletions sql/item_sum.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3553,6 +3553,39 @@ 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 (fix_fields_impl(thd, ref))
return TRUE;

DaveGosselin-MariaDB marked this conversation as resolved.
Show resolved Hide resolved
if (check_sum_func(thd, ref))
return TRUE;

base_flags|= item_base_t::FIXED;
return FALSE;
}


/*****************************************************************************
GROUP_CONCAT function

Expand Down Expand Up @@ -3933,7 +3966,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),
Expand Down Expand Up @@ -3996,7 +4029,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),
Expand Down Expand Up @@ -4265,28 +4298,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))
Expand Down Expand Up @@ -4326,10 +4339,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;
}

Expand Down Expand Up @@ -4637,7 +4646,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();
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be Item_sum_str::cleanup() here.


Expand Down Expand Up @@ -4707,7 +4716,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)
Expand All @@ -4716,7 +4725,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)
Expand Down
158 changes: 89 additions & 69 deletions sql/item_sum.h
Original file line number Diff line number Diff line change
Expand Up @@ -1953,9 +1953,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;
Expand Down Expand Up @@ -2034,27 +2090,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;}
Expand All @@ -2074,56 +2111,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;
Expand All @@ -2140,27 +2165,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(") };
Expand All @@ -2173,5 +2184,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 */