Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 21 additions & 6 deletions src/adapter/src/catalog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1633,11 +1633,11 @@ impl ExprHumanizer for ConnCatalog<'_> {
Some(self.resolve_full_name(entry.name()).into_parts())
}

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

match typ {
Array(t) => format!("{}[]", self.humanize_scalar_type(t)),
Array(t) => format!("{}[]", self.humanize_scalar_type(t, postgres_compat)),
List {
custom_id: Some(item_id),
..
Expand All @@ -1650,12 +1650,15 @@ impl ExprHumanizer for ConnCatalog<'_> {
self.minimal_qualification(item.name()).to_string()
}
List { element_type, .. } => {
format!("{} list", self.humanize_scalar_type(element_type))
format!(
"{} list",
self.humanize_scalar_type(element_type, postgres_compat)
)
}
Map { value_type, .. } => format!(
"map[{}=>{}]",
self.humanize_scalar_type(&ScalarType::String),
self.humanize_scalar_type(value_type)
self.humanize_scalar_type(&ScalarType::String, postgres_compat),
self.humanize_scalar_type(value_type, postgres_compat)
),
Record {
custom_id: Some(item_id),
Expand All @@ -1668,10 +1671,22 @@ impl ExprHumanizer for ConnCatalog<'_> {
"record({})",
fields
.iter()
.map(|f| format!("{}: {}", f.0, self.humanize_column_type(&f.1)))
.map(|f| format!(
"{}: {}",
f.0,
self.humanize_column_type(&f.1, postgres_compat)
))
.join(",")
),
PgLegacyChar => "\"char\"".into(),
Char { length } if !postgres_compat => match length {
None => "char".into(),
Some(length) => format!("char({})", length.into_u32()),
},
VarChar { max_length } if !postgres_compat => match max_length {
None => "varchar".into(),
Some(length) => format!("varchar({})", length.into_u32()),
},
UInt16 => "uint2".into(),
UInt32 => "uint4".into(),
UInt64 => "uint8".into(),
Expand Down
4 changes: 3 additions & 1 deletion src/adapter/src/coord/timestamp_selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,9 @@ impl Coordinator {
.try_into()?,
_ => coord_bail!(
"can't use {} as a mz_timestamp for AS OF or UP TO",
catalog.for_session(session).humanize_column_type(&ty)
catalog
.for_session(session)
.humanize_column_type(&ty, false)
),
})
}
Expand Down
4 changes: 3 additions & 1 deletion src/compute-types/src/explain/text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,9 @@ impl DisplayText<PlanRenderingContext<'_, Plan>> for AvailableCollections {
"{}",
separated(
", ",
types.iter().map(|c| ctx.humanizer.humanize_column_type(c))
types
.iter()
.map(|c| ctx.humanizer.humanize_column_type(c, false))
)
)?;
writeln!(f, "]")?;
Expand Down
4 changes: 2 additions & 2 deletions src/expr-parser/src/catalog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ impl ExprHumanizer for TestCatalog {
self.humanize_id_unqualified(id).map(|name| vec![name])
}

fn humanize_scalar_type(&self, ty: &ScalarType) -> String {
DummyHumanizer.humanize_scalar_type(ty)
fn humanize_scalar_type(&self, ty: &ScalarType, postgres_compat: bool) -> String {
DummyHumanizer.humanize_scalar_type(ty, postgres_compat)
}

fn column_names_for_id(&self, id: GlobalId) -> Option<Vec<String>> {
Expand Down
4 changes: 2 additions & 2 deletions src/expr-test-util/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ impl ExprHumanizer for TestCatalog {
self.humanize_id_unqualified(id).map(|name| vec![name])
}

fn humanize_scalar_type(&self, ty: &ScalarType) -> String {
DummyHumanizer.humanize_scalar_type(ty)
fn humanize_scalar_type(&self, ty: &ScalarType, postgres_compat: bool) -> String {
DummyHumanizer.humanize_scalar_type(ty, postgres_compat)
}

fn column_names_for_id(&self, _id: GlobalId) -> Option<Vec<String>> {
Expand Down
24 changes: 22 additions & 2 deletions src/expr/src/scalar/func/impls/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,17 @@ impl<'a> EagerUnaryFunc<'a> for CastStringToChar {

impl fmt::Display for CastStringToChar {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.write_str("text_to_char")
match self.length {
Some(length) => {
write!(
f,
"text_to_char[len={}, fail_on_len={}]",
length.into_u32(),
self.fail_on_len
)
}
None => f.write_str("text_to_char[len=unbounded]"),
}
}
}

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

impl fmt::Display for CastStringToVarChar {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.write_str("text_to_varchar")
match self.length {
Some(length) => {
write!(
f,
"text_to_varchar[len={}, fail_on_len={}]",
length.into_u32(),
self.fail_on_len
)
}
None => f.write_str("text_to_varchar[len=unbounded]"),
}
}
}

Expand Down
20 changes: 13 additions & 7 deletions src/repr/src/explain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,13 +438,19 @@ pub trait ExprHumanizer: fmt::Debug {
fn humanize_id_parts(&self, id: GlobalId) -> Option<Vec<String>>;

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

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

fn humanize_scalar_type(&self, ty: &ScalarType) -> String {
self.inner.humanize_scalar_type(ty)
fn humanize_scalar_type(&self, ty: &ScalarType, postgres_compat: bool) -> String {
self.inner.humanize_scalar_type(ty, postgres_compat)
}

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

fn humanize_scalar_type(&self, ty: &ScalarType) -> String {
fn humanize_scalar_type(&self, ty: &ScalarType, _postgres_compat: bool) -> String {
// The debug implementation is better than nothing.
format!("{:?}", ty)
}
Expand Down Expand Up @@ -683,7 +689,7 @@ impl<'a> Display for HumanizedAnalyses<'a> {
Some(types) => {
let types = types
.into_iter()
.map(|c| self.humanizer.humanize_column_type(c))
.map(|c| self.humanizer.humanize_column_type(c, false))
.collect::<Vec<_>>();

bracketed("(", ")", separated(", ", types)).to_string()
Expand Down
14 changes: 8 additions & 6 deletions src/sql/src/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1009,7 +1009,8 @@ where
let arg_types: Vec<_> = types
.into_iter()
.map(|ty| match ty {
CoercibleScalarType::Coerced(ty) => ecx.humanize_scalar_type(&ty),
// This will be used in error msgs, therefore we call with `postgres_compat` false.
CoercibleScalarType::Coerced(ty) => ecx.humanize_scalar_type(&ty, false),
CoercibleScalarType::Record(_) => "record".to_string(),
CoercibleScalarType::Uncoerced => "unknown".to_string(),
})
Expand Down Expand Up @@ -1809,7 +1810,8 @@ pub static PG_CATALOG_BUILTINS: LazyLock<BTreeMap<&'static str, Func>> = LazyLoc
let elem_type = match elem_type.array_of_self_elem_type() {
Ok(elem_type) => elem_type,
Err(elem_type) => bail_unsupported!(
format!("array_fill on {}", ecx.humanize_scalar_type(&elem_type))
// This will be used in error msgs, therefore we call with `postgres_compat` false.
format!("array_fill on {}", ecx.humanize_scalar_type(&elem_type, false))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
format!("array_fill on {}", ecx.humanize_scalar_type(&elem_type, false))
// This will be used in error msgs, therefore we call with `postgres_compat` false.
format!("array_fill on {}", ecx.humanize_scalar_type(&elem_type, false))

... and probably similar at other uses.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed this one. Generally, I didn't add this comment to all call sites, only to those where it was not immediately clear whether it's only used in an error msg.

),
};

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

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

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

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

Expand Down
4 changes: 2 additions & 2 deletions src/sql/src/plan/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -834,8 +834,8 @@ impl CoercibleScalarExpr {
sql_bail!(
"{} must have type {}, not type {}",
ecx.name,
ecx.humanize_scalar_type(ty),
ecx.humanize_scalar_type(&expr_ty),
ecx.humanize_scalar_type(ty, false),
ecx.humanize_scalar_type(&expr_ty, false),
);
}
Ok(expr)
Expand Down
Loading