Copilot commented on code in PR #12777:
URL: https://github.com/apache/gluten/pull/12777#discussion_r4079607606
##########
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)
Review Comment:
On Spark 4.x this suite does not override `spark.sql.ansi.enabled` (the
superclass only sets unrelated test settings), and Spark 4 defaults ANSI mode
to true. `DecimalCeilFloorTransformer` deliberately throws
`GlutenNotSupportException` whenever ANSI is enabled, so these queries will be
validated as unsupported and fall back to Spark;
`checkGlutenPlan[ProjectExecTransformer]` will then fail instead of exercising
the new native path. Run the offload assertions under `SQLConf.ANSI_ENABLED.key
-> "false"` (or move them to an explicitly non-ANSI suite), leaving the
overflow case as the ANSI test.
##########
gluten-substrait/src/main/scala/org/apache/gluten/expression/DecimalCeilFloorTransformer.scala:
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.gluten.expression
+
+import org.apache.gluten.backendsapi.BackendsApiManager
+import org.apache.gluten.exception.GlutenNotSupportException
+
+import org.apache.spark.sql.catalyst.expressions._
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.types.{DataType, DecimalType}
+
+/**
+ * Transformer for Spark `RoundCeil(decimal, scale)` and `RoundFloor(decimal,
scale)`. These power
+ * the 2-argument forms of `ceiling(x, scale)` / `floor(x, scale)` and
dispatch to the Velox
+ * `decimal_ceil` / `decimal_floor` special forms (substrait names `ceil` /
`floor`, remapped on the
+ * C++ side based on arity + decimal arg type).
+ *
+ * The output `DataType` is recomputed from the original Spark decimal input
type and the constant
+ * folded scale, matching Spark's `RoundBase.dataType` formula. Mirrors the
structure of
+ * `DecimalRoundTransformer`.
+ */
+case class DecimalCeilFloorTransformer(
+ substraitExprName: String,
+ child: ExpressionTransformer,
+ original: Expression,
+ scaleExpr: Expression)
+ extends BinaryExpressionTransformer {
+
+ // Velox's `decimal_ceil` / `decimal_floor` return NULL when the rounded
result exceeds the
+ // declared decimal precision, whereas Spark's `RoundBase` raises a
precision-overflow error
+ // under ANSI mode (e.g. DECIMAL(38, 0) at its maximum value rounded with a
negative scale).
+ // Offloading under ANSI would silently substitute NULL for that error, so
fall back to vanilla
+ // Spark and preserve the ANSI semantics. Under non-ANSI mode Spark also
returns NULL on
+ // overflow, matching Velox, so offloading is safe.
+ if (SQLConf.get.ansiEnabled) {
+ throw new GlutenNotSupportException(
+ s"${original.nodeName} on decimal is not offloaded under ANSI mode
because Velox returns " +
+ "NULL on precision overflow while Spark raises. Falling back to
Spark.")
Review Comment:
This guard rejects every decimal `RoundCeil`/`RoundFloor` while ANSI is
enabled, including scales/types whose output precision cannot overflow. In the
ANSI workflow, native ANSI execution is explicitly tested with
`spark.gluten.sql.ansiFallback.enabled=false`
(`.github/workflows/velox_backend_ansi.yml:185-187`), so these expressions
cannot execute natively and this does not address the `RoundCeil`/`RoundFloor`
ANSI items in #10134. Restrict the fallback to cases where the output precision
is capped and overflow is possible, or implement matching overflow errors in
Velox before rejecting the whole ANSI path.
--
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]
