Skip to content
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 56 additions & 1 deletion arrow-schema/src/datatype_display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,24 @@ impl fmt::Display for DataType {
Ok(())
}
Self::Union(union_fields, union_mode) => {
write!(f, "Union({union_fields:?}, {union_mode:?})")
write!(f, "Union({union_mode:?}, ")?;
if !union_fields.is_empty() {
let fields_str = union_fields
.iter()
.map(|v| {
let type_id = v.0;
let field = v.1;
let maybe_nullable = if field.is_nullable() { "nullable " } else { "" };
let data_type = field.data_type();
let metadata_str = format_metadata(field.metadata());
format!("{type_id:?}: {maybe_nullable}{data_type}{metadata_str}")
})
.collect::<Vec<_>>()
.join(", ");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to use ; as a separator ?
This way the , used to separate the metadata won't confuse the readers.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some investigation, I think we should continue using commas to separate different fields, following the DuckDB SQL style. For metadata, since it is displayed with brackets, it should not be too confusing.

write!(f, "{fields_str}")?;
}
write!(f, ")")?;
Ok(())
}
Self::Dictionary(data_type, data_type1) => {
write!(f, "Dictionary({data_type}, {data_type1})")
Expand Down Expand Up @@ -248,4 +265,42 @@ mod tests {
"FixedSizeList(4 x nullable Int32, metadata: {\"key2\": \"value2\"})";
assert_eq!(fixed_size_metadata_string, expected_metadata_string);
}

#[test]
fn test_display_union() {
let fields = vec![
Field::new("a", DataType::Int32, false),
Field::new("b", DataType::Utf8, true),
];
let type_ids = vec![0, 1];
let union_fields = type_ids
.into_iter()
.zip(fields.into_iter().map(Arc::new))
.collect();

let union_data_type = DataType::Union(union_fields, crate::UnionMode::Sparse);
let union_data_type_string = union_data_type.to_string();
let expected_string = "Union(Sparse, 0: Int32, 1: nullable Utf8)";
assert_eq!(union_data_type_string, expected_string);

// Test with metadata
let mut field_with_metadata = Field::new("b", DataType::Utf8, true);
let metadata = HashMap::from([("key".to_string(), "value".to_string())]);
field_with_metadata.set_metadata(metadata);
let union_fields_with_metadata = vec![
(0, Arc::new(Field::new("a", DataType::Int32, false))),
(1, Arc::new(field_with_metadata)),
]
.into_iter()
.collect();
let union_data_type_with_metadata =
DataType::Union(union_fields_with_metadata, crate::UnionMode::Sparse);
let union_data_type_with_metadata_string = union_data_type_with_metadata.to_string();
let expected_string_with_metadata =
"Union(Sparse, 0: Int32, 1: nullable Utf8, metadata: {\"key\": \"value\"})";
assert_eq!(
union_data_type_with_metadata_string,
expected_string_with_metadata
);
}
}
Loading