samuelcolvin commented on code in PR #6211:
URL: https://github.com/apache/arrow-rs/pull/6211#discussion_r1709759294
##########
arrow-cast/src/parse.rs:
##########
@@ -1013,9 +1020,9 @@ const NANOS_PER_HOUR: i64 = 60 * NANOS_PER_MINUTE;
const NANOS_PER_DAY: i64 = 24 * NANOS_PER_HOUR;
#[rustfmt::skip]
-#[derive(Clone, Copy)]
+#[derive(Debug, Clone, Copy)]
#[repr(u16)]
-enum IntervalUnit {
+pub enum IntervalUnit {
Review Comment:
has to be made public for `parse_interval_month_day_nano_config`
##########
arrow-cast/src/parse.rs:
##########
@@ -994,17 +994,24 @@ pub fn parse_interval_day_time(
Ok(IntervalDayTimeType::make_value(days, millis))
}
-pub fn parse_interval_month_day_nano(
+pub fn parse_interval_month_day_nano_config(
Review Comment:
the idea is we can use this method in datafusion, and set the `default_unit`
to seconds, without changing the behaviour of the arrow-rs API.
I'd also be happy to simply change the default used in
`parse_interval_month_day_nano`
##########
arrow-cast/src/parse.rs:
##########
@@ -1369,51 +1394,28 @@ fn parse_interval_components(
value: &str,
config: &IntervalParseConfig,
) -> Result<Vec<(IntervalAmount, IntervalUnit)>, ArrowError> {
- let parts = value.split_whitespace();
-
- let raw_amounts = parts.clone().step_by(2);
- let raw_units = parts.skip(1).step_by(2);
+ let raw_pairs = split_interval_components(value);
// parse amounts
- let (amounts, invalid_amounts) = raw_amounts
- .map(IntervalAmount::from_str)
- .partition::<Vec<_>, _>(Result::is_ok);
Review Comment:
this "partition on error when error if the invalid vec is not empty" is a
very unusual way to detect errors when iterating over something.
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
