Skip to content

Commit d45e709

Browse files
gatesngatesn
authored
Scalar display (#2951)
Previously we were delegating to display of ScalarValue, but this doesn't have sufficient type information. Old <img width="453" alt="Screenshot 2025-04-08 at 14 42 07" src="https://github.com/user-attachments/assets/f70435ce-47e6-4a14-84dc-b878e18ffc96" /> New <img width="449" alt="Screenshot 2025-04-08 at 14 41 44" src="https://github.com/user-attachments/assets/557f2b50-ed89-4a36-8c72-22c88347037f" />
1 parent c7dba8c commit d45e709

File tree

13 files changed

+164
-135
lines changed

13 files changed

+164
-135
lines changed

‎vortex-expr/src/lib.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -313,11 +313,11 @@ mod tests {
313313
"$~{col1, col2}"
314314
);
315315

316-
assert_eq!(lit(Scalar::from(0_u8)).to_string(), "0_u8");
317-
assert_eq!(lit(Scalar::from(0.0_f32)).to_string(), "0_f32");
316+
assert_eq!(lit(Scalar::from(0u8)).to_string(), "0u8");
317+
assert_eq!(lit(Scalar::from(0.0f32)).to_string(), "0f32");
318318
assert_eq!(
319319
lit(Scalar::from(i64::MAX)).to_string(),
320-
"9223372036854775807_i64"
320+
"9223372036854775807i64"
321321
);
322322
assert_eq!(lit(Scalar::from(true)).to_string(), "true");
323323
assert_eq!(
@@ -340,7 +340,7 @@ mod tests {
340340
vec![Scalar::from(32_u32), Scalar::from("rufus".to_string())]
341341
))
342342
.to_string(),
343-
"{dog:32_u32,cat:\"rufus\"}"
343+
"{dog: 32u32, cat: \"rufus\"}"
344344
);
345345
}
346346

‎vortex-expr/src/pruning.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -660,7 +660,7 @@ mod tests {
660660

661661
assert_eq!(
662662
PruningPredicate::try_new(&not_eq_expr).unwrap().to_string(),
663-
"PruningPredicate(($.a_min >= 42_i32), {a: {min}})"
663+
"PruningPredicate(($.a_min >= 42i32), {a: {min}})"
664664
);
665665
}
666666

‎vortex-scalar/src/binary.rs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
use std::fmt::{Display, Formatter};
12
use std::sync::Arc;
23

4+
use itertools::Itertools;
35
use vortex_buffer::ByteBuffer;
46
use vortex_dtype::{DType, Nullability};
57
use vortex_error::{VortexError, VortexExpect as _, VortexResult, vortex_bail, vortex_err};
@@ -12,6 +14,19 @@ pub struct BinaryScalar<'a> {
1214
value: Option<ByteBuffer>,
1315
}
1416

17+
impl Display for BinaryScalar<'_> {
18+
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
19+
match &self.value {
20+
None => write!(f, "null"),
21+
Some(v) => write!(
22+
f,
23+
"\"{}\"",
24+
v.as_slice().iter().map(|b| format!("{b:x}")).format(" ")
25+
),
26+
}
27+
}
28+
}
29+
1530
impl PartialEq for BinaryScalar<'_> {
1631
fn eq(&self, other: &Self) -> bool {
1732
self.dtype.eq_ignore_nullability(other.dtype) && self.value == other.value

‎vortex-scalar/src/bool.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use std::cmp::Ordering;
2+
use std::fmt::{Display, Formatter};
23

34
use vortex_dtype::Nullability::NonNullable;
45
use vortex_dtype::{DType, Nullability};
@@ -12,6 +13,15 @@ pub struct BoolScalar<'a> {
1213
value: Option<bool>,
1314
}
1415

16+
impl Display for BoolScalar<'_> {
17+
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
18+
match self.value {
19+
None => write!(f, "null"),
20+
Some(v) => write!(f, "{}", v),
21+
}
22+
}
23+
}
24+
1525
impl PartialEq for BoolScalar<'_> {
1626
fn eq(&self, other: &Self) -> bool {
1727
self.dtype.eq_ignore_nullability(other.dtype) && self.value == other.value

‎vortex-scalar/src/display.rs‎

Lines changed: 24 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -1,111 +1,20 @@
11
use std::fmt::{Display, Formatter};
22

3-
use itertools::Itertools;
43
use vortex_dtype::DType;
5-
use vortex_dtype::datetime::{TemporalMetadata, is_temporal_ext_type};
6-
use vortex_error::{VortexExpect, vortex_panic};
74

8-
use crate::binary::BinaryScalar;
9-
use crate::extension::ExtScalar;
10-
use crate::struct_::StructScalar;
11-
use crate::utf8::Utf8Scalar;
12-
use crate::{ListScalar, Scalar};
5+
use crate::Scalar;
136

147
impl Display for Scalar {
158
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
169
match self.dtype() {
17-
DType::Null | DType::Bool(_) | DType::Primitive(..) => Display::fmt(&self.value, f),
18-
DType::Utf8(_) => {
19-
match Utf8Scalar::try_from(self)
20-
.map_err(|_| std::fmt::Error)?
21-
.value()
22-
{
23-
None => write!(f, "null"),
24-
Some(bs) => write!(f, "\"{}\"", bs.as_str()),
25-
}
26-
}
27-
DType::Binary(_) => {
28-
match BinaryScalar::try_from(self)
29-
.map_err(|_| std::fmt::Error)?
30-
.value()
31-
{
32-
None => write!(f, "null"),
33-
Some(buf) => {
34-
write!(
35-
f,
36-
"\"{}\"",
37-
buf.as_slice().iter().map(|b| format!("{b:x}")).format(",")
38-
)
39-
}
40-
}
41-
}
42-
DType::Struct(dtype, _) => {
43-
let v = StructScalar::try_from(self).map_err(|_| std::fmt::Error)?;
44-
45-
if v.is_null() {
46-
write!(f, "null")
47-
} else {
48-
write!(f, "{{")?;
49-
let formatted_fields = dtype
50-
.names()
51-
.iter()
52-
.enumerate()
53-
.map(|(idx, name)| {
54-
let val = v.field_by_idx(idx).vortex_expect("not out of bounds");
55-
format!("{name}:{val}")
56-
})
57-
.format(",");
58-
write!(f, "{}", formatted_fields)?;
59-
write!(f, "}}")
60-
}
61-
}
62-
DType::List(..) => {
63-
let v = ListScalar::try_from(self).map_err(|_| std::fmt::Error)?;
64-
match v.elements() {
65-
None => write!(f, "null"),
66-
Some(elems) => {
67-
write!(f, "[{}]", elems.iter().format(","))
68-
}
69-
}
70-
}
71-
// Specialized handling for date/time/timestamp builtin extension types.
72-
DType::Extension(dtype) if is_temporal_ext_type(dtype.id()) => {
73-
let metadata =
74-
TemporalMetadata::try_from(dtype.as_ref()).map_err(|_| std::fmt::Error)?;
75-
let storage_scalar = self.as_extension().storage();
76-
77-
match storage_scalar.dtype() {
78-
DType::Null => {
79-
write!(f, "null")
80-
}
81-
DType::Primitive(..) => {
82-
let maybe_timestamp = storage_scalar
83-
.as_primitive()
84-
.as_::<i64>()
85-
.and_then(|maybe_timestamp| {
86-
maybe_timestamp.map(|v| metadata.to_jiff(v)).transpose()
87-
})
88-
.map_err(|_| std::fmt::Error)?;
89-
match maybe_timestamp {
90-
None => write!(f, "null"),
91-
Some(v) => write!(f, "{}", v),
92-
}
93-
}
94-
_ => {
95-
vortex_panic!(
96-
"Expected temporal extension data type to have Primitive or Null storage type"
97-
)
98-
}
99-
}
100-
}
101-
// Generic handling of unknown extension types.
102-
// TODO(aduffy): Allow extension authors plugin their own Scalar display.
103-
DType::Extension(..) => {
104-
let storage_value = ExtScalar::try_from(self)
105-
.map_err(|_| std::fmt::Error)?
106-
.storage();
107-
write!(f, "{}", storage_value)
108-
}
10+
DType::Null => write!(f, "null"),
11+
DType::Bool(_) => write!(f, "{}", self.as_bool()),
12+
DType::Primitive(..) => write!(f, "{}", self.as_primitive()),
13+
DType::Utf8(_) => write!(f, "{}", self.as_utf8()),
14+
DType::Binary(_) => write!(f, "{}", self.as_binary()),
15+
DType::Struct(..) => write!(f, "{}", self.as_struct()),
16+
DType::List(..) => write!(f, "{}", self.as_list()),
17+
DType::Extension(_) => write!(f, "{}", self.as_extension()),
10918
}
11019
}
11120
}
@@ -134,19 +43,19 @@ mod tests {
13443

13544
#[test]
13645
fn display_primitive() {
137-
assert_eq!(format!("{}", Scalar::from(0_u8)), "0_u8");
138-
assert_eq!(format!("{}", Scalar::from(255_u8)), "255_u8");
46+
assert_eq!(format!("{}", Scalar::from(0u8)), "0u8");
47+
assert_eq!(format!("{}", Scalar::from(255u8)), "255u8");
13948

140-
assert_eq!(format!("{}", Scalar::from(0_u16)), "0_u16");
141-
assert_eq!(format!("{}", Scalar::from(!0_u16)), "65535_u16");
49+
assert_eq!(format!("{}", Scalar::from(0u16)), "0u16");
50+
assert_eq!(format!("{}", Scalar::from(!0u16)), "65535u16");
14251

143-
assert_eq!(format!("{}", Scalar::from(0_u32)), "0_u32");
144-
assert_eq!(format!("{}", Scalar::from(!0_u32)), "4294967295_u32");
52+
assert_eq!(format!("{}", Scalar::from(0u32)), "0u32");
53+
assert_eq!(format!("{}", Scalar::from(!0u32)), "4294967295u32");
14554

146-
assert_eq!(format!("{}", Scalar::from(0_u64)), "0_u64");
55+
assert_eq!(format!("{}", Scalar::from(0u64)), "0u64");
14756
assert_eq!(
148-
format!("{}", Scalar::from(!0_u64)),
149-
"18446744073709551615_u64"
57+
format!("{}", Scalar::from(!0u64)),
58+
"18446744073709551615u64"
15059
);
15160

15261
assert_eq!(
@@ -174,7 +83,7 @@ mod tests {
17483
NonNullable
17584
)
17685
),
177-
"\"48,65,6c,6c,6f,20,57,6f,72,6c,64,21\""
86+
"\"48 65 6c 6c 6f 20 57 6f 72 6c 64 21\""
17887
);
17988
assert_eq!(format!("{}", Scalar::null(DType::Binary(Nullable))), "null");
18089
}
@@ -209,12 +118,12 @@ mod tests {
209118
"{}",
210119
Scalar::struct_(dtype(), vec![Scalar::null_typed::<u32>()])
211120
),
212-
"{foo:null}"
121+
"{foo: null}"
213122
);
214123

215124
assert_eq!(
216125
format!("{}", Scalar::struct_(dtype(), vec![Scalar::from(32_u32)])),
217-
"{foo:32_u32}"
126+
"{foo: 32u32}"
218127
);
219128
}
220129

@@ -242,23 +151,23 @@ mod tests {
242151
vec![Scalar::null(f1), Scalar::null(f2.clone())]
243152
)
244153
),
245-
"{foo:null,bar:null}"
154+
"{foo: null, bar: null}"
246155
);
247156

248157
assert_eq!(
249158
format!(
250159
"{}",
251160
Scalar::struct_(dtype.clone(), vec![Scalar::from(true), Scalar::null(f2)])
252161
),
253-
"{foo:true,bar:null}"
162+
"{foo: true, bar: null}"
254163
);
255164

256165
assert_eq!(
257166
format!(
258167
"{}",
259168
Scalar::struct_(dtype, vec![Scalar::from(true), Scalar::from(32_u32)])
260169
),
261-
"{foo:true,bar:32_u32}"
170+
"{foo: true, bar: 32u32}"
262171
);
263172
}
264173

‎vortex-scalar/src/extension.rs‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
use std::fmt::{Display, Formatter};
12
use std::hash::Hash;
23
use std::sync::Arc;
34

5+
use vortex_dtype::datetime::{TemporalMetadata, is_temporal_ext_type};
46
use vortex_dtype::{DType, ExtDType};
57
use vortex_error::{VortexError, VortexResult, vortex_bail};
68

@@ -11,6 +13,32 @@ pub struct ExtScalar<'a> {
1113
value: &'a ScalarValue,
1214
}
1315

16+
impl Display for ExtScalar<'_> {
17+
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
18+
// Specialized handling for date/time/timestamp builtin extension types.
19+
if is_temporal_ext_type(self.ext_dtype().id()) {
20+
let metadata =
21+
TemporalMetadata::try_from(self.ext_dtype()).map_err(|_| std::fmt::Error)?;
22+
23+
let maybe_timestamp = self
24+
.storage()
25+
.as_primitive()
26+
.as_::<i64>()
27+
.and_then(|maybe_timestamp| {
28+
maybe_timestamp.map(|v| metadata.to_jiff(v)).transpose()
29+
})
30+
.map_err(|_| std::fmt::Error)?;
31+
32+
match maybe_timestamp {
33+
None => write!(f, "null"),
34+
Some(v) => write!(f, "{}", v),
35+
}
36+
} else {
37+
write!(f, "{}({})", self.ext_dtype().id(), self.storage())
38+
}
39+
}
40+
}
41+
1442
impl PartialEq for ExtScalar<'_> {
1543
fn eq(&self, other: &Self) -> bool {
1644
self.ext_dtype.eq_ignore_nullability(other.ext_dtype) && self.storage() == other.storage()

‎vortex-scalar/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,7 @@ mod test {
532532
result.as_ref().is_err_and(|err| {
533533
err
534534
.to_string()
535-
.contains("Can't cast u16 scalar 1000_u16 to u8 (cause: Cannot read primitive value U16(1000) as u8")
535+
.contains("Can't cast u16 scalar 1000u16 to u8 (cause: Cannot read primitive value U16(1000) as u8")
536536
}),
537537
"{:?}",
538538
result

‎vortex-scalar/src/list.rs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use std::fmt::{Display, Formatter};
12
use std::hash::Hash;
23
use std::ops::Deref;
34
use std::sync::Arc;
@@ -16,6 +17,24 @@ pub struct ListScalar<'a> {
1617
elements: Option<Arc<[ScalarValue]>>,
1718
}
1819

20+
impl Display for ListScalar<'_> {
21+
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
22+
match &self.elements {
23+
None => write!(f, "null"),
24+
Some(elems) => {
25+
write!(
26+
f,
27+
"[{}]",
28+
elems
29+
.iter()
30+
.map(|e| Scalar::new(self.element_dtype().clone(), e.clone()))
31+
.format(", ")
32+
)
33+
}
34+
}
35+
}
36+
}
37+
1938
impl PartialEq for ListScalar<'_> {
2039
fn eq(&self, other: &Self) -> bool {
2140
self.dtype.eq_ignore_nullability(other.dtype) && self.elements() == other.elements()

‎vortex-scalar/src/primitive.rs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::any::type_name;
22
use std::cmp::Ordering;
3-
use std::fmt::{Debug, Display};
3+
use std::fmt::{Debug, Display, Formatter};
44
use std::ops::{Add, Sub};
55

66
use num_traits::{CheckedAdd, CheckedDiv, CheckedMul, CheckedSub, FromPrimitive};
@@ -20,6 +20,15 @@ pub struct PrimitiveScalar<'a> {
2020
pvalue: Option<PValue>,
2121
}
2222

23+
impl Display for PrimitiveScalar<'_> {
24+
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
25+
match self.pvalue {
26+
None => write!(f, "null"),
27+
Some(pv) => write!(f, "{}", pv),
28+
}
29+
}
30+
}
31+
2332
impl PartialEq for PrimitiveScalar<'_> {
2433
fn eq(&self, other: &Self) -> bool {
2534
self.dtype.eq_ignore_nullability(other.dtype) && self.pvalue == other.pvalue
@@ -354,7 +363,7 @@ pub enum BinaryNumericOperator {
354363
}
355364

356365
impl Display for BinaryNumericOperator {
357-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
366+
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
358367
Debug::fmt(self, f)
359368
}
360369
}

0 commit comments

Comments
 (0)