This is an automated email from the ASF dual-hosted git repository.
chengchengjin pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-gluten.git
The following commit(s) were added to refs/heads/main by this push:
new c497071af7 [VL] Fix decimal arithmetic offload (#10689)
c497071af7 is described below
commit c497071af79ff2c93319d075529a8bd401e6d96b
Author: Zhen Li <[email protected]>
AuthorDate: Fri Sep 12 18:57:14 2025 +0800
[VL] Fix decimal arithmetic offload (#10689)
---
.../gluten/backendsapi/velox/VeloxBackend.scala | 2 --
.../backendsapi/velox/VeloxSparkPlanExecApi.scala | 15 +++++----------
.../gluten/functions/MathFunctionsValidateSuite.scala | 19 +++++++++++++++++++
.../gluten/backendsapi/BackendSettingsApi.scala | 2 --
.../apache/gluten/backendsapi/SparkPlanExecApi.scala | 2 ++
.../gluten/expression/ExpressionConverter.scala | 12 +++++++-----
.../apache/gluten/utils/DecimalArithmeticUtil.scala | 11 -----------
7 files changed, 33 insertions(+), 30 deletions(-)
diff --git
a/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxBackend.scala
b/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxBackend.scala
index 31b5a56f4f..266de9cc06 100644
---
a/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxBackend.scala
+++
b/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxBackend.scala
@@ -523,8 +523,6 @@ object VeloxBackendSettings extends BackendSettingsApi {
override def staticPartitionWriteOnly(): Boolean = true
- override def allowDecimalArithmetic: Boolean = true
-
override def enableNativeWriteFiles(): Boolean = {
GlutenConfig.get.enableNativeWriter.getOrElse(
SparkShimLoader.getSparkShims.enableNativeWriteFilesByDefault()
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 a822da014f..463514fb5c 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
@@ -149,16 +149,6 @@ class VeloxSparkPlanExecApi extends SparkPlanExecApi {
ExpressionMappings.expressionsMap(classOf[TryEval]),
Seq(GenericExpressionTransformer(checkArithmeticExprName, Seq(left,
right), original)),
original)
- } else if (
- left.dataType.isInstanceOf[DecimalType] &&
- right.dataType.isInstanceOf[DecimalType] &&
- !SQLConf.get.decimalOperationsAllowPrecisionLoss
- ) {
- if (SparkShimLoader.getSparkShims.withAnsiEvalMode(original)) {
- throw new GlutenNotSupportException(s"$substraitExprName with ansi
mode is not supported")
- }
- val newName = substraitExprName + "_deny_precision_loss"
- GenericExpressionTransformer(newName, Seq(left, right), original)
} else if (SparkShimLoader.getSparkShims.withAnsiEvalMode(original)) {
GenericExpressionTransformer(checkArithmeticExprName, Seq(left, right),
original)
} else {
@@ -166,6 +156,11 @@ class VeloxSparkPlanExecApi extends SparkPlanExecApi {
}
}
+ override def getDecimalArithmeticExprName(exprName: String): String = if (
+ !SQLConf.get.decimalOperationsAllowPrecisionLoss
+ ) { exprName + "_deny_precision_loss" }
+ else { exprName }
+
/** Transform map_entries to Substrait. */
override def genMapEntriesTransformer(
substraitExprName: String,
diff --git
a/backends-velox/src/test/scala/org/apache/gluten/functions/MathFunctionsValidateSuite.scala
b/backends-velox/src/test/scala/org/apache/gluten/functions/MathFunctionsValidateSuite.scala
index 6657d3d99f..3840ccc422 100644
---
a/backends-velox/src/test/scala/org/apache/gluten/functions/MathFunctionsValidateSuite.scala
+++
b/backends-velox/src/test/scala/org/apache/gluten/functions/MathFunctionsValidateSuite.scala
@@ -401,4 +401,23 @@ abstract class MathFunctionsValidateSuite extends
FunctionsValidateSuite {
}
checkLengthAndPlan(df, 1)
}
+
+ test("decimal arithmetic") {
+ withTempView("t") {
+ sql("""
+ |SELECT
+ |CAST('1234567890123456789012345.12345678901' AS DECIMAL(38,11))
AS a,
+ |CAST('1234567890123456789012345.02345678901' AS DECIMAL(38,11))
AS b;""".stripMargin)
+ .createOrReplaceTempView("t")
+
+ Seq("true", "false").foreach {
+ enabled =>
+ withSQLConf("spark.sql.decimalOperations.allowPrecisionLoss" ->
enabled) {
+ runQueryAndCompare("SELECT a - b, a + b, a * b, a / b FROM t") {
+ checkGlutenOperatorMatch[ProjectExecTransformer]
+ }
+ }
+ }
+ }
+ }
}
diff --git
a/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/BackendSettingsApi.scala
b/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/BackendSettingsApi.scala
index a8c2d75fe7..2d95821fd3 100644
---
a/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/BackendSettingsApi.scala
+++
b/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/BackendSettingsApi.scala
@@ -105,8 +105,6 @@ trait BackendSettingsApi {
def rescaleDecimalArithmetic: Boolean = false
- def allowDecimalArithmetic: Boolean = true
-
/**
* After https://github.com/apache/spark/pull/36698, every arithmetic should
report the accurate
* result decimal type and implement `CheckOverflow` by itself. <p/>
Regardless of whether there
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 e4de765672..addfd1800a 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
@@ -242,6 +242,8 @@ trait SparkPlanExecApi {
GenericExpressionTransformer(substraitExprName, Seq(left, right), original)
}
+ def getDecimalArithmeticExprName(exprName: String): String = exprName
+
/** Transform map_entries to Substrait. */
def genMapEntriesTransformer(
substraitExprName: String,
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 4c101e638b..ceee7d4799 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
@@ -541,8 +541,9 @@ object ExpressionConverter extends SQLConfHelper with
Logging {
case CheckOverflow(b: BinaryArithmetic, decimalType, _)
if !BackendsApiManager.getSettings.transformCheckOverflow &&
DecimalArithmeticUtil.isDecimalArithmetic(b) =>
- DecimalArithmeticUtil.checkAllowDecimalArithmetic()
- val arithmeticExprName = getAndCheckSubstraitName(b, expressionsMap)
+ val arithmeticExprName =
+
BackendsApiManager.getSparkPlanExecApiInstance.getDecimalArithmeticExprName(
+ getAndCheckSubstraitName(b, expressionsMap))
val left =
replaceWithExpressionTransformer0(b.left, attributeSeq,
expressionsMap)
val right =
@@ -558,17 +559,18 @@ object ExpressionConverter extends SQLConfHelper with
Logging {
"CheckOverflowInTableInsert is used in ANSI mode, but Gluten does
not support ANSI mode."
)
case b: BinaryArithmetic if DecimalArithmeticUtil.isDecimalArithmetic(b)
=>
- DecimalArithmeticUtil.checkAllowDecimalArithmetic()
+ val exprName =
BackendsApiManager.getSparkPlanExecApiInstance.getDecimalArithmeticExprName(
+ substraitExprName)
if (!BackendsApiManager.getSettings.transformCheckOverflow) {
GenericExpressionTransformer(
- substraitExprName,
+ exprName,
expr.children.map(replaceWithExpressionTransformer0(_,
attributeSeq, expressionsMap)),
expr
)
} else {
// Without the rescale and remove cast, result is right for high
version Spark,
// but performance regression in velox
- genRescaleDecimalTransformer(substraitExprName, b, attributeSeq,
expressionsMap)
+ genRescaleDecimalTransformer(exprName, b, attributeSeq,
expressionsMap)
}
case n: NaNvl =>
BackendsApiManager.getSparkPlanExecApiInstance.genNaNvlTransformer(
diff --git
a/gluten-substrait/src/main/scala/org/apache/gluten/utils/DecimalArithmeticUtil.scala
b/gluten-substrait/src/main/scala/org/apache/gluten/utils/DecimalArithmeticUtil.scala
index 91ecc90098..df5ed47e83 100644
---
a/gluten-substrait/src/main/scala/org/apache/gluten/utils/DecimalArithmeticUtil.scala
+++
b/gluten-substrait/src/main/scala/org/apache/gluten/utils/DecimalArithmeticUtil.scala
@@ -16,9 +16,7 @@
*/
package org.apache.gluten.utils
-import org.apache.gluten.backendsapi.BackendsApiManager
import org.apache.gluten.exception.GlutenNotSupportException
-import org.apache.gluten.expression.ExpressionConverter.conf
import org.apache.gluten.sql.shims.SparkShimLoader
import org.apache.spark.sql.catalyst.expressions.{Add, BinaryArithmetic, Cast,
Divide, Expression, Literal, Multiply, Pmod, PromotePrecision, Remainder,
Subtract}
@@ -221,13 +219,4 @@ object DecimalArithmeticUtil {
val widerType = SparkShimLoader.getSparkShims.widerDecimalType(left, right)
widerType.equals(wider)
}
-
- def checkAllowDecimalArithmetic(): Unit = {
- // PrecisionLoss=false: velox not support
- if (!BackendsApiManager.getSettings.allowDecimalArithmetic) {
- throw new GlutenNotSupportException(
- s"Not support ${SQLConf.DECIMAL_OPERATIONS_ALLOW_PREC_LOSS.key} " +
- s"${conf.decimalOperationsAllowPrecisionLoss} mode")
- }
- }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
