search

substrait-io / substrait-java

Apache License 2.0
72 stars 70 forks source link

feat: enable support from/to pojo/protobuf for extended expressions #206

Closed davisusanibar closed 6 months ago

davisusanibar commented 7 months ago

To closes https://github.com/substrait-io/substrait-java/issues/127

danepitkin commented 7 months ago

Thanks, Victor, for pointing out there is still some work to do to implement AggregateFunctions. It seems like it may be best to push AggregateFunction support to a follow up issue to keep this PR small.

davisusanibar commented 7 months ago

Thanks, Victor, for pointing out there is still some work to do to implement AggregateFunctions. It seems like it may be best to push AggregateFunction support to a follow up issue to keep this PR small.

Hi Dane, just doing my last attempt to try to cover AggregateFunction ... I could suggest that it is needed to have a uniform API that won't change that dramatically later.

danepitkin commented 6 months ago

I'll let @vbarua comment on if there is more work needed for AggregateFunctions, as I'm not yet that familiar with them. If there is more work required, we should do it in a follow up PR to keep this one small. The Expression support LGTM and I wouldn't want to hold up merging it, especially if the SQL -> ExtendedExpression PR would benefit from it!

danepitkin commented 6 months ago

Thanks, @davisusanibar ! Nice work!!