Conversation
sunchao
left a comment
There was a problem hiding this comment.
Summary
- Prior state and problem: The cast guide covered primitive types without explaining complex-type casts.
- Design approach: Extend the shared template with recursive support rules, a six-row table, exceptions, and cross-references.
- Correctness / compatibility analysis: Checked Comet’s support decisions and native casts against Spark
Cast.scalain 3.4.3, 3.5.9, 4.0.4, 4.1.3, and 4.2.0. The disclosed dependency on #6319 remains unresolved. Lines 191–192 describe behavior that is not present at this head. - Key design decisions: Reusing the template keeps generated pages consistent. Positional struct matching and the distinction between native support and Spark execution are appropriate.
- Implementation sketch: One documentation file changes, adding 37 lines and removing one. No runtime abstraction or execution overhead is introduced.
- Behavioral changes worth calling out: The page documents array restrictions, legacy string formatting, and the existing Try-mode map-key issue #5995. Runtime behavior is unchanged.
- Suggested improvements: Preserve the acknowledged merge order by landing #6319 first. At this head, a non-foldable
STRUCT<d:DATE>column containing2024-01-15, cast toSTRUCT<d:INT>in Legacy mode, still follows the native path and produces19737instead of a null field.CometCast.isSupportedaccepts the nested cast, and the nativeDate32→Int32branch reinterprets the day count. This substantiates the existing blocker without duplicating it as a new finding.
No introduced P1/P2 issues found within this review beyond the already-disclosed prerequisite.
Reviewed the entire diff from ce455f32d948355e073638e81009ecc3e5dea349 to f743e924a0d9a238f22319e49349e54a3f6b57c6. The PR is not a draft. Snapshot and live discussion checks found no reviews, comments, or threads. Routed skill: .ai/skills/review-comet-pr/SKILL.md. No sibling skill applies to this docs-only diff.
Exact-head CI: six checks succeeded, including Preflight and CodeQL, fourteen were skipped, and Required Checks remained queued. No failures were reported.
Validation: git diff --check passed. Markdown parsing confirmed all six table rows and five local anchor references. Full Sphinx/GenerateDocs builds and JVM/native runtime tests were not run. Compatibility conclusions and the dependency example were verified by source inspection.
Which issue does this PR close?
Closes #2743.
Rationale for this change
The cast compatibility page only covers casts between primitive types. #2760 was closed until the
complex-type casts had test coverage, which the #4248 work has since added.
What changes are included in this PR?
A new Complex Types section in the cast compatibility template, which is copied into every Spark
version's page:
value by key and value, so a complex-type cast is compatible only when every cast it contains
is compatible.
struct to string, map to map, and map to string, which has no native path.
ARRAY<DATE>,DATEstruct fields and map values cast to numeric or booleantypes, and
spark.sql.legacy.castComplexTypesToString.enabled.The
DATEstruct field and map value bullet describes the behavior after #6319, so this shouldmerge after that PR.
How are these changes tested?
Documentation only. I checked each claim against
CometCast.isSupported, and against Spark with aprobe over every container shape (struct field, map value, array element, array of struct) for
each primitive target type in Legacy mode. That probe is what turned up #6316. The #5995 failure
reproduces on
main. The page renders throughGenerateDocsfor Spark 4.1, and prettier passes.