Skip to content

Commit c962d78

Browse files
ggevayggevay
authored
Merge pull request #31533 from ggevay/char-varchar-printing
Fix EXPLAIN around Char and Varchar
2 parents 349ebc4 + 377a61e commit c962d78

File tree

24 files changed

+278
-101
lines changed

24 files changed

+278
-101
lines changed

‎src/adapter/src/catalog.rs‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1633,11 +1633,11 @@ impl ExprHumanizer for ConnCatalog<'_> {
16331633
Some(self.resolve_full_name(entry.name()).into_parts())
16341634
}
16351635

1636-
fn humanize_scalar_type(&self, typ: &ScalarType) -> String {
1636+
fn humanize_scalar_type(&self, typ: &ScalarType, postgres_compat: bool) -> String {
16371637
use ScalarType::*;
16381638

16391639
match typ {
1640-
Array(t) => format!("{}[]", self.humanize_scalar_type(t)),
1640+
Array(t) => format!("{}[]", self.humanize_scalar_type(t, postgres_compat)),
16411641
List {
16421642
custom_id: Some(item_id),
16431643
..
@@ -1650,12 +1650,15 @@ impl ExprHumanizer for ConnCatalog<'_> {
16501650
self.minimal_qualification(item.name()).to_string()
16511651
}
16521652
List { element_type, .. } => {
1653-
format!("{} list", self.humanize_scalar_type(element_type))
1653+
format!(
1654+
"{} list",
1655+
self.humanize_scalar_type(element_type, postgres_compat)
1656+
)
16541657
}
16551658
Map { value_type, .. } => format!(
16561659
"map[{}=>{}]",
1657-
self.humanize_scalar_type(&ScalarType::String),
1658-
self.humanize_scalar_type(value_type)
1660+
self.humanize_scalar_type(&ScalarType::String, postgres_compat),
1661+
self.humanize_scalar_type(value_type, postgres_compat)
16591662
),
16601663
Record {
16611664
custom_id: Some(item_id),
@@ -1668,10 +1671,22 @@ impl ExprHumanizer for ConnCatalog<'_> {
16681671
"record({})",
16691672
fields
16701673
.iter()
1671-
.map(|f| format!("{}: {}", f.0, self.humanize_column_type(&f.1)))
1674+
.map(|f| format!(
1675+
"{}: {}",
1676+
f.0,
1677+
self.humanize_column_type(&f.1, postgres_compat)
1678+
))
16721679
.join(",")
16731680
),
16741681
PgLegacyChar => "\"char\"".into(),
1682+
Char { length } if !postgres_compat => match length {
1683+
None => "char".into(),
1684+
Some(length) => format!("char({})", length.into_u32()),
1685+
},
1686+
VarChar { max_length } if !postgres_compat => match max_length {
1687+
None => "varchar".into(),
1688+
Some(length) => format!("varchar({})", length.into_u32()),
1689+
},
16751690
UInt16 => "uint2".into(),
16761691
UInt32 => "uint4".into(),
16771692
UInt64 => "uint8".into(),

‎src/adapter/src/coord/timestamp_selection.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -647,7 +647,9 @@ impl Coordinator {
647647
.try_into()?,
648648
_ => coord_bail!(
649649
"can't use {} as a mz_timestamp for AS OF or UP TO",
650-
catalog.for_session(session).humanize_column_type(&ty)
650+
catalog
651+
.for_session(session)
652+
.humanize_column_type(&ty, false)
651653
),
652654
})
653655
}

‎src/compute-types/src/explain/text.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,9 @@ impl DisplayText<PlanRenderingContext<'_, Plan>> for AvailableCollections {
458458
"{}",
459459
separated(
460460
", ",
461-
types.iter().map(|c| ctx.humanizer.humanize_column_type(c))
461+
types
462+
.iter()
463+
.map(|c| ctx.humanizer.humanize_column_type(c, false))
462464
)
463465
)?;
464466
writeln!(f, "]")?;

‎src/expr-parser/src/catalog.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,8 @@ impl ExprHumanizer for TestCatalog {
8383
self.humanize_id_unqualified(id).map(|name| vec![name])
8484
}
8585

86-
fn humanize_scalar_type(&self, ty: &ScalarType) -> String {
87-
DummyHumanizer.humanize_scalar_type(ty)
86+
fn humanize_scalar_type(&self, ty: &ScalarType, postgres_compat: bool) -> String {
87+
DummyHumanizer.humanize_scalar_type(ty, postgres_compat)
8888
}
8989

9090
fn column_names_for_id(&self, id: GlobalId) -> Option<Vec<String>> {

‎src/expr-test-util/src/lib.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,8 @@ impl ExprHumanizer for TestCatalog {
182182
self.humanize_id_unqualified(id).map(|name| vec![name])
183183
}
184184

185-
fn humanize_scalar_type(&self, ty: &ScalarType) -> String {
186-
DummyHumanizer.humanize_scalar_type(ty)
185+
fn humanize_scalar_type(&self, ty: &ScalarType, postgres_compat: bool) -> String {
186+
DummyHumanizer.humanize_scalar_type(ty, postgres_compat)
187187
}
188188

189189
fn column_names_for_id(&self, _id: GlobalId) -> Option<Vec<String>> {

‎src/expr/src/scalar/func/impls/string.rs‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,17 @@ impl<'a> EagerUnaryFunc<'a> for CastStringToChar {
579579

580580
impl fmt::Display for CastStringToChar {
581581
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
582-
f.write_str("text_to_char")
582+
match self.length {
583+
Some(length) => {
584+
write!(
585+
f,
586+
"text_to_char[len={}, fail_on_len={}]",
587+
length.into_u32(),
588+
self.fail_on_len
589+
)
590+
}
591+
None => f.write_str("text_to_char[len=unbounded]"),
592+
}
583593
}
584594
}
585595

@@ -707,7 +717,17 @@ impl<'a> EagerUnaryFunc<'a> for CastStringToVarChar {
707717

708718
impl fmt::Display for CastStringToVarChar {
709719
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
710-
f.write_str("text_to_varchar")
720+
match self.length {
721+
Some(length) => {
722+
write!(
723+
f,
724+
"text_to_varchar[len={}, fail_on_len={}]",
725+
length.into_u32(),
726+
self.fail_on_len
727+
)
728+
}
729+
None => f.write_str("text_to_varchar[len=unbounded]"),
730+
}
711731
}
712732
}
713733

‎src/repr/src/explain.rs‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -438,13 +438,19 @@ pub trait ExprHumanizer: fmt::Debug {
438438
fn humanize_id_parts(&self, id: GlobalId) -> Option<Vec<String>>;
439439

440440
/// Returns a human-readable name for the specified scalar type.
441-
fn humanize_scalar_type(&self, ty: &ScalarType) -> String;
441+
/// Used in, e.g., EXPLAIN and error msgs, in which case exact Postgres compatibility is less
442+
/// important than showing as much detail as possible. Also used in `pg_typeof`, where Postgres
443+
/// compatibility is more important.
444+
fn humanize_scalar_type(&self, ty: &ScalarType, postgres_compat: bool) -> String;
442445

443446
/// Returns a human-readable name for the specified column type.
444-
fn humanize_column_type(&self, typ: &ColumnType) -> String {
447+
/// Used in, e.g., EXPLAIN and error msgs, in which case exact Postgres compatibility is less
448+
/// important than showing as much detail as possible. Also used in `pg_typeof`, where Postgres
449+
/// compatibility is more important.
450+
fn humanize_column_type(&self, typ: &ColumnType, postgres_compat: bool) -> String {
445451
format!(
446452
"{}{}",
447-
self.humanize_scalar_type(&typ.scalar_type),
453+
self.humanize_scalar_type(&typ.scalar_type, postgres_compat),
448454
if typ.nullable { "?" } else { "" }
449455
)
450456
}
@@ -505,8 +511,8 @@ impl<'a> ExprHumanizer for ExprHumanizerExt<'a> {
505511
}
506512
}
507513

508-
fn humanize_scalar_type(&self, ty: &ScalarType) -> String {
509-
self.inner.humanize_scalar_type(ty)
514+
fn humanize_scalar_type(&self, ty: &ScalarType, postgres_compat: bool) -> String {
515+
self.inner.humanize_scalar_type(ty, postgres_compat)
510516
}
511517

512518
fn column_names_for_id(&self, id: GlobalId) -> Option<Vec<String>> {
@@ -572,7 +578,7 @@ impl ExprHumanizer for DummyHumanizer {
572578
None
573579
}
574580

575-
fn humanize_scalar_type(&self, ty: &ScalarType) -> String {
581+
fn humanize_scalar_type(&self, ty: &ScalarType, _postgres_compat: bool) -> String {
576582
// The debug implementation is better than nothing.
577583
format!("{:?}", ty)
578584
}
@@ -683,7 +689,7 @@ impl<'a> Display for HumanizedAnalyses<'a> {
683689
Some(types) => {
684690
let types = types
685691
.into_iter()
686-
.map(|c| self.humanizer.humanize_column_type(c))
692+
.map(|c| self.humanizer.humanize_column_type(c, false))
687693
.collect::<Vec<_>>();
688694

689695
bracketed("(", ")", separated(", ", types)).to_string()

‎src/sql/src/func.rs‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,7 +1009,8 @@ where
10091009
let arg_types: Vec<_> = types
10101010
.into_iter()
10111011
.map(|ty| match ty {
1012-
CoercibleScalarType::Coerced(ty) => ecx.humanize_scalar_type(&ty),
1012+
// This will be used in error msgs, therefore we call with `postgres_compat` false.
1013+
CoercibleScalarType::Coerced(ty) => ecx.humanize_scalar_type(&ty, false),
10131014
CoercibleScalarType::Record(_) => "record".to_string(),
10141015
CoercibleScalarType::Uncoerced => "unknown".to_string(),
10151016
})
@@ -1809,7 +1810,8 @@ pub static PG_CATALOG_BUILTINS: LazyLock<BTreeMap<&'static str, Func>> = LazyLoc
18091810
let elem_type = match elem_type.array_of_self_elem_type() {
18101811
Ok(elem_type) => elem_type,
18111812
Err(elem_type) => bail_unsupported!(
1812-
format!("array_fill on {}", ecx.humanize_scalar_type(&elem_type))
1813+
// This will be used in error msgs, therefore we call with `postgres_compat` false.
1814+
format!("array_fill on {}", ecx.humanize_scalar_type(&elem_type, false))
18131815
),
18141816
};
18151817

@@ -1825,7 +1827,7 @@ pub static PG_CATALOG_BUILTINS: LazyLock<BTreeMap<&'static str, Func>> = LazyLoc
18251827
let elem_type = match elem_type.array_of_self_elem_type() {
18261828
Ok(elem_type) => elem_type,
18271829
Err(elem_type) => bail_unsupported!(
1828-
format!("array_fill on {}", ecx.humanize_scalar_type(&elem_type))
1830+
format!("array_fill on {}", ecx.humanize_scalar_type(&elem_type, false))
18291831
),
18301832
};
18311833

@@ -2520,7 +2522,7 @@ pub static PG_CATALOG_BUILTINS: LazyLock<BTreeMap<&'static str, Func>> = LazyLoc
25202522
let name = match ecx.scalar_type(&exprs[0]) {
25212523
CoercibleScalarType::Uncoerced => "unknown".to_string(),
25222524
CoercibleScalarType::Record(_) => "record".to_string(),
2523-
CoercibleScalarType::Coerced(ty) => ecx.humanize_scalar_type(&ty),
2525+
CoercibleScalarType::Coerced(ty) => ecx.humanize_scalar_type(&ty, true),
25242526
};
25252527

25262528
// For consistency with other functions, verify that
@@ -3023,7 +3025,7 @@ pub static PG_CATALOG_BUILTINS: LazyLock<BTreeMap<&'static str, Func>> = LazyLoc
30233025
let elem_type = match elem_type.array_of_self_elem_type() {
30243026
Ok(elem_type) => elem_type,
30253027
Err(elem_type) => bail_unsupported!(
3026-
format!("array_agg on {}", ecx.humanize_scalar_type(&elem_type))
3028+
format!("array_agg on {}", ecx.humanize_scalar_type(&elem_type, false))
30273029
),
30283030
};
30293031

@@ -3759,7 +3761,7 @@ pub static MZ_CATALOG_BUILTINS: LazyLock<BTreeMap<&'static str, Func>> = LazyLoc
37593761
let err = || {
37603762
Err(sql_err!(
37613763
"function map_build({}) does not exist",
3762-
ecx.humanize_scalar_type(&ty.clone())
3764+
ecx.humanize_scalar_type(&ty.clone(), false)
37633765
))
37643766
};
37653767

‎src/sql/src/plan/hir.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -834,8 +834,8 @@ impl CoercibleScalarExpr {
834834
sql_bail!(
835835
"{} must have type {}, not type {}",
836836
ecx.name,
837-
ecx.humanize_scalar_type(ty),
838-
ecx.humanize_scalar_type(&expr_ty),
837+
ecx.humanize_scalar_type(ty, false),
838+
ecx.humanize_scalar_type(&expr_ty, false),
839839
);
840840
}
841841
Ok(expr)

0 commit comments

Comments
 (0)