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

AggregationPhase: AGGREGATION_PHASE_UNSPECIFIED not understood substrait-java #319

Open
mbwhite opened this issue Nov 29, 2024 · 4 comments

Comments

@mbwhite
Copy link
Contributor

mbwhite commented Nov 29, 2024

The protobuf defines the AGGREGATION_PHASE_UNSPECIFIED but does say it implies INTERMEDIATE_TO_RESULT

https://github.com/substrait-io/substrait/blob/55683fb1078bd50b9fe711a8e0084910f34199e6/proto/substrait/algebra.proto#L1582

substrait-java treats this as unknown, when it isn't really. (in fact duckdb produces this)

testing fix for this locally

@EpsilonPrime
Copy link
Member

AGGREGATION_PHASE_UNSPECIFIED is the default value. So if the field is not set consumers will encounter it (although one could argue that it should always be set). As stated above, consumers should likely deal with it in some manner.

@mbwhite
Copy link
Contributor Author

mbwhite commented Dec 3, 2024

Just to clairfy - that the substrait-java throws an exception if it encounters this field; this is part that is being fixed.

Up to consumers as you say to handle the actually meaning.

@Blizzara
Copy link
Contributor

Hmm, given the proto docs say

// Implies `INTERMEDIATE_TO_RESULT`.

does it make sense to let consumers handle it, or should we just treat it as INTERMEDIATE_TO_RESULT directly in substrait-java?

@mbwhite
Copy link
Contributor Author

mbwhite commented Dec 17, 2024

@Blizzara the pr just enables the the value to be correctly returned to the user to do as they wish.

The issue was that an exception was thrown;

The PR has approval and is waiting for merge

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

No branches or pull requests

3 participants