andygrove opened a new pull request, #4847:
URL: https://github.com/apache/datafusion-comet/pull/4847

   ## 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. This is a genuine 
incompatibility of the native path and 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.
   
   Reporting the format limitation as an incompatible reason was the 
misclassification noted in #4074. This PR separates the two so the generated 
compatibility docs describe each accurately.
   
   `CometFromUnixTime` mixes in `CodegenDispatchFallback`, so at the default 
`allowIncompatible=false` a non-default format still runs natively via JVM 
codegen dispatch (Spark-exact) rather than falling back to Spark. To preserve 
that, `getSupportLevel` intentionally stays `Incompatible` for every case 
(returning `Unsupported` would bypass codegen dispatch and force a full Spark 
fallback). The format limitation is surfaced to the docs via 
`getUnsupportedReasons`; it describes the opt-in native path, which covers only 
the default pattern.
   
   ## What changes are included in this PR?
   
   - Split the conflated reason string into `timestampRangeReason` (kept in 
`getIncompatibleReasons`) and `formatReason` (now returned from 
`getUnsupportedReasons`).
   - `getSupportLevel` returns `Incompatible` for all cases, with the 
timestamp-range reason for the default-format path and the existing collation 
reason for non-default collations. Runtime behavior is unchanged.
   - Added a regression test asserting that a non-default format stays a Comet 
operator (via codegen dispatch) at the default `allowIncompatible=false`.
   
   The generated `datetime.md` compatibility page is regenerated by CI and is 
not included here.
   
   ## How are these changes tested?
   
   `CometExpressionSuite`: the existing `from_unixtime` test plus a new test 
confirming a non-default format (`from_unixtime(_5, 'yyyy')`) runs fully native 
via codegen dispatch and matches Spark.
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to