From 75a258e6957e546b88132d818e5d6c65bdac0fe2 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 21 Jan 2026 15:57:20 -0800 Subject: [PATCH 1/3] put pg_catalog implicitly on the search path --- core/context.go | 26 ++++++++++++++++++++++++- server/functions/pg_table_is_visible.go | 4 ++-- server/functions/pg_type_is_visible.go | 4 ++-- server/functions/regclass.go | 6 +++--- server/node/create_trigger.go | 4 +--- testing/go/prepared_statement_test.go | 7 ++----- 6 files changed, 35 insertions(+), 16 deletions(-) diff --git a/core/context.go b/core/context.go index 1a9422f39b..bd2b6f52bc 100644 --- a/core/context.go +++ b/core/context.go @@ -199,7 +199,7 @@ func GetSqlTableFromContext(ctx *sql.Context, databaseName string, tableName dol var searchPath []string if len(tableName.Schema) == 0 { // If a schema was not provided, then we'll use the search path - searchPath, err = resolve.SearchPath(ctx) + searchPath, err = SearchPath(ctx) if err != nil { return nil, err } @@ -228,6 +228,30 @@ func GetSqlTableFromContext(ctx *sql.Context, databaseName string, tableName dol return nil, nil } +// SearchPath returns the effective schema search path for the current session +func SearchPath(ctx *sql.Context) ([]string, error) { + path, err := resolve.SearchPath(ctx) + if err != nil { + return nil, err + } + + // pg_catalog is *always* implicitly part of the search path as the first element, unless it's specifically + // included later. This allows users to override built-in names with user-defined names, but they have to + // opt in to that behavior. + hasPgCatalog := false + for _, schema := range path { + if schema == "pg_catalog" { + hasPgCatalog = true + break + } + } + + if !hasPgCatalog { + path = append([]string{"pg_catalog"}, path...) + } + return path, nil +} + // GetExtensionsCollectionFromContext returns the extensions collection from the given context. Will always return a // collection if no error is returned. func GetExtensionsCollectionFromContext(ctx *sql.Context, database string) (*extensions.Collection, error) { diff --git a/server/functions/pg_table_is_visible.go b/server/functions/pg_table_is_visible.go index 3f64715b04..88d27ee003 100644 --- a/server/functions/pg_table_is_visible.go +++ b/server/functions/pg_table_is_visible.go @@ -15,7 +15,7 @@ package functions import ( - "github.com/dolthub/dolt/go/libraries/doltcore/sqle/resolve" + "github.com/dolthub/doltgresql/core" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/doltgresql/core/id" @@ -38,7 +38,7 @@ var pg_table_is_visible_oid = framework.Function1{ Strict: true, Callable: func(ctx *sql.Context, _ [2]*pgtypes.DoltgresType, val any) (any, error) { oidVal := val.(id.Id) - paths, err := resolve.SearchPath(ctx) + paths, err := core.SearchPath(ctx) if err != nil { return false, err } diff --git a/server/functions/pg_type_is_visible.go b/server/functions/pg_type_is_visible.go index 040496cbc8..9701419428 100644 --- a/server/functions/pg_type_is_visible.go +++ b/server/functions/pg_type_is_visible.go @@ -15,7 +15,7 @@ package functions import ( - "github.com/dolthub/dolt/go/libraries/doltcore/sqle/resolve" + "github.com/dolthub/doltgresql/core" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/doltgresql/core/id" @@ -48,7 +48,7 @@ var pg_type_is_visible = framework.Function1{ schemaName := oidVal.Segment(0) // Get the current search path - searchPath, err := resolve.SearchPath(ctx) + searchPath, err := core.SearchPath(ctx) if err != nil { return false, err } diff --git a/server/functions/regclass.go b/server/functions/regclass.go index 808c256812..f9bdee70e3 100644 --- a/server/functions/regclass.go +++ b/server/functions/regclass.go @@ -19,7 +19,7 @@ import ( "strconv" "github.com/cockroachdb/errors" - "github.com/dolthub/dolt/go/libraries/doltcore/sqle/resolve" + "github.com/dolthub/doltgresql/core" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/doltgresql/core/id" @@ -65,7 +65,7 @@ var regclassin = framework.Function1{ switch len(sections) { case 1: database = ctx.GetCurrentDatabase() - searchSchemas, err = resolve.SearchPath(ctx) + searchSchemas, err = core.SearchPath(ctx) if err != nil { return id.Null, err } @@ -79,7 +79,7 @@ var regclassin = framework.Function1{ searchSchemas = []string{sections[2]} relationName = sections[4] default: - return id.Null, errors.Errorf("regclass failed validation") + return id.Null, errors.Errorf("unexpected input for regclass: %s", input) } // Iterate over all of the items to find which relation matches. diff --git a/server/node/create_trigger.go b/server/node/create_trigger.go index 04114212c6..8286f3920c 100644 --- a/server/node/create_trigger.go +++ b/server/node/create_trigger.go @@ -16,7 +16,6 @@ package node import ( "github.com/cockroachdb/errors" - "github.com/dolthub/dolt/go/libraries/doltcore/sqle/resolve" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/plan" vitess "github.com/dolthub/vitess/go/vt/sqlparser" @@ -180,11 +179,10 @@ func loadFunction(ctx *sql.Context, funcCollection *functions.Collection, funcID return functions.Function{}, err } } else { - searchPaths, err := resolve.SearchPath(ctx) + searchPaths, err := core.SearchPath(ctx) if err != nil { return functions.Function{}, err } - searchPaths = append(searchPaths, "pg_catalog") // This isn't included in the search path but functions use it for _, searchPath := range searchPaths { function, err = funcCollection.GetFunction(ctx, id.NewFunction(searchPath, funcID.FunctionName())) if err != nil { diff --git a/testing/go/prepared_statement_test.go b/testing/go/prepared_statement_test.go index aa5524c9b9..daf81fb628 100755 --- a/testing/go/prepared_statement_test.go +++ b/testing/go/prepared_statement_test.go @@ -1334,15 +1334,14 @@ var pgCatalogTests = []ScriptTest{ }, }, { - Name: "pg_class", + Name: "pg_class", + Focus: true, SetUpScript: []string{ `CREATE SCHEMA testschema;`, `CREATE TABLE testschema.testtable (id int primary key, v1 text)`, }, Assertions: []ScriptTestAssertion{ { - // https://github.com/dolthub/doltgresql/issues/2217 - Skip: true, Query: `SELECT c.oid,d.description,pg_catalog.pg_get_expr(c.relpartbound, c.oid) as partition_expr, pg_catalog.pg_get_partkeydef(c.oid) as partition_key FROM pg_catalog.pg_class c LEFT OUTER JOIN pg_catalog.pg_description d ON d.objoid=c.oid AND d.objsubid=0 AND d.classoid='pg_class'::regclass @@ -1351,8 +1350,6 @@ WHERE c.relnamespace=$1 AND c.relkind not in ('i','I','c') and c.oid not in (sel Expected: []sql.Row{{1712283605, nil, nil, ""}}, }, { - // https://github.com/dolthub/doltgresql/issues/2217 - Skip: true, Query: `SELECT d.description from pg_catalog.pg_description d WHERE d.classoid='pg_class'::regclass`, // TODO: add expected values }, From bf46d10db495363e330e28604b4c2118b0ea9648 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 21 Jan 2026 17:08:34 -0800 Subject: [PATCH 2/3] Added a long TODO note explaining buggy behavior for fresh servers --- testing/go/prepared_statement_test.go | 28 ++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/testing/go/prepared_statement_test.go b/testing/go/prepared_statement_test.go index daf81fb628..727a2a2f98 100755 --- a/testing/go/prepared_statement_test.go +++ b/testing/go/prepared_statement_test.go @@ -1334,13 +1334,35 @@ var pgCatalogTests = []ScriptTest{ }, }, { - Name: "pg_class", - Focus: true, + Name: "pg_class", SetUpScript: []string{ `CREATE SCHEMA testschema;`, `CREATE TABLE testschema.testtable (id int primary key, v1 text)`, }, Assertions: []ScriptTestAssertion{ + { + Query: "select distinct relnamespace from pg_catalog.pg_class c INNER JOIN pg_catalog.pg_namespace n ON c.relnamespace = n.oid WHERE n.nspname=$1;", + BindVars: []any{"testschema"}, + Expected: []sql.Row{{2638679668}}, + }, + // TODO: when this test is run in isolation without the query above, the below query returns no rows. This is + // because the process of converting an OID to its internal ID can only proceed in one direction: from internal + // to OID. When the above query runs, it causes the internal ID for the testschema namespace to be cached, + // allowing the reverse lookup to succeed in subsequent queries. For OIDs that have not yet been cached in this + // manner, lookups by their OID will fail. This doesn't impact all queries since many of them get an index + // lookup on OID, which has the side effect of converting everything to numeric IDs anyway. But for queries + // that use a normal comparison function for an OID literal value, the conversion to an internal ID of the + // appropriate type (e.g. id.Namespace) cannot happen in the |oidin| function in some cases because the internal + // to OID mapping hasn't yet been established for that schema element, so the comparison fails, yielding + // incorrect results. + // To fix this, we need to correctly seed the internal ID cache with all schema elements in the database. + { + Query: `SELECT c.oid,pg_catalog.pg_get_expr(c.relpartbound, c.oid) as partition_expr, pg_catalog.pg_get_partkeydef(c.oid) as partition_key +FROM pg_catalog.pg_class c +WHERE c.relnamespace=$1 AND c.relkind not in ('i','I','c') and c.oid not in (select oid from pg_catalog.pg_class where left(relname, 5) = 'dolt_');`, + BindVars: []any{2638679668}, + Expected: []sql.Row{{1712283605, nil, ""}}, + }, { Query: `SELECT c.oid,d.description,pg_catalog.pg_get_expr(c.relpartbound, c.oid) as partition_expr, pg_catalog.pg_get_partkeydef(c.oid) as partition_key FROM pg_catalog.pg_class c @@ -1351,7 +1373,7 @@ WHERE c.relnamespace=$1 AND c.relkind not in ('i','I','c') and c.oid not in (sel }, { Query: `SELECT d.description from pg_catalog.pg_description d WHERE d.classoid='pg_class'::regclass`, - // TODO: add expected values + // TODO: add expected values (pg_description not yet implemented) }, { Query: `select c.oid,pg_catalog.pg_total_relation_size(c.oid) as total_rel_size,pg_catalog.pg_relation_size(c.oid) as rel_size FROM pg_class c WHERE c.relnamespace=$1 and c.oid not in (select oid from pg_catalog.pg_class where left(relname, 5) = 'dolt_');`, From fa280fb1055cc9735aefae7b06d5b92d63d0917d Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 21 Jan 2026 17:09:23 -0800 Subject: [PATCH 3/3] formatting --- server/functions/pg_table_is_visible.go | 3 +-- server/functions/pg_type_is_visible.go | 3 ++- server/functions/regclass.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/functions/pg_table_is_visible.go b/server/functions/pg_table_is_visible.go index 88d27ee003..737ef12693 100644 --- a/server/functions/pg_table_is_visible.go +++ b/server/functions/pg_table_is_visible.go @@ -15,11 +15,10 @@ package functions import ( - "github.com/dolthub/doltgresql/core" "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/doltgresql/core" "github.com/dolthub/doltgresql/core/id" - "github.com/dolthub/doltgresql/server/functions/framework" pgtypes "github.com/dolthub/doltgresql/server/types" ) diff --git a/server/functions/pg_type_is_visible.go b/server/functions/pg_type_is_visible.go index 9701419428..7755ae220f 100644 --- a/server/functions/pg_type_is_visible.go +++ b/server/functions/pg_type_is_visible.go @@ -15,9 +15,10 @@ package functions import ( - "github.com/dolthub/doltgresql/core" "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/doltgresql/core" + "github.com/dolthub/doltgresql/core/id" "github.com/dolthub/doltgresql/server/functions/framework" pgtypes "github.com/dolthub/doltgresql/server/types" diff --git a/server/functions/regclass.go b/server/functions/regclass.go index f9bdee70e3..fb66c8b2e4 100644 --- a/server/functions/regclass.go +++ b/server/functions/regclass.go @@ -19,9 +19,9 @@ import ( "strconv" "github.com/cockroachdb/errors" - "github.com/dolthub/doltgresql/core" "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/doltgresql/core" "github.com/dolthub/doltgresql/core/id" "github.com/dolthub/doltgresql/server/functions/framework" "github.com/dolthub/doltgresql/server/settings"