From a5c19d9aa4552bc1b026b5c25fafb948ac72e00d Mon Sep 17 00:00:00 2001 From: noroshi Date: Sat, 14 Feb 2026 06:21:18 +0000 Subject: [PATCH] [VL] Support ANSI mode decimal Add/Subtract with checked overflow Routes decimal `Add` and `Subtract` to Velox's `checked_add` and `checked_subtract` functions when ANSI mode is enabled (`nullOnOverflow = false`). These checked variants throw on overflow instead of returning null, matching Spark's ANSI behavior. Depends on facebookincubator/velox#16302 which adds `checked_add` and `checked_subtract` support for decimal types. --- .../gluten/execution/VeloxLiteralSuite.scala | 1 + .../ArithmeticAnsiValidateSuite.scala | 42 +++++++++++++++++++ .../expression/ExpressionConverter.scala | 24 ++++++++++- 3 files changed, 65 insertions(+), 2 deletions(-) diff --git a/backends-velox/src/test/scala/org/apache/gluten/execution/VeloxLiteralSuite.scala b/backends-velox/src/test/scala/org/apache/gluten/execution/VeloxLiteralSuite.scala index cf2e7257f528..7dc6b5241d21 100644 --- a/backends-velox/src/test/scala/org/apache/gluten/execution/VeloxLiteralSuite.scala +++ b/backends-velox/src/test/scala/org/apache/gluten/execution/VeloxLiteralSuite.scala @@ -139,4 +139,5 @@ class VeloxLiteralSuite extends VeloxWholeStageTransformerSuite { validateFallbackResult("SELECT struct(cast(null as struct))") validateFallbackResult("SELECT array(struct(1, 'a'), null)") } + } diff --git a/backends-velox/src/test/scala/org/apache/gluten/functions/ArithmeticAnsiValidateSuite.scala b/backends-velox/src/test/scala/org/apache/gluten/functions/ArithmeticAnsiValidateSuite.scala index a1633c4cb490..83c88d1bd3d1 100644 --- a/backends-velox/src/test/scala/org/apache/gluten/functions/ArithmeticAnsiValidateSuite.scala +++ b/backends-velox/src/test/scala/org/apache/gluten/functions/ArithmeticAnsiValidateSuite.scala @@ -100,4 +100,46 @@ class ArithmeticAnsiValidateSuite extends FunctionsValidateSuite { } } + test("decimal add overflow") { + // Normal decimal add should succeed and match Spark results + runQueryAndCompare( + "SELECT CAST(1.0 AS DECIMAL(10,2)) + CAST(2.0 AS DECIMAL(10,2))") { + checkGlutenPlan[ProjectExecTransformer] + } + + // Overflow: max DECIMAL(38,0) + 1 should throw in ANSI mode + if (isSparkVersionGE("4.0")) { + intercept[SparkException] { + sql("SELECT CAST(99999999999999999999999999999999999999 AS DECIMAL(38,0)) + " + + "CAST(1 AS DECIMAL(38,0))").collect() + } + } else { + intercept[ArithmeticException] { + sql("SELECT CAST(99999999999999999999999999999999999999 AS DECIMAL(38,0)) + " + + "CAST(1 AS DECIMAL(38,0))").collect() + } + } + } + + test("decimal subtract overflow") { + // Normal decimal subtract should succeed and match Spark results + runQueryAndCompare( + "SELECT CAST(5.0 AS DECIMAL(10,2)) - CAST(2.0 AS DECIMAL(10,2))") { + checkGlutenPlan[ProjectExecTransformer] + } + + // Overflow: -max DECIMAL(38,0) - 1 should throw in ANSI mode + if (isSparkVersionGE("4.0")) { + intercept[SparkException] { + sql("SELECT CAST(-99999999999999999999999999999999999999 AS DECIMAL(38,0)) - " + + "CAST(1 AS DECIMAL(38,0))").collect() + } + } else { + intercept[ArithmeticException] { + sql("SELECT CAST(-99999999999999999999999999999999999999 AS DECIMAL(38,0)) - " + + "CAST(1 AS DECIMAL(38,0))").collect() + } + } + } + } diff --git a/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala b/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala index cc8d204cd53c..ac55a91ac903 100644 --- a/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala +++ b/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala @@ -621,17 +621,37 @@ object ExpressionConverter extends SQLConfHelper with Logging { substraitExprName, expr.children.map(replaceWithExpressionTransformer0(_, attributeSeq, expressionsMap)), expr) - case CheckOverflow(b: BinaryArithmetic, decimalType, _) + case CheckOverflow(b: BinaryArithmetic, decimalType, nullOnOverflow) if !BackendsApiManager.getSettings.transformCheckOverflow && DecimalArithmeticUtil.isDecimalArithmetic(b) => - val arithmeticExprName = + val baseExprName = BackendsApiManager.getSparkPlanExecApiInstance.getDecimalArithmeticExprName( getAndCheckSubstraitName(b, expressionsMap)) + // When nullOnOverflow is false, it's ANSI mode - use checked_ prefix for overflow errors + val arithmeticExprName = if (!nullOnOverflow) { + "checked_" + baseExprName + } else { + baseExprName + } val left = replaceWithExpressionTransformer0(b.left, attributeSeq, expressionsMap) val right = replaceWithExpressionTransformer0(b.right, attributeSeq, expressionsMap) DecimalArithmeticExpressionTransformer(arithmeticExprName, left, right, decimalType, b) + // Velox path: ANSI mode decimal Add/Subtract uses checked_ variants + // that throw on overflow instead of returning null. + case c @ CheckOverflow(b: BinaryArithmetic, _, nullOnOverflow) + if BackendsApiManager.getSettings.transformCheckOverflow && + DecimalArithmeticUtil.isDecimalArithmetic(b) && + !nullOnOverflow && + (b.isInstanceOf[Add] || b.isInstanceOf[Subtract]) => + val baseExprName = + BackendsApiManager.getSparkPlanExecApiInstance.getDecimalArithmeticExprName( + getAndCheckSubstraitName(b, expressionsMap)) + val checkedExprName = "checked_" + baseExprName + val childTransformer = + genRescaleDecimalTransformer(checkedExprName, b, attributeSeq, expressionsMap) + CheckOverflowTransformer(substraitExprName, childTransformer, c) case c: CheckOverflow => CheckOverflowTransformer( substraitExprName,