fix: report FromUnixTime non-default format as Unsupported#4847
Merged
andygrove merged 11 commits intoJul 8, 2026
Conversation
CodegenDispatchFallback already routes Incompatible cases through the JVM codegen dispatcher so the projection stays inside the Comet pipeline instead of falling back to Spark. Apply the same routing to Unsupported cases: when getSupportLevel returns Unsupported and the serde mixes in CodegenDispatchFallback, run Spark's own doGenCode via the dispatcher before resorting to Spark fallback. Affects Concat (non-string children), SortArray (nested Struct/Null children), ArrayIntersect (collated strings), TruncDate and TruncTimestamp (formats outside the native set). Also refresh the expression compatibility docs: drop "faster native" wording (no measured claim), clarify that the default path runs in the JVM via Spark codegen, and render Unsupported reasons differently for CodegenDispatchFallback serdes (always JVM dispatch) vs everything else (always Spark fallback).
The Unsupported and Incompatible arms both pattern-match on CodegenDispatchFallback and call emitJvmCodegenDispatch. Lift that pattern into a single helper that returns the matched handler alongside the dispatched expression, so the Incompatible arm can reach nativeOptInConfigKeyOverride for its [COMET-INFO] hint without re-matching the same value.
…h codegen dispatch Concat, SortArray, TruncDate, and TruncTimestamp now route their previously Unsupported cases through the JVM codegen dispatcher and stay native instead of falling back to Spark. Update the test expectations to assert native execution and a matching answer rather than a fallback reason.
…led fallback Add a SQL test exercising the non-boolean-literal ascendingOrder branch of SortArray, which routes through codegen dispatch and stays native, and a dedicated test with spark.comet.exec.scalaUDF.codegen.enabled=false confirming the Unsupported CodegenDispatchFallback cases (Concat, SortArray, TruncDate) fall back to Spark cleanly. Log at debug level when an Unsupported case routes through the dispatcher.
Spark 3.4/3.5's SortArray requires the ascendingOrder argument to be a boolean literal and rejects a non-literal expression at analysis time, so the sort_array(arr, true AND true) / (false OR false) cases fail on those versions. Move them into sort_array_min_spark_4_0.sql gated with MinSparkVersion 4.0, where Spark accepts foldable non-literal sort orders.
A non-default datetime pattern has no native DataFusion implementation, so it is now classified as Unsupported rather than folded into an Incompatible reason. With apache#4728, an Unsupported result on a CodegenDispatchFallback serde still stays in the Comet pipeline via JVM codegen dispatch, so this does not regress to a Spark fallback. Splits the conflated reason string into the timestamp-range incompatibility (kept in getIncompatibleReasons) and the format limitation (now in getUnsupportedReasons), and reuses the latter for the convert() fallback. Closes apache#4575
c331b0e to
5a160f8
Compare
The original support-level logic is correct: from_unixtime reports a non-default format as Unsupported (no native path). A collation can only appear on the format argument, so a collated format is always a non-default format and is therefore Unsupported (format reason) rather than Incompatible (collation reason), unlike date_format whose native path handles arbitrary formats. The CometCollationSuite from_unixtime test asserted the collation reason, which never fires; update it to expect the format reason.
c8c86f8 to
cc2babe
Compare
A collation can only appear on the format argument, and any collated format is a non-default format, which is already reported as Unsupported. The collation branch in getSupportLevel and the collation entry in getIncompatibleReasons could never fire, so remove them.
mbutrovich
approved these changes
Jul 8, 2026
mbutrovich
left a comment
Contributor
There was a problem hiding this comment.
Looks right to me, thanks @andygrove!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes #4575.
Rationale for this change
CometFromUnixTimefolded two unrelated limitations into a singleIncompatiblereason string:yyyy-MM-dd HH:mm:ssis supported natively. A non-default pattern has no native (DataFusion) implementation at all.Reporting the format limitation as an incompatibility was the misclassification noted in #4074. A non-default format has no native path, so it is genuinely
Unsupported. Since #4728, anUnsupportedresult on aCodegenDispatchFallbackserde still stays in the Comet pipeline via JVM codegen dispatch (Spark's owndoGenCode), so classifying it asUnsupporteddoes not regress to a Spark fallback.What changes are included in this PR?
getSupportLevelreturnsUnsupportedfor a non-default format andIncompatible(timestamp-range) for the default format.timestampRangeReason(kept ingetIncompatibleReasons) andformatReason(now returned fromgetUnsupportedReasons), and reusesformatReasonfor theconvert()fallback message.from_unixtime. A collation can only appear on the format argument, and any collated format is a non-default format that is alreadyUnsupported, so the collation branch and itsgetIncompatibleReasonsentry could never fire. This is unlikedate_format, whose native path handles arbitrary formats and is genuinelyIncompatiblefor non-default collations.allowIncompatible=false.CometCollationSuitefrom_unixtimetest (Spark 4.0 and 4.1) to expect the format reason, since a collated format is now reported asUnsupportedrather thanIncompatiblefor collation.The generated
datetime.mdcompatibility page is regenerated by CI and is not included here.How are these changes tested?
CometExpressionSuite'sfrom_unixtimetest, extended with an assertion thatfrom_unixtime(_5, 'yyyy')runs fully native via codegen dispatch and matches Spark at the default config.CometCollationSuite(Spark 4.0 and 4.1) verifies a collated format falls back with the format reason when codegen dispatch is disabled.