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..fa25526bbc 100644 --- a/spark/src/main/scala/org/apache/comet/serde/unixtime.scala +++ b/spark/src/main/scala/org/apache/comet/serde/unixtime.scala @@ -31,19 +31,26 @@ object CometFromUnixTime extends CometExpressionSerde[FromUnixTime] with Codegen private val collationReason = DatetimeCollation.reason("from_unixtime") - 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)" + override def getIncompatibleReasons(): Seq[String] = Seq( + "DataFusion's valid timestamp range differs from Spark" + + " (https://github.com/apache/datafusion/issues/16594)") ++ + DatetimeCollation.incompatibleReasons("from_unixtime") - override def getIncompatibleReasons(): Seq[String] = - Seq(formatReason) ++ DatetimeCollation.incompatibleReasons("from_unixtime") + override def getCompatibleNotes(): Seq[String] = Seq( + "Only the default datetime format pattern `yyyy-MM-dd HH:mm:ss` runs natively via " + + "DataFusion's `to_char`. Non-default patterns route through Spark's own " + + "`FromUnixTime.doGenCode` via the Arrow-direct codegen dispatcher when " + + "`spark.comet.exec.scalaUDF.codegen.enabled=true` (the default). When the codegen " + + "dispatcher is disabled the operator falls back to Spark in those cases.") override def getSupportLevel(expr: FromUnixTime): SupportLevel = { if (DatetimeCollation.hasNonDefaultCollation(expr)) { Incompatible(Some(collationReason)) + } else if (expr.format != Literal(TimestampFormatter.defaultPattern())) { + Incompatible( + Some("Only the default datetime pattern `yyyy-MM-dd HH:mm:ss` is supported natively")) } else { - Incompatible(Some(formatReason)) + Incompatible(None) } } @@ -51,25 +58,25 @@ object CometFromUnixTime extends CometExpressionSerde[FromUnixTime] with Codegen expr: FromUnixTime, inputs: Seq[Attribute], binding: Boolean): Option[ExprOuterClass.Expr] = { - val secExpr = exprToProtoInternal(expr.sec, inputs, binding) - // TODO: DataFusion toChar does not support Spark datetime pattern format - // https://github.com/apache/datafusion/issues/16577 - // https://github.com/apache/datafusion/issues/14536 - // After fixing these issues, use provided `format` instead of the manual replacement below - val formatExpr = exprToProtoInternal(Literal("%Y-%m-%d %H:%M:%S"), inputs, binding) - val timeZone = exprToProtoInternal(Literal(expr.timeZoneId.orNull), inputs, binding) - - if (expr.format != Literal(TimestampFormatter.defaultPattern)) { - withFallbackReason(expr, "Datetime pattern format is unsupported") - None - } else if (secExpr.isDefined && formatExpr.isDefined) { - val timestampExpr = - scalarFunctionExprToProto("from_unixtime", Seq(secExpr, timeZone): _*) - val optExpr = scalarFunctionExprToProto("to_char", Seq(timestampExpr, formatExpr): _*) - optExprWithFallbackReason(optExpr, expr, expr.sec, expr.format) + if (expr.format != Literal(TimestampFormatter.defaultPattern())) { + CometScalaUDF.emitJvmCodegenDispatch(expr, inputs, binding) } else { - withFallbackReason(expr, expr.sec, expr.format) - None + val secExpr = exprToProtoInternal(expr.sec, inputs, binding) + // TODO: DataFusion toChar does not support Spark datetime pattern format + // https://github.com/apache/datafusion/issues/16577 + // https://github.com/apache/datafusion/issues/14536 + // After fixing these issues, use provided `format` instead of the manual replacement below + val formatExpr = exprToProtoInternal(Literal("%Y-%m-%d %H:%M:%S"), inputs, binding) + val timeZone = exprToProtoInternal(Literal(expr.timeZoneId.orNull), inputs, binding) + if (secExpr.isDefined && formatExpr.isDefined) { + val timestampExpr = + scalarFunctionExprToProto("from_unixtime", Seq(secExpr, timeZone): _*) + val optExpr = scalarFunctionExprToProto("to_char", Seq(timestampExpr, formatExpr): _*) + optExprWithFallbackReason(optExpr, expr, expr.sec, expr.format) + } else { + withFallbackReason(expr, expr.sec, expr.format) + None + } } } } diff --git a/spark/src/test/resources/sql-tests/expressions/datetime/from_unix_time_enabled.sql b/spark/src/test/resources/sql-tests/expressions/datetime/from_unix_time_enabled.sql index 1821a6a01e..07ffebf69b 100644 --- a/spark/src/test/resources/sql-tests/expressions/datetime/from_unix_time_enabled.sql +++ b/spark/src/test/resources/sql-tests/expressions/datetime/from_unix_time_enabled.sql @@ -26,16 +26,17 @@ CREATE TABLE test_from_unix_time_enabled(t long) USING parquet statement INSERT INTO test_from_unix_time_enabled VALUES (0), (1718451045), (-1), (NULL), (2147483647) --- Even with allowIncompatible=true, the default datetime pattern is unsupported natively -query spark_answer_only +-- With allowIncompatible=true, the default datetime pattern runs natively +query SELECT from_unixtime(t) FROM test_from_unix_time_enabled -query spark_answer_only +-- Non-default format patterns are handled by the JVM codegen dispatcher +query SELECT from_unixtime(t, 'yyyy-MM-dd') FROM test_from_unix_time_enabled -- literal arguments -query spark_answer_only +query SELECT from_unixtime(0) -query spark_answer_only +query SELECT from_unixtime(1718451045, 'yyyy-MM-dd')