(spark) branch master updated: [SPARK-59719][SQL] Mark CheckOverflow as null-intolerant to match MakeDecimal

[do...@apache.org]

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

dongjoon-hyun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/master by this push:
     new 9929c6f0387a [SPARK-59719][SQL] Mark CheckOverflow as null-intolerant to match MakeDecimal
9929c6f0387a is described below

commit 9929c6f0387ab11538c94880b5ece7aee17aeec1
Author: david-mollitor-db <da...@databricks.com>
AuthorDate: Wed Sep 23 12:35:27 2026 -0700

    [SPARK-59719][SQL] Mark CheckOverflow as null-intolerant to match MakeDecimal
    
    ### What changes were proposed in this pull request?
    
    Declare `CheckOverflow` null-intolerant, matching its sibling `MakeDecimal`:
    
    ```scala
    override def nullIntolerant: Boolean = true
    ```
    
    `CheckOverflow` extends `UnaryExpression` and overrides `nullSafeEval`, so `UnaryExpression.eval` returns `null` whenever the child is `null` -- i.e. it already satisfies the null-intolerance contract ("any null input results in null output"). It has simply never been declared null-intolerant. Unlike its siblings `MakeDecimal` and `UnscaledValue`, `CheckOverflow` never mixed in the old `NullIntolerant` trait, so this is a pre-existing gap, not a regression: SPARK-50241 only turned tha [...]
    
    `Cast` -- which shares the same "throws under ANSI overflow / returns null otherwise" shape -- already declares `nullIntolerant = true`, as do `MakeDecimal`, `UnscaledValue`, and the arithmetic expressions. This aligns `CheckOverflow` with them.
    
    ### Why are the changes needed?
    
    `nullIntolerant` is consumed by the optimizer to:
    - fold a null-literal input to a null literal (`NullPropagation`), and
    - infer `IsNotNull(child)` constraints and push down `IsNotNull` filters (`QueryPlanConstraints.scanNullIntolerantAttribute` / `InferFiltersFromConstraints`).
    
    `CheckOverflow` wraps decimal results throughout the plan (e.g. the decimal serializers in `SerializerBuildHelper`), so declaring the property it already honors lets these optimizations see through it, consistent with `MakeDecimal`. This is a follow-up to the nullability alignment in SPARK-59643 (#58913), addressing a related consistency gap raised in that review.
    
    The direction the optimizer relies on holds even though `CheckOverflow` with `nullOnOverflow = true` can also return `null` from a non-null input on overflow: the contract only requires "null in => null out", not the converse -- the same situation as `Cast`.
    
    ### Does this PR introduce _any_ user-facing change?
    
    No. This only declares an optimizer-visible property that `CheckOverflow` already honored at runtime; computed values and error behavior are unchanged. It can enable additional `IsNotNull` filter inference / null folding in query plans, but those are semantics-preserving: an inferred `IsNotNull` only removes rows whose `CheckOverflow` result would already be null, and null folding only fires for null-literal inputs, which never overflow or throw.
    
    ### How was this patch tested?
    
    New tests for the newly-enabled optimizer behavior:
    - `InferFiltersFromConstraintsSuite`: `IsNotNull('d)` is inferred from a filter `CheckOverflow('d, ...) > 0`.
    - `NullPropagationSuite` (new): `CheckOverflow(Literal(null, ...), ...)` folds to a null literal under a NullPropagation-only batch, so the fold is attributable to null-intolerance.
    
    Existing suites pass with no plan or golden-file churn: `DecimalExpressionSuite`, `PlannerSuite`, `AlignUpdateAssignmentsSuite`, `AlignMergeAssignmentsSuite`, plus the TPCDS plan-stability suites, `ExplainSuite`, and the `explain*.sql` golden files (verified the added `IsNotNull` inference does not change those plans).
    
    ### Was this patch authored or co-authored using generative AI tooling?
    
    Generated-by: Claude Opus 4.8
    
    Closes #58972 from david-mollitor-db/checkoverflow-nullintolerant.
    
    Authored-by: david-mollitor-db <da...@databricks.com>
    Signed-off-by: Dongjoon Hyun <do...@apache.org>
---
 .../catalyst/expressions/decimalExpressions.scala  |  2 +
 .../InferFiltersFromConstraintsSuite.scala         | 14 +++++++
 .../catalyst/optimizer/NullPropagationSuite.scala  | 46 ++++++++++++++++++++++
 3 files changed, 62 insertions(+)

diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/decimalExpressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/decimalExpressions.scala
index f24c90768150..0cc8d8e38d4d 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/decimalExpressions.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/decimalExpressions.scala
@@ -117,6 +117,8 @@ case class CheckOverflow(
     dataType: DecimalType,
     nullOnOverflow: Boolean) extends UnaryExpression with SupportQueryContext {
 
+  override def nullIntolerant: Boolean = true
+
   override def nullable: Boolean = true
 
   override def nullSafeEval(input: Any): Any =
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala
index bfed9427c863..0640642489a6 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala
@@ -451,4 +451,18 @@ class InferFiltersFromConstraintsSuite extends PlanTest {
     assert(joinFound, "Expected a Join node in the optimized plan")
   }
 
+  test("SPARK-59719: infer IsNotNull through null-intolerant CheckOverflow") {
+    val dt = DecimalType(18, 0)
+    val decimalRelation = LocalRelation($"d".decimal(18, 0))
+    val query = decimalRelation
+      .where(CheckOverflow($"d", dt, nullOnOverflow = true) > Literal.create(Decimal(0), dt))
+      .analyze
+    val optimized = Optimize.execute(query)
+    val correctAnswer = decimalRelation
+      .where(IsNotNull($"d") &&
+        (CheckOverflow($"d", dt, nullOnOverflow = true) > Literal.create(Decimal(0), dt)))
+      .analyze
+    comparePlans(optimized, correctAnswer)
+  }
+
 }
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/NullPropagationSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/NullPropagationSuite.scala
new file mode 100644
index 000000000000..9f311e7becb8
--- /dev/null
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/NullPropagationSuite.scala
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.optimizer
+
+import org.apache.spark.sql.catalyst.dsl.expressions._
+import org.apache.spark.sql.catalyst.dsl.plans._
+import org.apache.spark.sql.catalyst.expressions.{CheckOverflow, Literal}
+import org.apache.spark.sql.catalyst.plans.PlanTest
+import org.apache.spark.sql.catalyst.plans.logical.{LocalRelation, LogicalPlan}
+import org.apache.spark.sql.catalyst.rules.RuleExecutor
+import org.apache.spark.sql.types.DecimalType
+
+class NullPropagationSuite extends PlanTest {
+
+  object Optimize extends RuleExecutor[LogicalPlan] {
+    val batches = Batch("NullPropagation", FixedPoint(50), NullPropagation) :: Nil
+  }
+
+  test("SPARK-59719: fold a null-intolerant CheckOverflow over a null literal to null") {
+    // With only NullPropagation in the batch, the fold happens because CheckOverflow is
+    // null-intolerant: a null child implies a null result.
+    val dt = DecimalType(18, 0)
+    val relation = LocalRelation($"a".int)
+    val query = relation
+      .select(CheckOverflow(Literal.create(null, dt), dt, nullOnOverflow = true).as("c"))
+      .analyze
+    val optimized = Optimize.execute(query)
+    val correctAnswer = relation.select(Literal.create(null, dt).as("c")).analyze
+    comparePlans(optimized, correctAnswer)
+  }
+}


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