fix: route nested DATE to numeric casts in structs and maps through the codegen dispatcher - #6319
Conversation
…he codegen dispatcher
sunchao
left a comment
There was a problem hiding this comment.
Summary
- Prior state and problem: Legacy nested
DATEcasts could return epoch days or fail instead of producing Spark’s null results. - Design approach: Mark affected struct and map casts
Unsupported, allowing the existingCodegenDispatchFallbackto execute Spark’s cast implementation. - Correctness / compatibility analysis: The guards propagate through nested containers and match the relevant Spark sources for 3.4.3, 3.5.9, 4.0.4, 4.1.3 and 4.2.0. No introduced P1/P2 issues found within this review.
- Key design decisions: Preserving the native
Date32-to-Int32reinterpretation protectsunix_date. ReusingisAlwaysCastToNullkeeps the change limited to legacy behavior. - Implementation sketch: Add checks for struct fields and map keys/values, with SQL regression coverage and support-level assertions. This uses the existing recursion and dispatcher without adding another abstraction.
- Behavioral changes worth calling out: Affected expressions now produce Spark-compatible nested nulls through JVM dispatch, or ordinary Spark fallback when dispatch is unavailable. Other casts retain their existing paths. No performance benchmark was run.
- Suggested improvements: None at the P1/P2 threshold.
Reviewed the complete three-file diff at 8317753d0c0cf9c2ce80df7610d26561b0fef1bd against ce455f32d948355e073638e81009ecc3e5dea349. The PR is not a draft. Existing reviews, comments and review threads were empty. Routed skills: review-comet-pr and review-comet-expression-pr.
Exact-head CI: Preflight, change detection, CodeQL, Actions analysis and metadata checks passed. Linux lint jobs and the label-triggered Spark 4.1 native/JVM build remained queued. No failures were reported, but no completed build/test verdict was available.
Validation: A disposable Spark 4.1.3 probe passed 280 nested-cast cases through both interpreted evaluation and generated code, plus 56 ANSI rejection checks. git diff --check passed. Full Comet/native suites were not run: this checkout lacks built artifacts, and an isolated Comet compilation attempt encountered empty cached Comet JARs. The author’s reported Comet test results were not independently reproduced.
Which issue does this PR close?
Closes #6316.
Rationale for this change
In Legacy mode Spark returns
NULLfor a cast fromDATEto a numeric or boolean type, includingwhen the date is a struct field or a map value. Comet ran those nested casts natively.
DATEtoINTreturned the day count, silently, and the other targets failed the query withNative cast invoked for unsupported cast from Date32 to ....A top-level cast is fine, because
CometCast.convertreplaces it with a null literal. That is alsowhy
isSupportedreportsDATEto a numeric type asCompatible. The struct and map arms reusethat level for their fields, but a nested cast does reach the native kernel. Arrays of dates
already had their own guard.
The kernel itself cannot just return null.
(Date32, Int32)is a reinterpret thatunix_daterelies on, since it serializes a native
Cast(Date -> Int).I found this while writing the complex-type cast docs for #2743.
What changes are included in this PR?
CometCast.isSupportedreports a struct or map cast asUnsupportedwhen one of its field, keyor value casts is one that
convertonly handles at the top level (isAlwaysCastToNull).CodegenDispatchFallbackthen runs Spark's own cast inside the Comet pipeline, so the plan staysin Comet and the result matches Spark.
DATEcasts, toTIMESTAMP,TIMESTAMP_NTZandSTRING, are unchanged and staynative.
How are these changes tested?
cast_complex.sqlcastDATEstruct fields, map values and array-of-structfields to
INT,BIGINT,BOOLEAN,DOUBLE,DECIMAL(10,2)andTINYINT, withTIMESTAMPand
STRINGas a control. Without the fix the first one returns[[19737,first],1]where Sparkreturns
[[null,first],1].CometNativeCastSuitetest pins the support levels. It also fails without the fix.CometNativeCastSuiteand thecast_complex*SQL files pass on Spark 4.1, and the newtests pass on Spark 3.5 / Scala 2.12.