Copilot commented on code in PR #12777:
URL: https://github.com/apache/gluten/pull/12777#discussion_r4022710343
##########
backends-velox/src/test/scala/org/apache/gluten/functions/MathFunctionsValidateSuite.scala:
##########
@@ -122,6 +122,30 @@ class MathFunctionsValidateSuite extends
FunctionsValidateSuite {
}
}
+ test("2-arg ceiling / floor on decimals (RoundCeil / RoundFloor)") {
+ // 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.
Review Comment:
This suite is shared by the Spark 3.4/3.5/4.0/4.1 profiles (the supported
profiles are listed in `pom.xml:982`), but the repository's scale-argument
CEIL/FLOOR SQL cases exist only under `gluten-ut/spark40` and `spark41`. On
Spark 3.4/3.5 these two-argument calls are not valid SQL and this test will
fail during analysis; gate it with `SparkVersionUtil.gteSpark40` or move it to
a Spark 4-specific suite.
##########
cpp/velox/substrait/SubstraitParser.cc:
##########
@@ -287,6 +287,13 @@ std::string SubstraitParser::mapToVeloxFunction(const
std::string& substraitFunc
if (substraitFunction == "round") {
return "decimal_round";
}
+ // Spark RoundCeil / RoundFloor are emitted with substrait names "ceil"
+ // and "floor" but require dispatch to the 2-arg decimal special forms.
+ // The unary forms `ceil(decimal)` / `floor(decimal)` keep their original
+ // name (handled by simple-function registration).
+ if (numArgs == 2 && (substraitFunction == "ceil" || substraitFunction ==
"floor")) {
+ return "decimal_" + substraitFunction;
Review Comment:
This remap changes Spark's overflow behavior for decimal directional
rounding. Velox's `decimal_ceil`/`decimal_floor` special forms return a NULL
value when the rounded result exceeds the declared decimal precision, while
Spark's `RoundBase` decimal path raises a precision-overflow exception (for
example, `DECIMAL(38,0)` at its maximum value rounded with scale `-1`). Since
this branch offloads every 2-arg decimal call, it can silently return NULL
instead of the Spark/ANSI error; please either provide matching overflow
handling or retain overflow-prone cases on Spark and add a regression test.
--
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]
