(spark) branch branch-4.3 updated: [SPARK-59019][SQL] Make RewriteWithExpression non-excludable

[pt...@apache.org]

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

peter-toth pushed a commit to branch branch-4.3
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-4.3 by this push:
     new 03f4276dd4e1 [SPARK-59019][SQL] Make RewriteWithExpression non-excludable
03f4276dd4e1 is described below

commit 03f4276dd4e1f65aa4becad4ebc20659c633aef8
Author: Shrirang Mhalgi <sh...@gmail.com>
AuthorDate: Mon Aug 31 09:49:53 2026 +0200

    [SPARK-59019][SQL] Make RewriteWithExpression non-excludable
    
    ### What changes were proposed in this pull request?
    
    Add `RewriteWithExpression` to the `nonExcludableRules` list in `Optimizer.scala`.
    
    ### Why are the changes needed?
    
    `With` expressions are `Unevaluable` — they are internal tree constructs for common subexpression elimination that must be rewritten before physical planning. `FinishAnalysis` replaces `RuntimeReplaceable` expressions (e.g., `Between`) with `With` nodes; `RewriteWithExpression` must then rewrite those nodes into executable form. If excluded via `spark.sql.optimizer.excludedRules`, the `With` nodes survive into codegen and throw `INTERNAL_ERROR: Cannot generate code for expression: wit [...]
    
    ### Does this PR introduce _any_ user-facing change?
    
    Yes. Users who previously set `spark.sql.optimizer.excludedRules=org.apache.spark.sql.catalyst.optimizer.RewriteWithExpression` will no longer see the rule excluded (it becomes a no-op config entry). This is strictly better — the previous behavior was a crash.
    
    ### How was this patch tested?
    
    Added a regression test in `SQLQuerySuite` that runs a `BETWEEN` query with `RewriteWithExpression` in `excludedRules` and asserts correct results.
    
    ### Was this patch authored or co-authored using generative AI tooling?
    
    Yes. Co-Authored using Claude Opus 4.8
    
    Closes #58323 from shrirangmhalgi/SPARK-59019-rewrite-with-non-excludable.
    
    Authored-by: Shrirang Mhalgi <sh...@gmail.com>
    Signed-off-by: Peter Toth <pe...@gmail.com>
    (cherry picked from commit f93e29cab139f165ef9110eb5f3ba91545e5fc7d)
    Signed-off-by: Peter Toth <pe...@gmail.com>
---
 .../apache/spark/sql/catalyst/optimizer/Optimizer.scala  |  3 +++
 .../test/scala/org/apache/spark/sql/SQLQuerySuite.scala  | 16 +++++++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
index 1eb51916dd8b..0a8ee18be268 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
@@ -317,6 +317,9 @@ abstract class Optimizer(catalogManager: CatalogManager)
       // execution, so it must never be excludable.
       ConvertToCatalyst.ruleName,
       FinishAnalysis.ruleName,
+      // ReplaceExpressions (in FinishAnalysis) turns Between/NullIf into the Unevaluable
+      // With expression; excluding this rule leaks it into codegen and fails with INTERNAL_ERROR.
+      RewriteWithExpression.ruleName,
       RewriteDistinctAggregates.ruleName,
       ReplaceDeduplicateWithAggregate.ruleName,
       ReplaceIntersectWithSemiJoin.ruleName,
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
index 8075a8a0776b..713fa39a9243 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
@@ -35,7 +35,7 @@ import org.apache.spark.sql.catalyst.ExtendedAnalysisException
 import org.apache.spark.sql.catalyst.expressions.{CodegenObjectFactoryMode, GenericRow, Hex}
 import org.apache.spark.sql.catalyst.expressions.Cast._
 import org.apache.spark.sql.catalyst.expressions.aggregate.{Complete, Partial}
-import org.apache.spark.sql.catalyst.optimizer.{ConvertToLocalRelation, NestedColumnAliasingSuite}
+import org.apache.spark.sql.catalyst.optimizer.{ConvertToLocalRelation, NestedColumnAliasingSuite, RewriteWithExpression}
 import org.apache.spark.sql.catalyst.parser.ParseException
 import org.apache.spark.sql.catalyst.plans.logical.{LocalLimit, Project, RepartitionByExpression, Sort}
 import org.apache.spark.sql.connector.catalog.CatalogManager
@@ -5315,6 +5315,20 @@ class SQLQuerySuite extends SharedSparkSession with AdaptiveSparkPlanHelper
         checkToRDD = false)
     }
   }
+
+  test("SPARK-59019: BETWEEN succeeds when RewriteWithExpression is in excludedRules") {
+    // RewriteWithExpression is non-excludable, so adding it to excludedRules has no effect.
+    // Before the fix, this threw INTERNAL_ERROR because With nodes reached codegen.
+    withSQLConf(SQLConf.OPTIMIZER_EXCLUDED_RULES.key ->
+      RewriteWithExpression.ruleName) {
+      checkAnswer(
+        sql("SELECT x BETWEEN 1 AND 2 AS in_range FROM (VALUES (1)) AS t(x)"),
+        Row(true))
+      checkAnswer(
+        sql("SELECT x BETWEEN 1 AND 2 AS in_range FROM (VALUES (3)) AS t(x)"),
+        Row(false))
+    }
+  }
 }
 
 case class Foo(bar: Option[String])


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