Skip to content

Conversation

@alamb
Copy link
Contributor

@alamb alamb commented Sep 8, 2025

Which issue does this PR close?

Rationale for this change

I believe the change in #17165 is needed to prevent a regression in DataFusion 50 (as we backported a bug fix to 49, but the code had changed on master)

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

…7165)

* fix: Implement AggregateUDFImpl::reverse_expr for StringAgg

* Add a test with two invocations of aggregateion

---------

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
@alamb alamb marked this pull request as ready for review September 8, 2025 11:09
@github-actions github-actions bot added sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation labels Sep 8, 2025
Copy link
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

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

lgtm thanks @alamb
Hm, wondering why branch protection rules didn't work, it is still possible to merge without approval 🤔

@alamb
Copy link
Contributor Author

alamb commented Sep 8, 2025

lgtm thanks @alamb Hm, wondering why branch protection rules didn't work, it is still possible to merge without approval 🤔

Yeah, I don't understand either, it looks like it should be ok

datafusion/.asf.yaml

Lines 59 to 61 in b084aa4

branch-50:
required_pull_request_reviews:
required_approving_review_count: 1

@comphead
Copy link
Contributor

comphead commented Sep 8, 2025

Filed discussion https://github.com/orgs/community/discussions/172811

@alamb alamb merged commit 9e7141f into apache:branch-50 Sep 8, 2025
28 checks passed
@alamb
Copy link
Contributor Author

alamb commented Sep 8, 2025

Thanks @comphead and @nuno-faria

@alamb alamb deleted the branch-50 branch September 8, 2025 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants