Jefffrey commented on code in PR #11572:
URL: https://github.com/apache/arrow-rs/pull/11572#discussion_r4387458324
##########
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:
could we add a test for not whole units such as 0.5 months, 0.5 decades, etc.
i think they should also be rejected
##########
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:
does `nulls: F` strictly need to be a closure?
##########
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) {
Review Comment:
i suppose parsing bare units is consistent with the interval logic 👍
##########
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:
```suggestion
```
lets not copy this comment over, especially as in this case we havent
verified its true
##########
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))
Review Comment:
is there potential for overflow when using this existing parser? as in, if
we have `10000000000 seconds` can this successfully parse into
`Duration(seconds)`, since I believe this function internally accumulates as
nanoseconds?
--
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]
