yadavay-amzn commented on code in PR #56417:
URL: https://github.com/apache/spark/pull/56417#discussion_r3738116299


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/SortResolver.scala:
##########
@@ -135,6 +135,14 @@ class SortResolver(operatorResolver: Resolver, 
expressionResolver: ExpressionRes
 
     val resolvedChild = operatorResolver.resolve(unresolvedSort.child)
 
+    // ORDER BY over grouping analytics (CUBE/ROLLUP/GROUPING SETS) is not yet 
supported in
+    // single-pass because the expanded Aggregate's ExprIds get out of sync 
when ORDER BY
+    // inserts missing expressions. Fall back to legacy for correct results 
(SPARK-57346).
+    if (operatorResolutionContextStack.current.hasGroupingAnalytics) {

Review Comment:
   Thanks, you're right. The flag was leaking through `pop()` past 
`isSubqueryRoot` contexts. Fixed so `hasGroupingAnalytics` stops at subquery 
roots; the outer Sort/Having guard now only sees direct aggregate lineage. 
`HavingResolver` picks up the same fix since it reads the same stack. Added 
regression tests for your ORDER BY repro and a HAVING analog.
   



-- 
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]

Reply via email to