rionmonster opened a new pull request, #29667: URL: https://github.com/apache/flink/pull/29667
## What is the purpose of the change `ARRAY_AGG(x ORDER BY y)` passes validation, although the docs say ORDER BY is not supported in `ARRAY_AGG`. This was noted in the review of FLINK-21949 (#23411) and resolved there with a note in the docs. Today direct execution silently ignores the ordering, and compiling the query to a plan fails with `Symbol class 'org.apache.calcite.sql.SqlSyntax$3' has no serializable representation`. The ordered call keeps Calcite's `SqlLibraryOperators.ARRAY_AGG`, because `SqlFunction#deriveType` swaps in Flink's operator only on the copy that `ReturnTypes#stripOrderBy` creates. `ArrayAggFunction` ignores the collation that `SqlToRelConverter` derives, and `SymbolUtil` has no mapping for `SqlSyntax.ORDERED_FUNCTION`. Mapping the syntax or fixing the operator swap would only make a plan that drops the ordering serializable, so this PR rejects the clause during validation. Supporting ORDER BY in `ARRAY_AGG` is tracked separately in FLINK-41261. The check sits in `FlinkCalciteSqlValidator#performUnconditionalRewrites`, next to the existing argument checks, because that is the first place that sees the call whichever operator it carries. `STRING_AGG`, `GROUP_CONCAT` and `ARRAY_CONCAT_AGG` share Calcite's parser rule but are already rejected by Flink's parser, so only `ARRAY_AGG` needs the check. There is no feature flag. The rejected queries never returned the requested ordering, and the docs already say the clause is not supported. ## Brief change log - `FlinkCalciteSqlValidator` rejects `ARRAY_AGG` calls that carry an ORDER BY with a `ValidationException`. ## Verifying this change This change added tests and can be verified as follows: - `FlinkCalciteSqlValidatorTest#testArrayAggWithOrderBy` covers ORDER BY in `ARRAY_AGG`, combined with `DISTINCT` and `RESPECT NULLS`, and inside a subquery. All cases fail without the fix. - `FlinkCalciteSqlValidatorTest#testArrayAggWithoutOrderBy` checks that `ARRAY_AGG` without ORDER BY still validates. - Checked manually: before the fix, `compilePlanSql(...).asJsonString()` failed with the error above in streaming and batch mode, and direct execution returned arrival order. After the fix, both report the new error. `ArrayAggFunctionITCase` still passes. ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): no - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: no - The serializers: no - The runtime per-record code paths (performance sensitive): no - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no - The S3 file system connector: no ## Documentation - Does this pull request introduce a new feature? no - If yes, how is the feature documented? not applicable. The docs already say ORDER BY is not supported in `ARRAY_AGG`. ##### Was generative AI tooling used to co-author this PR? - [X] Yes (please specify the tool below) Generated-by: Claude Code 2.1.312 (Claude Opus 5.5) -- 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]
