n-young commented on code in PR #60090:
URL: https://github.com/apache/spark/pull/60090#discussion_r4484921517
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -4935,19 +4949,47 @@ object ResolveUnresolvedHaving extends
Rule[LogicalPlan] {
*
* Returns None unless the condition can be evaluated below every Window in
the chain.
*/
- private def insertFilterBeforeWindow(filter: Filter): Option[LogicalPlan] =
filter.child match {
- case project: Project =>
- insertFilterBeforeWindow(filter.copy(child = project.child))
+ private def insertFilterBeforeWindow(filter: Filter): Option[LogicalPlan] = {
+ if
(SQLConf.get.getConf(SQLConf.LEGACY_EVALUATE_HAVING_AFTER_INNER_WINDOW)) {
+ insertFilterBeforeOuterWindow(filter)
+ } else {
+ insertFilterBeforeAllWindows(filter, foundWindow = false)
+ }
+ }
+
+ private def insertFilterBeforeAllWindows(
+ filter: Filter,
+ foundWindow: Boolean): Option[LogicalPlan] = filter.child match {
+ case project: Project if
filter.condition.references.subsetOf(project.child.outputSet) =>
+ insertFilterBeforeAllWindows(filter.copy(child = project.child),
foundWindow)
.map(child => project.withNewChildren(Seq(child)))
- case generate: Generate =>
- insertFilterBeforeWindow(filter.copy(child = generate.child))
+ case generate: Generate if
filter.condition.references.subsetOf(generate.child.outputSet) =>
+ insertFilterBeforeAllWindows(filter.copy(child = generate.child),
foundWindow)
.map(child => generate.withNewChildren(Seq(child)))
- case window: Window =>
- insertFilterBeforeWindowChain(filter, window)
+ case window: Window if
filter.condition.references.subsetOf(window.child.outputSet) =>
+ insertFilterBeforeAllWindows(filter.copy(child = window.child),
foundWindow = true)
+ .map(child => window.withNewChildren(Seq(child)))
+ case _ if foundWindow &&
filter.condition.references.subsetOf(filter.child.outputSet) =>
Review Comment:
Fixed the all-or-none placement. `insertFilterBeforeAllWindows` now refuses
a stop point while any `Window` remains below the candidate filter. Added
generator regressions where `HAVING` references the lower window alias with
both SELECT-list orders, under both config values. In every case the filter
remains above the full window chain and `group_count` is `2`.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -3860,6 +3860,25 @@ class Analyzer(
Project(windowOps.output ++ newExpressionsWithWindowFunctions, windowOps)
} // end of addWindow
+ /**
+ * Extracts all nested window expressions and places the HAVING filter
below every Window.
+ */
+ private def addWindowWithHaving(
Review Comment:
Fixed the grouping-analytics path. With
`spark.sql.legacy.evaluateHavingAfterInnerWindow=false`,
`tryResolveHavingCondition` keeps a resolved `UnresolvedHaving` only when the
aggregate contains window outputs and the condition does not reference one of
them; `ExtractWindowExpressions` then places the filter directly above the
constructed aggregate. Conditions referencing a window output keep the previous
analysis behavior. Added ROLLUP nested-window, GROUPING SETS single-window,
window-alias negative, grouping-function, HAVING-only aggregate, and
scalar-subquery coverage, plus legacy-true cases. Default results are `rank_sum
= 1` and `group_count = 1`; legacy results remain `6` and `3`. The config doc
and migration guide now cover grouping analytics.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -3860,6 +3860,25 @@ class Analyzer(
Project(windowOps.output ++ newExpressionsWithWindowFunctions, windowOps)
} // end of addWindow
+ /**
+ * Extracts all nested window expressions and places the HAVING filter
below every Window.
Review Comment:
Updated the Scaladoc to describe both branches: the default extracts every
nested window layer before placing `HAVING`, while the legacy setting extracts
only the outermost layer here and allows inner windows to be extracted below
the filter later.
--
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]
