This is an automated email from the ASF dual-hosted git repository.
andygrove pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git
The following commit(s) were added to refs/heads/main by this push:
new 5372d76148 fix: report FromUnixTime non-default format as Unsupported
(#4847)
5372d76148 is described below
commit 5372d7614836abca5a2c414945154d399c0fdcef
Author: Andy Grove <[email protected]>
AuthorDate: Wed Jul 8 08:16:07 2026 -0600
fix: report FromUnixTime non-default format as Unsupported (#4847)
---
.../scala/org/apache/comet/serde/unixtime.scala | 32 ++++++++++++++++------
.../org/apache/comet/CometExpressionSuite.scala | 7 +++++
.../org/apache/spark/sql/CometCollationSuite.scala | 5 +++-
.../org/apache/spark/sql/CometCollationSuite.scala | 5 +++-
4 files changed, 38 insertions(+), 11 deletions(-)
diff --git a/spark/src/main/scala/org/apache/comet/serde/unixtime.scala
b/spark/src/main/scala/org/apache/comet/serde/unixtime.scala
index 467058ccea..256baffa95 100644
--- a/spark/src/main/scala/org/apache/comet/serde/unixtime.scala
+++ b/spark/src/main/scala/org/apache/comet/serde/unixtime.scala
@@ -29,21 +29,35 @@ import
org.apache.comet.serde.QueryPlanSerde.{exprToProtoInternal, optExprWithFa
// https://github.com/apache/datafusion/issues/16594
object CometFromUnixTime extends CometExpressionSerde[FromUnixTime] with
CodegenDispatchFallback {
- private val collationReason = DatetimeCollation.reason("from_unixtime")
+ // Applies even to the default format: Comet executes natively but
DataFusion's valid timestamp
+ // range differs from Spark, so results can differ outside that range.
+ private val timestampRangeReason =
+ "DataFusion's valid timestamp range differs from Spark" +
+ " (https://github.com/apache/datafusion/issues/16594)"
+ // The native (DataFusion) path covers only the default pattern; documented
as an unsupported
+ // limitation of that path rather than an incompatibility (see #4575).
private val formatReason =
- "Only supports the default datetime format pattern `yyyy-MM-dd HH:mm:ss`."
+
- " DataFusion's valid timestamp range differs from Spark" +
- " (https://github.com/apache/datafusion/issues/16594)"
+ "Only the default datetime format pattern `yyyy-MM-dd HH:mm:ss` is
supported"
override def getIncompatibleReasons(): Seq[String] =
- Seq(formatReason) ++ DatetimeCollation.incompatibleReasons("from_unixtime")
+ Seq(timestampRangeReason)
+ override def getUnsupportedReasons(): Seq[String] =
+ Seq(formatReason)
+
+ // A non-default format has no native (DataFusion) path, so it is
`Unsupported`. Because
+ // `CodegenDispatchFallback` is mixed in, an `Unsupported` result still
stays in the Comet
+ // pipeline via JVM codegen dispatch (Spark's own `doGenCode`) rather than
falling back to Spark.
+ //
+ // Unlike the other datetime expressions, from_unixtime needs no collation
guard: a collation can
+ // only appear on the format argument, and any collated format is a
non-default format, which is
+ // already `Unsupported` here.
override def getSupportLevel(expr: FromUnixTime): SupportLevel = {
- if (DatetimeCollation.hasNonDefaultCollation(expr)) {
- Incompatible(Some(collationReason))
+ if (expr.format != Literal(TimestampFormatter.defaultPattern)) {
+ Unsupported(Some(formatReason))
} else {
- Incompatible(Some(formatReason))
+ Incompatible(Some(timestampRangeReason))
}
}
@@ -60,7 +74,7 @@ object CometFromUnixTime extends
CometExpressionSerde[FromUnixTime] with Codegen
val timeZone = exprToProtoInternal(Literal(expr.timeZoneId.orNull),
inputs, binding)
if (expr.format != Literal(TimestampFormatter.defaultPattern)) {
- withFallbackReason(expr, "Datetime pattern format is unsupported")
+ withFallbackReason(expr, formatReason)
None
} else if (secExpr.isDefined && formatExpr.isDefined) {
val timestampExpr =
diff --git a/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala
b/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala
index 33bd58c55e..d6955fb78b 100644
--- a/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala
+++ b/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala
@@ -1860,6 +1860,13 @@ class CometExpressionSuite extends CometTestBase with
AdaptiveSparkPlanHelper {
// After fixing these issues, change checkSparkAnswer to
checkSparkAnswerAndOperator
checkSparkAnswer(s"SELECT from_unixtime(_5, 'yyyy') FROM $table
$where")
checkSparkAnswer(s"SELECT from_unixtime(_8, 'yyyy') FROM $table
$where")
+ // A non-default format is Unsupported (no native DataFusion
path), but at the default
+ // allowIncompatible=false it stays a Comet operator via
CodegenDispatchFallback
+ // (Spark's own codegen) rather than falling back to Spark. See
#4575.
+ withSQLConf(
+ CometConf.getExprAllowIncompatConfigKey(classOf[FromUnixTime])
-> "false") {
+ checkSparkAnswerAndOperator(s"SELECT from_unixtime(_5, 'yyyy')
FROM $table $where")
+ }
withSQLConf(SESSION_LOCAL_TIMEZONE.key -> "Asia/Kathmandu") {
checkSparkAnswerAndOperator(s"SELECT from_unixtime(_5) FROM
$table $where")
checkSparkAnswerAndOperator(s"SELECT from_unixtime(_8) FROM
$table $where")
diff --git
a/spark/src/test/spark-4.0/org/apache/spark/sql/CometCollationSuite.scala
b/spark/src/test/spark-4.0/org/apache/spark/sql/CometCollationSuite.scala
index 4ac077d240..40e1d270cb 100644
--- a/spark/src/test/spark-4.0/org/apache/spark/sql/CometCollationSuite.scala
+++ b/spark/src/test/spark-4.0/org/apache/spark/sql/CometCollationSuite.scala
@@ -306,9 +306,12 @@ class CometCollationSuite extends CometTestBase {
}
test("from_unixtime rejects non-UTF8_BINARY collated format (issue #4646)") {
+ // A collation can only appear on the format argument, so a collated
format is a non-default
+ // format. from_unixtime has no native path for non-default formats, so it
is reported as
+ // Unsupported (the format reason) rather than Incompatible (the collation
reason).
checkDatetimeFallback(
"SELECT from_unixtime(_10, _6 COLLATE utf8_lcase) FROM
datetime_collation_tbl",
- "from_unixtime does not support non-UTF8_BINARY collations")
+ "Only the default datetime format pattern `yyyy-MM-dd HH:mm:ss` is
supported")
}
test("make_timestamp rejects non-UTF8_BINARY collated timezone (issue
#4646)") {
diff --git
a/spark/src/test/spark-4.1/org/apache/spark/sql/CometCollationSuite.scala
b/spark/src/test/spark-4.1/org/apache/spark/sql/CometCollationSuite.scala
index e1b9713d75..05b488d8cd 100644
--- a/spark/src/test/spark-4.1/org/apache/spark/sql/CometCollationSuite.scala
+++ b/spark/src/test/spark-4.1/org/apache/spark/sql/CometCollationSuite.scala
@@ -127,9 +127,12 @@ class CometCollationSuite extends CometTestBase {
}
test("from_unixtime rejects non-UTF8_BINARY collated format (issue #4646)") {
+ // A collation can only appear on the format argument, so a collated
format is a non-default
+ // format. from_unixtime has no native path for non-default formats, so it
is reported as
+ // Unsupported (the format reason) rather than Incompatible (the collation
reason).
checkDatetimeFallback(
"SELECT from_unixtime(_10, _6 COLLATE utf8_lcase) FROM
datetime_collation_tbl",
- "from_unixtime does not support non-UTF8_BINARY collations")
+ "Only the default datetime format pattern `yyyy-MM-dd HH:mm:ss` is
supported")
}
test("make_timestamp rejects non-UTF8_BINARY collated timezone (issue
#4646)") {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
