feat(pojo): add UNSPECIFIED value to AggregationPhase enum #320
Conversation
EpsilonPrime
changed the title
| @@ -940,6 +940,7 @@ public static AggregationInvocation fromProto(AggregateFunction.AggregationInvoc | |||
| } | |||
|
|
|||
| enum AggregationPhase { | |||
| UNSPECIFIED(io.substrait.proto.AggregationPhase.AGGREGATION_PHASE_UNSPECIFIED), | |||
There was a problem hiding this comment.
There are probably a lot of switches that need to be updated as appropriate. A search for INITIAL_TO_RESULT found many potential instances across Java and Scala.
There was a problem hiding this comment.
The enumerations here are mapping from the protobuf enumerations; in this case here one of the valid values in the protobuf was missing from the Java enumerations;
If a plan contained that it would fail immediately; which in the case of a plan coming from duckdb it did.
There was a problem hiding this comment.
There are probably a lot of switches that need to be updated as appropriate.
There's actually a Java warning enabled for exhaustive switch statements, which should trigger CI failures. I didn't see any pattern matching blocks in Java which would be affected.
I did find one in Scala running ./gradlew check locally. I filed #321 for the fact that it didn't fail in CI.
Signed-off-by: MBWhite <[email protected]>
mbwhite
force-pushed
the
aggregate_phase
branch
from
4b4bac5 to
116f1a7
Compare
|
@mbwhite I found one block in the Scala code missing a pattern for the UNSPECIFIED, which I've added. If my changes look good to you I can merge this in. |
vbarua
changed the title
Resolves issue #319