Skip to content

Commit 5e8648f

Browse files
qrilkajayzhan211
authored andcommitted
Unify formatting of both groups and files up to 5 elements (apache#6637)
This fixes inconsistency between rustdocs for Display of files and file groups by applying the same function formatting up to 5 elements. Also adds corresponding unit tests.
1 parent 4c7d318 commit 5e8648f

File tree

1 file changed

+68
-23
lines changed
  • datafusion/core/src/datasource/physical_plan

1 file changed

+68
-23
lines changed

datafusion/core/src/datasource/physical_plan/mod.rs

Lines changed: 68 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -312,27 +312,19 @@ struct FileGroupsDisplay<'a>(&'a [Vec<PartitionedFile>]);
312312

313313
impl<'a> Display for FileGroupsDisplay<'a> {
314314
fn fmt(&self, f: &mut Formatter) -> FmtResult {
315-
let n_group = self.0.len();
316-
let groups = if n_group == 1 { "group" } else { "groups" };
317-
write!(f, "{{{n_group} {groups}: [")?;
315+
let n_groups = self.0.len();
316+
let groups = if n_groups == 1 { "group" } else { "groups" };
317+
write!(f, "{{{n_groups} {groups}: [")?;
318318
// To avoid showing too many partitions
319319
let max_groups = 5;
320-
for (idx, group) in self.0.iter().take(max_groups).enumerate() {
321-
if idx > 0 {
322-
write!(f, ", ")?;
323-
}
324-
write!(f, "{}", FileGroupDisplay(group))?;
325-
}
326-
// Remaining elements are showed as `...` (to indicate there is more)
327-
if n_group > max_groups {
328-
write!(f, ", ...")?;
329-
}
330-
write!(f, "]}}")?;
331-
Ok(())
320+
fmt_up_to_n_elements(self.0, max_groups, f, |group, f| {
321+
write!(f, "{}", FileGroupDisplay(group))
322+
})?;
323+
write!(f, "]}}")
332324
}
333325
}
334326

335-
/// A wrapper to customize partitioned file display
327+
/// A wrapper to customize partitioned group of files display
336328
///
337329
/// Prints in the format:
338330
/// ```text
@@ -343,20 +335,42 @@ pub(crate) struct FileGroupDisplay<'a>(pub &'a [PartitionedFile]);
343335

344336
impl<'a> Display for FileGroupDisplay<'a> {
345337
fn fmt(&self, f: &mut Formatter) -> FmtResult {
346-
let group = self.0;
347338
write!(f, "[")?;
348-
for (idx, pf) in group.iter().enumerate() {
349-
if idx > 0 {
350-
write!(f, ", ")?;
351-
}
339+
// To avoid showing too many files
340+
let max_files = 5;
341+
fmt_up_to_n_elements(self.0, max_files, f, |pf, f| {
352342
write!(f, "{}", pf.object_meta.location.as_ref())?;
353343
if let Some(range) = pf.range.as_ref() {
354344
write!(f, ":{}..{}", range.start, range.end)?;
355345
}
346+
Ok(())
347+
})?;
348+
write!(f, "]")
349+
}
350+
}
351+
352+
/// helper to format an array of up to N elements
353+
fn fmt_up_to_n_elements<E, F>(
354+
elements: &[E],
355+
n: usize,
356+
f: &mut Formatter,
357+
format_element: F,
358+
) -> FmtResult
359+
where
360+
F: Fn(&E, &mut Formatter) -> FmtResult,
361+
{
362+
let len = elements.len();
363+
for (idx, element) in elements.iter().take(n).enumerate() {
364+
if idx > 0 {
365+
write!(f, ", ")?;
356366
}
357-
write!(f, "]")?;
358-
Ok(())
367+
format_element(element, f)?;
368+
}
369+
// Remaining elements are showed as `...` (to indicate there is more)
370+
if len > n {
371+
write!(f, ", ...")?;
359372
}
373+
Ok(())
360374
}
361375

362376
/// A wrapper to customize partitioned file display
@@ -1298,6 +1312,22 @@ mod tests {
12981312
assert_eq!(&FileGroupsDisplay(&files).to_string(), expected);
12991313
}
13001314

1315+
#[test]
1316+
fn file_groups_display_too_many() {
1317+
let files = [
1318+
vec![partitioned_file("foo"), partitioned_file("bar")],
1319+
vec![partitioned_file("baz")],
1320+
vec![partitioned_file("qux")],
1321+
vec![partitioned_file("quux")],
1322+
vec![partitioned_file("quuux")],
1323+
vec![partitioned_file("quuuux")],
1324+
vec![],
1325+
];
1326+
1327+
let expected = "{7 groups: [[foo, bar], [baz], [qux], [quux], [quuux], ...]}";
1328+
assert_eq!(&FileGroupsDisplay(&files).to_string(), expected);
1329+
}
1330+
13011331
#[test]
13021332
fn file_group_display_many() {
13031333
let files = vec![partitioned_file("foo"), partitioned_file("bar")];
@@ -1306,6 +1336,21 @@ mod tests {
13061336
assert_eq!(&FileGroupDisplay(&files).to_string(), expected);
13071337
}
13081338

1339+
#[test]
1340+
fn file_group_display_too_many() {
1341+
let files = vec![
1342+
partitioned_file("foo"),
1343+
partitioned_file("bar"),
1344+
partitioned_file("baz"),
1345+
partitioned_file("qux"),
1346+
partitioned_file("quux"),
1347+
partitioned_file("quuux"),
1348+
];
1349+
1350+
let expected = "[foo, bar, baz, qux, quux, ...]";
1351+
assert_eq!(&FileGroupDisplay(&files).to_string(), expected);
1352+
}
1353+
13091354
/// create a PartitionedFile for testing
13101355
fn partitioned_file(path: &str) -> PartitionedFile {
13111356
let object_meta = ObjectMeta {

0 commit comments

Comments
 (0)