Skip to content
Open
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
28 changes: 21 additions & 7 deletions src/catalog/src/builtin/mz_internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3687,21 +3687,35 @@ pub static PG_DESCRIPTION_ALL_DATABASES: LazyLock<BuiltinView> = LazyLock::new(|
column_comments: BTreeMap::new(),
sql: "
(
-- The classoid of a comment is the oid of the pg_catalog system catalog
-- that conceptually stores the commented object: pg_class for relations,
-- pg_type for types, pg_namespace for schemas. We scope the lookup to the
-- pg_catalog schema; otherwise a user-created object named e.g. `pg_class`
-- makes the scalar subqueries below match multiple rows and the whole view
-- errors for everyone. PostgreSQL's pg_description is a real catalog table
-- and is unaffected by such user objects, and so are we.
WITH pg_catalog_class AS (
SELECT oid, relname, database_name
FROM mz_internal.pg_class_all_databases
WHERE relnamespace = (
SELECT oid FROM mz_internal.pg_namespace_all_databases WHERE nspname = 'pg_catalog'
)
),
-- Gather all of the class oid's for objects that can have comments.
WITH pg_classoids AS (
pg_classoids AS (
SELECT oid, database_name as oid_database_name,
(SELECT oid FROM mz_internal.pg_class_all_databases WHERE relname = 'pg_class') AS classoid,
(SELECT database_name FROM mz_internal.pg_class_all_databases WHERE relname = 'pg_class') AS class_database_name
(SELECT oid FROM pg_catalog_class WHERE relname = 'pg_class') AS classoid,
(SELECT database_name FROM pg_catalog_class WHERE relname = 'pg_class') AS class_database_name
FROM mz_internal.pg_class_all_databases
UNION ALL
SELECT oid, database_name as oid_database_name,
(SELECT oid FROM mz_internal.pg_class_all_databases WHERE relname = 'pg_type') AS classoid,
(SELECT database_name FROM mz_internal.pg_class_all_databases WHERE relname = 'pg_type') AS class_database_name
(SELECT oid FROM pg_catalog_class WHERE relname = 'pg_type') AS classoid,
(SELECT database_name FROM pg_catalog_class WHERE relname = 'pg_type') AS class_database_name
FROM mz_internal.pg_type_all_databases
UNION ALL
SELECT oid, database_name as oid_database_name,
(SELECT oid FROM mz_internal.pg_class_all_databases WHERE relname = 'pg_namespace') AS classoid,
(SELECT database_name FROM mz_internal.pg_class_all_databases WHERE relname = 'pg_namespace') AS class_database_name
(SELECT oid FROM pg_catalog_class WHERE relname = 'pg_namespace') AS classoid,
(SELECT database_name FROM pg_catalog_class WHERE relname = 'pg_namespace') AS class_database_name
FROM mz_internal.pg_namespace_all_databases
),

Expand Down
117 changes: 68 additions & 49 deletions test/sqllogictest/catalog_server_explain.slt
Original file line number Diff line number Diff line change
Expand Up @@ -3413,82 +3413,101 @@ mz_internal.pg_description_all_databases_ind:
mz_internal.pg_description_all_databases:
→With
cte l0 =
→Arranged mz_internal.pg_class_all_databases
cte l1 =
→Differential Join %1[#0] » %0:l0[#1{relname}]
→Arranged l0
→Differential Join %1[#0] » %0:pg_namespace_all_databases[#1{nspname}]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ggevay I think you added these recently? I see that we're doing things differently now, which makes sense because we have an additional lookup. But couldn't say whether that's fine or not

→Arranged mz_internal.pg_namespace_all_databases
→Arrange (#0)
→Constant (1 row)
cte l1 =
→Differential Join %0:pg_class_all_databases[#2{relnamespace}] » %1[#0]
→Arrange (#2{relnamespace})
→Fused with Child Map/Filter/Project
Project: #1, #0, #2, #23
→Arranged mz_internal.pg_class_all_databases
Key: (#1{relname})
→Arrange (#0)
→Union
→Stream l0
→Table Function guard_subquery_size(#0)
→Accumulable GroupAggregate
Simple aggregates: count(*)
→Fused with Child Map/Filter/Project
Project: ()
→Read l0
cte l2 =
→Fused with Child Map/Filter/Project
Project: #0, #2
Filter: (#1{relname} = "pg_class")
→Read l1
cte l3 =
→Accumulable GroupAggregate
Simple aggregates: count(*)
→Fused with Child Map/Filter/Project
Project: ()
→Read l1
cte l3 =
→Read l2
cte l4 =
→Union
→Fused with Child Map/Filter/Project
Project: #0
→Read l1
→Read l2
→Table Function guard_subquery_size(#0)
→Arranged l2
cte l4 =
→Arranged l3
cte l5 =
→Union
→Fused with Child Map/Filter/Project
Project: #1
→Read l1
→Read l2
→Table Function guard_subquery_size(#0)
→Arranged l2
cte l5 =
→Differential Join %1[#0] » %0:l0[#1{relname}]
→Arranged l0
→Arrange (#0)
→Constant (1 row)
→Arranged l3
cte l6 =
→Fused with Child Map/Filter/Project
Project: #0, #2
Filter: (#1{relname} = "pg_type")
→Read l1
cte l7 =
→Accumulable GroupAggregate
Simple aggregates: count(*)
→Fused with Child Map/Filter/Project
Project: ()
→Read l5
cte l7 =
→Read l6
cte l8 =
→Union
→Fused with Child Map/Filter/Project
Project: #0
→Read l5
→Read l6
→Table Function guard_subquery_size(#0)
→Arranged l6
cte l8 =
→Arranged l7
cte l9 =
→Union
→Fused with Child Map/Filter/Project
Project: #1
→Read l5
→Read l6
→Table Function guard_subquery_size(#0)
→Arranged l6
cte l9 =
→Differential Join %1[#0] » %0:l0[#1{relname}]
→Arranged l0
→Arrange (#0)
→Constant (1 row)
→Arranged l7
cte l10 =
→Fused with Child Map/Filter/Project
Project: #0, #2
Filter: (#1{relname} = "pg_namespace")
→Read l1
cte l11 =
→Accumulable GroupAggregate
Simple aggregates: count(*)
→Fused with Child Map/Filter/Project
Project: ()
→Read l9
cte l11 =
→Read l10
cte l12 =
→Union
→Fused with Child Map/Filter/Project
Project: #0
→Read l9
→Read l10
→Table Function guard_subquery_size(#0)
→Arranged l10
cte l12 =
→Arranged l11
cte l13 =
→Union
→Fused with Child Map/Filter/Project
Project: #1
→Read l9
→Read l10
→Table Function guard_subquery_size(#0)
→Arranged l10
→Arranged l11
→Return
→Delta Join [%0[#0{oid}] » %1[#1{oid}] » %2:mz_comments[#0{id}, lower(#1{object_type})]] [%1[#0{id}, lower(#2{type})] » %2:mz_comments[#0{id}, lower(#1{object_type})] » %0[#0{oid}]] [%2:mz_comments[#0{id}, lower(#1{object_type})] » %1[#0{id}, lower(#2{type})] » %0[#0{oid}]]
path %0:
Expand All @@ -3509,7 +3528,7 @@ mz_internal.pg_description_all_databases:
Key: (#1{relname})
→Arrange (empty key)
→Union
→Stream l3
→Stream l4
→Map/Filter/Project
Project: #0
Map: null
Expand All @@ -3519,11 +3538,11 @@ mz_internal.pg_description_all_databases:
→Distinct GroupAggregate
→Fused with Child Map/Filter/Project
Project: ()
→Read l3
→Read l4
→Constant (1 row)
→Arrange (empty key)
→Union
→Stream l4
→Stream l5
→Map/Filter/Project
Project: #0
Map: null
Expand All @@ -3533,7 +3552,7 @@ mz_internal.pg_description_all_databases:
→Distinct GroupAggregate
→Fused with Child Map/Filter/Project
Project: ()
→Read l4
→Read l5
→Constant (1 row)
→Delta Cross Join [%0:pg_type_all_databases[×] » %1[×] » %2[×]] [%1[×] » %0:pg_type_all_databases[×] » %2[×]] [%2[×] » %0:pg_type_all_databases[×] » %1[×]]
→Arrange (empty key)
Expand All @@ -3543,7 +3562,7 @@ mz_internal.pg_description_all_databases:
Key: (#0{oid})
→Arrange (empty key)
→Union
→Stream l7
→Stream l8
→Map/Filter/Project
Project: #0
Map: null
Expand All @@ -3553,11 +3572,11 @@ mz_internal.pg_description_all_databases:
→Distinct GroupAggregate
→Fused with Child Map/Filter/Project
Project: ()
→Read l7
→Read l8
→Constant (1 row)
→Arrange (empty key)
→Union
→Stream l8
→Stream l9
→Map/Filter/Project
Project: #0
Map: null
Expand All @@ -3567,7 +3586,7 @@ mz_internal.pg_description_all_databases:
→Distinct GroupAggregate
→Fused with Child Map/Filter/Project
Project: ()
→Read l8
→Read l9
→Constant (1 row)
→Delta Cross Join [%0:pg_namespace_all_databases[×] » %1[×] » %2[×]] [%1[×] » %0:pg_namespace_all_databases[×] » %2[×]] [%2[×] » %0:pg_namespace_all_databases[×] » %1[×]]
→Arrange (empty key)
Expand All @@ -3577,7 +3596,7 @@ mz_internal.pg_description_all_databases:
Key: (#1{nspname})
→Arrange (empty key)
→Union
→Stream l11
→Stream l12
→Map/Filter/Project
Project: #0
Map: null
Expand All @@ -3587,11 +3606,11 @@ mz_internal.pg_description_all_databases:
→Distinct GroupAggregate
→Fused with Child Map/Filter/Project
Project: ()
→Read l11
→Read l12
→Constant (1 row)
→Arrange (empty key)
→Union
→Stream l12
→Stream l13
→Map/Filter/Project
Project: #0
Map: null
Expand All @@ -3601,7 +3620,7 @@ mz_internal.pg_description_all_databases:
→Distinct GroupAggregate
→Fused with Child Map/Filter/Project
Project: ()
→Read l12
→Read l13
→Constant (1 row)
→Arrange (#0{id}, lower(#2{type})) (#1{oid})
→Union
Expand All @@ -3618,8 +3637,8 @@ mz_internal.pg_description_all_databases:
→Arranged mz_internal.mz_comments

Used Indexes:
- mz_internal.pg_namespace_all_databases_ind (*** full scan ***)
- mz_internal.pg_class_all_databases_ind (*** full scan ***, lookup)
- mz_internal.pg_namespace_all_databases_ind (*** full scan ***, lookup)
- mz_internal.pg_class_all_databases_ind (*** full scan ***)
- mz_internal.pg_type_all_databases_ind (*** full scan ***)
- mz_internal.mz_comments_ind (*** full scan ***)
- mz_catalog.mz_schemas_ind (*** full scan ***)
Expand Down
40 changes: 40 additions & 0 deletions test/sqllogictest/comment.slt
Original file line number Diff line number Diff line change
Expand Up @@ -657,3 +657,43 @@ database NULL main_db
query IT
SELECT objsubid, description FROM pg_description WHERE objoid >= 20000;
----

# Regression test for SQL-278: pg_catalog.pg_description scopes its
# pg_class/pg_type/pg_namespace classoid lookups to the pg_catalog schema. A
# user-created object named after one of those catalogs (in any schema) used to
# make the scalar subqueries match multiple rows, breaking pg_description for
# everyone. PostgreSQL's pg_description is unaffected by such user objects.

statement ok
CREATE TABLE has_comment (a int)

statement ok
COMMENT ON TABLE has_comment IS 'useful_comment'

# User objects shadowing the pg_catalog system catalog names.
statement ok
CREATE TABLE public.pg_class (x int)

statement ok
CREATE TABLE public.pg_type (x int)

statement ok
CREATE TABLE public.pg_namespace (x int)

# pg_description must still return the comment, not error.
query IT
SELECT objsubid, description FROM pg_description WHERE objoid >= 20000;
----
0 useful_comment

statement ok
DROP TABLE public.pg_class

statement ok
DROP TABLE public.pg_type

statement ok
DROP TABLE public.pg_namespace

statement ok
DROP TABLE has_comment
Loading