This is an automated email from the ASF dual-hosted git repository.
Jefffrey pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new bc2db0f5fc fix: remove stale Interval -> Int64 arm from can_cast_types
(#10939)
bc2db0f5fc is described below
commit bc2db0f5fce8780c6805e7d37ef78019cbeb3753
Author: Dylan Pulver <[email protected]>
AuthorDate: Tue Sep 15 06:44:02 2026 +0200
fix: remove stale Interval -> Int64 arm from can_cast_types (#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?
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 answer, and
`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 5). The measurements
quoted above were run against this branch.
---------
Co-authored-by: Dylan Pulver <[email protected]>
---
arrow-cast/src/cast/mod.rs | 58 ++++++++++++++++++++++++++++++++++++++++------
arrow/tests/array_cast.rs | 2 +-
2 files changed, 52 insertions(+), 8 deletions(-)
diff --git a/arrow-cast/src/cast/mod.rs b/arrow-cast/src/cast/mod.rs
index 7ba864336e..03abb14d27 100644
--- a/arrow-cast/src/cast/mod.rs
+++ b/arrow-cast/src/cast/mod.rs
@@ -321,13 +321,7 @@ pub fn can_cast_types(from_type: &DataType, to_type:
&DataType) -> bool {
(_, Duration(_)) if from_type.is_numeric() => true,
(Duration(_), _) if to_type.is_numeric() => true,
(Duration(_), Duration(_)) => true,
- (Interval(from_type), Int64) => {
- match from_type {
- YearMonth => true,
- DayTime => true,
- MonthDayNano => false, // Native type is i128
- }
- }
+ // No `(Interval(_), Int64)` arm: `cast_with_options` implements no
such cast.
(Int32, Interval(to_type)) => match to_type {
YearMonth => true,
DayTime => false,
@@ -12450,6 +12444,56 @@ mod tests {
assert_eq!(casted_array.value(0), IntervalMonthDayNano::new(0, 123,
0));
}
+ #[test]
+ fn test_can_cast_interval_to_int64_matches_cast() {
+ // Assert the two agree for every interval unit, rather than hard
coding the answer.
+ let arrays: Vec<ArrayRef> = vec![
+ Arc::new(IntervalYearMonthArray::from(vec![12])),
+ Arc::new(IntervalDayTimeArray::from(vec![IntervalDayTime::new(1,
2)])),
+ Arc::new(IntervalMonthDayNanoArray::from(vec![
+ IntervalMonthDayNano::new(1, 2, 3),
+ ])),
+ ];
+ for array in arrays {
+ let from = array.data_type();
+ assert_eq!(
+ can_cast_types(from, &DataType::Int64),
+ cast(&array, &DataType::Int64).is_ok(),
+ "can_cast_types disagrees with cast for {from} -> Int64"
+ );
+ }
+ }
+
+ #[test]
+ fn test_cast_union_to_int64_skips_uncastable_interval_child() {
+ // `resolve_child_array` picks the first child `can_cast_types`
accepts, so an
+ // over-permissive answer made it pick the interval child instead of
the `Utf8` one.
+ let fields = UnionFields::try_new(
+ [0, 1],
+ [
+ Field::new("iv", DataType::Interval(IntervalUnit::YearMonth),
true),
+ Field::new("s", DataType::Utf8, true),
+ ],
+ )
+ .unwrap();
+ let union = UnionArray::try_new(
+ fields,
+ vec![0, 1, 0].into(),
+ None,
+ vec![
+ Arc::new(IntervalYearMonthArray::from(vec![Some(12), None,
Some(24)])),
+ Arc::new(StringArray::from(vec![None, Some("77"), None])),
+ ],
+ )
+ .unwrap();
+
+ let casted = cast(&union, &DataType::Int64).unwrap();
+ let casted = casted.as_primitive::<Int64Type>();
+ assert!(casted.is_null(0));
+ assert_eq!(casted.value(1), 77);
+ assert!(casted.is_null(2));
+ }
+
#[test]
fn test_cast_below_unixtimestamp() {
let valid = StringArray::from(vec