Re: [PR] refactor: move AttributeReference, FromUnixTime and KnownFloatingPointNormalized expression support checks to getSupportLevel [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove commented on code in PR #4745:
URL: https://github.com/apache/datafusion-comet/pull/4745#discussion_r3493136495


##########
spark/src/main/scala/org/apache/comet/serde/unixtime.scala:
##########
@@ -22,19 +22,28 @@ package org.apache.comet.serde
 import org.apache.spark.sql.catalyst.expressions.{Attribute, FromUnixTime, Literal}
 import org.apache.spark.sql.catalyst.util.TimestampFormatter
 
+import org.apache.comet.CometConf
 import org.apache.comet.CometSparkSessionExtensions.withFallbackReason
 import org.apache.comet.serde.QueryPlanSerde.{exprToProtoInternal, optExprWithFallbackReason, scalarFunctionExprToProto}
 
 // TODO: DataFusion supports only -8334601211038 <= sec <= 8210266876799
 // https://github.com/apache/datafusion/issues/16594
 object CometFromUnixTime extends CometExpressionSerde[FromUnixTime] with CodegenDispatchFallback {
+  private val unsupportedFormatReason = "Datetime pattern format is unsupported"
 
   override def getIncompatibleReasons(): Seq[String] = Seq(
     "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 getSupportLevel(expr: FromUnixTime): SupportLevel = Incompatible(None)
+  override def getSupportLevel(expr: FromUnixTime): SupportLevel = {
+    if (expr.format == Literal(TimestampFormatter.defaultPattern()) ||
+      !CometConf.isExprAllowIncompat(getExprConfigName(expr))) {

Review Comment:
   I'm confused about the reference to `isExprAllowIncompat` here. Typically we would just return `Incompatible` and then rely on the serde framework checkign this flag.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org