You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "jackylee-ch (via GitHub)" <gi...@apache.org> on 2026/08/04 05:43:02 UTC

[PR] [core] Validate data types for max/min aggregate functions [paimon]

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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [core] Validate data types for max/min aggregate functions [paimon]

Posted by "jackylee-ch (via GitHub)" <gi...@apache.org>.
jackylee-ch commented on PR #9013:
URL: https://github.com/apache/paimon/pull/9013#issuecomment-5190981832

   It seems upstream has spotless problem, I have create a PR #9049 to fix it.


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [core] Validate data types for max/min aggregate functions [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #9013:
URL: https://github.com/apache/paimon/pull/9013


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [core] Validate data types for max/min aggregate functions [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #9013:
URL: https://github.com/apache/paimon/pull/9013#issuecomment-5212732292

   +1


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org