david-mollitor-db opened a new pull request, #58913:
URL: https://github.com/apache/spark/pull/58913
### What changes were proposed in this pull request?
`CheckOverflow` declared `override def nullable: Boolean = true`
unconditionally. Its sibling `MakeDecimal` (immediately above it in
`decimalExpressions.scala`) already declares the accurate form `child.nullable
|| nullOnOverflow` and guards its generated `isNull` assignment accordingly.
This PR aligns `CheckOverflow` with `MakeDecimal`:
```scala
override def nullable: Boolean = child.nullable || nullOnOverflow
```
and, in `doGenCode`, only re-derives `isNull` from the value when the
expression is nullable (mirroring `MakeDecimal`) — otherwise `nullSafeCodeGen`
makes `ev.isNull` a literal and the assignment would be invalid:
```scala
val setIsNull = if (nullable) s"\n${ev.isNull} = ${ev.value} == null;" else
""
```
### Why are the changes needed?
`CheckOverflow` rounds a decimal to a target precision/scale; on overflow it
returns `null` when `nullOnOverflow` is true and **throws** when it is false
(ANSI). So the result is `null` only when the input is `null`, or when
`nullOnOverflow` is true and an overflow occurs — i.e. `child.nullable ||
nullOnOverflow`. Declaring it unconditionally nullable is inaccurate (a decimal
expression wrapped in `CheckOverflow` over non-nullable inputs is reported
nullable under ANSI when it can never be null) and inconsistent with
`MakeDecimal`, which already does this.
### Does this PR introduce _any_ user-facing change?
Schema metadata only: a decimal expression wrapped in `CheckOverflow` over
non-nullable inputs may now report `nullable = false` under ANSI, whereas
before it reported `nullable = true`. Computed values and error behavior are
unchanged — overflow still throws under ANSI and returns `null` otherwise.
### How was this patch tested?
Existing suites, all pass with no golden-file changes:
- `DecimalExpressionSuite` (directly exercises `CheckOverflow`),
`ArithmeticExpressionSuite`
- `SQLQueryTestSuite`: `decimalArithmeticOperations.sql`
`.sql.out` golden files are unaffected because their schema lines use
`df.schema.catalogString`, which does not encode nullability.
### 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]
