Describe the bug
CometToUnixTimestamp and CometMakeTimestamp both extend CometCodegenDispatch, whose convert unconditionally emits CometScalaUDF.emitJvmCodegenDispatch (spark/src/main/scala/org/apache/comet/serde/CometScalaUDF.scala:202-211). Neither expression has a native kernel at all. Both nevertheless return Incompatible when an argument carries a non-default collation:
CometMakeTimestamp, spark/src/main/scala/org/apache/comet/serde/datetime.scala:867-883
CometToUnixTimestamp, spark/src/main/scala/org/apache/comet/serde/datetime.scala:895-911
That Incompatible has two user-visible effects, and both of them promise a native implementation that does not exist.
In EXPLAIN, the Incompatible arm of QueryPlanSerde.exprToProtoInternal tags the node with withInfo(expr, NativeOptIn.message(...)) before routing it through the dispatcher, so the user is told to set a flag that changes nothing.
In the generated compatibility guide, getIncompatibleReasons() is rendered under the nativeOptIn header from GenerateDocs.renderExpressionCompatNotes: "By default, X is evaluated in the JVM using Spark's own code-generated implementation (run inside the Comet pipeline), which matches Spark exactly. Set ...allowIncompatible=true to opt into Comet's native implementation instead, which has the following differences from Spark:". There is no native implementation to opt into.
Steps to reproduce
On Spark 4.0+, over a Parquet table t(_1 string, _2 string, _3 string) holding ('2024-06-15', 'yyyy-MM-dd', 'America/Los_Angeles'), with ConstantFolding excluded, run each query twice, once with the expression's allowIncompatible at false and once at true:
SELECT to_unix_timestamp(_1, _2 COLLATE UTF8_LCASE) FROM t
SELECT make_timestamp(2024, 6, 15, 12, 0, 0, _3 COLLATE UTF8_LCASE) FROM t
The routing is byte-identical across the two settings. Only the hint disappears. On Spark 4.1.3:
ToUnixTimestamp allowIncompatible=false native=[] dispatched=[collate, resolvedcollation, to_unix_timestamp]
CometProject [COMET-INFO: A native implementation of ToUnixTimestamp is available.
Set spark.comet.expression.ToUnixTimestamp.allowIncompatible=true to enable it. ...]
Accelerated expressions: 0 native, 3 codegen dispatch.
ToUnixTimestamp allowIncompatible=true native=[] dispatched=[collate, resolvedcollation, to_unix_timestamp]
CometProject
Accelerated expressions: 0 native, 3 codegen dispatch.
MakeTimestamp allowIncompatible=false native=[] dispatched=[cast, collate, make_timestamp, resolvedcollation]
CometProject [COMET-INFO: A native implementation of MakeTimestamp is available.
Set spark.comet.expression.MakeTimestamp.allowIncompatible=true to enable it. ...]
Accelerated expressions: 0 native, 4 codegen dispatch.
MakeTimestamp allowIncompatible=true native=[] dispatched=[cast, collate, make_timestamp, resolvedcollation]
CometProject
Accelerated expressions: 0 native, 4 codegen dispatch.
Expected behavior
Results are correct in every case, so this is about diagnostics and documentation rather than answers.
A collated argument on an expression with no native path should not report Incompatible, because Incompatible is the level that means "Comet has a native implementation that differs from Spark, and you can opt into it". The natural fix is to drop the collation gate from these two serdes entirely and let them report Compatible, since CometCodegenDispatch always runs Spark's own generated code and therefore already honours collation exactly. Reporting Unsupported instead would also silence the hint, but it would misdescribe the situation, because these expressions are fully supported.
Either way, getIncompatibleReasons() should go too, so the compatibility page stops advertising a native opt-in for these two.
Additional context
This surfaced while reviewing #5789, which makes the same correction for unix_timestamp. That PR removes the collation gate from CometUnixTimestamp on the grounds that Spark's ToTimestamp only reads the format argument in the StringType branch, so the format's collation cannot change the answer for date and timestamp inputs. ToUnixTimestamp is the same ToTimestamp subclass, and neither it nor MakeTimestamp has a native path for the gate to protect in the first place.
These are the only two serdes in the tree that combine CometCodegenDispatch with a non-Compatible support level, so the fix is contained. CometMakeInterval also reports Incompatible, but it is a plain CometExpressionSerde with a real native path, so its opt-in hint is genuine and should stay.
Existing coverage to keep green: the to_unix_timestamp and make_timestamp cases in routing_datetime_collation_enabled.sql and routing_datetime_collation_disabled.sql (added by #5950) assert the dispatch and fallback routes, and CometCollationSuite on Spark 4.0 and 4.1 has a make_timestamp rejects non-UTF8_BINARY collated timezone and a to_unix_timestamp rejects non-UTF8_BINARY collated format test, both asserting the fallback reason text that would go away.
Describe the bug
CometToUnixTimestampandCometMakeTimestampboth extendCometCodegenDispatch, whoseconvertunconditionally emitsCometScalaUDF.emitJvmCodegenDispatch(spark/src/main/scala/org/apache/comet/serde/CometScalaUDF.scala:202-211). Neither expression has a native kernel at all. Both nevertheless returnIncompatiblewhen an argument carries a non-default collation:CometMakeTimestamp,spark/src/main/scala/org/apache/comet/serde/datetime.scala:867-883CometToUnixTimestamp,spark/src/main/scala/org/apache/comet/serde/datetime.scala:895-911That
Incompatiblehas two user-visible effects, and both of them promise a native implementation that does not exist.In
EXPLAIN, theIncompatiblearm ofQueryPlanSerde.exprToProtoInternaltags the node withwithInfo(expr, NativeOptIn.message(...))before routing it through the dispatcher, so the user is told to set a flag that changes nothing.In the generated compatibility guide,
getIncompatibleReasons()is rendered under thenativeOptInheader fromGenerateDocs.renderExpressionCompatNotes: "By default,Xis evaluated in the JVM using Spark's own code-generated implementation (run inside the Comet pipeline), which matches Spark exactly. Set...allowIncompatible=trueto opt into Comet's native implementation instead, which has the following differences from Spark:". There is no native implementation to opt into.Steps to reproduce
On Spark 4.0+, over a Parquet table
t(_1 string, _2 string, _3 string)holding('2024-06-15', 'yyyy-MM-dd', 'America/Los_Angeles'), withConstantFoldingexcluded, run each query twice, once with the expression'sallowIncompatibleatfalseand once attrue:The routing is byte-identical across the two settings. Only the hint disappears. On Spark 4.1.3:
Expected behavior
Results are correct in every case, so this is about diagnostics and documentation rather than answers.
A collated argument on an expression with no native path should not report
Incompatible, becauseIncompatibleis the level that means "Comet has a native implementation that differs from Spark, and you can opt into it". The natural fix is to drop the collation gate from these two serdes entirely and let them reportCompatible, sinceCometCodegenDispatchalways runs Spark's own generated code and therefore already honours collation exactly. ReportingUnsupportedinstead would also silence the hint, but it would misdescribe the situation, because these expressions are fully supported.Either way,
getIncompatibleReasons()should go too, so the compatibility page stops advertising a native opt-in for these two.Additional context
This surfaced while reviewing #5789, which makes the same correction for
unix_timestamp. That PR removes the collation gate fromCometUnixTimestampon the grounds that Spark'sToTimestamponly reads the format argument in theStringTypebranch, so the format's collation cannot change the answer for date and timestamp inputs.ToUnixTimestampis the sameToTimestampsubclass, and neither it norMakeTimestamphas a native path for the gate to protect in the first place.These are the only two serdes in the tree that combine
CometCodegenDispatchwith a non-Compatiblesupport level, so the fix is contained.CometMakeIntervalalso reportsIncompatible, but it is a plainCometExpressionSerdewith a real native path, so its opt-in hint is genuine and should stay.Existing coverage to keep green: the
to_unix_timestampandmake_timestampcases inrouting_datetime_collation_enabled.sqlandrouting_datetime_collation_disabled.sql(added by #5950) assert the dispatch and fallback routes, andCometCollationSuiteon Spark 4.0 and 4.1 has amake_timestamp rejects non-UTF8_BINARY collated timezoneand ato_unix_timestamp rejects non-UTF8_BINARY collated formattest, both asserting the fallback reason text that would go away.