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)") {