You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/10/04 17:48:32 UTC
(datafusion-comet) 01/01: fix: align wide-decimal ANSI overflow value with Spark (#5276)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-5276-76f436c0f957742bf6e50a7e7514796c2a6fbe2b
in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git
commit 0d729a9b11b0745964bfdbe1ec41842b913d413c
Author: sam-1112 <75...@users.noreply.github.com>
AuthorDate: Sun Oct 4 17:48:09 2026 +0000
fix: align wide-decimal ANSI overflow value with Spark (#5276)
* fix: align wide-decimal ANSI overflow value with Spark (#5211)
* fix: match Spark decimal overflow error value formatting
* fix: allow too_many_arguments in wide decimal overflow helper
---
.../user-guide/latest/compatibility/index.md | 2 -
.../src/math_funcs/wide_decimal_binary_expr.rs | 99 ++++++++++++++++++++--
.../sql/comet/shims/ShimSparkErrorConverter.scala | 3 +-
.../sql/comet/shims/ShimSparkErrorConverter.scala | 3 +-
.../sql/comet/shims/ShimSparkErrorConverter.scala | 3 +-
.../org/apache/comet/CometExpressionSuite.scala | 15 ++--
6 files changed, 104 insertions(+), 21 deletions(-)
diff --git a/docs/source/user-guide/latest/compatibility/index.md b/docs/source/user-guide/latest/compatibility/index.md
index 9bae431207..017140b14d 100644
--- a/docs/source/user-guide/latest/compatibility/index.md
+++ b/docs/source/user-guide/latest/compatibility/index.md
@@ -116,8 +116,6 @@ divergence:
example `byte overflow`) where Spark raises `BINARY_ARITHMETIC_OVERFLOW`, and integral
`ARITHMETIC_OVERFLOW` messages omit Spark's `try_` suggestion
([#6217](https://github.com/apache/datafusion-comet/issues/6217)).
-- Wide-decimal arithmetic overflow reports a different value in the error message than Spark
- ([#5211](https://github.com/apache/datafusion-comet/issues/5211)).
- Spark 4.2 introduced additional ANSI arithmetic overflow behavior differences that Comet does
not yet track ([#4967](https://github.com/apache/datafusion-comet/issues/4967)).
diff --git a/native/spark-expr/src/math_funcs/wide_decimal_binary_expr.rs b/native/spark-expr/src/math_funcs/wide_decimal_binary_expr.rs
index 45ada3ff33..a501a01278 100644
--- a/native/spark-expr/src/math_funcs/wide_decimal_binary_expr.rs
+++ b/native/spark-expr/src/math_funcs/wide_decimal_binary_expr.rs
@@ -264,6 +264,9 @@ impl PhysicalExpr for WideDecimalBinaryExpr {
neg_bound,
p_out,
s_out,
+ raw,
+ max_scale,
+ op,
eval_mode,
&overflowed,
)
@@ -299,6 +302,9 @@ impl PhysicalExpr for WideDecimalBinaryExpr {
neg_bound,
p_out,
s_out,
+ raw,
+ natural_scale,
+ op,
eval_mode,
&overflowed,
)
@@ -358,30 +364,42 @@ impl PhysicalExpr for WideDecimalBinaryExpr {
}
}
-/// Check if the i256 result fits in the output precision. In Ansi mode, return an error
-/// on overflow. In Legacy/Try mode, record the overflow and return i128::MAX as a sentinel
+/// Check if the rescaled i256 result fits in the output precision. In Ansi mode, return an
+/// error on overflow. In Legacy/Try mode, record the overflow and return i128::MAX as a sentinel
/// value that will be nullified by `null_if_overflow_precision`.
+///
+/// ANSI overflow messages format `report_value` at `report_scale` (Spark's pre-toPrecision
+/// intermediate), not the rescaled result. Multiplication also applies Spark's
+/// MathContext(39, DOWN).
#[inline]
+#[allow(clippy::too_many_arguments)]
fn check_overflow_and_convert(
result: i256,
bound: i256,
neg_bound: i256,
precision: u8,
scale: i8,
+ report_value: i256,
+ report_scale: i8,
+ op: WideDecimalOp,
eval_mode: EvalMode,
overflowed: &Cell<bool>,
) -> Result<i128, ArrowError> {
if result > bound || result < neg_bound {
if eval_mode == EvalMode::Ansi {
- let unscaled = result.to_string();
- // Arrow's formatter truncates to its precision argument. This value is already
- // known to overflow, so pass its actual digit count to preserve every digit.
- // Spark reports the pre-toPrecision value instead; see
- // https://github.com/apache/datafusion-comet/issues/5211.
- let digits = unscaled.trim_start_matches('-').len();
+ let value = match op {
+ WideDecimalOp::Multiply => {
+ spark_multiply_overflow_value(&report_value.to_string(), report_scale)?
+ }
+ WideDecimalOp::Add | WideDecimalOp::Subtract => {
+ let unscaled = report_value.to_string();
+ let digits = unscaled.trim_start_matches('-').len();
+ format_decimal_str(&unscaled, digits, report_scale)
+ }
+ };
return Err(ArrowError::ExternalError(Box::new(
SparkError::NumericValueOutOfRange {
- value: format_decimal_str(&unscaled, digits, scale),
+ value,
precision,
scale,
},
@@ -397,6 +415,34 @@ fn check_overflow_and_convert(
}
}
+/// Emulate Spark multiply's `MathContext(39, DOWN)` before `toPlainString()`.
+fn spark_multiply_overflow_value(unscaled: &str, scale: i8) -> Result<String, ArrowError> {
+ const MC_PRECISION: usize = 39; // DecimalType.MAX_PRECISION + 1
+ let negative = unscaled.starts_with('-');
+ let digits = unscaled.trim_start_matches('-');
+ if digits.is_empty() {
+ return Ok("0".to_string());
+ }
+
+ if digits.len() <= MC_PRECISION {
+ return Ok(format_decimal_str(unscaled, digits.len(), scale));
+ }
+
+ let truncated = &digits[..MC_PRECISION];
+ let dropped = digits.len() - MC_PRECISION;
+ let new_scale = i8::try_from(scale as i16 - dropped as i16).map_err(|_| {
+ ArrowError::ComputeError(format!(
+ "Spark multiply overflow value scale is out of range: scale={scale}, dropped={dropped}"
+ ))
+ })?;
+ let truncated = if negative {
+ format!("-{truncated}")
+ } else {
+ truncated.to_string()
+ };
+ Ok(format_decimal_str(&truncated, MC_PRECISION, new_scale))
+}
+
#[cfg(test)]
mod tests {
use super::*;
@@ -587,6 +633,41 @@ mod tests {
WideDecimalOp::Multiply,
1,
0,
+ "9.90",
+ ),
+ (
+ make_batch(
+ vec![Some(11_000_000_000_000_000_000)],
+ 20,
+ 0,
+ vec![Some(11_000_000_000_000_000_000)],
+ 20,
+ 0,
+ ),
+ WideDecimalOp::Multiply,
+ 38,
+ 6,
+ "121000000000000000000000000000000000000",
+ ),
+ (
+ make_batch(
+ vec![Some(11_000_000_000_000_000_000i128 * 10i128.pow(18))],
+ 38,
+ 18,
+ vec![Some(11_000_000_000_000_000_000i128 * 10i128.pow(18))],
+ 38,
+ 18,
+ ),
+ WideDecimalOp::Multiply,
+ 38,
+ 6,
+ "121000000000000000000000000000000000000",
+ ),
+ (
+ make_batch(vec![Some(5)], 38, 0, vec![Some(5)], 38, 0),
+ WideDecimalOp::Add,
+ 1,
+ 2,
"10",
),
];
diff --git a/spark/src/main/spark-3.4/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala b/spark/src/main/spark-3.4/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala
index dc48e1b53b..001b327956 100644
--- a/spark/src/main/spark-3.4/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala
+++ b/spark/src/main/spark-3.4/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala
@@ -105,7 +105,8 @@ trait ShimSparkErrorConverter {
Some(QueryExecutionErrors.overflowInSumOfDecimalError(sqlCtx(context)))
case "NumericValueOutOfRange" =>
- val decimal = Decimal(params("value").toString)
+ // Use Java BigDecimal to avoid Scala BigDecimal's DECIMAL128 MathContext rewriting.
+ val decimal = Decimal(new java.math.BigDecimal(params("value").toString))
Some(
QueryExecutionErrors.cannotChangeDecimalPrecisionError(
decimal,
diff --git a/spark/src/main/spark-3.5/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala b/spark/src/main/spark-3.5/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala
index 128844ac7f..b148543853 100644
--- a/spark/src/main/spark-3.5/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala
+++ b/spark/src/main/spark-3.5/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala
@@ -105,7 +105,8 @@ trait ShimSparkErrorConverter {
Some(QueryExecutionErrors.overflowInSumOfDecimalError(sqlCtx(context)))
case "NumericValueOutOfRange" =>
- val decimal = Decimal(params("value").toString)
+ // Use Java BigDecimal to avoid Scala BigDecimal's DECIMAL128 MathContext rewriting.
+ val decimal = Decimal(new java.math.BigDecimal(params("value").toString))
Some(
QueryExecutionErrors.cannotChangeDecimalPrecisionError(
decimal,
diff --git a/spark/src/main/spark-4.x/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala b/spark/src/main/spark-4.x/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala
index 8ff9ea0112..dd9024e8ff 100644
--- a/spark/src/main/spark-4.x/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala
+++ b/spark/src/main/spark-4.x/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala
@@ -126,7 +126,8 @@ trait ShimSparkErrorConverter {
.overflowInSumOfDecimalError(context.headOption.orNull, s"try_$functionName"))
case "NumericValueOutOfRange" =>
- val decimal = Decimal(params("value").toString)
+ // Use Java BigDecimal to avoid Scala BigDecimal's DECIMAL128 MathContext rewriting.
+ val decimal = Decimal(new java.math.BigDecimal(params("value").toString))
Some(
QueryExecutionErrors.cannotChangeDecimalPrecisionError(
decimal,
diff --git a/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala b/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala
index af8aa7f0b9..04af0d8505 100644
--- a/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala
+++ b/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala
@@ -1807,10 +1807,8 @@ class CometExpressionSuite extends CometTestBase with AdaptiveSparkPlanHelper {
}
test("scalar decimal overflow - ANSI mode throws ArithmeticException") {
- // 1.1e19 * 1.1e19 = 1.21e38 overflows DECIMAL(38,0). With ANSI mode on, both Spark and
- // Comet must throw — Comet must not panic or silently return null. Spark reports
- // NUMERIC_VALUE_OUT_OF_RANGE; Comet's WideDecimalBinaryExpr catches the overflow first
- // and must surface the same structured Spark error.
+ // 1.1e19 * 1.1e19 overflows Decimal(38, 6); ANSI must raise NUMERIC_VALUE_OUT_OF_RANGE
+ // with the same pre-toPrecision `value` as Spark (#5211).
withSQLConf(CometConf.COMET_ENABLED.key -> "true", SQLConf.ANSI_ENABLED.key -> "true") {
withParquetTable(Seq((BigDecimal("11000000000000000000"), 0)), "tbl") {
val res = sql("SELECT _1 * _1 FROM tbl")
@@ -1819,14 +1817,17 @@ class CometExpressionSuite extends CometTestBase with AdaptiveSparkPlanHelper {
case (Some(sparkExc), Some(cometExc)) =>
val expected = arithmeticError(sparkExc)
val actual = arithmeticError(cometExc)
- // Spark formats its pre-toPrecision Decimal, while Comet formats the rescaled i256
- // value (https://github.com/apache/datafusion-comet/issues/5211). This regression
- // covers the structured error fields and query context.
assert(actual.getErrorClass == expected.getErrorClass)
assert(actual.getSqlState == expected.getSqlState)
assert(
actual.getQueryContext.map(_.fragment()).toSeq ==
expected.getQueryContext.map(_.fragment()).toSeq)
+ val actualValue = actual.getMessageParameters.get("value")
+ val expectedValue = expected.getMessageParameters.get("value")
+ assert(
+ actualValue == expectedValue,
+ s"value mismatch: comet=$actualValue spark=$expectedValue")
+ assert(!actualValue.contains(".000000"))
case _ =>
fail("Expected exception for decimal overflow in ANSI mode")
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org