ulysses-you commented on PR #57221: URL: https://github.com/apache/spark/pull/57221#issuecomment-4976091250
Thanks @viirya, both addressed in 6db19e30e75: 1. **You're right, the `EliminateSorts` justification doesn't hold** -- I verified it end-to-end. `EliminateSorts` only recurses through `Sort`/`Join`/order-irrelevant-`Aggregate` parents, and `CollapseRepartition` removes only a global sort under `RepartitionByExpression` (or any sort under `RebalancePartitions`), never a sort under a round-robin `Repartition`. So `SELECT /*+ REPARTITION(3) */ * FROM (SELECT key FROM t SORT BY key)` does survive to the physical plan as `Exchange RoundRobinPartitioning(3) <- Sort(local)` and reaches the new branch. One caveat: the source must be unsorted, otherwise the local sort is elided as redundant before it can dangle (with `spark.range` the source is already ordered by `id`, so no sort materializes). I replaced the direct-invocation local-sort case with a `checkSorts`-style end-to-end test over an unsorted `toDF` source (exercises the rule's real position in both prep chains), kept the direct-invocation test for the global-sort-preservation case, and fixed the incorrect comment. 2. **Reworded the `QueryExecution.preparations` comment** to state that running `RemoveRedundantSorts` after `DisableUnnecessaryBucketedScan` is a correctness requirement (disabling a bucketed scan drops its output ordering, which must be reflected before sort-removal decisions), with the AQE-order consistency demoted to a parenthetical so a future reorder doesn't look harmless. Both SPARK-58099 tests pass in the AE and non-AE variants. -- 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]
