From 3a464601de54f9fab6a362c4c9fa93747de4c65d Mon Sep 17 00:00:00 2001 From: Kazuyuki Tanimura Date: Mon, 30 Mar 2026 19:25:15 -0700 Subject: [PATCH 1/2] test: ceil and floor works correctly for Decimal128 --- native/spark-expr/src/math_funcs/ceil.rs | 7 +++--- native/spark-expr/src/math_funcs/floor.rs | 7 +++--- .../scala/org/apache/comet/serde/math.scala | 24 ------------------- .../apache/comet/CometExpressionSuite.scala | 4 ++++ 4 files changed, 10 insertions(+), 32 deletions(-) diff --git a/native/spark-expr/src/math_funcs/ceil.rs b/native/spark-expr/src/math_funcs/ceil.rs index b5bc1c31bd..e1f0e69e5f 100644 --- a/native/spark-expr/src/math_funcs/ceil.rs +++ b/native/spark-expr/src/math_funcs/ceil.rs @@ -164,7 +164,6 @@ mod test { // https://github.com/apache/datafusion-comet/issues/1729 #[test] - #[ignore] fn test_ceil_decimal128_array() -> Result<()> { let array = Decimal128Array::from(vec![ Some(12345), // 123.45 @@ -178,9 +177,9 @@ mod test { unreachable!() }; let expected = Decimal128Array::from(vec![ - Some(12400), // 124.00 - Some(12500), // 125.00 - Some(-12900), // -129.00 + Some(124), // 124.00 + Some(125), // 125.00 + Some(-129), // -129.00 None, ]) .with_precision_and_scale(5, 2)?; diff --git a/native/spark-expr/src/math_funcs/floor.rs b/native/spark-expr/src/math_funcs/floor.rs index 2028000e19..5cb1f16e3b 100644 --- a/native/spark-expr/src/math_funcs/floor.rs +++ b/native/spark-expr/src/math_funcs/floor.rs @@ -164,7 +164,6 @@ mod test { // https://github.com/apache/datafusion-comet/issues/1729 #[test] - #[ignore] fn test_floor_decimal128_array() -> Result<()> { let array = Decimal128Array::from(vec![ Some(12345), // 123.45 @@ -178,9 +177,9 @@ mod test { unreachable!() }; let expected = Decimal128Array::from(vec![ - Some(12300), // 123.00 - Some(12500), // 125.00 - Some(-13000), // -130.00 + Some(123), // 123.00 + Some(125), // 125.00 + Some(-130), // -130.00 None, ]) .with_precision_and_scale(5, 2)?; diff --git a/spark/src/main/scala/org/apache/comet/serde/math.scala b/spark/src/main/scala/org/apache/comet/serde/math.scala index 5a0393142a..ddc953857d 100644 --- a/spark/src/main/scala/org/apache/comet/serde/math.scala +++ b/spark/src/main/scala/org/apache/comet/serde/math.scala @@ -38,18 +38,6 @@ object CometAtan2 extends CometExpressionSerde[Atan2] { } object CometCeil extends CometExpressionSerde[Ceil] { - - override def getSupportLevel(expr: Ceil): SupportLevel = { - expr.child.dataType match { - case _: DecimalType => - Incompatible( - Some( - "Incorrect results for Decimal type inputs" + - " (https://github.com/apache/datafusion-comet/issues/1729)")) - case _ => Compatible() - } - } - override def convert( expr: Ceil, inputs: Seq[Attribute], @@ -70,18 +58,6 @@ object CometCeil extends CometExpressionSerde[Ceil] { } object CometFloor extends CometExpressionSerde[Floor] { - - override def getSupportLevel(expr: Floor): SupportLevel = { - expr.child.dataType match { - case _: DecimalType => - Incompatible( - Some( - "Incorrect results for Decimal type inputs" + - " (https://github.com/apache/datafusion-comet/issues/1729)")) - case _ => Compatible() - } - } - override def convert( expr: Floor, inputs: Seq[Attribute], diff --git a/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala b/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala index 9fdd5a6777..8aa88d615b 100644 --- a/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala +++ b/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala @@ -1487,6 +1487,10 @@ class CometExpressionSuite extends CometTestBase with AdaptiveSparkPlanHelper { checkSparkAnswerAndOperator(s"SELECT floor(cast(${n} as decimal(38, 18))) FROM tbl") checkSparkAnswerAndOperator(s"SELECT floor(cast(${n} as decimal(20, 0))) FROM tbl") } + for (n <- Seq("123.45", "125.00", "-129.99")) { + checkSparkAnswerAndOperator(s"SELECT ceil(cast(${n} as decimal(5, 2))) FROM tbl") + checkSparkAnswerAndOperator(s"SELECT floor(cast(${n} as decimal(5, 2))) FROM tbl") + } } } } From ae9fab27264a4fe0b5e5935e11b240f65ce21fa5 Mon Sep 17 00:00:00 2001 From: Kazuyuki Tanimura Date: Mon, 30 Mar 2026 23:13:22 -0700 Subject: [PATCH 2/2] doc --- docs/source/user-guide/latest/compatibility.md | 2 -- docs/source/user-guide/latest/expressions.md | 4 ++-- spark/src/test/resources/sql-tests/expressions/math/ceil.sql | 1 - spark/src/test/resources/sql-tests/expressions/math/floor.sql | 1 - .../test/scala/org/apache/comet/CometExpressionSuite.scala | 4 +--- 5 files changed, 3 insertions(+), 9 deletions(-) diff --git a/docs/source/user-guide/latest/compatibility.md b/docs/source/user-guide/latest/compatibility.md index 3ec6656187..a5a424e858 100644 --- a/docs/source/user-guide/latest/compatibility.md +++ b/docs/source/user-guide/latest/compatibility.md @@ -86,8 +86,6 @@ the [Comet Supported Expressions Guide](expressions.md) for more information on ### Math Expressions -- **Ceil, Floor**: Incorrect results for Decimal type inputs. - [#1729](https://github.com/apache/datafusion-comet/issues/1729) - **Tan**: `tan(-0.0)` produces `0.0` instead of `-0.0`. [#1897](https://github.com/apache/datafusion-comet/issues/1897) diff --git a/docs/source/user-guide/latest/expressions.md b/docs/source/user-guide/latest/expressions.md index c3aca6f67a..ed8083c0e1 100644 --- a/docs/source/user-guide/latest/expressions.md +++ b/docs/source/user-guide/latest/expressions.md @@ -130,14 +130,14 @@ Expressions that are not Spark-compatible will fall back to Spark by default and | Atan | `atan` | Yes | | | Atan2 | `atan2` | Yes | | | BRound | `bround` | Yes | | -| Ceil | `ceil` | No | Incorrect results for Decimal type inputs ([#1729](https://github.com/apache/datafusion-comet/issues/1729)) | +| Ceil | `ceil` | No | | | Cos | `cos` | Yes | | | Cosh | `cosh` | Yes | | | Cot | `cot` | Yes | | | Divide | `/` | Yes | | | Exp | `exp` | Yes | | | Expm1 | `expm1` | Yes | | -| Floor | `floor` | No | Incorrect results for Decimal type inputs ([#1729](https://github.com/apache/datafusion-comet/issues/1729)) | +| Floor | `floor` | No | | | Hex | `hex` | Yes | | | IntegralDivide | `div` | Yes | | | IsNaN | `isnan` | Yes | | diff --git a/spark/src/test/resources/sql-tests/expressions/math/ceil.sql b/spark/src/test/resources/sql-tests/expressions/math/ceil.sql index c11c42bedb..fade75d28a 100644 --- a/spark/src/test/resources/sql-tests/expressions/math/ceil.sql +++ b/spark/src/test/resources/sql-tests/expressions/math/ceil.sql @@ -15,7 +15,6 @@ -- specific language governing permissions and limitations -- under the License. --- Config: spark.comet.expression.Ceil.allowIncompatible=true -- ConfigMatrix: parquet.enable.dictionary=false,true statement diff --git a/spark/src/test/resources/sql-tests/expressions/math/floor.sql b/spark/src/test/resources/sql-tests/expressions/math/floor.sql index 62fa2a4045..3960002846 100644 --- a/spark/src/test/resources/sql-tests/expressions/math/floor.sql +++ b/spark/src/test/resources/sql-tests/expressions/math/floor.sql @@ -15,7 +15,6 @@ -- specific language governing permissions and limitations -- under the License. --- Config: spark.comet.expression.Floor.allowIncompatible=true -- ConfigMatrix: parquet.enable.dictionary=false,true statement diff --git a/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala b/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala index 8aa88d615b..739ec66f3d 100644 --- a/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala +++ b/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala @@ -1445,9 +1445,7 @@ class CometExpressionSuite extends CometTestBase with AdaptiveSparkPlanHelper { test("ceil and floor") { Seq("true", "false").foreach { dictionary => withSQLConf( - "parquet.enable.dictionary" -> dictionary, - CometConf.getExprAllowIncompatConfigKey(classOf[Ceil]) -> "true", - CometConf.getExprAllowIncompatConfigKey(classOf[Floor]) -> "true") { + "parquet.enable.dictionary" -> dictionary) { withParquetTable( (-5 until 5).map(i => (i.toDouble + 0.3, i.toDouble + 0.8)), "tbl",