From de3d4cba062f02189f9c1beee427db313cddc885 Mon Sep 17 00:00:00 2001 From: David Mollitor Date: Fri, 18 Sep 2026 17:07:48 +0000 Subject: [PATCH] [SPARK-59643][SQL] Tighten CheckOverflow nullability to match MakeDecimal CheckOverflow declared `nullable = true` unconditionally, while its sibling MakeDecimal (immediately above it in the same file) already declares the accurate form `child.nullable || nullOnOverflow`. CheckOverflow returns null only when its input is null, or when an overflow occurs with `nullOnOverflow = true` (under ANSI, `nullOnOverflow` is false and an overflow throws). Align CheckOverflow with MakeDecimal: override def nullable: Boolean = child.nullable || nullOnOverflow and, in doGenCode, only re-derive `isNull` from the value when the expression is nullable (mirroring MakeDecimal) -- otherwise nullSafeCodeGen makes `ev.isNull` a literal and the assignment would be invalid. No behavioral change: overflow still throws under ANSI and returns null otherwise. Only the declared nullability tightens, so a CheckOverflow-wrapped decimal over non-null inputs is now reported non-nullable under ANSI. Co-authored-by: Isaac --- .../sql/catalyst/expressions/decimalExpressions.scala | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) 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 f24c907681502..6d2f6ee5ed251 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,7 +117,9 @@ case class CheckOverflow( dataType: DecimalType, nullOnOverflow: Boolean) extends UnaryExpression with SupportQueryContext { - override def nullable: Boolean = true + // When `nullOnOverflow` is false, an overflow throws instead of producing null, so the result is + // null only when the input is. When true, an overflow yields null regardless of the input. + override def nullable: Boolean = child.nullable || nullOnOverflow override def nullSafeEval(input: Any): Any = input.asInstanceOf[Decimal].toPrecision( @@ -130,11 +132,14 @@ case class CheckOverflow( override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { val errorContextCode = getContextOrNullCode(ctx, !nullOnOverflow) nullSafeCodeGen(ctx, ev, eval => { + // Mirror MakeDecimal above: `ev.isNull` is a literal when this expression is non-nullable + // (under `!nullOnOverflow` a non-null input never overflows to null -- it throws), so only + // re-derive it from the value when the expression is actually nullable. + val setIsNull = if (nullable) s"\n${ev.isNull} = ${ev.value} == null;" else "" // scalastyle:off line.size.limit s""" |${ev.value} = $eval.toPrecision( - | ${dataType.precision}, ${dataType.scale}, Decimal.ROUND_HALF_UP(), $nullOnOverflow, $errorContextCode); - |${ev.isNull} = ${ev.value} == null; + | ${dataType.precision}, ${dataType.scale}, Decimal.ROUND_HALF_UP(), $nullOnOverflow, $errorContextCode);$setIsNull """.stripMargin // scalastyle:on line.size.limit })