sql: resolve built-in function OIDs in memory for reg* casts#171295
Draft
ZhouXing19 wants to merge 1 commit into
Draft
sql: resolve built-in function OIDs in memory for reg* casts#171295ZhouXing19 wants to merge 1 commit into
ZhouXing19 wants to merge 1 commit into
Conversation
Casting an integer to regproc or regprocedure and then to text routes through resolveOID, which runs a pg_catalog.pg_proc query to compute a Postgres-style search-path visibility bit and schema-qualify the name when it is not visible through the current search_path. That query determines visibility by looking up pg_proc by name, but pg_proc's by-name population is built from builtins.AllBuiltinNames, which excludes "private" builtins such as the regression-aggregate helper functions (e.g. final_regr_syy, OID 56). As a result, casting such a function's OID produced inconsistent output: in a normal session the visibility lookup found no match and emitted a spuriously schema-qualified name (pg_catalog.final_regr_syy), while in a context with no database in scope the catalog query errored and the OID fell back to its bare numeric form (56). The unoptimized-query-oracle roachtest observed both forms for the same logical cast and failed. Postgres emits the bare name for these functions, since pg_catalog is always implicitly in the search path. Resolve built-in function OIDs from the in-memory function registry instead, mirroring the existing built-in handling for regtype OIDs. This keeps the cast deterministic regardless of session and execution context and matches Postgres. The change lives in (*planner).ResolveOIDFromOID, the single point shared by both the OID cast path and the 'name'::reg* parse path, so the two continue to produce identical DOid names (the optimizer interns DOids by OID alone). User-defined functions still flow through resolveOID for search-path-aware schema qualification. Resolves: cockroachdb#171257 Epic: CRDB-62230 Release note: None Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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 change is |
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
2 participants
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.
Casting an integer to
regprocorregprocedureand then to text routesthrough
resolveOID, which runs apg_catalog.pg_procquery to compute aPostgres-style search-path visibility bit and schema-qualify the name when it
is not visible through the current
search_path. That query determinesvisibility by looking up
pg_procby name, butpg_proc's by-name populationis built from
builtins.AllBuiltinNames, which excludes "private" builtins suchas the regression-aggregate helper functions (e.g.
final_regr_syy, OID 56).As a result, casting such a function's OID produced inconsistent output: in a
normal session the visibility lookup found no match and emitted a spuriously
schema-qualified name (
pg_catalog.final_regr_syy), while in a context with nodatabase in scope the catalog query errored and the OID fell back to its bare
numeric form (
56). Theunoptimized-query-oracleroachtest observed bothforms for the same logical cast and failed. Postgres emits the bare name for
these functions, since
pg_catalogis always implicitly in the search path.This change resolves built-in function OIDs from the in-memory function registry
instead, mirroring the existing built-in handling for
regtypeOIDs. It keepsthe cast deterministic regardless of session and execution context and matches
Postgres. The fix lives in
(*planner).ResolveOIDFromOID, the single pointshared by both the OID cast path and the
'name'::reg*parse path, so the twocontinue to produce identical
DOidnames (the optimizer internsDOids by OIDalone). User-defined functions still flow through
resolveOIDforsearch-path-aware schema qualification.
Fixes: #171257
Epic: CRDB-62230
Release note: None
🤖 Generated with Claude Code