(spark) branch branch-4.0 updated: [SPARK-53360][SQL] Once strategy with ConstantFolding's idempotence should not be broken

[vi...@apache.org]

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

viirya pushed a commit to branch branch-4.0
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-4.0 by this push:
     new 326052ec8280 [SPARK-53360][SQL] Once strategy with ConstantFolding's idempotence should not be broken
326052ec8280 is described below

commit 326052ec8280d8bf8ee1904504be3b62a72d3d29
Author: Liang-Chi Hsieh <vi...@gmail.com>
AuthorDate: Sat Aug 23 12:07:22 2025 -0700

    [SPARK-53360][SQL] Once strategy with ConstantFolding's idempotence should not be broken
    
    This patch fixes the optimization rule `ConstantFolding`'s idempotence.
    
    Currently the optimization rule `ConstantFolding` is not idempotent. So if the rule is used in any batch with `Once` strategy like `ConstantFoldingSuite` did, the `Once` strategy's idempotence will be broken. We encounter this issue internally.
    
    No
    
    Unit test
    
    No
    
    Closes #52106 from viirya/SPARK-53360.
    
    Authored-by: Liang-Chi Hsieh <vi...@gmail.com>
    Signed-off-by: Liang-Chi Hsieh <vi...@gmail.com>
    (cherry picked from commit e28f42700611f6acd2de7dc84ecdf4f5b86b8f71)
    Signed-off-by: Liang-Chi Hsieh <vi...@gmail.com>
---
 .../spark/sql/catalyst/optimizer/expressions.scala | 33 ++++++++++++++--------
 .../catalyst/optimizer/ConstantFoldingSuite.scala  | 13 +++++++++
 2 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala
index e867953bcf28..fbb9afe46860 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala
@@ -58,6 +58,19 @@ object ConstantFolding extends Rule[LogicalPlan] {
     case _ => false
   }
 
+  private def tryFold(expr: Expression, isConditionalBranch: Boolean): Expression = {
+    try {
+      Literal.create(expr.freshCopyIfContainsStatefulExpression().eval(EmptyRow), expr.dataType)
+    } catch {
+      case NonFatal(_) if isConditionalBranch =>
+        // When doing constant folding inside conditional expressions, we should not fail
+        // during expression evaluation, as the branch we are evaluating may not be reached at
+        // runtime, and we shouldn't fail the query, to match the original behavior.
+        expr.setTagValue(FAILED_TO_EVALUATE, ())
+        expr
+    }
+  }
+
   private def constantFolding(
       e: Expression,
       isConditionalBranch: Boolean = false): Expression = e match {
@@ -77,17 +90,7 @@ object ConstantFolding extends Rule[LogicalPlan] {
     case e if e.getTagValue(FAILED_TO_EVALUATE).isDefined => e
 
     // Fold expressions that are foldable.
-    case e if e.foldable =>
-      try {
-        Literal.create(e.freshCopyIfContainsStatefulExpression().eval(EmptyRow), e.dataType)
-      } catch {
-        case NonFatal(_) if isConditionalBranch =>
-          // When doing constant folding inside conditional expressions, we should not fail
-          // during expression evaluation, as the branch we are evaluating may not be reached at
-          // runtime, and we shouldn't fail the query, to match the original behavior.
-          e.setTagValue(FAILED_TO_EVALUATE, ())
-          e
-      }
+    case e if e.foldable => tryFold(e, isConditionalBranch)
 
     // Don't replace ScalarSubquery if its plan is an aggregate that may suffer from a COUNT bug.
     case s @ ScalarSubquery(_, _, _, _, _, mayHaveCountBug, _)
@@ -99,7 +102,13 @@ object ConstantFolding extends Rule[LogicalPlan] {
     case s: ScalarSubquery if s.plan.maxRows.contains(0) =>
       Literal(null, s.dataType)
 
-    case other => other.mapChildren(constantFolding(_, isConditionalBranch))
+    case other =>
+      val newOther = other.mapChildren(constantFolding(_, isConditionalBranch))
+      if (newOther.foldable) {
+        tryFold(newOther, isConditionalBranch)
+      } else {
+        newOther
+      }
   }
 
   def apply(plan: LogicalPlan): LogicalPlan = plan.transformWithPruning(AlwaysProcess.fn, ruleId) {
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ConstantFoldingSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ConstantFoldingSuite.scala
index d8d58ea6aa90..0489308a1ab2 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ConstantFoldingSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ConstantFoldingSuite.scala
@@ -437,6 +437,19 @@ class ConstantFoldingSuite extends PlanTest {
       Optimize.execute(oneRowScalarSubquery),
       oneRowScalarSubquery)
   }
+
+  test("SPARK-53360: Once strategy with ConstantFolding's idempotence should not be broken") {
+    val emptyRelation = LocalRelation($"a".int)
+    val nullIntLit = Literal(null, IntegerType)
+
+    Seq(EqualTo, LessThan, GreaterThan).foreach { comparison =>
+      comparePlans(
+        Optimize.execute(testRelation
+          .select(comparison($"a",
+            Multiply(ScalarSubquery(emptyRelation), Literal(1, IntegerType))).as("o")).analyze),
+        testRelation.select(comparison($"a", nullIntLit).as("o")).analyze)
+    }
+  }
 }
 
 case class SerializableBoxedInt(intVal: Int) {


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