From 60755203ebeb325328c64ed42f1c065b18d78a1c Mon Sep 17 00:00:00 2001
From: Dan King <dan@spiraldb.com>
Date: Tue, 8 Oct 2024 09:43:54 -0400
Subject: [PATCH] fix: rely on ScalarValue Display in Scalar Display (#978)

Curious for your thoughts on Utf8 and Buffer. It feels a little wrong to
change them to truncate by default? I think I really want two kinds of
`Display`, one that's lossless and one that's lossy and meant to be
"small".
---
 vortex-scalar/src/display.rs | 42 +++++++++++++-----------------------
 1 file changed, 15 insertions(+), 27 deletions(-)

diff --git a/vortex-scalar/src/display.rs b/vortex-scalar/src/display.rs
index d3355638ad..b9dced5ecf 100644
--- a/vortex-scalar/src/display.rs
+++ b/vortex-scalar/src/display.rs
@@ -2,12 +2,10 @@ use std::fmt::{Display, Formatter};
 
 use itertools::Itertools;
 use vortex_datetime_dtype::{is_temporal_ext_type, TemporalMetadata};
-use vortex_dtype::{match_each_native_ptype, DType};
+use vortex_dtype::DType;
 
 use crate::binary::BinaryScalar;
-use crate::bool::BoolScalar;
 use crate::extension::ExtScalar;
-use crate::primitive::PrimitiveScalar;
 use crate::struct_::StructScalar;
 use crate::utf8::Utf8Scalar;
 use crate::{PValue, Scalar, ScalarValue};
@@ -15,20 +13,7 @@ use crate::{PValue, Scalar, ScalarValue};
 impl Display for Scalar {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         match self.dtype() {
-            DType::Null => write!(f, "null"),
-            DType::Bool(_) => match BoolScalar::try_from(self)
-                .map_err(|_| std::fmt::Error)?
-                .value()
-            {
-                None => write!(f, "null"),
-                Some(b) => write!(f, "{}", b),
-            },
-            DType::Primitive(ptype, _) => match_each_native_ptype!(ptype, |$T| {
-                match PrimitiveScalar::try_from(self).expect("primitive").typed_value::<$T>() {
-                    None => write!(f, "null"),
-                    Some(v) => write!(f, "{}", v),
-                }
-            }),
+            DType::Null | DType::Bool(_) | DType::Primitive(..) => Display::fmt(&self.value, f),
             DType::Utf8(_) => {
                 match Utf8Scalar::try_from(self)
                     .map_err(|_| std::fmt::Error)?
@@ -123,17 +108,20 @@ mod tests {
 
     #[test]
     fn display_primitive() {
-        assert_eq!(format!("{}", Scalar::from(0_u8)), "0");
-        assert_eq!(format!("{}", Scalar::from(255_u8)), "255");
+        assert_eq!(format!("{}", Scalar::from(0_u8)), "0_u8");
+        assert_eq!(format!("{}", Scalar::from(255_u8)), "255_u8");
 
-        assert_eq!(format!("{}", Scalar::from(0_u16)), "0");
-        assert_eq!(format!("{}", Scalar::from(!0_u16)), "65535");
+        assert_eq!(format!("{}", Scalar::from(0_u16)), "0_u16");
+        assert_eq!(format!("{}", Scalar::from(!0_u16)), "65535_u16");
 
-        assert_eq!(format!("{}", Scalar::from(0_u32)), "0");
-        assert_eq!(format!("{}", Scalar::from(!0_u32)), "4294967295");
+        assert_eq!(format!("{}", Scalar::from(0_u32)), "0_u32");
+        assert_eq!(format!("{}", Scalar::from(!0_u32)), "4294967295_u32");
 
-        assert_eq!(format!("{}", Scalar::from(0_u64)), "0");
-        assert_eq!(format!("{}", Scalar::from(!0_u64)), "18446744073709551615");
+        assert_eq!(format!("{}", Scalar::from(0_u64)), "0_u64");
+        assert_eq!(
+            format!("{}", Scalar::from(!0_u64)),
+            "18446744073709551615_u64"
+        );
 
         assert_eq!(
             format!("{}", Scalar::null(DType::Primitive(PType::U8, Nullable))),
@@ -194,7 +182,7 @@ mod tests {
                 "{}",
                 Scalar::r#struct(dtype(), vec![ScalarValue::Primitive(PValue::U32(32))])
             ),
-            "{foo:32}"
+            "{foo:32_u32}"
         );
     }
 
@@ -239,7 +227,7 @@ mod tests {
                     ]
                 )
             ),
-            "{foo:true,bar:32}"
+            "{foo:true,bar:32_u32}"
         );
     }