shrirangmhalgi commented on code in PR #56417:
URL: https://github.com/apache/spark/pull/56417#discussion_r3409002306
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/ExprUtils.scala:
##########
@@ -217,7 +217,14 @@ object ExprUtils extends EvalHelper with QueryErrorsBase {
}
}
- a.groupingExpressions.foreach(checkValidGroupingExprs)
+ // Skip grouping-expression validation when unexpanded BaseGroupingSets
are present:
+ // calling .dataType on them would throw. They are validated
post-expansion instead.
+ // Aggregate-expression validation (nested agg, missing group-by, etc.)
still runs.
+ val skipGroupingChecks =
Review Comment:
The `skipGroupingChecks` skip applies to all callers of
`assertValidAggregation`, not just the single-pass resolver. If a future caller
passes an unexpanded `Aggregate` without subsequently validating the expanded
form, the grouping-expression checks (type-is-orderable, no nested aggs in
grouping) are silently lost. Would it be safer to make the skip explicit via a
parameter (e.g., `skipGroupingExprChecks: Boolean = false`) so callers opt in
to the "validate post-expansion" contract?
--
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]
