Rich-T-kid commented on code in PR #10939:
URL: https://github.com/apache/arrow-rs/pull/10939#discussion_r3905097785
##########
arrow-cast/src/cast/mod.rs:
##########
@@ -321,13 +321,10 @@ 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
- }
- }
+ // Note: there is deliberately no `(Interval(_), Int64)` arm. No
interval unit has an
+ // unambiguous `i64` value (`YearMonth` is a count of months,
`DayTime` packs days and
+ // milliseconds, `MonthDayNano` is 128 bits wide), and
`cast_with_options` implements no
+ // such cast. Cast via `Duration` instead.
Review Comment:
I don't think this is strictly needed. we don't need to explain why certain
cast don't exist.
if you think its still warranted I think we should shorted this.
##########
arrow-cast/src/cast/mod.rs:
##########
@@ -12451,6 +12448,60 @@ mod tests {
assert_eq!(casted_array.value(0), IntervalMonthDayNano::new(0, 123,
0));
}
+ #[test]
+ fn test_can_cast_interval_to_int64_matches_cast() {
Review Comment:
similar Idea for these test, I think we can add one test that asserts
1. `can_cast_types()` reports false for interval(yearMonth/dayTime) -> int64
2. `cast(yearMonth/dayTime,int64)` fails with an error as expected
--
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]
