Skip to content

fix: report FromUnixTime non-default format as Unsupported#4847

Merged
andygrove merged 11 commits into
apache:mainfrom
andygrove:fix-4575-fromunixtime-support-level
Jul 8, 2026
Merged

fix: report FromUnixTime non-default format as Unsupported#4847
andygrove merged 11 commits into
apache:mainfrom
andygrove:fix-4575-fromunixtime-support-level

Conversation

@andygrove

@andygrove andygrove commented Jul 7, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

Closes #4575.

Rationale for this change

CometFromUnixTime folded two unrelated limitations into a single Incompatible reason string:

  1. DataFusion's valid timestamp range differs from Spark. A genuine incompatibility of the native path that applies even to the default format.
  2. Only the default datetime pattern yyyy-MM-dd HH:mm:ss is 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, an Unsupported result on a CodegenDispatchFallback serde still stays in the Comet pipeline via JVM codegen dispatch (Spark's own doGenCode), so classifying it as Unsupported does not regress to a Spark fallback.

What changes are included in this PR?

  • getSupportLevel returns Unsupported for a non-default format and Incompatible (timestamp-range) for the default format.
  • Splits the conflated reason string into timestampRangeReason (kept in getIncompatibleReasons) and formatReason (now returned from getUnsupportedReasons), and reuses formatReason for the convert() fallback message.
  • Removes the unreachable collation guard from from_unixtime. A collation can only appear on the format argument, and any collated format is a non-default format that is already Unsupported, so the collation branch and its getIncompatibleReasons entry could never fire. This is unlike date_format, whose native path handles arbitrary formats and is genuinely Incompatible for non-default collations.
  • Adds a regression test asserting a non-default format stays a Comet operator via codegen dispatch at the default allowIncompatible=false.
  • Updates the CometCollationSuite from_unixtime test (Spark 4.0 and 4.1) to expect the format reason, since a collated format is now reported as Unsupported rather than Incompatible for collation.

The generated datetime.md compatibility page is regenerated by CI and is not included here.

How are these changes tested?

CometExpressionSuite's from_unixtime test, extended with an assertion that from_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.

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.
@andygrove andygrove marked this pull request as draft July 7, 2026 16:42
andygrove added 2 commits July 7, 2026 10:44
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
@andygrove andygrove force-pushed the fix-4575-fromunixtime-support-level branch from c331b0e to 5a160f8 Compare July 7, 2026 16:49
@andygrove andygrove changed the title fix: report FromUnixTime non-default format as unsupported in docs fix: report FromUnixTime non-default format as Unsupported Jul 7, 2026
andygrove added 2 commits July 7, 2026 13:14
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.
@andygrove andygrove force-pushed the fix-4575-fromunixtime-support-level branch from c8c86f8 to cc2babe Compare July 7, 2026 19:43
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.
@andygrove andygrove marked this pull request as ready for review July 7, 2026 19:54

@mbutrovich mbutrovich left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks right to me, thanks @andygrove!

@andygrove andygrove merged commit 5372d76 into apache:main Jul 8, 2026
70 checks passed
@andygrove andygrove deleted the fix-4575-fromunixtime-support-level branch July 8, 2026 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CometFromUnixTime reports unsupported format patterns as Incompatible instead of Unsupported

2 participants