david-mollitor-db opened a new pull request, #58972:
URL: https://github.com/apache/spark/pull/58972
### 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"). The property was simply never declared: when the
`NullIntolerant` marker trait was replaced by a `def` (SPARK-50241), the
adjacent `UnscaledValue` and `MakeDecimal` were updated but `CheckOverflow` was
missed.
`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?
Existing suites pass unchanged, with no plan or golden-file churn:
- catalyst: `DecimalAggregatesSuite`, `DecimalExpressionSuite`,
`InferFiltersFromConstraintsSuite`
- sql/core: `PlannerSuite`, `AlignUpdateAssignmentsSuite`,
`AlignMergeAssignmentsSuite`
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Isaac
This pull request and its description were written by Isaac.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
