jackylee-ch opened a new pull request, #9013:
URL: https://github.com/apache/paimon/pull/9013
### Purpose
`FieldMaxAggFactory` and `FieldMinAggFactory` create their aggregator
without checking the column type. `FieldMaxAgg#agg` / `FieldMinAgg#agg`
delegate to `InternalRowUtils.compare`, which only handles ordered types, so
configuring `fields.<f>.aggregate-function = max` on an ARRAY / MAP / ROW /
VARIANT / BLOB / VECTOR column is accepted and then fails during merging with
`IllegalArgumentException: Incomparable type: ARRAY` — a message that names
neither the field nor the aggregate function. This is more likely now that
multimodal tables put such columns next to scalar ones.
#4446 moved aggregate type checking into the factories precisely so these
failures surface when the function is created, and #7485 followed the same
pattern for `listagg`. max/min were never migrated; they are 2 of the 4
remaining factories with no type check (the others, `first_value` /
`last_value`, accept all types by design).
`compare` also had no BOOLEAN branch, even though a boolean column has an
obvious ordering, `TypeCheckUtils.isComparable` classifies it as comparable,
and the codegen comparator (`GenerateUtils#compareEnabledType`) already
implements it. Rejecting boolean instead of supporting it would have been a
behaviour regression for `max_pt` too, which uses the same `compare`.
### Changes
- `InternalRowUtils.compare`: add the BOOLEAN case (`false < true`),
matching the codegen comparator.
- `FieldMaxAggFactory` / `FieldMinAggFactory`: reject non-comparable types
via `TypeCheckUtils.isComparable`, in the style of `FieldSumAggFactory`.
- Update the max/min supported-type lists in `aggregation.mdx`, which
omitted BOOLEAN, BINARY and VARBINARY (BINARY/VARBINARY were already supported
by `compare`).
The failure point moves from merge time to merge-function construction, i.e.
the same point at which `sum` on a STRING column is already rejected today. No
previously-working configuration stops working.
### Tests
- `FieldAggregatorTest#testFieldMaxMinAggWithIncomparableTypeShouldFail`
covers all six non-comparable roots for both factories.
- `FieldAggregatorTest#testFieldMaxMinAggWithBooleanType` covers the new
BOOLEAN ordering through the aggregators.
- `InternalRowUtilsTest#testCompare` extended with BOOLEAN.
```
mvn -pl paimon-common -Pfast-build -Dtest=InternalRowUtilsTest test
-> 6 passed
mvn -pl paimon-core -Pfast-build -Dtest=FieldAggregatorTest test
-> 95 passed
mvn -pl paimon-core -Pfast-build -Dtest='AggregationITCase*,*Aggregat*Test'
test -> 120 passed
mvn -pl paimon-core -Pfast-build -Dtest=SchemaValidationTest test
-> 51 passed
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
