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

Take sorts into account inside aggregate functions #279

Closed
wants to merge 2 commits into from

Conversation

gabotechs
Copy link

Aggregate functions can take inner ORDER BY statements that will sort the underlaying data before the aggregation, for example:

SELECT string_agg("foo", ', ' ORDER BY "foo" DESC) FROM "tbl"

In substrait this is reflected as a sorts field inside an aggregate function's measure. This PR adds support for loading that field in ProtoTypeConverter

@CLAassistant
Copy link

CLAassistant commented Jul 3, 2024

CLA assistant check
All committers have signed the CLA.

@gabotechs
Copy link
Author

I'm unsure about how to contribute tests for this, any guidance there is welcomed

@vbarua
Copy link
Member

vbarua commented Jul 3, 2024

For testing this, I would suggest adding a sort to the aggregate roundtrip test in
https://github.com/substrait-io/substrait-java/blob/3e553eee981feb11a64b6c2fef6daf1fe377945a/core/src/test/java/io/substrait/type/proto/AggregateRoundtripTest.java

to make sure that sorts can be read from protos and output to protos.

@gabotechs
Copy link
Author

No longer working on this

@gabotechs gabotechs closed this Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants