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

Conversation

DaveGosselin-MariaDB
Copy link
Member

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.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@DaveGosselin-MariaDB DaveGosselin-MariaDB requested review from abarkov and removed request for vuvova September 20, 2024 12:02
@DaveGosselin-MariaDB DaveGosselin-MariaDB self-assigned this Oct 2, 2024
Copy link
Contributor

@abarkov abarkov left a comment

Choose a reason for hiding this comment

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

The patch looks ok for me. I only found some minor issues (see comments).

sql/item_sum.cc Outdated Show resolved Hide resolved
sql/item_sum.cc Show resolved Hide resolved
@@ -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();
}

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.

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.
@DaveGosselin-MariaDB DaveGosselin-MariaDB merged commit 2674c47 into bb-11.7-mdev-34120-gis-functions Oct 24, 2024
12 of 16 checks passed
@DaveGosselin-MariaDB DaveGosselin-MariaDB deleted the 11.7-mdev-34940 branch October 24, 2024 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants