Skip to content

Commit f93e29c

Browse files
shrirangmhalgipeter-toth
authored andcommitted
[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: with(...)`. ### 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 <shrirangmhalgi@gmail.com> Signed-off-by: Peter Toth <peter.toth@gmail.com>
1 parent af0d5c3 commit f93e29c

2 files changed

Lines changed: 18 additions & 1 deletion

File tree

‎sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,9 @@ abstract class Optimizer(catalogManager: CatalogManager)
321321
// execution, so it must never be excludable.
322322
ConvertToCatalyst.ruleName,
323323
FinishAnalysis.ruleName,
324+
// ReplaceExpressions (in FinishAnalysis) turns Between/NullIf into the Unevaluable
325+
// With expression; excluding this rule leaks it into codegen and fails with INTERNAL_ERROR.
326+
RewriteWithExpression.ruleName,
324327
RewriteDistinctAggregates.ruleName,
325328
ReplaceDeduplicateWithAggregate.ruleName,
326329
ReplaceIntersectWithSemiJoin.ruleName,

‎sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import org.apache.spark.sql.catalyst.ExtendedAnalysisException
3535
import org.apache.spark.sql.catalyst.expressions.{CodegenObjectFactoryMode, GenericRow, Hex}
3636
import org.apache.spark.sql.catalyst.expressions.Cast._
3737
import org.apache.spark.sql.catalyst.expressions.aggregate.{Complete, Partial}
38-
import org.apache.spark.sql.catalyst.optimizer.{ConvertToLocalRelation, NestedColumnAliasingSuite}
38+
import org.apache.spark.sql.catalyst.optimizer.{ConvertToLocalRelation, NestedColumnAliasingSuite, RewriteWithExpression}
3939
import org.apache.spark.sql.catalyst.parser.ParseException
4040
import org.apache.spark.sql.catalyst.plans.logical.{LocalLimit, Project, RepartitionByExpression, Sort}
4141
import org.apache.spark.sql.connector.catalog.CatalogManager
@@ -5315,6 +5315,20 @@ class SQLQuerySuite extends SharedSparkSession with AdaptiveSparkPlanHelper
53155315
checkToRDD = false)
53165316
}
53175317
}
5318+
5319+
test("SPARK-59019: BETWEEN succeeds when RewriteWithExpression is in excludedRules") {
5320+
// RewriteWithExpression is non-excludable, so adding it to excludedRules has no effect.
5321+
// Before the fix, this threw INTERNAL_ERROR because With nodes reached codegen.
5322+
withSQLConf(SQLConf.OPTIMIZER_EXCLUDED_RULES.key ->
5323+
RewriteWithExpression.ruleName) {
5324+
checkAnswer(
5325+
sql("SELECT x BETWEEN 1 AND 2 AS in_range FROM (VALUES (1)) AS t(x)"),
5326+
Row(true))
5327+
checkAnswer(
5328+
sql("SELECT x BETWEEN 1 AND 2 AS in_range FROM (VALUES (3)) AS t(x)"),
5329+
Row(false))
5330+
}
5331+
}
53185332
}
53195333

53205334
case class Foo(bar: Option[String])

0 commit comments

Comments
 (0)