xumingming commented on PR #56987: URL: https://github.com/apache/spark/pull/56987#issuecomment-4905574265
@yadavay-amzn thanks for the review. I added the two coverage tests: 1. **Strict-subset child partitioning** (`GROUP BY a, b` over a join on `a` only) — confirms that `HashPartitioning(a)` satisfying `ClusteredDistribution(a, b)` is enough for the collapse. 2. **Global aggregate over a single-partition child** — confirms the `Some(Nil)` / `AllTuples` path. I did not add the computed-grouping-key case. When I tried the suggested shape (`GROUP BY a + b` over a join on `a + b`), Spark inserts a `Project` that aliases the computed expression before the partial aggregate, so the partial aggregate groups on the aliased attribute rather than the raw expression. Because the join output is partitioned on the raw expression and the aggregate groups on the alias, `EnsureRequirements` still inserts a shuffle, and the rule does not fire. So that path does not actually provide additional rule coverage under the current planner. Both new tests pass in non-AQE and AQE modes, and `sql/Test/scalastyle` is clean. Please take another look. -- 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]
