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..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/dolt/go/libraries/doltcore/sqle/resolve" "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" ) @@ -38,7 +37,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..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/dolt/go/libraries/doltcore/sqle/resolve" "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" @@ -48,7 +49,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..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/dolt/go/libraries/doltcore/sqle/resolve" "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" @@ -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..727a2a2f98 100755 --- a/testing/go/prepared_statement_test.go +++ b/testing/go/prepared_statement_test.go @@ -1341,8 +1341,29 @@ var pgCatalogTests = []ScriptTest{ }, Assertions: []ScriptTestAssertion{ { - // https://github.com/dolthub/doltgresql/issues/2217 - Skip: true, + 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 LEFT OUTER JOIN pg_catalog.pg_description d ON d.objoid=c.oid AND d.objsubid=0 AND d.classoid='pg_class'::regclass @@ -1351,10 +1372,8 @@ 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 + // 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_');`,