Skip to content

fix: route nested DATE to numeric casts in structs and maps through the codegen dispatcher - #6319

Merged
andygrove merged 1 commit into
apache:mainfrom
andygrove:fix-nested-date-casts
Sep 30, 2026
Merged

andygrove merged 1 commit into
apache:mainfrom
andygrove:fix-nested-date-casts

Conversation

@andygrove

Copy link
Copy Markdown
Member

Which issue does this PR close?

Closes #6316.

Rationale for this change

In Legacy mode Spark returns NULL for a cast from DATE to a numeric or boolean type, including
when the date is a struct field or a map value. Comet ran those nested casts natively. DATE to
INT returned the day count, silently, and the other targets failed the query with
Native cast invoked for unsupported cast from Date32 to ....

A top-level cast is fine, because CometCast.convert replaces it with a null literal. That is also
why isSupported reports DATE to a numeric type as Compatible. The struct and map arms reuse
that 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 that unix_date
relies 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.isSupported reports a struct or map cast as Unsupported when one of its field, key
    or value casts is one that convert only handles at the top level (isAlwaysCastToNull).
    CodegenDispatchFallback then runs Spark's own cast inside the Comet pipeline, so the plan stays
    in Comet and the result matches Spark.
  • Other nested DATE casts, to TIMESTAMP, TIMESTAMP_NTZ and STRING, are unchanged and stay
    native.

How are these changes tested?

  • New queries in cast_complex.sql cast DATE struct fields, map values and array-of-struct
    fields to INT, BIGINT, BOOLEAN, DOUBLE, DECIMAL(10,2) and TINYINT, with TIMESTAMP
    and STRING as a control. Without the fix the first one returns [[19737,first],1] where Spark
    returns [[null,first],1].
  • A new CometNativeCastSuite test pins the support levels. It also fails without the fix.
  • The full CometNativeCastSuite and the cast_complex* SQL files pass on Spark 4.1, and the new
    tests pass on Spark 3.5 / Scala 2.12.

@andygrove andygrove added the run-spark-4.1-tests Run the Spark 4.1 SQL tests on this pull request instead of waiting for the merge queue label Sep 28, 2026
@github-actions github-actions Bot added bug Something isn't working area:expressions Expression evaluation labels Sep 28, 2026

@sunchao sunchao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Summary

  • Prior state and problem: Legacy nested DATE casts could return epoch days or fail instead of producing Spark’s null results.
  • Design approach: Mark affected struct and map casts Unsupported, allowing the existing CodegenDispatchFallback to 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-Int32 reinterpretation protects unix_date. Reusing isAlwaysCastToNull keeps 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.

@andygrove
andygrove added this pull request to the merge queue Sep 30, 2026
Merged via the queue into apache:main with commit 5967c37 Sep 30, 2026
64 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:expressions Expression evaluation bug Something isn't working run-spark-4.1-tests Run the Spark 4.1 SQL tests on this pull request instead of waiting for the merge queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nested DATE to numeric casts in structs and maps return the day count or fail

2 participants