peter-toth opened a new pull request, #59306: URL: https://github.com/apache/spark/pull/59306
### What changes were proposed in this pull request? `GroupPartitionsExec` now freezes the ordering its k-way merge merges by, when the merge is planned: - `sortedMergeOrdering: Option[Seq[SortOrder]]` replaces the `enableSortedMerge: Boolean` field. `enableSortedMerge` stays, as a method. - `tryEnableSortedMerge` captures the child's ordering. That is the ordering `EnsureRequirements` checks against the parent's requirement. - Every rebuild over the same child passes it on: `copy`, `withNewChildInternal` and `EnsureRequirements.withJoinKeyPositions`. Only `tryEnableSortedMerge` captures it. - `withKeyPositionsFor` turns a merging node away, since it rebuilds the node over another child. No merging node reaches it today. - For a merging node, `kWayMergeIsFeasible`, `kWayMergeOrdering` and the merge branch of `outputOrdering` read the frozen ordering instead of `child.outputOrdering`. ### Why are the changes needed? SPARK-59279 froze the merge decision, but the node still read the ordering it merges by from `child.outputOrdering`. When the source reports no ordering, the scan derives one from the partition keys under `spark.sql.sources.v2.bucketing.partitionKeyOrdering.enabled`, which it reads live. So turning that conf off after planning emptied the ordering of a planned merge. What the node did then depends on when its instance was made: - Without AQE the instance was made before the change. It still merged, by an empty ordering, so it treated every row as equal. - With AQE the instance is made when the result stage is created, after the change. It found the merge infeasible and concatenated the splits instead. Either way, a parent that relied on the merge read its rows out of order. For example, take a table partitioned by `(identity(id), days(arrive_time), identity(name))`, with `allowKeysSubsetOfPartitionKeys` and `preserveOrderingOnCoalesce` on. The scan derives the ordering `[id, name]`, while Spark sorts the splits by the full key. `ROW_NUMBER() OVER (PARTITION BY id ORDER BY name)` plans a k-way merge and no sort. With the conf turned off before the query runs, the rows of `id` 1 were numbered in split order: `[1, aa, 2]` and `[1, bb, 1]` instead of `[1, aa, 1]` and `[1, bb, 2]`. ### Does this PR introduce _any_ user-facing change? Yes. A query planned with the k-way merge keeps the merge's ordering when `spark.sql.sources.v2.bucketing.partitionKeyOrdering.enabled` changes after planning. It used to give wrong results then, such as wrong `ROW_NUMBER` values. The bug exists since 4.2.0, which has both the k-way merge (SPARK-55715) and the key-derived ordering (SPARK-56241). ### How was this patch tested? - New test in `KeyGroupedPartitioningSuite`, "SPARK-60044: a planned k-way merge keeps its ordering after a later config change". It plans the example above, turns the conf off, then checks the rows and that the merge still reports `[id, name]`, in both AQE modes. It fails without the fix with the wrong row numbers above. - New test in `EnsureRequirementsSuite`: pushing join key positions into a merging node keeps its frozen ordering. - New test in `GroupPartitionsExecSuite`: `withKeyPositionsFor` turns a merging node away. It fails without the check. - The tests that build `GroupPartitionsExec` with the merge on now pass the ordering. - Ran `GroupPartitionsExecSuite` (34), the `KeyGroupedPartitioning*` suites (233), `EnsureRequirementsSuite` (80) and `CombineAdjacentAggregationSuite` (10), plus `dev/lint-scala`. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
