substrait-io / substrait-java

Apache License 2.0
77 stars 72 forks source link

Take `sorts` into account inside aggregate functions #279

Closed gabotechs closed 1 month ago

gabotechs commented 4 months ago

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 commented 4 months ago

CLA assistant check
All committers have signed the CLA.

gabotechs commented 4 months ago

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

vbarua commented 4 months ago

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 commented 1 month ago

No longer working on this