This is an automated email from the ASF dual-hosted git repository.
philo-he pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gluten.git
The following commit(s) were added to refs/heads/main by this push:
new 45fa7d606c [GLUTEN-10134][VL] Throw on decimal Add/Subtract/Multiply
overflow in ANSI mode (#13170)
45fa7d606c is described below
commit 45fa7d606cb1c1dc34fe3eb5111802768f1173dc
Author: rdtr <[email protected]>
AuthorDate: Sun Oct 11 00:00:18 2026 -0700
[GLUTEN-10134][VL] Throw on decimal Add/Subtract/Multiply overflow in ANSI
mode (#13170)
---
.../backendsapi/bolt/BoltSparkPlanExecApi.scala | 1 +
.../backendsapi/velox/VeloxSparkPlanExecApi.scala | 24 +++-
.../functions/ArithmeticAnsiValidateSuite.scala | 151 ++++++++++++++++++---
.../gluten/backendsapi/SparkPlanExecApi.scala | 12 +-
.../gluten/expression/ExpressionConverter.scala | 1 +
5 files changed, 161 insertions(+), 28 deletions(-)
diff --git
a/backends-bolt/src/main/scala/org/apache/gluten/backendsapi/bolt/BoltSparkPlanExecApi.scala
b/backends-bolt/src/main/scala/org/apache/gluten/backendsapi/bolt/BoltSparkPlanExecApi.scala
index 006435066d..331e984455 100644
---
a/backends-bolt/src/main/scala/org/apache/gluten/backendsapi/bolt/BoltSparkPlanExecApi.scala
+++
b/backends-bolt/src/main/scala/org/apache/gluten/backendsapi/bolt/BoltSparkPlanExecApi.scala
@@ -164,6 +164,7 @@ class BoltSparkPlanExecApi extends SparkPlanExecApi {
override def getDecimalArithmeticExprName(
exprName: String,
+ original: BinaryArithmetic,
allowPrecisionLoss: Boolean): String =
if (!allowPrecisionLoss) { exprName + "_deny_precision_loss" }
else { exprName }
diff --git
a/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala
b/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala
index a7fd67ea81..640e42690b 100644
---
a/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala
+++
b/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala
@@ -165,9 +165,27 @@ class VeloxSparkPlanExecApi extends SparkPlanExecApi with
Logging {
}
}
- override def getDecimalArithmeticExprName(exprName: String,
allowPrecisionLoss: Boolean): String =
- if (!allowPrecisionLoss) { exprName + "_deny_precision_loss" }
- else { exprName }
+ /**
+ * In ANSI mode, decimal Add, Subtract and Multiply map to Velox's checked_
functions, which throw
+ * on overflow instead of returning null.
+ */
+ override def getDecimalArithmeticExprName(
+ exprName: String,
+ original: BinaryArithmetic,
+ allowPrecisionLoss: Boolean): String = {
+ val name = if (ExpressionUtils.withAnsiEvalMode(original)) {
+ original match {
+ case _: Add => ExpressionNames.CHECKED_ADD
+ case _: Subtract => ExpressionNames.CHECKED_SUBTRACT
+ case _: Multiply => ExpressionNames.CHECKED_MULTIPLY
+ case _ => exprName
+ }
+ } else {
+ exprName
+ }
+ if (!allowPrecisionLoss) { name + "_deny_precision_loss" }
+ else { name }
+ }
/** Transform map_entries to Substrait. */
override def genMapEntriesTransformer(
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 9b7a14a51f..795e6a16c4 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
@@ -33,22 +33,38 @@ class ArithmeticAnsiValidateSuite extends
FunctionsValidateSuite {
.set(SQLConf.ANSI_ENABLED.key, "true")
}
+ private val maxDecimal = "CAST(%s AS DECIMAL(38,0))".format("9" * 38)
+ private val minDecimal = "CAST(-%s AS DECIMAL(38,0))".format("9" * 38)
+
+ // Reads `columns` from a table and evaluates `expr` on them, so the
arithmetic runs in Velox.
+ // With ANSI mode on, the overflow throws Velox's `error`. With ANSI mode
off, the result
+ // matches Spark: NULL for decimals and a wrapped value for integers.
+ private def checkOverflow(columns: String, expr: String, error: String):
Unit = {
+ withTempPath {
+ path =>
+ sql(s"SELECT $columns").write.parquet(path.getCanonicalPath)
+ withTempView("overflow_tab") {
+
spark.read.parquet(path.getCanonicalPath).createOrReplaceTempView("overflow_tab")
+ val query = s"SELECT $expr FROM overflow_tab"
+ withSQLConf(SQLConf.ANSI_ENABLED.key -> "true") {
+ val e = intercept[SparkException](sql(query).collect())
+ assert(e.getMessage.contains(error), e.getMessage)
+ }
+ withSQLConf(SQLConf.ANSI_ENABLED.key -> "false") {
+ runQueryAndCompare(query) {
+ checkGlutenPlan[ProjectExecTransformer]
+ }
+ }
+ }
+ }
+ }
+
test("add") {
runQueryAndCompare("SELECT int_field1 + 100 FROM datatab WHERE int_field1
IS NOT NULL") {
checkGlutenPlan[ProjectExecTransformer]
}
- val df = sql("SELECT 2147483647 + 1")
-
- if (isSparkVersionGE("4.0")) {
- intercept[SparkException] {
- df.collect()
- }
- } else {
- intercept[ArithmeticException] {
- df.collect()
- }
- }
+ checkOverflow("2147483647 AS a, 1 AS b", "a + b", "Arithmetic overflow")
}
test("subtract") {
@@ -62,16 +78,7 @@ class ArithmeticAnsiValidateSuite extends
FunctionsValidateSuite {
checkGlutenPlan[ProjectExecTransformer]
}
- val df = sql("SELECT 2147483647 + 1")
- if (isSparkVersionGE("4.0")) {
- intercept[SparkException] {
- df.collect()
- }
- } else {
- intercept[ArithmeticException] {
- df.collect()
- }
- }
+ checkOverflow("2147483647 AS a, 2 AS b", "a * b", "Arithmetic overflow")
}
test("divide") {
@@ -93,4 +100,106 @@ 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
+ checkOverflow(
+ s"$maxDecimal AS a, CAST(1 AS DECIMAL(38,0)) AS b",
+ "a + b",
+ "Decimal overflow in add")
+ }
+
+ 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
+ checkOverflow(
+ s"$minDecimal AS a, CAST(1 AS DECIMAL(38,0)) AS b",
+ "a - b",
+ "Decimal overflow in subtract")
+ }
+
+ test("decimal try_add") {
+ // Normal case should match Spark results
+ runQueryAndCompare(
+ "SELECT try_add(CAST(1.0 AS DECIMAL(10,2)), CAST(2.0 AS
DECIMAL(10,2)))") {
+ checkGlutenPlan[ProjectExecTransformer]
+ }
+ // Overflow should return null
+ runQueryAndCompare(
+ "SELECT try_add(CAST(99999999999999999999999999999999999999 AS
DECIMAL(38,0)), " +
+ "CAST(1 AS DECIMAL(38,0)))") {
+ checkGlutenPlan[ProjectExecTransformer]
+ }
+ }
+
+ test("decimal try_subtract") {
+ // Normal case should match Spark results
+ runQueryAndCompare(
+ "SELECT try_subtract(CAST(5.0 AS DECIMAL(10,2)), CAST(2.0 AS
DECIMAL(10,2)))") {
+ checkGlutenPlan[ProjectExecTransformer]
+ }
+ // Overflow should return null
+ runQueryAndCompare(
+ "SELECT try_subtract(CAST(-99999999999999999999999999999999999999 AS
DECIMAL(38,0)), " +
+ "CAST(1 AS DECIMAL(38,0)))") {
+ checkGlutenPlan[ProjectExecTransformer]
+ }
+ }
+
+ test("decimal multiply overflow") {
+ // Normal decimal multiply should succeed and match Spark results
+ runQueryAndCompare(
+ "SELECT CAST(2.0 AS DECIMAL(10,2)) * CAST(3.0 AS DECIMAL(10,2))") {
+ checkGlutenPlan[ProjectExecTransformer]
+ }
+
+ // Overflow: max DECIMAL(38,0) * 2
+ checkOverflow(
+ s"$maxDecimal AS a, CAST(2 AS DECIMAL(38,0)) AS b",
+ "a * b",
+ "Decimal overflow in multiply")
+ }
+
+ test("decimal try_multiply") {
+ // Normal case should match Spark results
+ runQueryAndCompare(
+ "SELECT try_multiply(CAST(2.0 AS DECIMAL(10,2)), CAST(3.0 AS
DECIMAL(10,2)))") {
+ checkGlutenPlan[ProjectExecTransformer]
+ }
+ // Overflow should return null
+ runQueryAndCompare(
+ "SELECT try_multiply(CAST(99999999999999999999999999999999999999 AS
DECIMAL(38,0)), " +
+ "CAST(2 AS DECIMAL(38,0)))") {
+ checkGlutenPlan[ProjectExecTransformer]
+ }
+ }
+
+ test("decimal overflow with allowPrecisionLoss disabled") {
+ // Uses checked_*_deny_precision_loss. A missing function would fail
validation and fall back
+ // to Spark, which also throws on overflow, so check that the query runs
on Velox first.
+ withSQLConf(SQLConf.DECIMAL_OPERATIONS_ALLOW_PREC_LOSS.key -> "false") {
+ runQueryAndCompare(
+ "SELECT CAST(1.5 AS DECIMAL(10,2)) + CAST(2.25 AS DECIMAL(10,2)), " +
+ "CAST(1.5 AS DECIMAL(10,2)) - CAST(2.25 AS DECIMAL(10,2)), " +
+ "CAST(1.5 AS DECIMAL(10,2)) * CAST(2.25 AS DECIMAL(10,2))") {
+ checkGlutenPlan[ProjectExecTransformer]
+ }
+
+ val columns = s"$maxDecimal AS max, $minDecimal AS min, " +
+ "CAST(1 AS DECIMAL(38,0)) AS one, CAST(2 AS DECIMAL(38,0)) AS two"
+ checkOverflow(columns, "max + one", "Decimal overflow in add")
+ checkOverflow(columns, "min - one", "Decimal overflow in subtract")
+ checkOverflow(columns, "max * two", "Decimal overflow in multiply")
+ }
+ }
}
diff --git
a/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala
b/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala
index 0141320030..1bbd92f3c5 100644
---
a/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala
+++
b/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala
@@ -280,10 +280,14 @@ trait SparkPlanExecApi {
GenericExpressionTransformer(substraitExprName, Seq(left, right), original)
}
- // Default: ignore allowPrecisionLoss and return exprName unchanged.
Non-Velox backends
- // (e.g. ClickHouse) do not use the _deny_precision_loss naming convention;
they handle
- // decimal precision through their own mechanisms. VeloxSparkPlanExecApi
overrides this.
- def getDecimalArithmeticExprName(exprName: String, allowPrecisionLoss:
Boolean): String = exprName
+ // Default: ignore 'original' and allowPrecisionLoss, and return exprName
unchanged. Non-Velox
+ // backends (e.g. ClickHouse) do not use the checked_ and
_deny_precision_loss naming
+ // conventions; they handle decimal overflow and precision through their own
mechanisms.
+ // VeloxSparkPlanExecApi overrides this.
+ def getDecimalArithmeticExprName(
+ exprName: String,
+ original: BinaryArithmetic,
+ allowPrecisionLoss: Boolean): String = exprName
/** Transform map_entries to Substrait. */
def genMapEntriesTransformer(
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 97e21a375b..9bfd90cdd7 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
@@ -671,6 +671,7 @@ object ExpressionConverter extends SQLConfHelper with
Logging {
case b: BinaryArithmetic if DecimalArithmeticUtil.isDecimalArithmetic(b)
=>
val exprName =
BackendsApiManager.getSparkPlanExecApiInstance.getDecimalArithmeticExprName(
substraitExprName,
+ b,
SparkShimLoader.getSparkShims.decimalAllowPrecisionLoss(b))
if (!BackendsApiManager.getSettings.transformCheckOverflow) {
GenericExpressionTransformer(
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
