dylanpulver opened a new pull request, #10939: URL: https://github.com/apache/arrow-rs/pull/10939
# Which issue does this PR close? - Closes #10938. # Rationale for this change `can_cast_types` promised `Interval(YearMonth) -> Int64` and `Interval(DayTime) -> Int64`; `cast_with_options` has no such arm and errors. #5769 removed the cast side deliberately and left the `can_cast_types` arm behind, so this restores the intended state rather than adding a cast. The user-visible effect is in `cast/union.rs::resolve_child_array`, which picks the first union child `can_cast_types` accepts: a union with an interval child and a `Utf8` child failed to cast to `Int64` even though the `Utf8` child casts fine. If you would rather have `Interval -> Int64` actually implemented, this is the wrong direction and I am happy to close it — that would be a feature rather than a fix. # What changes are included in this PR? The `(Interval(_), Int64)` arm is removed from `can_cast_types`. `arrow/tests/array_cast.rs::get_all_types()` gets `Int64` back: it currently reads `Int8, Int16, Int32, UInt64, UInt8, UInt16, UInt32, UInt64`, so `Int64` was never used as a cast target. Line 568 of that file has the intended sequence. # Are these changes tested? Yes, and the existing integration test does the load-bearing part once `Int64` is restored. Matched pair, `cargo test -p arrow --features="chrono-tz prettyprint" --test array_cast test_can_cast_types`: passes on unmodified main; with only `Int64` restored and no source change it fails with `from Interval(YearMonth) to Int64 but can_cast_types reported true`; with both changes it passes. Restoring `Int64` exposed that one mismatch and no others. Two unit tests added in `arrow-cast`. `test_can_cast_interval_to_int64_matches_cast` asserts `can_cast_types == cast(..).is_ok()` for all three interval units rather than hard coding the expected answer. `test_cast_union_to_int64_skips_uncastable_interval_child` covers the union path. Reverting the source with the tests in place fails both of them and the integration test. A half fix (`YearMonth => false`, `DayTime => true`) still passes the union test but fails the enumerating one on `Interval(DayTime)`, which is why the enumeration is there — `test_can_cast_types` panics on the first mismatch and would only ever have shown `YearMonth`. `cargo fmt --all -- --check` clean, `cargo clippy -p arrow-cast --all-targets -- -D warnings` clean, `cargo test -p arrow-cast` 379 passed + 11 doc tests, `--test array_cast` 10 passed. rustc 1.97.1, matching `rust-toolchain.toml`. Not tested: full workspace, miri, `force_validate`, `--all-features`, benchmarks. My sweep for other stale arms of this class was dispatch-level over null arrays, so value-level cast bugs were not covered. # Are there any user-facing changes? `can_cast_types(Interval(YearMonth) | Interval(DayTime), Int64)` returns `false` instead of `true`. Nothing could have relied on the `true`, since the cast always failed. Casting a union to `Int64` now succeeds in cases that previously errored. --- AI disclosure, per CONTRIBUTING: the investigation, the patch and the tests were produced with AI assistance (Claude Opus 4.8). The measurements quoted above were run against this branch. -- 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]
