[core] Validate data types for max/min aggregate functions - #9013
Open
jackylee-ch wants to merge 1 commit into
Open
[core] Validate data types for max/min aggregate functions#9013jackylee-ch wants to merge 1 commit into
jackylee-ch wants to merge 1 commit into
Conversation
jackylee-ch
force-pushed
the
core-minmax-agg-validation
branch
3 times, most recently
from
August 5, 2026 10:04
1fb5c22 to
3875e90
Compare
### Purpose `FieldMaxAggFactory` / `FieldMinAggFactory` create the aggregator without checking the column type. `FieldMaxAgg#agg` delegates to `InternalRowUtils.compare`, which only handles ordered types, so `fields.<f>.aggregate-function = max` on an ARRAY / MAP / MULTISET / ROW / VARIANT / BLOB / VECTOR column is accepted and only fails later during merging with `Incomparable type: ARRAY`, naming neither the field nor the function. apache#4446 moved this kind of check into the factories and apache#7485 did the same for `listagg`; max/min were never migrated. `compare` also had no BOOLEAN case, although `TypeCheckUtils.isComparable` treats BOOLEAN as comparable and the codegen comparator already implements it, so BOOLEAN is added rather than rejected. With that, the set of types the factory admits equals the set `compare` can order; a test pins the invariant. ### Tests `FieldAggregatorTest`, `InternalRowUtilsTest`.
jackylee-ch
force-pushed
the
core-minmax-agg-validation
branch
from
August 5, 2026 11:23
3875e90 to
760a5f7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
FieldMaxAggFactory/FieldMinAggFactorycreate the aggregator without checking the column type.FieldMaxAgg#aggdelegates toInternalRowUtils.compare, which only handles ordered types, sofields.<f>.aggregate-function = maxon an ARRAY / MAP / ROW / VARIANT / BLOB / VECTOR column is accepted and only fails later during merging withIncomparable type: ARRAY, naming neither the field nor the function. #4446 moved this kind of check into the factories and #7485 did the same forlistagg; max/min were never migrated.comparealso had no BOOLEAN case, althoughTypeCheckUtils.isComparabletreats BOOLEAN as comparable and the codegen comparator already implements it, so BOOLEAN is added rather than rejected. The doc lists for max/min were missing BOOLEAN, BINARY and VARBINARY.Tests
FieldAggregatorTest,InternalRowUtilsTest.