miguelcorderopamphile commented on code in PR #11572:
URL: https://github.com/apache/arrow-rs/pull/11572#discussion_r4396025116
##########
arrow-cast/src/cast/string.rs:
##########
@@ -132,6 +136,111 @@ fn parse_string_iter<
Ok(Arc::new(array) as ArrayRef)
}
+/// Parse a string to a duration, accepting integer strings (interpreted
+/// in the target unit) and interval strings with units, such as
+/// `"1 hour 10 minutes"`.
+fn parse_duration<D>(string: &str) -> Result<i64, ArrowError>
+where
+ D: ArrowTemporalType<Native = i64> + Parser,
+{
+ if let Some(duration) = D::parse(string) {
+ return Ok(duration);
+ }
+
+ let (unit, scale) = match D::DATA_TYPE {
+ DataType::Duration(TimeUnit::Second) => (IntervalParseUnit::Second,
1_000_000_000),
+ DataType::Duration(TimeUnit::Millisecond) =>
(IntervalParseUnit::Millisecond, 1_000_000),
+ DataType::Duration(TimeUnit::Microsecond) =>
(IntervalParseUnit::Microsecond, 1_000),
+ DataType::Duration(TimeUnit::Nanosecond) =>
(IntervalParseUnit::Nanosecond, 1),
+ _ => unreachable!(),
+ };
+
+ let interval = parse_interval_month_day_nano_config(string,
IntervalParseConfig::new(unit))
+ .map_err(|_| {
+ ArrowError::CastError(format!(
+ "Cannot cast string '{string}' to value of {} type",
+ D::DATA_TYPE
+ ))
+ })?;
+
+ if interval.months != 0 {
+ return Err(ArrowError::ComputeError(
+ "Cannot convert interval containing non-zero months to
duration".to_string(),
+ ));
+ }
+
+ let nanos = NANOS_PER_DAY as i128 * interval.days as i128 +
interval.nanoseconds as i128;
+ i64::try_from(nanos / scale as i128).map_err(|_| {
+ ArrowError::CastError(format!(
+ "Cannot cast string '{string}' to value of {} type",
+ D::DATA_TYPE
+ ))
+ })
+}
+
+/// Casts generic string arrays to a Duration type
+pub(crate) fn cast_string_to_duration<D, O>(
+ array: &dyn Array,
+ cast_options: &CastOptions,
+) -> Result<ArrayRef, ArrowError>
+where
+ D: ArrowTemporalType<Native = i64> + Parser,
+ O: OffsetSizeTrait,
+{
+ let string_array = array.as_string::<O>();
+ parse_duration_iter::<D, _, _>(string_array.iter(), cast_options, || {
+ string_array.nulls().cloned()
+ })
+}
+
+/// Casts string view arrays to a Duration type
+pub(crate) fn cast_view_to_duration<D>(
+ array: &dyn Array,
+ cast_options: &CastOptions,
+) -> Result<ArrayRef, ArrowError>
+where
+ D: ArrowTemporalType<Native = i64> + Parser,
+{
+ let string_view_array = array.as_string_view();
+ parse_duration_iter::<D, _, _>(string_view_array.iter(), cast_options, || {
+ string_view_array.nulls().cloned()
+ })
+}
+
+fn parse_duration_iter<'a, D, I, F>(
+ iter: I,
+ cast_options: &CastOptions,
+ nulls: F,
Review Comment:
It doesn't. The closure only deferred the clone in the safe path; passing
`Option<NullBuffer>` directly is simpler. Done in 887ac2d72.
##########
arrow-cast/src/cast/string.rs:
##########
@@ -132,6 +136,111 @@ fn parse_string_iter<
Ok(Arc::new(array) as ArrayRef)
}
+/// Parse a string to a duration, accepting integer strings (interpreted
+/// in the target unit) and interval strings with units, such as
+/// `"1 hour 10 minutes"`.
+fn parse_duration<D>(string: &str) -> Result<i64, ArrowError>
+where
+ D: ArrowTemporalType<Native = i64> + Parser,
+{
+ if let Some(duration) = D::parse(string) {
+ return Ok(duration);
+ }
+
+ let (unit, scale) = match D::DATA_TYPE {
+ DataType::Duration(TimeUnit::Second) => (IntervalParseUnit::Second,
1_000_000_000),
+ DataType::Duration(TimeUnit::Millisecond) =>
(IntervalParseUnit::Millisecond, 1_000_000),
+ DataType::Duration(TimeUnit::Microsecond) =>
(IntervalParseUnit::Microsecond, 1_000),
+ DataType::Duration(TimeUnit::Nanosecond) =>
(IntervalParseUnit::Nanosecond, 1),
+ _ => unreachable!(),
+ };
+
+ let interval = parse_interval_month_day_nano_config(string,
IntervalParseConfig::new(unit))
+ .map_err(|_| {
+ ArrowError::CastError(format!(
+ "Cannot cast string '{string}' to value of {} type",
+ D::DATA_TYPE
+ ))
+ })?;
+
+ if interval.months != 0 {
+ return Err(ArrowError::ComputeError(
+ "Cannot convert interval containing non-zero months to
duration".to_string(),
+ ));
+ }
+
+ let nanos = NANOS_PER_DAY as i128 * interval.days as i128 +
interval.nanoseconds as i128;
+ i64::try_from(nanos / scale as i128).map_err(|_| {
+ ArrowError::CastError(format!(
+ "Cannot cast string '{string}' to value of {} type",
+ D::DATA_TYPE
+ ))
+ })
+}
+
+/// Casts generic string arrays to a Duration type
+pub(crate) fn cast_string_to_duration<D, O>(
+ array: &dyn Array,
+ cast_options: &CastOptions,
+) -> Result<ArrayRef, ArrowError>
+where
+ D: ArrowTemporalType<Native = i64> + Parser,
+ O: OffsetSizeTrait,
+{
+ let string_array = array.as_string::<O>();
+ parse_duration_iter::<D, _, _>(string_array.iter(), cast_options, || {
+ string_array.nulls().cloned()
+ })
+}
+
+/// Casts string view arrays to a Duration type
+pub(crate) fn cast_view_to_duration<D>(
+ array: &dyn Array,
+ cast_options: &CastOptions,
+) -> Result<ArrayRef, ArrowError>
+where
+ D: ArrowTemporalType<Native = i64> + Parser,
+{
+ let string_view_array = array.as_string_view();
+ parse_duration_iter::<D, _, _>(string_view_array.iter(), cast_options, || {
+ string_view_array.nulls().cloned()
+ })
+}
+
+fn parse_duration_iter<'a, D, I, F>(
+ iter: I,
+ cast_options: &CastOptions,
+ nulls: F,
+) -> Result<ArrayRef, ArrowError>
+where
+ D: ArrowTemporalType<Native = i64> + Parser,
+ I: Iterator<Item = Option<&'a str>>,
+ F: FnOnce() -> Option<NullBuffer>,
+{
+ let array = if cast_options.safe {
+ let iter = iter.map(|x| {
+ let v = x?;
+ parse_duration::<D>(v).ok()
+ });
+
+ // Benefit:
+ // 20% performance improvement
+ // Soundness:
+ // The iterator is trustedLen because it comes from an
`StringArray`.
Review Comment:
Removed in 887ac2d72. The 20% figure was measured for `parse_string_iter`; I
hadn't verified it for this path, so it shouldn't be copied over.
##########
arrow-cast/src/cast/mod.rs:
##########
@@ -5526,6 +5563,101 @@ mod tests {
);
}
+ #[test]
+ fn test_cast_string_to_duration() {
+ let array = StringArray::from(vec![Some("1"), Some("-2"), None,
Some("invalid")]);
+ let to_type = DataType::Duration(TimeUnit::Second);
+ assert!(can_cast_types(array.data_type(), &to_type));
+
+ let casted = cast(&array, &to_type).unwrap();
+ assert_eq!(
+ casted.as_primitive::<DurationSecondType>(),
+ &DurationSecondArray::from(vec![Some(1), Some(-2), None, None])
+ );
+
+ let options = CastOptions {
+ safe: false,
+ format_options: FormatOptions::default(),
+ };
+ let err = cast_with_options(&array, &to_type, &options).unwrap_err();
+ assert_eq!(
+ err.to_string(),
+ "Cast error: Cannot cast string 'invalid' to value of Duration(s)
type"
+ );
+ }
+
+ #[test]
+ fn test_cast_string_with_units_to_duration() {
+ let array = StringArray::from(vec![
+ Some("1 hour 10 minutes"),
+ Some("90 seconds"),
+ Some("0.5 hours"),
+ Some("-1 hour -30 minutes"),
+ Some("2 days"),
+ Some("1 week"),
+ Some("45 days 14 hours 2 mins 34 secs"),
+ None,
+ Some("2 months"),
Review Comment:
Good catch. `0.5 months` was accepted as 15 days: the interval parser spills
fractional months into `days`, so the `interval.months != 0` check never saw
it. `0.5 decades` and `0.5 years` were already rejected because they become
months, but I've added coverage for all of them.
The cast now uses an opt-in
`IntervalParseConfig::reject_fractional_calendar_units()`, so `Interval` keeps
its existing spill semantics. The test covers `0.5 months`, `-0.5 months`, `1.5
months`, `0.5 years`, `0.5 decades`, with `15 days`, `0.5 days` and `0.5 weeks`
as controls. Safe casts return null; unsafe casts return a `CastError`. Done in
887ac2d72.
--
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]
