minni31 commented on code in PR #12777:
URL: https://github.com/apache/gluten/pull/12777#discussion_r4141175166
##########
gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala:
##########
@@ -390,6 +390,18 @@ object ExpressionConverter extends SQLConfHelper with
Logging {
substraitExprName,
replaceWithExpressionTransformer0(r.child, attributeSeq,
expressionsMap),
r)
+ case rc: RoundCeil if rc.child.dataType.isInstanceOf[DecimalType] =>
+ DecimalCeilFloorTransformer(
+ substraitExprName,
+ replaceWithExpressionTransformer0(rc.child, attributeSeq,
expressionsMap),
+ rc,
Review Comment:
Fixed in 9075346d4. The `RoundCeil`/`RoundFloor` decimal specialization has
been removed from the shared `ExpressionConverter` and moved into Velox's
existing `extraExpressionConverter` hook. Bolt and ClickHouse now retain their
prior generic conversion/fallback behavior, while only Velox constructs
`DecimalCeilFloorTransformer` and emits the special-form path.
##########
backends-velox/src/test/scala/org/apache/gluten/functions/MathFunctionsValidateSuite.scala:
##########
@@ -122,6 +123,51 @@ class MathFunctionsValidateSuite extends
FunctionsValidateSuite {
}
}
+ test("2-arg ceiling / floor on decimals (RoundCeil / RoundFloor)") {
+ // The 2-argument ceiling/floor SQL forms only exist on Spark 4.0+; on
Spark 3.4/3.5 they are
+ // invalid and would fail during analysis, so skip the test on those
profiles.
+ assume(SparkVersionUtil.gteSpark40)
+ // The 2-arg forms produce Spark RoundCeil / RoundFloor and dispatch to
the Velox
+ // decimal_ceil / decimal_floor special forms. The projection is native
only when the
+ // expression offloads, so checkGlutenPlan[ProjectExecTransformer] doubles
as an offload
+ // assertion; runQueryAndCompare additionally validates results against
vanilla Spark.
+ runQueryAndCompare(
+ "SELECT ceiling(cast(l_quantity as decimal(12, 2)), 1) FROM lineitem
limit 10") {
+ checkGlutenPlan[ProjectExecTransformer]
+ }
+ runQueryAndCompare(
+ "SELECT floor(cast(l_quantity as decimal(12, 2)), 1) FROM lineitem limit
10") {
+ checkGlutenPlan[ProjectExecTransformer]
+ }
+ // Negative scale rounds to the left of the decimal point.
+ runQueryAndCompare(
+ "SELECT ceiling(cast(l_extendedprice as decimal(20, 4)), -2) FROM
lineitem limit 10") {
+ checkGlutenPlan[ProjectExecTransformer]
+ }
+ runQueryAndCompare(
+ "SELECT floor(cast(l_extendedprice as decimal(20, 4)), -2) FROM lineitem
limit 10") {
+ checkGlutenPlan[ProjectExecTransformer]
+ }
+ }
+
+ test("2-arg ceiling / floor on decimals falls back under ANSI overflow") {
+ assume(SparkVersionUtil.gteSpark40)
+ // Velox's decimal_ceil / decimal_floor return NULL when the rounded
result overflows the
+ // declared precision, whereas Spark raises under ANSI mode. Even with
ANSI fallback disabled
+ // (native ANSI execution opted in), this op must fall back to Spark so
the overflow raises
+ // instead of silently producing NULL. DECIMAL(38, 0) at its maximum value
rounded with a
+ // negative scale overflows the 38-digit output precision.
+ withSQLConf(
+ SQLConf.ANSI_ENABLED.key -> "true",
+ GlutenConfig.GLUTEN_ANSI_FALLBACK_ENABLED.key -> "false") {
+ val overflowSql =
+ "SELECT ceiling(cast('99999999999999999999999999999999999999' as
decimal(38, 0)), -1)"
+ intercept[Exception] {
+ spark.sql(overflowSql).collect()
Review Comment:
This is covered by the current test. Before entering `intercept`, it
materializes `df.queryExecution.executedPlan` and asserts that
`ProjectExecTransformer` is absent, so analysis or an unhandled plan-conversion
failure cannot satisfy the assertion. The intercepted exception from
`collect()` is then required to be an `ArithmeticException` directly or through
its cause chain, which handles Spark 4's direct `SparkArithmeticException` and
Spark 3.x's `SparkException` wrapper without accepting an arbitrary failure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
