From 38a149ca1168e78cb2744b70d992b8662e7fd9e4 Mon Sep 17 00:00:00 2001 From: ZhouXing19 Date: Mon, 1 Jun 2026 13:24:28 +0000 Subject: [PATCH] sql: resolve built-in function OIDs in memory for reg* casts 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: #171257 Epic: CRDB-62230 Release note: None Co-Authored-By: Claude Opus 4.8 (1M context) --- .../logictest/testdata/logic_test/pgoidtype | 21 ++++++++++++++ pkg/sql/resolve_oid.go | 29 +++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/pkg/sql/logictest/testdata/logic_test/pgoidtype b/pkg/sql/logictest/testdata/logic_test/pgoidtype index 74aebbce0816..babaa8ece33a 100644 --- a/pkg/sql/logictest/testdata/logic_test/pgoidtype +++ b/pkg/sql/logictest/testdata/logic_test/pgoidtype @@ -508,6 +508,27 @@ SELECT ---- 12345 12345 12345 12345 12345 +# Regression test for #171257. A built-in function OID that names a "private" +# helper function (defined via makePrivate, e.g. the regression-aggregate final +# functions) must cast to its bare pg_catalog name -- not a spuriously +# "pg_catalog."-qualified name, and never the bare numeric OID. These helpers are +# excluded from builtins.AllBuiltinNames, so the by-name pg_catalog.pg_proc +# population omits them; the visibility lookup therefore found no match and +# wrongly schema-qualified them (or, with no database in scope, errored and let +# the OID render as a number). OID 56 is final_regr_syy; see +# pkg/sql/sem/builtins/fixed_oids.go. +query TTT +SELECT 56::REGPROC::STRING, 56::REGPROCEDURE::STRING, 56::OID::REGPROC::STRING +---- +final_regr_syy final_regr_syy final_regr_syy + +# The 'name'::reg* parse path must agree with the OID cast path: the optimizer +# interns DOids by OID alone, so divergent names would corrupt round-trips. +query TT +SELECT 'final_regr_syy'::REGPROC::STRING, 'final_regr_syy'::REGPROC::OID::REGPROC::STRING +---- +final_regr_syy final_regr_syy + query T PREPARE regression_56193 AS SELECT $1::regclass; EXECUTE regression_56193('regression_53686"'::regclass) diff --git a/pkg/sql/resolve_oid.go b/pkg/sql/resolve_oid.go index d853317fd3a2..f8e1ffb2ab7a 100644 --- a/pkg/sql/resolve_oid.go +++ b/pkg/sql/resolve_oid.go @@ -16,6 +16,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/lexbase" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" + "github.com/cockroachdb/cockroach/pkg/sql/sem/catid" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" "github.com/cockroachdb/cockroach/pkg/sql/types" @@ -38,6 +39,34 @@ func (p *planner) ResolveOIDFromString( func (p *planner) ResolveOIDFromOID( ctx context.Context, resultType *types.T, toResolve *tree.DOid, ) (_ *tree.DOid, errSafeToIgnore bool, _ error) { + // Built-in functions live in pg_catalog, which is always implicitly in the + // search path, so Postgres's regprocout emits their bare name. Resolve them + // from the in-memory function registry rather than through resolveOID's + // catalog query, which determines visibility by looking up + // pg_catalog.pg_proc by name. The by-name population of that virtual table + // is built from builtins.AllBuiltinNames and omits private aggregate/window + // helper functions (e.g. final_regr_syy), so resolveOID's visibility check + // finds no match and spuriously schema-qualifies them; worse, when no + // database is in scope the query errors and the OID renders as a bare + // number. Resolving in memory keeps the cast deterministic regardless of + // session and execution context. User-defined functions still flow through + // resolveOID for search-path-aware schema qualification. + switch resultType.Oid() { + case oid.T_regproc, oid.T_regprocedure: + if !catid.IsOIDUserDefined(toResolve.Oid) { + name, _, err := p.ResolveFunctionByOID(ctx, toResolve.Oid) + if err != nil { + if errors.Is(err, tree.ErrRoutineUndefined) { + // The OID is in the built-in range but names no known + // function; leave the DOid nameless so it renders as the + // bare numeric OID, matching Postgres. + return tree.NewDOidWithType(toResolve.Oid, resultType), true, nil //nolint:returnerrcheck + } + return nil, false, err + } + return tree.NewDOidWithTypeAndName(toResolve.Oid, resultType, name.Object()), true, nil + } + } return resolveOID( ctx, p.Txn(), p.InternalSQLTxn(),