Improve error messages with nicer formatting of Date and Time types (#19954)

* Follow-up to: #17565
* Related: apache/arrow-rs#8290

## Rationale for this change
I believe that error messages should be as readable as possible. Aim for
`rustc` more than `gcc`.

`Display` is the nice, user-facing formatter. `Debug` is for… well,
debugging.

## What changes are included in this PR?
Change a bunch of `{:?}` format string to `{}`. I'm sure I missed a lot
of them, because I know of no way to enforce this without
* rust-lang/rust-clippy#8581

## Are these changes tested?
I assume CI runs `cargo test` :)

## Are there any user-facing changes?
Yes! Error messages should be a bit more readable now.

---------

Co-authored-by: Tim Saucer <timsaucer@gmail.com>

Author: emilk

datafusion/common/src/types/native.rs

@@ -186,7 +186,57 @@ pub enum NativeType {
 
 impl Display for NativeType {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{self:?}") // TODO: nicer formatting
+        // Match the format used by arrow::datatypes::DataType's Display impl
+        match self {
+            Self::Null => write!(f, "Null"),
+            Self::Boolean => write!(f, "Boolean"),
+            Self::Int8 => write!(f, "Int8"),
+            Self::Int16 => write!(f, "Int16"),
+            Self::Int32 => write!(f, "Int32"),
+            Self::Int64 => write!(f, "Int64"),
+            Self::UInt8 => write!(f, "UInt8"),
+            Self::UInt16 => write!(f, "UInt16"),
+            Self::UInt32 => write!(f, "UInt32"),
+            Self::UInt64 => write!(f, "UInt64"),
+            Self::Float16 => write!(f, "Float16"),
+            Self::Float32 => write!(f, "Float32"),
+            Self::Float64 => write!(f, "Float64"),
+            Self::Timestamp(unit, Some(tz)) => write!(f, "Timestamp({unit}, {tz:?})"),
+            Self::Timestamp(unit, None) => write!(f, "Timestamp({unit})"),
+            Self::Date => write!(f, "Date"),
+            Self::Time(unit) => write!(f, "Time({unit})"),
+            Self::Duration(unit) => write!(f, "Duration({unit})"),
+            Self::Interval(unit) => write!(f, "Interval({unit:?})"),
+            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::FixedSizeList(field, size) => {
+                write!(f, "FixedSizeList({size} x {})", 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)?;
+                }
+                write!(f, ")")
+            }
+            Self::Union(fields) => {
+                write!(f, "Union(")?;
+                for (i, (type_id, field)) in fields.iter().enumerate() {
+                    if i > 0 {
+                        write!(f, ", ")?;
+                    }
+                    write!(f, "{type_id}: ({:?}: {})", field.name, field.logical_type)?;
+                }
+                write!(f, ")")
+            }
+            Self::Decimal(precision, scale) => write!(f, "Decimal({precision}, {scale})"),
+            Self::Map(field) => write!(f, "Map({})", field.logical_type),
+        }
     }
 }
 

datafusion/core/src/datasource/file_format/avro.rs

@@ -95,7 +95,7 @@ mod tests {
             .schema()
             .fields()
             .iter()
-            .map(|f| format!("{}: {:?}", f.name(), f.data_type()))
+            .map(|f| format!("{}: {}", f.name(), f.data_type()))
             .collect();
         assert_eq!(
             vec![
@@ -109,7 +109,7 @@ mod tests {
                 "double_col: Float64",
                 "date_string_col: Binary",
                 "string_col: Binary",
-                "timestamp_col: Timestamp(Microsecond, None)",
+                "timestamp_col: Timestamp(µs)",
             ],
             x
         );

datafusion/core/src/datasource/file_format/parquet.rs

@@ -815,7 +815,7 @@ mod tests {
             .schema()
             .fields()
             .iter()
-            .map(|f| format!("{}: {:?}", f.name(), f.data_type()))
+            .map(|f| format!("{}: {}", f.name(), f.data_type()))
             .collect();
         let y = x.join("\n");
         assert_eq!(expected, y);
@@ -841,7 +841,7 @@ mod tests {
              double_col: Float64\n\
              date_string_col: Binary\n\
              string_col: Binary\n\
-             timestamp_col: Timestamp(Nanosecond, None)";
+             timestamp_col: Timestamp(ns)";
         _run_read_alltypes_plain_parquet(ForceViews::No, no_views).await?;
 
         let with_views = "id: Int32\n\
@@ -854,7 +854,7 @@ mod tests {
              double_col: Float64\n\
              date_string_col: BinaryView\n\
              string_col: BinaryView\n\
-             timestamp_col: Timestamp(Nanosecond, None)";
+             timestamp_col: Timestamp(ns)";
         _run_read_alltypes_plain_parquet(ForceViews::Yes, with_views).await?;
 
         Ok(())

datafusion/core/src/physical_planner.rs

@@ -2753,7 +2753,7 @@ impl<'a> OptimizationInvariantChecker<'a> {
             && !is_allowed_schema_change(previous_schema.as_ref(), plan.schema().as_ref())
         {
             internal_err!(
-                "PhysicalOptimizer rule '{}' failed. Schema mismatch. Expected original schema: {:?}, got new schema: {:?}",
+                "PhysicalOptimizer rule '{}' failed. Schema mismatch. Expected original schema: {}, got new schema: {}",
                 self.rule.name(),
                 previous_schema,
                 plan.schema()

datafusion/core/tests/dataframe/mod.rs

@@ -4802,7 +4802,7 @@ async fn unnest_with_redundant_columns() -> Result<()> {
         @r"
     Projection: shapes.shape_id [shape_id:UInt32]
       Unnest: lists[shape_id2|depth=1] structs[] [shape_id:UInt32, shape_id2:UInt32;N]
-        Aggregate: groupBy=[[shapes.shape_id]], aggr=[[array_agg(shapes.shape_id) AS shape_id2]] [shape_id:UInt32, shape_id2:List(Field { data_type: UInt32, nullable: true });N]
+        Aggregate: groupBy=[[shapes.shape_id]], aggr=[[array_agg(shapes.shape_id) AS shape_id2]] [shape_id:UInt32, shape_id2:List(UInt32);N]
           TableScan: shapes projection=[shape_id] [shape_id:UInt32]
     "
     );

datafusion/datasource/src/sink.rs

@@ -94,7 +94,7 @@ pub struct DataSinkExec {
 
 impl Debug for DataSinkExec {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "DataSinkExec schema: {:?}", self.count_schema)
+        write!(f, "DataSinkExec schema: {}", self.count_schema)
     }
 }
 

datafusion/expr/src/logical_plan/display.rs

@@ -117,13 +117,7 @@ pub fn display_schema(schema: &Schema) -> impl fmt::Display + '_ {
                     write!(f, ", ")?;
                 }
                 let nullable_str = if field.is_nullable() { ";N" } else { "" };
-                write!(
-                    f,
-                    "{}:{:?}{}",
-                    field.name(),
-                    field.data_type(),
-                    nullable_str
-                )?;
+                write!(f, "{}:{}{}", field.name(), field.data_type(), nullable_str)?;
             }
             write!(f, "]")
         }

datafusion/expr/src/type_coercion/functions.rs

@@ -1068,7 +1068,7 @@ mod tests {
         .unwrap_err();
         assert_contains!(
             got.to_string(),
-            "Function 'test' expects NativeType::Numeric but received NativeType::Timestamp(Second, None)"
+            "Function 'test' expects NativeType::Numeric but received NativeType::Timestamp(s)"
         );
 
         Ok(())

datafusion/functions-nested/src/array_has.rs

@@ -262,7 +262,7 @@ impl<'a> TryFrom<&'a dyn Array> for ArrayWrapper<'a> {
             DataType::FixedSizeList(_, _) => Ok(ArrayWrapper::FixedSizeList(
                 as_fixed_size_list_array(value)?,
             )),
-            _ => exec_err!("array_has does not support type '{:?}'.", value.data_type()),
+            _ => exec_err!("array_has does not support type '{}'.", value.data_type()),
         }
     }
 }

datafusion/functions-nested/src/flatten.rs

@@ -208,7 +208,7 @@ fn flatten_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
         }
         Null => Ok(Arc::clone(array)),
         _ => {
-            exec_err!("flatten does not support type '{:?}'", array.data_type())
+            exec_err!("flatten does not support type '{}'", array.data_type())
         }
     }
 }