Improve formatting of datatypes

datafusion/common/src/types/logical.rs

@@ -100,7 +100,10 @@ impl fmt::Debug for dyn LogicalType {
 
 impl std::fmt::Display for dyn LogicalType {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{self:?}")
+        match self.signature() {
+            TypeSignature::Native(_) => write!(f, "{}", self.native()),
+            TypeSignature::Extension { name, .. } => write!(f, "{name}"),
+        }
     }
 }
 
@@ -132,3 +135,118 @@ impl Hash for dyn LogicalType {
         self.signature().hash(state);
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::types::{
+        LogicalField, LogicalFields, logical_boolean, logical_date, logical_float32,
+        logical_float64, logical_int32, logical_int64, logical_null, logical_string,
+    };
+    use arrow::datatypes::{DataType, Field, Fields};
+    use insta::assert_snapshot;
+
+    #[test]
+    fn test_logical_type_display_simple() {
+        assert_snapshot!(logical_null(), @"Null");
+        assert_snapshot!(logical_boolean(), @"Boolean");
+        assert_snapshot!(logical_int32(), @"Int32");
+        assert_snapshot!(logical_int64(), @"Int64");
+        assert_snapshot!(logical_float32(), @"Float32");
+        assert_snapshot!(logical_float64(), @"Float64");
+        assert_snapshot!(logical_string(), @"String");
+        assert_snapshot!(logical_date(), @"Date");
+    }
+
+    #[test]
+    fn test_logical_type_display_list() {
+        let list_type: Arc<dyn LogicalType> = Arc::new(NativeType::List(Arc::new(
+            LogicalField::from(&Field::new("item", DataType::Int32, true)),
+        )));
+        assert_snapshot!(list_type, @"List(Int32)");
+    }
+
+    #[test]
+    fn test_logical_type_display_struct() {
+        let struct_type: Arc<dyn LogicalType> = Arc::new(NativeType::Struct(
+            LogicalFields::from(&Fields::from(vec![
+                Field::new("x", DataType::Float64, false),
+                Field::new("y", DataType::Float64, true),
+            ])),
+        ));
+        assert_snapshot!(struct_type, @r#"Struct("x": non-null Float64, "y": Float64)"#);
+    }
+
+    #[test]
+    fn test_logical_type_display_fixed_size_list() {
+        let fsl_type: Arc<dyn LogicalType> = Arc::new(NativeType::FixedSizeList(
+            Arc::new(LogicalField::from(&Field::new(
+                "item",
+                DataType::Float32,
+                false,
+            ))),
+            3,
+        ));
+        assert_snapshot!(fsl_type, @"FixedSizeList(3 x non-null Float32)");
+    }
+
+    #[test]
+    fn test_logical_type_display_map() {
+        let map_type: Arc<dyn LogicalType> = Arc::new(NativeType::Map(Arc::new(
+            LogicalField::from(&Field::new("entries", DataType::Utf8, false)),
+        )));
+        assert_snapshot!(map_type, @"Map(non-null String)");
+    }
+
+    #[test]
+    fn test_logical_type_display_union() {
+        use arrow::datatypes::UnionFields;
+
+        let union_fields = UnionFields::try_new(
+            vec![0, 1],
+            vec![
+                Field::new("int_val", DataType::Int32, false),
+                Field::new("str_val", DataType::Utf8, true),
+            ],
+        )
+        .unwrap();
+        let union_type: Arc<dyn LogicalType> = Arc::new(NativeType::Union(
+            crate::types::LogicalUnionFields::from(&union_fields),
+        ));
+        assert_snapshot!(union_type, @r#"Union(0: ("int_val": non-null Int32), 1: ("str_val": String))"#);
+    }
+
+    #[test]
+    fn test_logical_type_display_nullable_vs_non_nullable() {
+        let nullable_list: Arc<dyn LogicalType> = Arc::new(NativeType::List(Arc::new(
+            LogicalField::from(&Field::new("item", DataType::Int32, true)),
+        )));
+        let non_nullable_list: Arc<dyn LogicalType> =
+            Arc::new(NativeType::List(Arc::new(LogicalField::from(&Field::new(
+                "item",
+                DataType::Int32,
+                false,
+            )))));
+
+        assert_snapshot!(nullable_list, @"List(Int32)");
+        assert_snapshot!(non_nullable_list, @"List(non-null Int32)");
+    }
+
+    #[test]
+    fn test_logical_type_display_extension() {
+        struct JsonType;
+        impl LogicalType for JsonType {
+            fn native(&self) -> &NativeType {
+                &NativeType::String
+            }
+            fn signature(&self) -> TypeSignature<'_> {
+                TypeSignature::Extension {
+                    name: "JSON",
+                    parameters: &[],
+                }
+            }
+        }
+        let json: Arc<dyn LogicalType> = Arc::new(JsonType);
+        assert_snapshot!(json, @"JSON");
+    }
+}

datafusion/common/src/types/native.rs

@@ -184,6 +184,16 @@ pub enum NativeType {
     Map(LogicalFieldRef),
 }
 
+/// Format a [`LogicalField`] for display, matching [`arrow::datatypes::DataType`]'s
+/// Display convention of showing a `"non-null "` prefix for non-nullable fields.
+fn format_logical_field(
+    f: &mut std::fmt::Formatter<'_>,
+    field: &LogicalField,
+) -> std::fmt::Result {
+    let non_null = if field.nullable { "" } else { "non-null " };
+    write!(f, "{:?}: {non_null}{}", field.name, field.logical_type)
+}
+
 impl Display for NativeType {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         // Match the format used by arrow::datatypes::DataType's Display impl
@@ -210,17 +220,25 @@ impl Display for NativeType {
             Self::Binary => write!(f, "Binary"),
             Self::FixedSizeBinary(size) => write!(f, "FixedSizeBinary({size})"),
             Self::String => write!(f, "String"),
-            Self::List(field) => write!(f, "List({})", field.logical_type),
+            Self::List(field) => {
+                let non_null = if field.nullable { "" } else { "non-null " };
+                write!(f, "List({non_null}{})", field.logical_type)
+            }
             Self::FixedSizeList(field, size) => {
-                write!(f, "FixedSizeList({size} x {})", field.logical_type)
+                let non_null = if field.nullable { "" } else { "non-null " };
+                write!(
+                    f,
+                    "FixedSizeList({size} x {non_null}{})",
+                    field.logical_type
+                )
             }
             Self::Struct(fields) => {
                 write!(f, "Struct(")?;
                 for (i, field) in fields.iter().enumerate() {
                     if i > 0 {
                         write!(f, ", ")?;
                     }
-                    write!(f, "{:?}: {}", field.name, field.logical_type)?;
+                    format_logical_field(f, field)?;
                 }
                 write!(f, ")")
             }
@@ -230,12 +248,17 @@ impl Display for NativeType {
                     if i > 0 {
                         write!(f, ", ")?;
                     }
-                    write!(f, "{type_id}: ({:?}: {})", field.name, field.logical_type)?;
+                    write!(f, "{type_id}: (")?;
+                    format_logical_field(f, field)?;
+                    write!(f, ")")?;
                 }
                 write!(f, ")")
             }
             Self::Decimal(precision, scale) => write!(f, "Decimal({precision}, {scale})"),
-            Self::Map(field) => write!(f, "Map({})", field.logical_type),
+            Self::Map(field) => {
+                let non_null = if field.nullable { "" } else { "non-null " };
+                write!(f, "Map({non_null}{})", field.logical_type)
+            }
         }
     }
 }
@@ -537,3 +560,90 @@ impl NativeType {
         matches!(self, Self::Float16 | Self::Float32 | Self::Float64)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::types::LogicalField;
+    use arrow::datatypes::Field;
+    use insta::assert_snapshot;
+
+    #[test]
+    fn test_native_type_display() {
+        assert_snapshot!(NativeType::Null, @"Null");
+        assert_snapshot!(NativeType::Boolean, @"Boolean");
+        assert_snapshot!(NativeType::Int8, @"Int8");
+        assert_snapshot!(NativeType::Int16, @"Int16");
+        assert_snapshot!(NativeType::Int32, @"Int32");
+        assert_snapshot!(NativeType::Int64, @"Int64");
+        assert_snapshot!(NativeType::UInt8, @"UInt8");
+        assert_snapshot!(NativeType::UInt16, @"UInt16");
+        assert_snapshot!(NativeType::UInt32, @"UInt32");
+        assert_snapshot!(NativeType::UInt64, @"UInt64");
+        assert_snapshot!(NativeType::Float16, @"Float16");
+        assert_snapshot!(NativeType::Float32, @"Float32");
+        assert_snapshot!(NativeType::Float64, @"Float64");
+        assert_snapshot!(NativeType::Date, @"Date");
+        assert_snapshot!(NativeType::Binary, @"Binary");
+        assert_snapshot!(NativeType::String, @"String");
+        assert_snapshot!(NativeType::FixedSizeBinary(16), @"FixedSizeBinary(16)");
+        assert_snapshot!(NativeType::Decimal(10, 2), @"Decimal(10, 2)");
+    }
+
+    #[test]
+    fn test_native_type_display_timestamp() {
+        assert_snapshot!(
+            NativeType::Timestamp(TimeUnit::Second, None),
+            @"Timestamp(s)"
+        );
+        assert_snapshot!(
+            NativeType::Timestamp(TimeUnit::Millisecond, None),
+            @"Timestamp(ms)"
+        );
+        assert_snapshot!(
+            NativeType::Timestamp(TimeUnit::Nanosecond, Some(Arc::from("UTC"))),
+            @r#"Timestamp(ns, "UTC")"#
+        );
+    }
+
+    #[test]
+    fn test_native_type_display_time_duration_interval() {
+        assert_snapshot!(NativeType::Time(TimeUnit::Microsecond), @"Time(µs)");
+        assert_snapshot!(NativeType::Duration(TimeUnit::Nanosecond), @"Duration(ns)");
+        assert_snapshot!(NativeType::Interval(IntervalUnit::YearMonth), @"Interval(YearMonth)");
+        assert_snapshot!(NativeType::Interval(IntervalUnit::MonthDayNano), @"Interval(MonthDayNano)");
+    }
+
+    #[test]
+    fn test_native_type_display_nested() {
+        let list = NativeType::List(Arc::new(LogicalField::from(&Field::new(
+            "item",
+            DataType::Int32,
+            true,
+        ))));
+        assert_snapshot!(list, @"List(Int32)");
+
+        let fixed_list = NativeType::FixedSizeList(
+            Arc::new(LogicalField::from(&Field::new(
+                "item",
+                DataType::Float64,
+                false,
+            ))),
+            3,
+        );
+        assert_snapshot!(fixed_list, @"FixedSizeList(3 x non-null Float64)");
+
+        let struct_type = NativeType::Struct(LogicalFields::from(&Fields::from(vec![
+            Field::new("name", DataType::Utf8, false),
+            Field::new("age", DataType::Int32, true),
+        ])));
+        assert_snapshot!(struct_type, @r#"Struct("name": non-null String, "age": Int32)"#);
+
+        let map = NativeType::Map(Arc::new(LogicalField::from(&Field::new(
+            "entries",
+            DataType::Utf8,
+            false,
+        ))));
+        assert_snapshot!(map, @"Map(non-null String)");
+    }
+}

datafusion/expr-common/Cargo.toml

@@ -46,3 +46,6 @@ datafusion-common = { workspace = true }
 indexmap = { workspace = true }
 itertools = { workspace = true }
 paste = { workspace = true }
+
+[dev-dependencies]
+insta = { workspace = true }