From 589b640999b07e6f357896e0350f5d184910173f Mon Sep 17 00:00:00 2001 From: minni31 Date: Fri, 14 Aug 2026 18:52:07 +0000 Subject: [PATCH] [VL] Offload 2-arg decimal ceiling/floor to Velox native execution Spark's `ceiling(x, scale)` / `floor(x, scale)` on decimal inputs produce `RoundCeil(decimal, scale)` / `RoundFloor(decimal, scale)`. These were mapped to the substrait `ceil` / `floor` function names but had no native execution path, so the 2-arg decimal form fell back to vanilla Spark. This wires them to the Velox `decimal_ceil` / `decimal_floor` special forms: - SubstraitParser: `mapToVeloxFunction` gains a `numArgs` parameter and remaps 2-arg `ceil` / `floor` on decimals to `decimal_ceil` / `decimal_floor`. Unary `ceil(decimal)` / `floor(decimal)` keep their existing name. - DecimalCeilFloorTransformer: new transformer mirroring DecimalRoundTransformer; recomputes the output DecimalType from the input type and constant-folded scale (matching Spark's RoundBase.dataType) and emits the scale as a literal. - ExpressionConverter: route decimal RoundCeil / RoundFloor through the new transformer. Addresses the RoundCeil / RoundFloor items in #10134. --- .../MathFunctionsValidateSuite.scala | 24 +++++++ cpp/velox/substrait/SubstraitParser.cc | 11 +++- cpp/velox/substrait/SubstraitParser.h | 7 ++- .../DecimalCeilFloorTransformer.scala | 63 +++++++++++++++++++ .../expression/ExpressionConverter.scala | 12 ++++ 5 files changed, 114 insertions(+), 3 deletions(-) create mode 100644 gluten-substrait/src/main/scala/org/apache/gluten/expression/DecimalCeilFloorTransformer.scala 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 81a9ad5cdba..3e199e4a4d5 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 @@ -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. + 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("cos") { runQueryAndCompare("SELECT cos(l_orderkey) from lineitem limit 1") { checkGlutenPlan[ProjectExecTransformer] diff --git a/cpp/velox/substrait/SubstraitParser.cc b/cpp/velox/substrait/SubstraitParser.cc index 54bf8d4f243..6a28d11a531 100644 --- a/cpp/velox/substrait/SubstraitParser.cc +++ b/cpp/velox/substrait/SubstraitParser.cc @@ -254,10 +254,10 @@ std::string SubstraitParser::findVeloxFunction( break; } } - return mapToVeloxFunction(funcName, isDecimal); + return mapToVeloxFunction(funcName, isDecimal, types.size()); } -std::string SubstraitParser::mapToVeloxFunction(const std::string& substraitFunction, bool isDecimal) { +std::string SubstraitParser::mapToVeloxFunction(const std::string& substraitFunction, bool isDecimal, size_t numArgs) { auto it = substraitVeloxFunctionMap_.find(substraitFunction); if (isDecimal) { if (substraitFunction == "lt" || substraitFunction == "lte" || substraitFunction == "gt" || @@ -267,6 +267,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; + } } if (it != substraitVeloxFunctionMap_.end()) { return it->second; diff --git a/cpp/velox/substrait/SubstraitParser.h b/cpp/velox/substrait/SubstraitParser.h index 1122f3dc9b6..d9cd1ef143d 100644 --- a/cpp/velox/substrait/SubstraitParser.h +++ b/cpp/velox/substrait/SubstraitParser.h @@ -80,7 +80,12 @@ class SubstraitParser { static std::string findVeloxFunction(const std::unordered_map& functionMap, uint64_t id); /// Map the Substrait function keyword into Velox function keyword. - static std::string mapToVeloxFunction(const std::string& substraitFunction, bool isDecimal); + /// `numArgs` is the number of substrait arguments observed in the call + /// signature. It is used to disambiguate overloads such as unary + /// `ceil(decimal)` (mapped to `ceil`) vs. 2-arg `ceil(decimal, int)` + /// (mapped to the special form `decimal_ceil`). Pass 0 when arity is + /// unknown -- in that case arity-sensitive remappings are skipped. + static std::string mapToVeloxFunction(const std::string& substraitFunction, bool isDecimal, size_t numArgs = 0); /// @brief Return whether a config is set as true in AdvancedExtension /// optimization. diff --git a/gluten-substrait/src/main/scala/org/apache/gluten/expression/DecimalCeilFloorTransformer.scala b/gluten-substrait/src/main/scala/org/apache/gluten/expression/DecimalCeilFloorTransformer.scala new file mode 100644 index 00000000000..e14018dd7b0 --- /dev/null +++ b/gluten-substrait/src/main/scala/org/apache/gluten/expression/DecimalCeilFloorTransformer.scala @@ -0,0 +1,63 @@ +/* + * 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.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 { + + private val toScale: Int = { + val evaluated = scaleExpr.eval(EmptyRow) + if (evaluated == null) { + throw new GlutenNotSupportException( + s"Scale expression evaluated to null for ${original.nodeName}. Falling back to Spark.") + } + evaluated.asInstanceOf[Int] + } + + override val dataType: DataType = original.children.head.dataType match { + case decimalType: DecimalType => + BackendsApiManager.getSparkPlanExecApiInstance.genDecimalRoundExpressionOutput( + decimalType, + toScale) + case other => + throw new GlutenNotSupportException( + s"Decimal type is expected for ${original.nodeName} but received ${other.typeName}.") + } + + override def left: ExpressionTransformer = child + override def right: ExpressionTransformer = LiteralTransformer(toScale) +} 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 b70bbc87994..7624bdaadfa 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 @@ -394,6 +394,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, + rc.scale) + case rf: RoundFloor if rf.child.dataType.isInstanceOf[DecimalType] => + DecimalCeilFloorTransformer( + substraitExprName, + replaceWithExpressionTransformer0(rf.child, attributeSeq, expressionsMap), + rf, + rf.scale) case t: ToUnixTimestamp => BackendsApiManager.getSparkPlanExecApiInstance.genToUnixTimestampTransformer( substraitExprName,