sql: reimplement pg_*_is_visible as Go builtins#171344
Closed
rafiss wants to merge 1 commit into
Closed
Conversation
pg_function_is_visible, pg_table_is_visible, and pg_type_is_visible are called
once per row by ORM and psql \d introspection queries. They were implemented as
SQL-body builtins whose Postgres-correct shadow check ("is this the first object
of its name on the search path?") scanned pg_proc / pg_class / pg_type. Because
those virtual tables are indexed only by OID, the shadow check forced a full
materialization of the catalog on every call: O(catalog) per call, and O(N^2)
under the per-row introspection pattern. This made catalog introspection very
slow and was the cause of the TestDescribe timeout in cockroachdb#171054.
This commit reimplements the three builtins in Go (pkg/sql/pg_is_visible.go),
exposed through new eval.Planner methods. Instead of scanning the virtual
catalogs, each builtin resolves the object's name and schema from its OID and
walks the current search path doing leased-cache-backed lookups, mirroring
Postgres's RelationIsVisible / TypeIsVisible / FunctionIsVisible:
* pg_function_is_visible resolves the OID's name and signature, then uses the
normal function resolver to find the candidate overloads and checks that the
first one (by search-path precedence) with matching argument types is the
target. This preserves Postgres's signature-aware behavior: same-named
functions with disjoint argument lists do not shadow each other.
* pg_table_is_visible and pg_type_is_visible walk the search path and check, per
schema, whether an object of the same name exists, stopping at the first match.
The per-call cost drops from a full catalog populate to a handful of
descriptor/namespace lookups, eliminating the quadratic blow-up.
Behavior is unchanged from the SQL implementation: visibility, cross-schema
shadowing, signature-aware function visibility, NULL for a non-existent OID, and
NULL for objects in another database all match. A few cases are handled
explicitly: relations and types in other databases (which have no row in the
per-database virtual catalogs) return NULL; virtual tables (e.g.
pg_catalog.pg_class) are resolved through pg_class so they remain visible; and
index entries, whose hashed OIDs cannot be reversed to a descriptor, fall back
to a pg_class lookup.
This restores the Go-builtin approach used before acf5006, which had moved
these to SQL bodies for a performance win that later regressed once the bodies
were made shadow-aware.
Resolves: cockroachdb#171054
Epic: none
Release note: None
Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
Contributor
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
Member
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
pg_function_is_visible, pg_table_is_visible, and pg_type_is_visible are called
once per row by ORM and psql \d introspection queries. They were implemented as
SQL-body builtins whose Postgres-correct shadow check ("is this the first object
of its name on the search path?") scanned pg_proc / pg_class / pg_type. Because
those virtual tables are indexed only by OID, the shadow check forced a full
materialization of the catalog on every call: O(catalog) per call, and O(N^2)
under the per-row introspection pattern. This made catalog introspection very
slow and was the cause of the TestDescribe timeout in #171054.
This commit reimplements the three builtins in Go (pkg/sql/pg_is_visible.go),
exposed through new eval.Planner methods. Instead of scanning the virtual
catalogs, each builtin resolves the object's name and schema from its OID and
walks the current search path doing leased-cache-backed lookups, mirroring
Postgres's RelationIsVisible / TypeIsVisible / FunctionIsVisible:
normal function resolver to find the candidate overloads and checks that the
first one (by search-path precedence) with matching argument types is the
target. This preserves Postgres's signature-aware behavior: same-named
functions with disjoint argument lists do not shadow each other.
schema, whether an object of the same name exists, stopping at the first match.
The per-call cost drops from a full catalog populate to a handful of
descriptor/namespace lookups, eliminating the quadratic blow-up.
Behavior is unchanged from the SQL implementation: visibility, cross-schema
shadowing, signature-aware function visibility, NULL for a non-existent OID, and
NULL for objects in another database all match. A few cases are handled
explicitly: relations and types in other databases (which have no row in the
per-database virtual catalogs) return NULL; virtual tables (e.g.
pg_catalog.pg_class) are resolved through pg_class so they remain visible; and
index entries, whose hashed OIDs cannot be reversed to a descriptor, fall back
to a pg_class lookup.
This restores the Go-builtin approach used before acf5006, which had moved
these to SQL bodies for a performance win that later regressed once the bodies
were made shadow-aware.
Performance
BenchmarkORMQueries/django_table_introspectioncallspg_table_is_visibleonce per row. Base master vs this PR (
./dev bench --count=6, compared withbenchstat):TestDescribe(psql\d/\df/\dTover the full catalog) drops from ~75s onmaster to ~10.6s with this PR.
Resolves: #171054
Informs: #108334
Epic: none
Release note: None