You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by ur...@apache.org on 2026/08/02 22:02:50 UTC

(spark) branch branch-4.x updated: [SPARK-58503][SQL] Evaluate HAVING before window functions with generators

This is an automated email from the ASF dual-hosted git repository.

uros-b pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-4.x by this push:
     new 69b205825c9d [SPARK-58503][SQL] Evaluate HAVING before window functions with generators
69b205825c9d is described below

commit 69b205825c9d1f729945091fb1d585e7bc5ac670
Author: Vladimir Golubev <vl...@databricks.com>
AuthorDate: Sun Aug 2 22:02:27 2026 +0000

    [SPARK-58503][SQL] Evaluate HAVING before window functions with generators
    
    ### What changes were proposed in this pull request?
    
    Place a resolved HAVING filter below the complete Window chain when generator extraction has
    inserted Project and Generate operators above it. If the condition's attributes are not available
    below every Window, retain the existing fallback placement.
    
    Add regression coverage for HAVING conditions on grouping columns and aggregate aliases, including
    multiple window specifications, and document the corrected behavior in the SQL migration guide.
    
    ### Why are the changes needed?
    
    Generator extraction can hide the Window chain from the analyzer's normal HAVING handling. The
    remaining fallback then places HAVING above Window, so window functions see groups that HAVING
    removes.
    
    ### Does this PR introduce _any_ user-facing change?
    
    Yes. Window functions are now evaluated after HAVING for queries that also select generators. For
    example, the window count from this query changes from 4 to the correct value 3:
    
    ```sql
    SELECT explode(array(a)), count(*) OVER (), a
    FROM VALUES (1), (2), (3), (NULL) AS t(a)
    GROUP BY a
    HAVING a IS NOT NULL;
    ```
    
    ### How was this patch tested?
    
    Added SQL execution and analyzer golden coverage and ran:
    
    ```
    build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z generators-resolution-edge-cases.sql"
    ```
    
    ### Was this patch authored or co-authored using generative AI tooling?
    
    Generated-by: OpenAI Codex (GPT-5)
    
    Closes #57704 from vladimirg-db/fix-having-before-window.
    
    Authored-by: Vladimir Golubev <vl...@databricks.com>
    Signed-off-by: Uros Bojanic <22...@users.noreply.github.com>
    (cherry picked from commit 22bffbbf0b7a1a15c3aec8b79dffce2f0afd87fa)
    Signed-off-by: Uros Bojanic <22...@users.noreply.github.com>
---
 docs/sql-migration-guide.md                        |  1 +
 .../spark/sql/catalyst/analysis/Analyzer.scala     | 45 +++++++++++++++-
 .../generators-resolution-edge-cases.sql.out       | 60 ++++++++++++++++++++++
 .../inputs/generators-resolution-edge-cases.sql    | 26 ++++++++++
 .../generators-resolution-edge-cases.sql.out       | 46 +++++++++++++++++
 5 files changed, 177 insertions(+), 1 deletion(-)

diff --git a/docs/sql-migration-guide.md b/docs/sql-migration-guide.md
index 62cefee4f62f..c04d20f44d5e 100644
--- a/docs/sql-migration-guide.md
+++ b/docs/sql-migration-guide.md
@@ -39,6 +39,7 @@ license: |
 - Since Spark 4.3, `unix_seconds`, `unix_millis`, and `unix_micros` accept `TIMESTAMP_NTZ` and the nanosecond-precision timestamp types directly, reading them with no time-zone shift. Previously these functions accepted only `TIMESTAMP_LTZ`; a `TIMESTAMP_NTZ` or nanosecond-timestamp argument was rejected with a `DATATYPE_MISMATCH` error. This is a new capability and does not change the result of any query that previously succeeded.
 - Since Spark 4.3, `hash()` and `xxhash64()` include the `days` field of `CalendarInterval` when computing the hash, so their output for interval values differs from earlier releases. Previously the codegen path dropped `days`, disagreeing with interpreted evaluation.
 - Since Spark 4.3, when a `SELECT` or `INSERT` statement references the same table more than once with different `WITH (...)` options (for example a self-join, or `INSERT INTO t WITH (...) SELECT * FROM t WITH (...)`), each reference now uses its own options instead of the second reference silently inheriting the first reference's options via the analyzer's relation cache.
+- Since Spark 4.3, `HAVING` is evaluated before window functions when the `SELECT` list also contains generator functions such as `explode`. Previously, window functions could include groups removed by `HAVING` and produce incorrect results.
 
 ## Upgrading from Spark SQL 4.1 to 4.2
 
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
index 0369a9ce1a5f..f156b191b610 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
@@ -4711,7 +4711,50 @@ object ResolveUnresolvedHaving extends Rule[LogicalPlan] {
     plan.resolveOperatorsWithPruning(_.containsPattern(UNRESOLVED_HAVING), ruleId) {
       case u @ UnresolvedHaving(havingCondition, child)
         if havingCondition.resolved && child.resolved =>
-        Filter(condition = havingCondition, child = child)
+        val filter = Filter(condition = havingCondition, child = child)
+        insertFilterBeforeWindow(filter).getOrElse(filter)
+    }
+  }
+
+  /**
+   * Searches through Project and Generate nodes for a Window chain and places HAVING below every
+   * Window in that chain. This restores SQL clause order for plans produced by queries such as:
+   *
+   * {{{
+   * SELECT explode(array(a)), count(*) OVER ()
+   * FROM VALUES (1), (2), (NULL) AS t(a)
+   * GROUP BY a
+   * HAVING a IS NOT NULL
+   * }}}
+   *
+   * 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))
+        .map(child => project.withNewChildren(Seq(child)))
+    case generate: Generate =>
+      insertFilterBeforeWindow(filter.copy(child = generate.child))
+        .map(child => generate.withNewChildren(Seq(child)))
+    case window: Window =>
+      insertFilterBeforeWindowChain(filter, window)
+    case _ =>
+      None
+  }
+
+  private def insertFilterBeforeWindowChain(
+      filter: Filter,
+      window: Window): Option[LogicalPlan] = {
+    if (!filter.condition.references.subsetOf(window.child.outputSet)) {
+      None
+    } else {
+      val child = window.child match {
+        case childWindow: Window =>
+          insertFilterBeforeWindowChain(filter, childWindow)
+        case child =>
+          Some(filter.copy(child = child))
+      }
+      child.map(child => window.withNewChildren(Seq(child)))
     }
   }
 }
diff --git a/sql/core/src/test/resources/sql-tests/analyzer-results/generators-resolution-edge-cases.sql.out b/sql/core/src/test/resources/sql-tests/analyzer-results/generators-resolution-edge-cases.sql.out
index 0b21d6e6b85d..00930b19cd41 100644
--- a/sql/core/src/test/resources/sql-tests/analyzer-results/generators-resolution-edge-cases.sql.out
+++ b/sql/core/src/test/resources/sql-tests/analyzer-results/generators-resolution-edge-cases.sql.out
@@ -781,3 +781,63 @@ org.apache.spark.sql.catalyst.ExtendedAnalysisException
     "fragment" : "val"
   } ]
 }
+
+
+-- !query
+SELECT explode(array(a)) AS col, count(*) OVER () AS cnt, a
+FROM VALUES (1), (2), (3), (NULL) AS t(a)
+GROUP BY a
+HAVING a IS NOT NULL
+-- !query analysis
+Project [col#x, cnt#xL, a#x]
++- Generate explode(_gen_input_0#x), false, [col#x]
+   +- Project [_gen_input_0#x, cnt#xL, a#x]
+      +- Project [_gen_input_0#x, a#x, cnt#xL, cnt#xL]
+         +- Window [count(1) windowspecdefinition(specifiedwindowframe(RowFrame, unboundedpreceding$(), unboundedfollowing$())) AS cnt#xL]
+            +- Filter isnotnull(a#x)
+               +- Aggregate [a#x], [array(a#x) AS _gen_input_0#x, a#x]
+                  +- SubqueryAlias t
+                     +- LocalRelation [a#x]
+
+
+-- !query
+SELECT explode(array(a)) AS col,
+       count(*) OVER () AS group_count,
+       count(*) AS row_count
+FROM VALUES (1), (1), (2), (3), (3) AS t(a)
+GROUP BY a
+HAVING row_count > 1
+-- !query analysis
+Project [col#x, group_count#xL, row_count#xL]
++- Generate explode(_gen_input_0#x), false, [col#x]
+   +- Project [_gen_input_0#x, group_count#xL, row_count#xL]
+      +- Project [_gen_input_0#x, row_count#xL, group_count#xL, group_count#xL]
+         +- Window [count(1) windowspecdefinition(specifiedwindowframe(RowFrame, unboundedpreceding$(), unboundedfollowing$())) AS group_count#xL]
+            +- Filter (row_count#xL > cast(1 as bigint))
+               +- Aggregate [a#x], [array(a#x) AS _gen_input_0#x, count(1) AS row_count#xL]
+                  +- SubqueryAlias t
+                     +- LocalRelation [a#x]
+
+
+-- !query
+SELECT explode(array(a)) AS col,
+       count(*) OVER () AS cnt,
+       count(*) OVER (
+         ORDER BY a
+         ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING
+       ) AS ordered_cnt,
+       a
+FROM VALUES (1), (2), (3), (NULL) AS t(a)
+GROUP BY a
+HAVING a IS NOT NULL
+-- !query analysis
+Project [col#x, cnt#xL, ordered_cnt#xL, a#x]
++- Generate explode(_gen_input_0#x), false, [col#x]
+   +- Project [_gen_input_0#x, cnt#xL, ordered_cnt#xL, a#x]
+      +- Project [_gen_input_0#x, a#x, cnt#xL, ordered_cnt#xL, cnt#xL, ordered_cnt#xL]
+         +- Window [count(1) windowspecdefinition(a#x ASC NULLS FIRST, specifiedwindowframe(RowFrame, unboundedpreceding$(), unboundedfollowing$())) AS ordered_cnt#xL], [a#x ASC NULLS FIRST]
+            +- Window [count(1) windowspecdefinition(specifiedwindowframe(RowFrame, unboundedpreceding$(), unboundedfollowing$())) AS cnt#xL]
+               +- Filter isnotnull(a#x)
+                  +- Aggregate [a#x], [array(a#x) AS _gen_input_0#x, a#x]
+                     +- SubqueryAlias t
+                        +- LocalRelation [a#x]
diff --git a/sql/core/src/test/resources/sql-tests/inputs/generators-resolution-edge-cases.sql b/sql/core/src/test/resources/sql-tests/inputs/generators-resolution-edge-cases.sql
index 67b5ae77c336..5928ccea56e7 100644
--- a/sql/core/src/test/resources/sql-tests/inputs/generators-resolution-edge-cases.sql
+++ b/sql/core/src/test/resources/sql-tests/inputs/generators-resolution-edge-cases.sql
@@ -215,3 +215,29 @@ SELECT posexplode(array('x', 'y')) as (pos, val), pos, val FROM (VALUES (42)) AS
 
 -- generator's multi-alias does not shadow table column with aggregate
 SELECT posexplode(array('x', 'y')) as (pos, val), pos, val, count(*) FROM (VALUES (42)) AS t(pos) GROUP BY pos;
+
+-- HAVING on a grouping key should be evaluated before a window function with a generator
+SELECT explode(array(a)) AS col, count(*) OVER () AS cnt, a
+FROM VALUES (1), (2), (3), (NULL) AS t(a)
+GROUP BY a
+HAVING a IS NOT NULL;
+
+-- HAVING on an aggregate should be evaluated before a window function with a generator
+SELECT explode(array(a)) AS col,
+       count(*) OVER () AS group_count,
+       count(*) AS row_count
+FROM VALUES (1), (1), (2), (3), (3) AS t(a)
+GROUP BY a
+HAVING row_count > 1;
+
+-- HAVING should be evaluated before multiple window functions with a generator
+SELECT explode(array(a)) AS col,
+       count(*) OVER () AS cnt,
+       count(*) OVER (
+         ORDER BY a
+         ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING
+       ) AS ordered_cnt,
+       a
+FROM VALUES (1), (2), (3), (NULL) AS t(a)
+GROUP BY a
+HAVING a IS NOT NULL;
diff --git a/sql/core/src/test/resources/sql-tests/results/generators-resolution-edge-cases.sql.out b/sql/core/src/test/resources/sql-tests/results/generators-resolution-edge-cases.sql.out
index 7dc50cfb4aac..7876b772b720 100644
--- a/sql/core/src/test/resources/sql-tests/results/generators-resolution-edge-cases.sql.out
+++ b/sql/core/src/test/resources/sql-tests/results/generators-resolution-edge-cases.sql.out
@@ -781,3 +781,49 @@ org.apache.spark.sql.catalyst.ExtendedAnalysisException
     "fragment" : "val"
   } ]
 }
+
+
+-- !query
+SELECT explode(array(a)) AS col, count(*) OVER () AS cnt, a
+FROM VALUES (1), (2), (3), (NULL) AS t(a)
+GROUP BY a
+HAVING a IS NOT NULL
+-- !query schema
+struct<col:int,cnt:bigint,a:int>
+-- !query output
+1	3	1
+2	3	2
+3	3	3
+
+
+-- !query
+SELECT explode(array(a)) AS col,
+       count(*) OVER () AS group_count,
+       count(*) AS row_count
+FROM VALUES (1), (1), (2), (3), (3) AS t(a)
+GROUP BY a
+HAVING row_count > 1
+-- !query schema
+struct<col:int,group_count:bigint,row_count:bigint>
+-- !query output
+1	2	2
+3	2	2
+
+
+-- !query
+SELECT explode(array(a)) AS col,
+       count(*) OVER () AS cnt,
+       count(*) OVER (
+         ORDER BY a
+         ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING
+       ) AS ordered_cnt,
+       a
+FROM VALUES (1), (2), (3), (NULL) AS t(a)
+GROUP BY a
+HAVING a IS NOT NULL
+-- !query schema
+struct<col:int,cnt:bigint,ordered_cnt:bigint,a:int>
+-- !query output
+1	3	3	1
+2	3	3	2
+3	3	3	3


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org