This is an automated email from the ASF dual-hosted git repository.
alamb 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 1c380b8336 fix: Panic in pretty_format function when displaying
DurationSecondsA… (#7534)
1c380b8336 is described below
commit 1c380b8336ec9be1cdbd3179f2d289bf6087f507
Author: Qi Zhu <[email protected]>
AuthorDate: Mon May 26 18:19:13 2025 +0800
fix: Panic in pretty_format function when displaying DurationSecondsA…
(#7534)
* fix: Panic in pretty_format function when displaying DurationSecondsArray
with i64::MIN / i64::MAX
* Address comments
* Avoid changes for infallible functions
* Fix compilation
* fix docs
---------
Co-authored-by: Andrew Lamb <[email protected]>
---
arrow-array/src/temporal_conversions.rs | 20 ++++++++++--
arrow-cast/src/display.rs | 41 ++++++++++++++++++++++---
arrow-cast/src/pretty.rs | 54 ++++++++++++++++++++++++++++++++-
3 files changed, 107 insertions(+), 8 deletions(-)
diff --git a/arrow-array/src/temporal_conversions.rs
b/arrow-array/src/temporal_conversions.rs
index 23f950d550..7a4c676029 100644
--- a/arrow-array/src/temporal_conversions.rs
+++ b/arrow-array/src/temporal_conversions.rs
@@ -217,14 +217,28 @@ pub(crate) fn split_second(v: i64, base: i64) -> (i64,
u32) {
/// converts a `i64` representing a `duration(s)` to [`Duration`]
#[inline]
+#[deprecated(since = "55.2.0", note = "Use `try_duration_s_to_duration`
instead")]
pub fn duration_s_to_duration(v: i64) -> Duration {
Duration::try_seconds(v).unwrap()
}
+/// converts a `i64` representing a `duration(s)` to [`Option<Duration>`]
+#[inline]
+pub fn try_duration_s_to_duration(v: i64) -> Option<Duration> {
+ Duration::try_seconds(v)
+}
+
/// converts a `i64` representing a `duration(ms)` to [`Duration`]
#[inline]
+#[deprecated(since = "55.2.0", note = "Use `try_duration_ms_to_duration`
instead")]
pub fn duration_ms_to_duration(v: i64) -> Duration {
- Duration::try_milliseconds(v).unwrap()
+ Duration::try_seconds(v).unwrap()
+}
+
+/// converts a `i64` representing a `duration(ms)` to [`Option<Duration>`]
+#[inline]
+pub fn try_duration_ms_to_duration(v: i64) -> Option<Duration> {
+ Duration::try_milliseconds(v)
}
/// converts a `i64` representing a `duration(us)` to [`Duration`]
@@ -296,8 +310,8 @@ pub fn as_time<T: ArrowPrimitiveType>(v: i64) ->
Option<NaiveTime> {
pub fn as_duration<T: ArrowPrimitiveType>(v: i64) -> Option<Duration> {
match T::DATA_TYPE {
DataType::Duration(unit) => match unit {
- TimeUnit::Second => Some(duration_s_to_duration(v)),
- TimeUnit::Millisecond => Some(duration_ms_to_duration(v)),
+ TimeUnit::Second => try_duration_s_to_duration(v),
+ TimeUnit::Millisecond => try_duration_ms_to_duration(v),
TimeUnit::Microsecond => Some(duration_us_to_duration(v)),
TimeUnit::Nanosecond => Some(duration_ns_to_duration(v)),
},
diff --git a/arrow-cast/src/display.rs b/arrow-cast/src/display.rs
index 3c61797c96..6761ac22fa 100644
--- a/arrow-cast/src/display.rs
+++ b/arrow-cast/src/display.rs
@@ -590,6 +590,12 @@ temporal_display!(time32ms_to_time, time_format,
Time32MillisecondType);
temporal_display!(time64us_to_time, time_format, Time64MicrosecondType);
temporal_display!(time64ns_to_time, time_format, Time64NanosecondType);
+/// Derive [`DisplayIndexState`] for `PrimitiveArray<$t>`
+///
+/// Arguments
+/// * `$convert` - function to convert the value to an `Duration`
+/// * `$t` - [`ArrowPrimitiveType`] of the array
+/// * `$scale` - scale of the duration (passed to `duration_fmt`)
macro_rules! duration_display {
($convert:ident, $t:ty, $scale:tt) => {
impl<'a> DisplayIndexState<'a> for &'a PrimitiveArray<$t> {
@@ -611,6 +617,34 @@ macro_rules! duration_display {
};
}
+/// Similar to [`duration_display`] but `$convert` returns an `Option`
+macro_rules! duration_option_display {
+ ($convert:ident, $t:ty, $scale:tt) => {
+ impl<'a> DisplayIndexState<'a> for &'a PrimitiveArray<$t> {
+ type State = DurationFormat;
+
+ fn prepare(&self, options: &FormatOptions<'a>) ->
Result<Self::State, ArrowError> {
+ Ok(options.duration_format)
+ }
+
+ fn write(&self, fmt: &Self::State, idx: usize, f: &mut dyn Write)
-> FormatResult {
+ let v = self.value(idx);
+ match fmt {
+ DurationFormat::ISO8601 => match $convert(v) {
+ Some(td) => write!(f, "{}", td)?,
+ None => write!(f, "<invalid>")?,
+ },
+ DurationFormat::Pretty => match $convert(v) {
+ Some(_) => duration_fmt!(f, v, $scale)?,
+ None => write!(f, "<invalid>")?,
+ },
+ }
+ Ok(())
+ }
+ }
+ };
+}
+
macro_rules! duration_fmt {
($f:ident, $v:expr, 0) => {{
let secs = $v;
@@ -657,8 +691,8 @@ macro_rules! duration_fmt {
}};
}
-duration_display!(duration_s_to_duration, DurationSecondType, 0);
-duration_display!(duration_ms_to_duration, DurationMillisecondType, 3);
+duration_option_display!(try_duration_s_to_duration, DurationSecondType, 0);
+duration_option_display!(try_duration_ms_to_duration, DurationMillisecondType,
3);
duration_display!(duration_us_to_duration, DurationMicrosecondType, 6);
duration_display!(duration_ns_to_duration, DurationNanosecondType, 9);
@@ -1071,9 +1105,8 @@ pub fn lexical_to_string<N: lexical_core::ToLexical>(n:
N) -> String {
#[cfg(test)]
mod tests {
- use arrow_array::builder::StringRunBuilder;
-
use super::*;
+ use arrow_array::builder::StringRunBuilder;
/// Test to verify options can be constant. See #4580
const TEST_CONST_OPTIONS: FormatOptions<'static> = FormatOptions::new()
diff --git a/arrow-cast/src/pretty.rs b/arrow-cast/src/pretty.rs
index 5223c9d7e1..1c0b7a5d44 100644
--- a/arrow-cast/src/pretty.rs
+++ b/arrow-cast/src/pretty.rs
@@ -221,7 +221,7 @@ mod tests {
use arrow_buffer::{IntervalDayTime, IntervalMonthDayNano, ScalarBuffer};
use arrow_schema::*;
- use crate::display::array_value_to_string;
+ use crate::display::{array_value_to_string, DurationFormat};
use super::*;
@@ -1186,4 +1186,56 @@ mod tests {
let actual: Vec<&str> = batch.lines().collect();
assert_eq!(expected_table, actual, "Actual result:\n{batch}");
}
+
+ #[test]
+ fn duration_pretty_and_iso_extremes() {
+ // Build [MIN, MAX, 3661, NULL]
+ let arr = DurationSecondArray::from(vec![Some(i64::MIN),
Some(i64::MAX), Some(3661), None]);
+ let array: ArrayRef = Arc::new(arr);
+
+ // Pretty formatting
+ let opts = FormatOptions::default().with_null("null");
+ let opts = opts.with_duration_format(DurationFormat::Pretty);
+ let pretty = pretty_format_columns_with_options("pretty",
&[array.clone()], &opts)
+ .unwrap()
+ .to_string();
+
+ // Expected output
+ let expected_pretty = vec![
+ "+------------------------------+",
+ "| pretty |",
+ "+------------------------------+",
+ "| <invalid> |",
+ "| <invalid> |",
+ "| 0 days 1 hours 1 mins 1 secs |",
+ "| null |",
+ "+------------------------------+",
+ ];
+
+ let actual: Vec<&str> = pretty.lines().collect();
+ assert_eq!(expected_pretty, actual, "Actual result:\n{pretty}");
+
+ // ISO8601 formatting
+ let opts_iso = FormatOptions::default()
+ .with_null("null")
+ .with_duration_format(DurationFormat::ISO8601);
+ let iso = pretty_format_columns_with_options("iso", &[array],
&opts_iso)
+ .unwrap()
+ .to_string();
+
+ // Expected output
+ let expected_iso = vec![
+ "+-----------+",
+ "| iso |",
+ "+-----------+",
+ "| <invalid> |",
+ "| <invalid> |",
+ "| PT3661S |",
+ "| null |",
+ "+-----------+",
+ ];
+
+ let actual: Vec<&str> = iso.lines().collect();
+ assert_eq!(expected_iso, actual, "Actual result:\n{iso}");
+ }
}
