adapter: correctly report custom types in mz_columns

For columns with a type of list, map, or record, the mz_columns table
was incorrectly reporting the generic psuedotype for the column, even
when the type was a named type in the catalog.

For example, given the following objects:

    CREATE TYPE my_rec AS (x int, y int);
    CREATE TABLE my_tab (f1 my_rec);

the mz_columns table would incorrectly report that column f1 had type
record (OID 2249), rather than type my_rec with a dynamic OID determined
at envd boot.

This commit is a surgical fix that fixes the immediate issue.

In the future, we may want to consider a deeper fix that makes the
conversion between a `ScalarType` and a `pg_repr::Type` less prone to
this sort of bug. That is a much larger refactor, however.

We'll also want to deprecate `mz_columns.type` in the future, for the
reasons noted in the comment in the patch. That too is left to future
work for expediency.

src/adapter/src/catalog/builtin_table_updates.rs

@@ -50,7 +50,7 @@ use mz_repr::adt::jsonb::Jsonb;
 use mz_repr::adt::mz_acl_item::{AclMode, MzAclItem, PrivilegeMap};
 use mz_repr::refresh_schedule::RefreshEvery;
 use mz_repr::role_id::RoleId;
-use mz_repr::{Datum, Diff, GlobalId, Row, RowPacker, Timestamp};
+use mz_repr::{Datum, Diff, GlobalId, Row, RowPacker, ScalarType, Timestamp};
 use mz_sql::ast::{CreateIndexStatement, Statement, UnresolvedItemName};
 use mz_sql::catalog::{
     CatalogCluster, CatalogDatabase, CatalogSchema, CatalogType, DefaultPrivilegeObject,
@@ -562,16 +562,46 @@ impl CatalogState {
                     .map(|d| Datum::String(d))
                     .unwrap_or(Datum::Null);
                 let pgtype = mz_pgrepr::Type::from(&column_type.scalar_type);
+                let (type_name, type_oid) = match &column_type.scalar_type {
+                    ScalarType::List {
+                        custom_id: Some(custom_id),
+                        ..
+                    }
+                    | ScalarType::Map {
+                        custom_id: Some(custom_id),
+                        ..
+                    }
+                    | ScalarType::Record {
+                        custom_id: Some(custom_id),
+                        ..
+                    } => {
+                        let entry = self.get_entry(custom_id);
+                        // NOTE(benesch): the `mz_columns.type text` field is
+                        // wrong. Types do not have a name that can be
+                        // represented as a single textual field. There can
+                        // create multiple types with the same name in different
+                        // schemas and databases. We should eventually deprecate
+                        // the `type` field in favor of a new a `type_id` field
+                        // that can be joined against `mz_types`. For now, in
+                        // the interest of pragmatism, we just use the type's
+                        // item name, and accept that there may be ambiguity if
+                        // the same type name is used in multiple schemas.
+                        let name = &*entry.name().item;
+                        let oid = entry.oid();
+                        (name, oid)
+                    }
+                    _ => (pgtype.name(), pgtype.oid()),
+                };
                 updates.push(BuiltinTableUpdate {
                     id: self.resolve_builtin_table(&MZ_COLUMNS),
                     row: Row::pack_slice(&[
                         Datum::String(&id.to_string()),
                         Datum::String(column_name.as_str()),
                         Datum::UInt64(u64::cast_from(i + 1)),
                         Datum::from(column_type.nullable),
-                        Datum::String(pgtype.name()),
+                        Datum::String(type_name),
                         default,
-                        Datum::UInt32(pgtype.oid()),
+                        Datum::UInt32(type_oid),
                         Datum::Int32(pgtype.typmod()),
                     ]),
                     diff,

test/sqllogictest/mz_columns.slt

@@ -0,0 +1,48 @@
+# Copyright Materialize, Inc. and contributors. All rights reserved.
+#
+# Use of this software is governed by the Business Source License
+# included in the LICENSE file at the root of this repository.
+#
+# As of the Change Date specified in that file, in accordance with
+# the Business Source License, use of this software will be governed
+# by the Apache License, Version 2.0.
+
+mode cockroach
+
+statement ok
+CREATE TYPE r AS (a int)
+
+statement ok
+CREATE TYPE l AS LIST (ELEMENT TYPE = int)
+
+statement ok
+CREATE TYPE m AS MAP (KEY TYPE = text, VALUE TYPE = int)
+
+statement ok
+CREATE VIEW v AS SELECT
+    row(1) AS ra,
+    row(1)::r AS rn,
+    list[]::int list AS la,
+    list[]::l AS ln,
+    map[]::map[text=>int] AS ma,
+    '{}'::m AS mn
+
+# We intentionally don't assert on the `c.type_oid` or `t.id` columns, as
+# IDs are not stable. Instead, we ensure that the `c.type_oid` column can be
+# used to look up the type in the `mz_types` table and that the ID is
+# as expected (system or user).
+query TTTIT
+SELECT
+    c.position, c.name, c.type, c.type_mod, left(t.id, 1)
+    FROM mz_columns c
+    JOIN mz_views v ON c.id = v.id
+    JOIN mz_types t ON c.type_oid = t.oid
+    WHERE v.name = 'v'
+    ORDER BY c.position
+----
+1  ra  record  -1  s
+2  rn  r       -1  u
+3  la  list    -1  s
+4  ln  l       -1  u
+5  ma  map     -1  s
+6  mn  m       -1  u