From b7de92c4d598d24a263fa6d89206a85e93ccfa4e Mon Sep 17 00:00:00 2001 From: ayusssmaan Date: Thu, 17 Sep 2026 19:33:05 +0530 Subject: [PATCH 1/5] fix: resolve extension-owned types in the external plan database's temp schema pgschema's temporary comparison schema during plan generation only sets search_path to ", public", so a bare reference to an extension-owned type installed in a non-public schema (e.g. pgvector's "vector" in a "domain" schema) fails with "type ... does not exist" when the desired-state SQL is applied there. This is issue #518's deeper, apply-time failure mode, which PR #544's pre-flight schema-consistency check does not address even when the plan and target databases agree on the extension's schema. Three related fixes, all rooted in the same cause (pgschema's per-object code only knows about the target schema, not about extension schemas): - external.go: extend the temp schema's search_path to include every schema hosting an installed extension on the plan database. - inspector.go: stripSameSchemaPrefix also strips a prefix matching any known extension schema, not just the routine's own schema - needed because a function's routineSchema *is* the temp schema name when introspecting the "new" side of a diff, so an extension-owned type's qualifier would otherwise survive stripped on one side and not the other, causing a spurious drop+recreate. - inspector.go: buildPrivileges applies the same extension-schema-aware stripping (via the existing StripSchemaQualifiers tokenizer already used for aggregate signatures) to FUNCTION/PROCEDURE object_name, which bypasses stripSameSchemaPrefix entirely by being rendered via pg_get_function_identity_arguments() directly in SQL. --- internal/postgres/external.go | 37 +++++++++- ir/inspector.go | 100 +++++++++++++++++++++++--- ir/inspector_extension_schema_test.go | 72 +++++++++++++++++++ 3 files changed, 199 insertions(+), 10 deletions(-) create mode 100644 ir/inspector_extension_schema_test.go diff --git a/internal/postgres/external.go b/internal/postgres/external.go index e79249e2..7b17d382 100644 --- a/internal/postgres/external.go +++ b/internal/postgres/external.go @@ -142,8 +142,41 @@ func (ed *ExternalDatabase) ApplySchema(ctx context.Context, schema string, sql } // Set search_path to the temporary schema, with public as fallback - // for resolving extension types installed in public schema (issue #197) - setSearchPathSQL := fmt.Sprintf("SET search_path TO \"%s\", public", ed.tempSchema) + // for resolving extension types installed in public schema (issue #197). + // + // Also append every other schema that hosts an installed extension (e.g. + // pgvector's "vector" living in a non-public schema like "domain"). + // Desired-state SQL for an extension-owned column is written unqualified, + // same as any other same-schema reference (e.g. "embedding vector(384)"), + // and stripSchemaQualifications only strips - it never adds - a schema + // prefix. Without the extension's real schema in this search_path, such a + // bare reference cannot resolve inside the temp schema at all, since the + // temp schema has no copy of the type and "public" may not either (issue + // #518's apply-time failure mode, not addressed by #544's pre-flight + // schema-consistency check alone). This is safe: NewExternalDatabase + // already validated that every extension shared between the plan and + // target databases lives in the same schema on both, so resolving a bare + // extension reference against the plan database's copy here is exactly + // equivalent to how it resolves on the real target. + extraSchemas, err := getExtensionSchemas(ed.db) + if err != nil { + return fmt.Errorf("failed to query extension schemas: %w", err) + } + searchPathParts := []string{quoteIdent(ed.tempSchema), "public"} + seen := map[string]bool{ed.tempSchema: true, "public": true} + extraSchemaNames := make([]string, 0, len(extraSchemas)) + for _, extSchema := range extraSchemas { + extraSchemaNames = append(extraSchemaNames, extSchema) + } + sort.Strings(extraSchemaNames) + for _, extSchema := range extraSchemaNames { + if seen[extSchema] { + continue + } + seen[extSchema] = true + searchPathParts = append(searchPathParts, quoteIdent(extSchema)) + } + setSearchPathSQL := fmt.Sprintf("SET search_path TO %s", strings.Join(searchPathParts, ", ")) if _, err := util.ExecContextWithLogging(ctx, conn, setSearchPathSQL, "set search_path for desired state"); err != nil { return fmt.Errorf("failed to set search_path: %w", err) } diff --git a/ir/inspector.go b/ir/inspector.go index 6233a088..5cf4bcd2 100644 --- a/ir/inspector.go +++ b/ir/inspector.go @@ -40,6 +40,16 @@ type Inspector struct { ignoreConfig *IgnoreConfig dataConfig *DataConfig loadRows bool + // extensionSchemas is the set of schema names hosting at least one + // installed extension on this connection, populated once per BuildIR call + // (see populateExtensionSchemas). Used alongside routineSchema in + // stripSameSchemaPrefix, and alongside targetSchema in buildPrivileges, so + // a function/procedure parameter or privilege object_name owned by an + // extension (e.g. pgvector's "vector") is recognized as same-schema even + // when the routine itself is currently being introspected from a + // different schema than the extension lives in - notably, pgschema's own + // temporary comparison schema during plan generation (issue #518). + extensionSchemas map[string]bool } // NewInspector creates a new schema inspector with optional ignore configuration @@ -78,6 +88,10 @@ func (i *Inspector) BuildIR(ctx context.Context, targetSchema string) (*IR, erro return nil, fmt.Errorf("failed to build schemas: %w", err) } + if err := i.populateExtensionSchemas(ctx); err != nil { + return nil, fmt.Errorf("failed to query extension schemas: %w", err) + } + if err := i.buildTables(ctx, schema, targetSchema); err != nil { return nil, fmt.Errorf("failed to build tables: %w", err) } @@ -1355,30 +1369,85 @@ func (i *Inspector) parseParametersFromSignature(signature string, routineSchema // This ensures consistent comparison between database inspection (which may return qualified // names) and source SQL (which typically uses unqualified names for same-schema types). func (i *Inspector) stripSameSchemaPrefix(typeName, routineSchema string) string { - if typeName == "" || routineSchema == "" { + if typeName == "" { return typeName } + if routineSchema != "" { + if stripped, ok := stripSchemaPrefixIfMatches(typeName, routineSchema); ok { + return stripped + } + } + + // Also strip a prefix matching any schema known to host an installed + // extension (see populateExtensionSchemas). An extension-owned type's + // schema is a property of the extension, not of the function using it - + // pgschema's temporary comparison schema during plan generation + // introspects functions as if their routineSchema were that temp schema, + // which would otherwise leave a genuinely same-extension type qualified + // while the live database renders it bare (issue #518). + for extSchema := range i.extensionSchemas { + if stripped, ok := stripSchemaPrefixIfMatches(typeName, extSchema); ok { + return stripped + } + } + + // No matching prefix - return as-is (could be cross-schema type or already unqualified) + return typeName +} + +// stripSchemaPrefixIfMatches strips a "schema." or "\"schema\"." prefix from +// typeName if it matches schemaName, reporting whether a strip happened. +func stripSchemaPrefixIfMatches(typeName, schemaName string) (string, bool) { // Remove quotes from schema name for comparison - unquotedSchema := routineSchema - if strings.HasPrefix(routineSchema, `"`) && strings.HasSuffix(routineSchema, `"`) { - unquotedSchema = routineSchema[1 : len(routineSchema)-1] + unquotedSchema := schemaName + if strings.HasPrefix(schemaName, `"`) && strings.HasSuffix(schemaName, `"`) { + unquotedSchema = schemaName[1 : len(schemaName)-1] } // Handle quoted schema prefix: "schema".typename quotedPrefix := fmt.Sprintf(`"%s".`, unquotedSchema) if strings.HasPrefix(typeName, quotedPrefix) { - return typeName[len(quotedPrefix):] + return typeName[len(quotedPrefix):], true } // Handle unquoted schema prefix: schema.typename unquotedPrefix := unquotedSchema + "." if strings.HasPrefix(typeName, unquotedPrefix) { - return typeName[len(unquotedPrefix):] + return typeName[len(unquotedPrefix):], true } - // No matching prefix - return as-is (could be cross-schema type or already unqualified) - return typeName + return typeName, false +} + +// populateExtensionSchemas records which schemas host at least one installed +// extension on this connection, for use by stripSameSchemaPrefix and +// buildPrivileges. Extensions are database-wide, so this runs once per +// BuildIR call rather than per schema/target. +func (i *Inspector) populateExtensionSchemas(ctx context.Context) error { + rows, err := i.db.QueryContext(ctx, + `SELECT DISTINCT n.nspname + FROM pg_catalog.pg_extension e + JOIN pg_catalog.pg_namespace n ON n.oid = e.extnamespace`) + if err != nil { + return err + } + defer rows.Close() + + extensionSchemas := make(map[string]bool) + for rows.Next() { + var schemaName string + if err := rows.Scan(&schemaName); err != nil { + return err + } + extensionSchemas[schemaName] = true + } + if err := rows.Err(); err != nil { + return err + } + + i.extensionSchemas = extensionSchemas + return nil } // stripSameSchemaPrefixFromList is stripSameSchemaPrefix for a comma-separated @@ -2294,6 +2363,21 @@ func (i *Inspector) buildPrivileges(ctx context.Context, schema *IR, targetSchem objectName := row.ObjectName.String objectType := row.ObjectType.String privilegeType := row.PrivilegeType.String + + // FUNCTION/PROCEDURE object_name is rendered by pg_get_function_identity_arguments + // directly in SQL (see GetPrivilegesForSchema), so it never goes through + // stripSameSchemaPrefix the way parseParametersFromSignature does. Strip it + // here too (routine's own schema, then any extension schema), so a privilege + // on the same function keys identically regardless of which schema it's + // currently being introspected from (issue #518 - see stripSameSchemaPrefix + // for the temp-schema/extension-schema rationale). Reuses the same + // StripSchemaQualifiers tokenizer already used for aggregate signatures. + if objectType == "FUNCTION" || objectType == "PROCEDURE" { + objectName = StripSchemaQualifiers(objectName, targetSchema) + for extSchema := range i.extensionSchemas { + objectName = StripSchemaQualifiers(objectName, extSchema) + } + } owner := row.Owner.String isGrantable := row.IsGrantable.Valid && row.IsGrantable.Bool diff --git a/ir/inspector_extension_schema_test.go b/ir/inspector_extension_schema_test.go new file mode 100644 index 00000000..5e5a4f02 --- /dev/null +++ b/ir/inspector_extension_schema_test.go @@ -0,0 +1,72 @@ +package ir + +import "testing" + +// stripSameSchemaPrefix must strip a prefix matching either the routine's own +// schema or any schema known to host an installed extension (issue #518): +// when introspecting pgschema's own temporary comparison schema, a function's +// routineSchema *is* that temp schema, so an extension-owned type's real +// schema qualifier (e.g. "domain.vector") never matches routineSchema and +// would otherwise survive unstripped - causing the temp-schema side and the +// real-target side of a diff to compare as different function signatures. +func TestStripSameSchemaPrefix_ExtensionSchemaAware(t *testing.T) { + tests := []struct { + name string + typeName string + routineSchema string + extensionSchemas map[string]bool + want string + }{ + { + name: "strips routine's own schema, no extension schemas known", + typeName: "domain.mytype", + routineSchema: "domain", + extensionSchemas: nil, + want: "mytype", + }, + { + name: "routine schema is the temp schema, but type is protected by an extension schema", + typeName: "domain.vector", + routineSchema: "pgschema_tmp_20260101_000000_abcd1234", + extensionSchemas: map[string]bool{"domain": true}, + want: "vector", + }, + { + name: "quoted extension schema qualifier", + typeName: `"domain".vector`, + routineSchema: "pgschema_tmp_xxx", + extensionSchemas: map[string]bool{"domain": true}, + want: "vector", + }, + { + name: "cross-schema type unaffected - schema matches neither routine nor any extension", + typeName: "utils.hstore", + routineSchema: "pgschema_tmp_xxx", + extensionSchemas: map[string]bool{"domain": true}, + want: "utils.hstore", + }, + { + name: "already-bare type unaffected", + typeName: "vector", + routineSchema: "pgschema_tmp_xxx", + extensionSchemas: map[string]bool{"domain": true}, + want: "vector", + }, + { + name: "empty type name", + typeName: "", + routineSchema: "domain", + extensionSchemas: map[string]bool{"domain": true}, + want: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + insp := &Inspector{extensionSchemas: tt.extensionSchemas} + if got := insp.stripSameSchemaPrefix(tt.typeName, tt.routineSchema); got != tt.want { + t.Errorf("stripSameSchemaPrefix(%q, %q) = %q, want %q", tt.typeName, tt.routineSchema, got, tt.want) + } + }) + } +} From fe2e540a739f9db81f9aca156be8fcf08c50cc3b Mon Sep 17 00:00:00 2001 From: ayusssmaan Date: Fri, 18 Sep 2026 12:09:33 +0530 Subject: [PATCH 2/5] fix: narrow extension-schema handling to address review feedback Addresses Greptile and Copilot review findings on the original commit: - external.go: only add the managed schema itself to the temp schema's search_path (and only if it hosts an installed extension), instead of every extension schema in the database. Adding unrelated extension schemas let bare cross-schema types resolve during planning that the real apply session (search_path ", public") could never resolve, and could explicitly relocate pg_catalog out of its default implicit-first search position when a bundled extension (e.g. plpgsql) lives there. - inspector.go: stripSameSchemaPrefix and buildPrivileges now only strip an extension schema qualifier when routineSchema/targetSchema is pgschema's own temp comparison schema (pgschema_tmp_*) - never for a routine's real, permanent schema, where an extension-owned type from a different schema is a genuine cross-schema reference that must stay qualified in generated DDL. - inspector.go: added extensionOwnedTypes, populated via pg_depend (deptype='e'), so stripping only applies to catalog-verified extension member types, not to every type that happens to live in a schema an extension also occupies (a schema can host both). - inspector.go: function return types (direct, SETOF, and TABLE(...)) now get the same extension-aware stripping as parameters, via a new stripSameSchemaPrefixFromReturnType - previously only parameters were covered, so a function returning an extension-owned type directly would still spuriously diff. Extended unit tests to cover all of the above, including the specific schema-hosts-extension-but-type-isn't-a-member and genuine-cross-schema-must-stay-qualified regressions the reviews caught. go build/vet/test ./... all pass. Re-verified end-to-end against the real ddms model files: plan/apply/re-plan still comes back clean after these changes. --- internal/postgres/external.go | 55 ++++--- ir/inspector.go | 204 +++++++++++++++++++++---- ir/inspector_extension_schema_test.go | 211 +++++++++++++++++++++----- 3 files changed, 375 insertions(+), 95 deletions(-) diff --git a/internal/postgres/external.go b/internal/postgres/external.go index 7b17d382..c6e06e7a 100644 --- a/internal/postgres/external.go +++ b/internal/postgres/external.go @@ -144,37 +144,42 @@ func (ed *ExternalDatabase) ApplySchema(ctx context.Context, schema string, sql // Set search_path to the temporary schema, with public as fallback // for resolving extension types installed in public schema (issue #197). // - // Also append every other schema that hosts an installed extension (e.g. - // pgvector's "vector" living in a non-public schema like "domain"). - // Desired-state SQL for an extension-owned column is written unqualified, - // same as any other same-schema reference (e.g. "embedding vector(384)"), - // and stripSchemaQualifications only strips - it never adds - a schema - // prefix. Without the extension's real schema in this search_path, such a - // bare reference cannot resolve inside the temp schema at all, since the - // temp schema has no copy of the type and "public" may not either (issue - // #518's apply-time failure mode, not addressed by #544's pre-flight - // schema-consistency check alone). This is safe: NewExternalDatabase - // already validated that every extension shared between the plan and - // target databases lives in the same schema on both, so resolving a bare - // extension reference against the plan database's copy here is exactly - // equivalent to how it resolves on the real target. + // Also append the managed schema itself, if (and only if) it hosts an + // installed extension (e.g. pgvector's "vector" living in a non-public + // schema like "domain"). Desired-state SQL for an extension-owned column + // is written unqualified, same as any other same-schema reference (e.g. + // "embedding vector(384)"), and stripSchemaQualifications only strips - + // it never adds - a schema prefix. Without the extension's real schema in + // this search_path, such a bare reference cannot resolve inside the temp + // schema at all, since the temp schema has no copy of the type and + // "public" may not either (issue #518's apply-time failure mode, not + // addressed by #544's pre-flight schema-consistency check alone). This is + // safe: NewExternalDatabase already validated that every extension shared + // between the plan and target databases lives in the same schema on + // both, so resolving a bare extension reference against the plan + // database's copy here is exactly equivalent to how it resolves on the + // real target. + // + // Deliberately NOT every extension schema in the database: the real + // apply session against the target only ever uses ", public" + // (see cmd/apply/apply.go), so adding an unrelated extension's schema + // here would let a cross-schema type resolve during planning that the + // real apply could never resolve, and would explicitly relocate + // pg_catalog out of its default implicit-first search position for the + // common case of a bundled extension (e.g. plpgsql) living there (PR + // #608 review feedback). extraSchemas, err := getExtensionSchemas(ed.db) if err != nil { return fmt.Errorf("failed to query extension schemas: %w", err) } searchPathParts := []string{quoteIdent(ed.tempSchema), "public"} - seen := map[string]bool{ed.tempSchema: true, "public": true} - extraSchemaNames := make([]string, 0, len(extraSchemas)) - for _, extSchema := range extraSchemas { - extraSchemaNames = append(extraSchemaNames, extSchema) - } - sort.Strings(extraSchemaNames) - for _, extSchema := range extraSchemaNames { - if seen[extSchema] { - continue + if schema != "public" { + for _, extSchema := range extraSchemas { + if extSchema == schema { + searchPathParts = append(searchPathParts, quoteIdent(schema)) + break + } } - seen[extSchema] = true - searchPathParts = append(searchPathParts, quoteIdent(extSchema)) } setSearchPathSQL := fmt.Sprintf("SET search_path TO %s", strings.Join(searchPathParts, ", ")) if _, err := util.ExecContextWithLogging(ctx, conn, setSearchPathSQL, "set search_path for desired state"); err != nil { diff --git a/ir/inspector.go b/ir/inspector.go index 5cf4bcd2..d9cee53c 100644 --- a/ir/inspector.go +++ b/ir/inspector.go @@ -5,6 +5,7 @@ import ( "database/sql" "encoding/json" "fmt" + "regexp" "sort" "strconv" "strings" @@ -42,14 +43,20 @@ type Inspector struct { loadRows bool // extensionSchemas is the set of schema names hosting at least one // installed extension on this connection, populated once per BuildIR call - // (see populateExtensionSchemas). Used alongside routineSchema in - // stripSameSchemaPrefix, and alongside targetSchema in buildPrivileges, so - // a function/procedure parameter or privilege object_name owned by an - // extension (e.g. pgvector's "vector") is recognized as same-schema even - // when the routine itself is currently being introspected from a - // different schema than the extension lives in - notably, pgschema's own - // temporary comparison schema during plan generation (issue #518). + // (see populateExtensionSchemas). Used only as a candidate list of + // schemas worth checking against extensionOwnedTypes below; membership in + // this set alone is never sufficient to strip a qualifier, since a schema + // can host both extension-owned and ordinary user-defined objects. extensionSchemas map[string]bool + // extensionOwnedTypes is the set of "schema.typename" pairs that are + // confirmed, catalog-verified members of an installed extension (via + // pg_depend, deptype='e'; see populateExtensionSchemas). Consulted by + // stripSameSchemaPrefix and buildPrivileges, but only when the schema + // currently being introspected is pgschema's own temporary comparison + // schema (pgschema_tmp_*) - never for a routine's real, permanent + // schema, where an extension-owned type from a different schema is a + // genuine cross-schema reference that must stay qualified (issue #518). + extensionOwnedTypes map[string]bool } // NewInspector creates a new schema inspector with optional ignore configuration @@ -1162,7 +1169,7 @@ func (i *Inspector) buildFunctions(ctx context.Context, schema *IR, targetSchema Schema: schemaName, Name: functionName, Definition: definition, - ReturnType: i.safeInterfaceToString(fn.DataType), + ReturnType: i.stripSameSchemaPrefixFromReturnType(i.safeInterfaceToString(fn.DataType), schemaName), Language: i.safeInterfaceToString(fn.ExternalLanguage), Comment: comment, Parameters: parameters, @@ -1379,16 +1386,42 @@ func (i *Inspector) stripSameSchemaPrefix(typeName, routineSchema string) string } } - // Also strip a prefix matching any schema known to host an installed - // extension (see populateExtensionSchemas). An extension-owned type's - // schema is a property of the extension, not of the function using it - - // pgschema's temporary comparison schema during plan generation - // introspects functions as if their routineSchema were that temp schema, - // which would otherwise leave a genuinely same-extension type qualified - // while the live database renders it bare (issue #518). - for extSchema := range i.extensionSchemas { - if stripped, ok := stripSchemaPrefixIfMatches(typeName, extSchema); ok { - return stripped + // Also strip a prefix matching a schema known to host an installed + // extension, but ONLY when routineSchema is pgschema's own temporary + // comparison schema (pgschema_tmp_*, see GenerateTempSchemaName) AND the + // specific type is a catalog-verified extension member (see + // populateExtensionSchemas) - never for a routine's genuine, permanent + // home schema, and never for an ordinary type that merely happens to + // live in a schema an extension also occupies. + // + // The temp-schema gate matters because both the embedded and external + // plan-DB providers apply desired-state SQL into exactly one such temp + // schema per plan/apply (see cmd/plan/plan.go), so a function's + // routineSchema during that introspection is always the temp schema name + // rather than its real target schema, which would otherwise leave a + // genuinely same-extension type qualified there while the live database + // renders it bare (issue #518). Without this gate, a genuinely + // cross-schema type reference on a real routine (e.g. a function in + // "app" taking a "domain.vector" parameter) would also get stripped, + // even though it must stay qualified since neither generated DDL nor the + // real target's search_path would otherwise be able to resolve it. + // + // The extension-membership check matters because a schema can host both + // extension-owned and ordinary user-defined objects (e.g. an "exts" + // schema with both pgvector's "vector" and an unrelated user-defined + // "exts.status" type) - only the former is safe to treat as + // automatically resolvable without qualification (PR #608 review + // feedback). + if strings.HasPrefix(routineSchema, "pgschema_tmp_") { + for extSchema := range i.extensionSchemas { + stripped, ok := stripSchemaPrefixIfMatches(typeName, extSchema) + if !ok { + continue + } + baseType := strings.TrimSuffix(stripped, "[]") + if i.extensionOwnedTypes[extSchema+"."+baseType] { + return stripped + } } } @@ -1396,6 +1429,53 @@ func (i *Inspector) stripSameSchemaPrefix(typeName, routineSchema string) string return typeName } +// stripSameSchemaPrefixFromReturnType applies stripSameSchemaPrefix to a +// function return type, decomposing SETOF and TABLE(...) forms the same way +// stripSchemaFromReturnType (ir/normalize.go) does, so an extension-owned +// type appearing directly, under SETOF, or as a TABLE(...) column type is +// normalized consistently with parameter types. Without this, a function +// declared as e.g. "RETURNS vector" would still compare as +// "RETURNS domain.vector" on the temp-schema side vs "RETURNS vector" on the +// real target side, causing the same spurious drop+recreate that motivated +// stripSameSchemaPrefix itself (PR #608 review feedback: return types were +// previously only normalized against the routine's own schema, later in +// normalizeFunction, which has no extension-schema knowledge). +func (i *Inspector) stripSameSchemaPrefixFromReturnType(returnType, routineSchema string) string { + if returnType == "" { + return returnType + } + + if len(returnType) > 6 && strings.EqualFold(returnType[:6], "SETOF ") { + rest := strings.TrimSpace(returnType[6:]) + stripped := i.stripSameSchemaPrefix(rest, routineSchema) + if stripped != rest { + return returnType[:6] + stripped + } + return returnType + } + + if strings.HasPrefix(returnType, "TABLE(") && strings.HasSuffix(returnType, ")") { + inner := returnType[6 : len(returnType)-1] + parts := splitTableColumns(inner) + var newParts []string + for _, part := range parts { + part = strings.TrimSpace(part) + if part == "" { + continue + } + name, typePart := splitColumnNameAndType(part) + if typePart != "" { + newParts = append(newParts, name+" "+i.stripSameSchemaPrefix(typePart, routineSchema)) + } else { + newParts = append(newParts, part) + } + } + return "TABLE(" + strings.Join(newParts, ", ") + ")" + } + + return i.stripSameSchemaPrefix(returnType, routineSchema) +} + // stripSchemaPrefixIfMatches strips a "schema." or "\"schema\"." prefix from // typeName if it matches schemaName, reporting whether a strip happened. func stripSchemaPrefixIfMatches(typeName, schemaName string) (string, bool) { @@ -1421,32 +1501,62 @@ func stripSchemaPrefixIfMatches(typeName, schemaName string) (string, bool) { } // populateExtensionSchemas records which schemas host at least one installed -// extension on this connection, for use by stripSameSchemaPrefix and -// buildPrivileges. Extensions are database-wide, so this runs once per -// BuildIR call rather than per schema/target. +// extension on this connection, and which specific types are catalog-verified +// members of an extension (e.g. pgvector's "vector"), for use by +// stripSameSchemaPrefix and buildPrivileges. Extensions are database-wide, so +// this runs once per BuildIR call rather than per schema/target. func (i *Inspector) populateExtensionSchemas(ctx context.Context) error { - rows, err := i.db.QueryContext(ctx, + schemaRows, err := i.db.QueryContext(ctx, `SELECT DISTINCT n.nspname FROM pg_catalog.pg_extension e JOIN pg_catalog.pg_namespace n ON n.oid = e.extnamespace`) if err != nil { return err } - defer rows.Close() + defer schemaRows.Close() extensionSchemas := make(map[string]bool) - for rows.Next() { + for schemaRows.Next() { var schemaName string - if err := rows.Scan(&schemaName); err != nil { + if err := schemaRows.Scan(&schemaName); err != nil { return err } extensionSchemas[schemaName] = true } - if err := rows.Err(); err != nil { + if err := schemaRows.Err(); err != nil { + return err + } + + // pg_depend with deptype='e' identifies objects that are genuine members + // of an extension (as opposed to merely living in a schema an extension + // happens to also occupy - a schema can hold both). + typeRows, err := i.db.QueryContext(ctx, + `SELECT n.nspname, t.typname + FROM pg_catalog.pg_depend d + JOIN pg_catalog.pg_type t ON t.oid = d.objid + JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace + WHERE d.deptype = 'e' + AND d.classid = 'pg_type'::regclass + AND d.refclassid = 'pg_extension'::regclass`) + if err != nil { + return err + } + defer typeRows.Close() + + extensionOwnedTypes := make(map[string]bool) + for typeRows.Next() { + var schemaName, typeName string + if err := typeRows.Scan(&schemaName, &typeName); err != nil { + return err + } + extensionOwnedTypes[schemaName+"."+typeName] = true + } + if err := typeRows.Err(); err != nil { return err } i.extensionSchemas = extensionSchemas + i.extensionOwnedTypes = extensionOwnedTypes return nil } @@ -1460,6 +1570,34 @@ func (i *Inspector) stripSameSchemaPrefixFromList(list, schema string) string { return StripSchemaQualifiers(list, schema) } +// stripExtensionMemberTypeQualifiers drops a "." qualifier from s +// only where the schema is a known extension schema AND the identifier +// immediately following it is a catalog-verified extension member (see +// populateExtensionSchemas / extensionOwnedTypes) - not merely because the +// schema hosts some extension. A schema can hold both extension-owned and +// ordinary user-defined objects (e.g. an "exts" schema with both pgvector's +// "vector" and an unrelated user-defined "exts.status" type); only the +// former is safe to treat as automatically resolvable without qualification +// (PR #608 review feedback). Callers gate this on the schema currently being +// introspected being pgschema's own temp comparison schema, same as +// stripSameSchemaPrefix. +func (i *Inspector) stripExtensionMemberTypeQualifiers(s string) string { + if s == "" || len(i.extensionOwnedTypes) == 0 { + return s + } + for extSchema := range i.extensionSchemas { + re := regexp.MustCompile(`\b` + regexp.QuoteMeta(extSchema) + `\.([A-Za-z_][A-Za-z0-9_$]*)`) + s = re.ReplaceAllStringFunc(s, func(match string) string { + ident := match[len(extSchema)+1:] + if i.extensionOwnedTypes[extSchema+"."+ident] { + return ident + } + return match + }) + } + return s +} + // oidToTypeName maps PostgreSQL type OIDs to standard SQL type names. // Reference: https://github.com/postgres/postgres/blob/master/src/include/catalog/pg_type.dat var oidToTypeName = map[int64]string{ @@ -2367,15 +2505,15 @@ func (i *Inspector) buildPrivileges(ctx context.Context, schema *IR, targetSchem // FUNCTION/PROCEDURE object_name is rendered by pg_get_function_identity_arguments // directly in SQL (see GetPrivilegesForSchema), so it never goes through // stripSameSchemaPrefix the way parseParametersFromSignature does. Strip it - // here too (routine's own schema, then any extension schema), so a privilege - // on the same function keys identically regardless of which schema it's - // currently being introspected from (issue #518 - see stripSameSchemaPrefix - // for the temp-schema/extension-schema rationale). Reuses the same - // StripSchemaQualifiers tokenizer already used for aggregate signatures. + // here too (routine's own schema always; any extension-member type only when + // targetSchema is pgschema's own temp comparison schema, matching + // stripSameSchemaPrefix's gating and its extension-membership check - see + // its comment for why both are required, or a genuinely cross-schema or + // non-member privilege signature would lose its qualifier). if objectType == "FUNCTION" || objectType == "PROCEDURE" { objectName = StripSchemaQualifiers(objectName, targetSchema) - for extSchema := range i.extensionSchemas { - objectName = StripSchemaQualifiers(objectName, extSchema) + if strings.HasPrefix(targetSchema, "pgschema_tmp_") { + objectName = i.stripExtensionMemberTypeQualifiers(objectName) } } owner := row.Owner.String diff --git a/ir/inspector_extension_schema_test.go b/ir/inspector_extension_schema_test.go index 5e5a4f02..d3003db0 100644 --- a/ir/inspector_extension_schema_test.go +++ b/ir/inspector_extension_schema_test.go @@ -3,19 +3,33 @@ package ir import "testing" // stripSameSchemaPrefix must strip a prefix matching either the routine's own -// schema or any schema known to host an installed extension (issue #518): -// when introspecting pgschema's own temporary comparison schema, a function's -// routineSchema *is* that temp schema, so an extension-owned type's real -// schema qualifier (e.g. "domain.vector") never matches routineSchema and -// would otherwise survive unstripped - causing the temp-schema side and the -// real-target side of a diff to compare as different function signatures. +// schema, or (only when routineSchema is pgschema's own temp comparison +// schema, pgschema_tmp_*, AND the specific type is a catalog-verified +// extension member) a schema known to host an installed extension (issue +// #518): when introspecting that temp schema, a function's routineSchema *is* +// the temp schema name, so an extension-owned type's real schema qualifier +// (e.g. "domain.vector") never matches routineSchema and would otherwise +// survive unstripped - causing the temp-schema side and the real-target side +// of a diff to compare as different function signatures. +// +// Critically, this must NOT fire for a routine's genuine, permanent schema: +// a function actually declared in "app" taking a "domain.vector" parameter +// is a real cross-schema reference and must stay qualified, or generated +// CREATE/DROP/GRANT DDL would reference an unresolvable bare "vector" (a +// regression caught in PR #608 review - see the temp-schema-prefix guard). +// +// It also must NOT fire for a type that merely lives in a schema an +// extension happens to occupy but isn't itself an extension member (a schema +// can host both) - another regression caught in PR #608 review, addressed by +// checking extensionOwnedTypes rather than extensionSchemas alone. func TestStripSameSchemaPrefix_ExtensionSchemaAware(t *testing.T) { tests := []struct { - name string - typeName string - routineSchema string - extensionSchemas map[string]bool - want string + name string + typeName string + routineSchema string + extensionSchemas map[string]bool + extensionOwnedTypes map[string]bool + want string }{ { name: "strips routine's own schema, no extension schemas known", @@ -25,48 +39,171 @@ func TestStripSameSchemaPrefix_ExtensionSchemaAware(t *testing.T) { want: "mytype", }, { - name: "routine schema is the temp schema, but type is protected by an extension schema", - typeName: "domain.vector", - routineSchema: "pgschema_tmp_20260101_000000_abcd1234", - extensionSchemas: map[string]bool{"domain": true}, - want: "vector", + name: "routine schema is the temp schema, and type is a confirmed extension member", + typeName: "domain.vector", + routineSchema: "pgschema_tmp_20260101_000000_abcd1234", + extensionSchemas: map[string]bool{"domain": true}, + extensionOwnedTypes: map[string]bool{"domain.vector": true}, + want: "vector", }, { - name: "quoted extension schema qualifier", - typeName: `"domain".vector`, - routineSchema: "pgschema_tmp_xxx", - extensionSchemas: map[string]bool{"domain": true}, - want: "vector", + name: "quoted extension schema qualifier", + typeName: `"domain".vector`, + routineSchema: "pgschema_tmp_xxx", + extensionSchemas: map[string]bool{"domain": true}, + extensionOwnedTypes: map[string]bool{"domain.vector": true}, + want: "vector", }, { - name: "cross-schema type unaffected - schema matches neither routine nor any extension", - typeName: "utils.hstore", - routineSchema: "pgschema_tmp_xxx", - extensionSchemas: map[string]bool{"domain": true}, - want: "utils.hstore", + name: "array of a confirmed extension member type", + typeName: "domain.vector[]", + routineSchema: "pgschema_tmp_xxx", + extensionSchemas: map[string]bool{"domain": true}, + extensionOwnedTypes: map[string]bool{"domain.vector": true}, + want: "vector[]", }, { - name: "already-bare type unaffected", - typeName: "vector", - routineSchema: "pgschema_tmp_xxx", - extensionSchemas: map[string]bool{"domain": true}, - want: "vector", + name: "genuine cross-schema reference in a real (non-temp) schema is preserved", + typeName: "domain.vector", + routineSchema: "app", + extensionSchemas: map[string]bool{"domain": true}, + extensionOwnedTypes: map[string]bool{"domain.vector": true}, + want: "domain.vector", }, { - name: "empty type name", - typeName: "", - routineSchema: "domain", - extensionSchemas: map[string]bool{"domain": true}, - want: "", + name: "schema hosts an extension, but this specific type is not a member", + typeName: "exts.status", + routineSchema: "pgschema_tmp_xxx", + extensionSchemas: map[string]bool{"exts": true}, + extensionOwnedTypes: map[string]bool{"exts.vector": true}, + want: "exts.status", + }, + { + name: "cross-schema type unaffected - schema matches neither routine nor any extension", + typeName: "utils.hstore", + routineSchema: "pgschema_tmp_xxx", + extensionSchemas: map[string]bool{"domain": true}, + extensionOwnedTypes: map[string]bool{"domain.vector": true}, + want: "utils.hstore", + }, + { + name: "already-bare type unaffected", + typeName: "vector", + routineSchema: "pgschema_tmp_xxx", + extensionSchemas: map[string]bool{"domain": true}, + extensionOwnedTypes: map[string]bool{"domain.vector": true}, + want: "vector", + }, + { + name: "empty type name", + typeName: "", + routineSchema: "domain", + extensionSchemas: map[string]bool{"domain": true}, + extensionOwnedTypes: map[string]bool{"domain.vector": true}, + want: "", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - insp := &Inspector{extensionSchemas: tt.extensionSchemas} + insp := &Inspector{extensionSchemas: tt.extensionSchemas, extensionOwnedTypes: tt.extensionOwnedTypes} if got := insp.stripSameSchemaPrefix(tt.typeName, tt.routineSchema); got != tt.want { t.Errorf("stripSameSchemaPrefix(%q, %q) = %q, want %q", tt.typeName, tt.routineSchema, got, tt.want) } }) } } + +// stripSameSchemaPrefixFromReturnType must decompose SETOF and TABLE(...) +// return types the same way ir/normalize.go's stripSchemaFromReturnType +// does, applying the extension-membership-aware stripSameSchemaPrefix to +// each contained type rather than a single top-level prefix check. Without +// this, "RETURNS vector" would compare as "domain.vector" (temp side) vs +// "vector" (real target side) and spuriously trigger a drop+recreate (PR +// #608 review feedback). +func TestStripSameSchemaPrefixFromReturnType(t *testing.T) { + insp := &Inspector{ + extensionSchemas: map[string]bool{"domain": true}, + extensionOwnedTypes: map[string]bool{"domain.vector": true}, + } + + tests := []struct { + name string + returnType string + routineSchema string + want string + }{ + { + name: "direct extension-owned return type in temp schema", + returnType: "domain.vector", + routineSchema: "pgschema_tmp_xxx", + want: "vector", + }, + { + name: "SETOF extension-owned return type in temp schema", + returnType: "SETOF domain.vector", + routineSchema: "pgschema_tmp_xxx", + want: "SETOF vector", + }, + { + name: "TABLE(...) column referencing an extension-owned type in temp schema", + returnType: "TABLE(id integer, embedding domain.vector)", + routineSchema: "pgschema_tmp_xxx", + want: "TABLE(id integer, embedding vector)", + }, + { + name: "direct extension-owned return type on a real, permanent schema is preserved", + returnType: "domain.vector", + routineSchema: "app", + want: "domain.vector", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := insp.stripSameSchemaPrefixFromReturnType(tt.returnType, tt.routineSchema); got != tt.want { + t.Errorf("stripSameSchemaPrefixFromReturnType(%q, %q) = %q, want %q", tt.returnType, tt.routineSchema, got, tt.want) + } + }) + } +} + +// stripExtensionMemberTypeQualifiers is buildPrivileges' equivalent of +// stripSameSchemaPrefix's extension-membership check, operating on a whole +// function/procedure identity-arguments string instead of a single type. +func TestStripExtensionMemberTypeQualifiers(t *testing.T) { + insp := &Inspector{ + extensionSchemas: map[string]bool{"domain": true}, + extensionOwnedTypes: map[string]bool{"domain.vector": true}, + } + + tests := []struct { + name string + in string + want string + }{ + { + name: "strips a confirmed extension member type in a signature", + in: "vector_search(query_embedding domain.vector)", + want: "vector_search(query_embedding vector)", + }, + { + name: "preserves a non-member type in the same extension schema", + in: "f(x domain.status)", + want: "f(x domain.status)", + }, + { + name: "leaves an unrelated schema untouched", + in: "g(x utils.hstore)", + want: "g(x utils.hstore)", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := insp.stripExtensionMemberTypeQualifiers(tt.in); got != tt.want { + t.Errorf("stripExtensionMemberTypeQualifiers(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } +} From 73ba2f51c725522975538da656591cfca5b16a3e Mon Sep 17 00:00:00 2001 From: ayusssmaan Date: Sun, 20 Sep 2026 12:24:41 +0530 Subject: [PATCH 3/5] fix: correct search_path ordering and quoted-identifier handling Addresses a fresh round of Copilot review feedback on the previous fix: - external.go: the managed schema was appended after "public" in the temp schema's search_path ("", public, ""), but the real apply session uses ", public" - managed schema first. If public had an object with the same bare name as the managed schema's extension type, the plan side would resolve it against public while apply resolves it against the managed schema. Reordered to "", "", public to match apply's priority. Extracted the construction logic into buildDesiredStateSearchPath, a pure function now directly unit-tested without needing a live DB connection. - ir/inspector.go: extensionOwnedTypes is keyed by the raw, unquoted pg_type.typname, but the membership check in stripSameSchemaPrefix's extension loop compared it against the still-quoted stripped value, so a quoted mixed-case extension member type never matched and stayed qualified on the temp side - the same spurious-recreate class of bug this fix exists to prevent. stripExtensionMemberTypeQualifiers had the same root cause, plus its regex never matched quoted identifiers at all. Both now unquote before the membership check while still returning the properly-quoted identifier as the strip result. Added regression tests for both (quoted mixed-case member types in both helpers, all 4 search_path ordering cases). go build/vet clean, full go test ./... green, no regressions. Re-verified end-to-end against the real ddms model files (both the raw pgschema CLI and the actual deploydb orchestrator pipeline) - still comes back "No changes detected." after these changes. --- internal/postgres/external.go | 42 +++++++++++++++----- internal/postgres/external_test.go | 56 +++++++++++++++++++++++++++ ir/inspector.go | 21 ++++++++-- ir/inspector_extension_schema_test.go | 23 ++++++++++- 4 files changed, 128 insertions(+), 14 deletions(-) create mode 100644 internal/postgres/external_test.go diff --git a/internal/postgres/external.go b/internal/postgres/external.go index c6e06e7a..51d2fca4 100644 --- a/internal/postgres/external.go +++ b/internal/postgres/external.go @@ -168,20 +168,19 @@ func (ed *ExternalDatabase) ApplySchema(ctx context.Context, schema string, sql // pg_catalog out of its default implicit-first search position for the // common case of a bundled extension (e.g. plpgsql) living there (PR // #608 review feedback). + // + // Inserted before "public" (not after): the real apply session's + // search_path is ", public", so the managed schema takes + // priority over public there. Appending it after public here would flip + // that priority for the plan-side lookup, letting a same-named object in + // public shadow the managed schema's extension type on the plan side + // while the real target resolves it the other way around (PR #608 + // review feedback). extraSchemas, err := getExtensionSchemas(ed.db) if err != nil { return fmt.Errorf("failed to query extension schemas: %w", err) } - searchPathParts := []string{quoteIdent(ed.tempSchema), "public"} - if schema != "public" { - for _, extSchema := range extraSchemas { - if extSchema == schema { - searchPathParts = append(searchPathParts, quoteIdent(schema)) - break - } - } - } - setSearchPathSQL := fmt.Sprintf("SET search_path TO %s", strings.Join(searchPathParts, ", ")) + setSearchPathSQL := fmt.Sprintf("SET search_path TO %s", buildDesiredStateSearchPath(ed.tempSchema, schema, extraSchemas)) if _, err := util.ExecContextWithLogging(ctx, conn, setSearchPathSQL, "set search_path for desired state"); err != nil { return fmt.Errorf("failed to set search_path: %w", err) } @@ -346,6 +345,29 @@ func quoteIdent(name string) string { return `"` + strings.ReplaceAll(name, `"`, `""`) + `"` } +// buildDesiredStateSearchPath builds the search_path used when applying +// desired-state SQL to the temporary comparison schema: the temp schema +// first, then the managed schema itself if it hosts an installed extension, +// then public - in that order, to mirror the real apply session's +// ", public" priority (see cmd/apply/apply.go). extensionSchemas is +// the extname -> schema map from getExtensionSchemas. Not every extension +// schema in the database is included, only the managed one, or a same-named +// object in public could shadow the extension type on the plan side while +// resolving the other way on the real target (PR #608 review feedback). +func buildDesiredStateSearchPath(tempSchema, schema string, extensionSchemas map[string]string) string { + parts := []string{quoteIdent(tempSchema)} + if schema != "public" { + for _, extSchema := range extensionSchemas { + if extSchema == schema { + parts = append(parts, quoteIdent(schema)) + break + } + } + } + parts = append(parts, "public") + return strings.Join(parts, ", ") +} + // detectMajorVersion queries the database to determine its PostgreSQL major version func detectMajorVersion(db *sql.DB) (int, error) { ctx := context.Background() diff --git a/internal/postgres/external_test.go b/internal/postgres/external_test.go new file mode 100644 index 00000000..6c9358c1 --- /dev/null +++ b/internal/postgres/external_test.go @@ -0,0 +1,56 @@ +package postgres + +import "testing" + +// buildDesiredStateSearchPath must order the managed schema before "public" +// (not after), so its priority matches the real apply session's +// ", public" search_path (see cmd/apply/apply.go). Putting it after +// public would let a same-named object in public shadow the managed +// schema's extension type during planning while the real target resolves +// it the other way around (PR #608 review feedback). +func TestBuildDesiredStateSearchPath(t *testing.T) { + tests := []struct { + name string + tempSchema string + schema string + extensionSchemas map[string]string + want string + }{ + { + name: "managed schema hosts an extension - inserted before public", + tempSchema: "pgschema_tmp_xxx", + schema: "domain", + extensionSchemas: map[string]string{"vector": "domain"}, + want: `"pgschema_tmp_xxx", "domain", public`, + }, + { + name: "managed schema does not host an extension", + tempSchema: "pgschema_tmp_xxx", + schema: "domain", + extensionSchemas: map[string]string{"vector": "other_schema"}, + want: `"pgschema_tmp_xxx", public`, + }, + { + name: "no extensions installed at all", + tempSchema: "pgschema_tmp_xxx", + schema: "domain", + extensionSchemas: map[string]string{}, + want: `"pgschema_tmp_xxx", public`, + }, + { + name: "managed schema is public - never duplicated", + tempSchema: "pgschema_tmp_xxx", + schema: "public", + extensionSchemas: map[string]string{"vector": "public"}, + want: `"pgschema_tmp_xxx", public`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := buildDesiredStateSearchPath(tt.tempSchema, tt.schema, tt.extensionSchemas); got != tt.want { + t.Errorf("buildDesiredStateSearchPath(%q, %q, %v) = %q, want %q", tt.tempSchema, tt.schema, tt.extensionSchemas, got, tt.want) + } + }) + } +} diff --git a/ir/inspector.go b/ir/inspector.go index d9cee53c..ab8c5da4 100644 --- a/ir/inspector.go +++ b/ir/inspector.go @@ -1418,7 +1418,14 @@ func (i *Inspector) stripSameSchemaPrefix(typeName, routineSchema string) string if !ok { continue } - baseType := strings.TrimSuffix(stripped, "[]") + // extensionOwnedTypes is keyed by the raw, unquoted pg_type.typname + // (see populateExtensionSchemas), so a quoted mixed-case type (e.g. + // stripped == "\"Vector\"") must be unquoted before the membership + // lookup - comparing the quoted form would never match and silently + // leave every quoted extension-member type qualified (PR #608 review + // feedback). stripped itself (still quoted, if it was quoted) is what + // gets returned, so a valid identifier is preserved either way. + baseType := unquoteIdentifier(strings.TrimSuffix(stripped, "[]")) if i.extensionOwnedTypes[extSchema+"."+baseType] { return stripped } @@ -1586,10 +1593,18 @@ func (i *Inspector) stripExtensionMemberTypeQualifiers(s string) string { return s } for extSchema := range i.extensionSchemas { - re := regexp.MustCompile(`\b` + regexp.QuoteMeta(extSchema) + `\.([A-Za-z_][A-Za-z0-9_$]*)`) + // The identifier group matches either a quoted identifier (allowing + // "" escapes) or a bare one, so a quoted mixed-case member type (e.g. + // domain."Vector") is captured too - not just plain lowercase names + // (PR #608 review feedback). + re := regexp.MustCompile(`\b` + regexp.QuoteMeta(extSchema) + `\.("(?:[^"]|"")*"|[A-Za-z_][A-Za-z0-9_$]*)`) s = re.ReplaceAllStringFunc(s, func(match string) string { ident := match[len(extSchema)+1:] - if i.extensionOwnedTypes[extSchema+"."+ident] { + // extensionOwnedTypes is keyed by the raw, unquoted pg_type.typname, + // so the membership check must unquote ident first; the returned + // value keeps ident as originally captured (quoted or not) so a + // valid identifier is preserved either way. + if i.extensionOwnedTypes[extSchema+"."+unquoteIdentifier(ident)] { return ident } return match diff --git a/ir/inspector_extension_schema_test.go b/ir/inspector_extension_schema_test.go index d3003db0..ff2fbc5c 100644 --- a/ir/inspector_extension_schema_test.go +++ b/ir/inspector_extension_schema_test.go @@ -62,6 +62,22 @@ func TestStripSameSchemaPrefix_ExtensionSchemaAware(t *testing.T) { extensionOwnedTypes: map[string]bool{"domain.vector": true}, want: "vector[]", }, + { + name: "quoted mixed-case extension member type - membership check must unquote", + typeName: `domain."Vector"`, + routineSchema: "pgschema_tmp_xxx", + extensionSchemas: map[string]bool{"domain": true}, + extensionOwnedTypes: map[string]bool{"domain.Vector": true}, + want: `"Vector"`, + }, + { + name: "quoted mixed-case array of a confirmed extension member type", + typeName: `domain."Vector"[]`, + routineSchema: "pgschema_tmp_xxx", + extensionSchemas: map[string]bool{"domain": true}, + extensionOwnedTypes: map[string]bool{"domain.Vector": true}, + want: `"Vector"[]`, + }, { name: "genuine cross-schema reference in a real (non-temp) schema is preserved", typeName: "domain.vector", @@ -174,7 +190,7 @@ func TestStripSameSchemaPrefixFromReturnType(t *testing.T) { func TestStripExtensionMemberTypeQualifiers(t *testing.T) { insp := &Inspector{ extensionSchemas: map[string]bool{"domain": true}, - extensionOwnedTypes: map[string]bool{"domain.vector": true}, + extensionOwnedTypes: map[string]bool{"domain.vector": true, "domain.Vector": true}, } tests := []struct { @@ -197,6 +213,11 @@ func TestStripExtensionMemberTypeQualifiers(t *testing.T) { in: "g(x utils.hstore)", want: "g(x utils.hstore)", }, + { + name: "strips a quoted mixed-case confirmed extension member type", + in: `h(x domain."Vector")`, + want: `h(x "Vector")`, + }, } for _, tt := range tests { From 5cfcedbed15c91df29cf8574012ec289c2437d15 Mon Sep 17 00:00:00 2001 From: ayusssmaan Date: Sun, 20 Sep 2026 14:54:23 +0530 Subject: [PATCH 4/5] fix: scope extension-type handling to the managed schema, not any temp-schema introspection Addresses a fresh round of review feedback: - ir/inspector.go (HIGH): gating the extension-schema fallback on routineSchema looking like a temp schema wasn't precise enough. Every function's routineSchema during temp-schema introspection is the same temp-schema literal regardless of which real schema it represents, so a function genuinely declared in a managed "app" schema that takes a parameter from a different schema's extension (e.g. "domain.vector", where pgvector lives in "domain" not "app") would also get stripped - a real cross-schema reference losing its qualifier. Fixed by threading the logical managed schema through the Inspector (new SetManagedSchema/managedSchema field, defaulting to targetSchema when unset) and scoping stripSameSchemaPrefix/buildPrivileges' extension handling to managedSchema specifically, never any other known extension schema and never based on routineSchema's shape. This is simpler than the previous temp-schema-prefix gate and subsumes it: on the real target side routineSchema always equals managedSchema by construction (BuildIR only returns objects within targetSchema), so the primary same-schema check already handles it; the fallback now only ever considers the one schema that's actually correct. cmd/util/connection.go's GetIRFromDatabase gained a managedSchema parameter (empty string defaults to the schema being introspected); only cmd/plan/plan.go's desired-state IR call passes a real value (config.Schema, distinct from the temp schema it actually connects to). - internal/postgres/external.go (MEDIUM): getExtensionSchemas only sees the plan database, and validateExtensionSchemas explicitly permits an extension present on only one side. Without cross-checking against what's confirmed on the target too, an extension installed on the plan database alone would let a bare type reference resolve during planning that the real target could never resolve at apply time - plan succeeds, apply fails. Fixed by storing ExternalDatabaseConfig's existing TargetExtensions on ExternalDatabase and filtering getExtensionSchemas' result through it (new filterConfirmedExtensionSchemas) before building the search path. Extended unit tests for both: a function within a differently-managed schema keeps a genuine cross-schema qualifier through stripSameSchemaPrefix, stripSameSchemaPrefixFromReturnType, and stripExtensionMemberTypeQualifiers alike; filterConfirmedExtensionSchemas covers plan-only, target-only, and both-sides-confirmed cases. go build/vet clean, full go test ./... green. Re-verified end-to-end against the real ddms model files (raw pgschema CLI and the actual deploydb orchestrator pipeline) - still "No changes detected." after these changes. --- cmd/apply/apply.go | 2 +- cmd/dump/dump.go | 2 +- cmd/plan/plan.go | 10 +- cmd/util/connection.go | 11 +- internal/postgres/external.go | 38 +++++- internal/postgres/external_test.go | 54 +++++++++ ir/inspector.go | 166 +++++++++++++++----------- ir/inspector_extension_schema_test.go | 142 ++++++++++++++++------ 8 files changed, 309 insertions(+), 116 deletions(-) diff --git a/cmd/apply/apply.go b/cmd/apply/apply.go index cc04bd2a..936178dc 100644 --- a/cmd/apply/apply.go +++ b/cmd/apply/apply.go @@ -457,7 +457,7 @@ func RunApply(cmd *cobra.Command, args []string) error { func validateSchemaFingerprint(migrationPlan *plan.Plan, host string, port int, db, user, password, sslmode, schema, applicationName string, ignoreConfig *ir.IgnoreConfig) error { // Get current state from target database with ignore config // This ensures ignored objects are excluded from fingerprint calculation - currentStateIR, err := util.GetIRFromDatabase(host, port, db, user, password, sslmode, schema, applicationName, ignoreConfig) + currentStateIR, err := util.GetIRFromDatabase(host, port, db, user, password, sslmode, schema, applicationName, ignoreConfig, "") if err != nil { return fmt.Errorf("failed to get current database state for fingerprint validation: %w", err) } diff --git a/cmd/dump/dump.go b/cmd/dump/dump.go index f497a41d..cadc22bc 100644 --- a/cmd/dump/dump.go +++ b/cmd/dump/dump.go @@ -79,7 +79,7 @@ func ExecuteDump(config *DumpConfig) (string, error) { } // Get IR from database using the shared utility - schemaIR, err := util.GetIRFromDatabase(config.Host, config.Port, config.DB, config.User, config.Password, config.SSLMode, config.Schema, "pgschema", ignoreConfig) + schemaIR, err := util.GetIRFromDatabase(config.Host, config.Port, config.DB, config.User, config.Password, config.SSLMode, config.Schema, "pgschema", ignoreConfig, "") if err != nil { return "", fmt.Errorf("failed to get database schema: %w", err) } diff --git a/cmd/plan/plan.go b/cmd/plan/plan.go index b6fa5606..5e1f8b7a 100644 --- a/cmd/plan/plan.go +++ b/cmd/plan/plan.go @@ -287,7 +287,7 @@ func GeneratePlan(config *PlanConfig, provider postgres.DesiredStateProvider) (* } // Get current state from target database - currentStateIR, err := util.GetIRFromDatabase(config.Host, config.Port, config.DB, config.User, config.Password, config.SSLMode, config.Schema, config.ApplicationName, ignoreConfig) + currentStateIR, err := util.GetIRFromDatabase(config.Host, config.Port, config.DB, config.User, config.Password, config.SSLMode, config.Schema, config.ApplicationName, ignoreConfig, "") if err != nil { return nil, fmt.Errorf("failed to get current state from database: %w", err) } @@ -357,7 +357,13 @@ func GeneratePlan(config *PlanConfig, provider postgres.DesiredStateProvider) (* providerSSLMode = "prefer" } } - desiredStateIR, err := util.GetIRFromDatabase(providerHost, providerPort, providerDB, providerUsername, providerPassword, providerSSLMode, schemaToInspect, config.ApplicationName, ignoreConfig) + // managedSchema is config.Schema (not schemaToInspect): when the provider + // uses a temporary comparison schema, schemaToInspect is that temp + // schema's literal name, but config.Schema is the real schema it stands + // in for - the Inspector needs both to correctly scope extension-owned + // type handling to the real schema, not the temp one (PR #608 review + // feedback; see Inspector.SetManagedSchema). + desiredStateIR, err := util.GetIRFromDatabase(providerHost, providerPort, providerDB, providerUsername, providerPassword, providerSSLMode, schemaToInspect, config.ApplicationName, ignoreConfig, config.Schema) if err != nil { return nil, fmt.Errorf("failed to get desired state: %w", err) } diff --git a/cmd/util/connection.go b/cmd/util/connection.go index 9eba0422..6496fdea 100644 --- a/cmd/util/connection.go +++ b/cmd/util/connection.go @@ -94,8 +94,12 @@ func ValidateSSLMode(mode string) error { } } -// GetIRFromDatabase gets the IR from a database with ignore configuration -func GetIRFromDatabase(host string, port int, db, user, password, sslmode, schemaName, applicationName string, ignoreConfig *ir.IgnoreConfig) (*ir.IR, error) { +// GetIRFromDatabase gets the IR from a database with ignore configuration. +// managedSchema is the logical schema this IR represents when it differs +// from schemaName - the schema actually being connected to and introspected +// (see Inspector.SetManagedSchema). Pass "" when they're the same, which is +// every caller except the desired-state/temp-schema comparison path. +func GetIRFromDatabase(host string, port int, db, user, password, sslmode, schemaName, applicationName string, ignoreConfig *ir.IgnoreConfig, managedSchema string) (*ir.IR, error) { if sslmode == "" { sslmode = "prefer" } @@ -121,6 +125,9 @@ func GetIRFromDatabase(host string, port int, db, user, password, sslmode, schem // Build IR using the IR system with ignore config inspector := ir.NewInspector(conn, ignoreConfig) + if managedSchema != "" { + inspector.SetManagedSchema(managedSchema) + } // Default to public schema if none specified targetSchema := schemaName diff --git a/internal/postgres/external.go b/internal/postgres/external.go index 15c7079c..ee6f253b 100644 --- a/internal/postgres/external.go +++ b/internal/postgres/external.go @@ -24,6 +24,14 @@ type ExternalDatabase struct { tempSchema string // Temporary schema name with timestamp suffix targetMajorVersion int // Expected major version (from target database) stubRoles []string // Roles created for ALTER DEFAULT PRIVILEGES (issue #553) + // targetExtensions is ExternalDatabaseConfig.TargetExtensions, kept for + // ApplySchema: getExtensionSchemas(ed.db) only sees what's installed on + // the plan database, so without cross-checking against what's actually + // on the target, an extension installed only on the plan side (e.g. for + // local testing convenience) would let a bare type reference resolve + // during planning that the real target could never resolve at apply + // time - plan succeeds, apply fails (PR #608 review feedback). + targetExtensions map[string]string } // ExternalDatabaseConfig holds configuration for connecting to an external database @@ -106,6 +114,7 @@ func NewExternalDatabase(config *ExternalDatabaseConfig) (*ExternalDatabase, err password: config.Password, tempSchema: tempSchema, targetMajorVersion: config.TargetMajorVersion, + targetExtensions: config.TargetExtensions, }, nil } @@ -180,7 +189,16 @@ func (ed *ExternalDatabase) ApplySchema(ctx context.Context, schema string, sql if err != nil { return fmt.Errorf("failed to query extension schemas: %w", err) } - setSearchPathSQL := fmt.Sprintf("SET search_path TO %s", buildDesiredStateSearchPath(ed.tempSchema, schema, extraSchemas)) + // getExtensionSchemas only sees the plan database. validateExtensionSchemas + // (in NewExternalDatabase) explicitly permits an extension present on only + // one side, so an extension installed on the plan database alone (e.g. for + // local testing convenience) is not itself an error - but it would be + // wrong to add its schema to search_path here: a bare type reference would + // then resolve during planning that the real target, lacking the + // extension entirely, could never resolve at apply time. Only trust an + // extension confirmed present on both sides (PR #608 review feedback). + confirmedSchemas := filterConfirmedExtensionSchemas(extraSchemas, ed.targetExtensions) + setSearchPathSQL := fmt.Sprintf("SET search_path TO %s", buildDesiredStateSearchPath(ed.tempSchema, schema, confirmedSchemas)) if _, err := util.ExecContextWithLogging(ctx, conn, setSearchPathSQL, "set search_path for desired state"); err != nil { return fmt.Errorf("failed to set search_path: %w", err) } @@ -374,6 +392,24 @@ func buildDesiredStateSearchPath(tempSchema, schema string, extensionSchemas map return strings.Join(parts, ", ") } +// filterConfirmedExtensionSchemas keeps only the entries of planSchemas +// (extname -> schema, from getExtensionSchemas on the plan database) whose +// extension name also appears in targetExtensions (extname -> schema, from +// the real target). getExtensionSchemas only sees the plan database, and +// validateExtensionSchemas explicitly permits an extension present on only +// one side - so without this filter, a plan-only extension would let a bare +// type reference resolve during planning that the real target could never +// resolve at apply time (PR #608 review feedback). +func filterConfirmedExtensionSchemas(planSchemas, targetExtensions map[string]string) map[string]string { + confirmed := make(map[string]string, len(planSchemas)) + for extName, extSchema := range planSchemas { + if _, onTarget := targetExtensions[extName]; onTarget { + confirmed[extName] = extSchema + } + } + return confirmed +} + // detectMajorVersion queries the database to determine its PostgreSQL major version func detectMajorVersion(db *sql.DB) (int, error) { ctx := context.Background() diff --git a/internal/postgres/external_test.go b/internal/postgres/external_test.go index 6c9358c1..4936f55f 100644 --- a/internal/postgres/external_test.go +++ b/internal/postgres/external_test.go @@ -54,3 +54,57 @@ func TestBuildDesiredStateSearchPath(t *testing.T) { }) } } + +// filterConfirmedExtensionSchemas must keep only extensions present in both +// maps. getExtensionSchemas only sees the plan database, and +// validateExtensionSchemas explicitly permits an extension present on only +// one side, so without this filter a plan-only extension would let a bare +// type reference resolve during planning that the real target could never +// resolve at apply time (PR #608 review feedback). +func TestFilterConfirmedExtensionSchemas(t *testing.T) { + tests := []struct { + name string + planSchemas map[string]string + targetExtensions map[string]string + want map[string]string + }{ + { + name: "extension present on both sides is kept", + planSchemas: map[string]string{"vector": "domain"}, + targetExtensions: map[string]string{"vector": "domain"}, + want: map[string]string{"vector": "domain"}, + }, + { + name: "plan-only extension is dropped", + planSchemas: map[string]string{"vector": "domain"}, + targetExtensions: map[string]string{}, + want: map[string]string{}, + }, + { + name: "target-only extension is irrelevant and absent from the result", + planSchemas: map[string]string{}, + targetExtensions: map[string]string{"vector": "domain"}, + want: map[string]string{}, + }, + { + name: "mixed - only the confirmed one survives", + planSchemas: map[string]string{"vector": "domain", "hstore": "exts"}, + targetExtensions: map[string]string{"vector": "domain"}, + want: map[string]string{"vector": "domain"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := filterConfirmedExtensionSchemas(tt.planSchemas, tt.targetExtensions) + if len(got) != len(tt.want) { + t.Fatalf("filterConfirmedExtensionSchemas(%v, %v) = %v, want %v", tt.planSchemas, tt.targetExtensions, got, tt.want) + } + for k, v := range tt.want { + if got[k] != v { + t.Errorf("filterConfirmedExtensionSchemas(%v, %v) = %v, want %v", tt.planSchemas, tt.targetExtensions, got, tt.want) + } + } + }) + } +} diff --git a/ir/inspector.go b/ir/inspector.go index 8dd3e523..1f8a338a 100644 --- a/ir/inspector.go +++ b/ir/inspector.go @@ -39,21 +39,37 @@ type Inspector struct { db *sql.DB queries *queries.Queries ignoreConfig *IgnoreConfig + // managedSchema is the logical schema pgschema is planning/applying, + // which can differ from targetSchema (the schema this particular + // BuildIR call actually introspects): when introspecting pgschema's own + // temporary comparison schema, targetSchema is that temp schema's + // literal name, but managedSchema is the real schema it stands in for + // (e.g. "domain"). Set via SetManagedSchema before calling BuildIR; + // defaults to targetSchema if never set, which is correct for every + // caller that introspects the schema it means to manage directly (dump, + // apply's current-state read, etc.) - only the desired-state/temp-schema + // path needs to set it explicitly. + managedSchema string // extensionSchemas is the set of schema names hosting at least one // installed extension on this connection, populated once per BuildIR call - // (see populateExtensionSchemas). Used only as a candidate list of - // schemas worth checking against extensionOwnedTypes below; membership in - // this set alone is never sufficient to strip a qualifier, since a schema - // can host both extension-owned and ordinary user-defined objects. + // (see populateExtensionSchemas). Only ever consulted for managedSchema + // itself, never any other schema - see stripSameSchemaPrefix. extensionSchemas map[string]bool // extensionOwnedTypes is the set of "schema.typename" pairs that are // confirmed, catalog-verified members of an installed extension (via // pg_depend, deptype='e'; see populateExtensionSchemas). Consulted by - // stripSameSchemaPrefix and buildPrivileges, but only when the schema - // currently being introspected is pgschema's own temporary comparison - // schema (pgschema_tmp_*) - never for a routine's real, permanent - // schema, where an extension-owned type from a different schema is a - // genuine cross-schema reference that must stay qualified (issue #518). + // stripSameSchemaPrefix and buildPrivileges, but only for a type + // qualified with managedSchema specifically - never an arbitrary schema + // that happens to host some extension elsewhere in the database, and + // never for a routine's real, permanent schema when it differs from + // managedSchema, where an extension-owned type from a different schema + // is a genuine cross-schema reference that must stay qualified (issue + // #518, PR #608 review feedback: this must be scoped to managedSchema, + // not "any known extension schema" or "any temp-schema introspection", + // or a genuinely cross-schema reference to another schema's extension + // type - e.g. a function in a managed "app" schema taking a + // "domain.vector" parameter - would also get incorrectly stripped while + // introspecting the temp schema). extensionOwnedTypes map[string]bool } @@ -66,8 +82,19 @@ func NewInspector(db *sql.DB, ignoreConfig *IgnoreConfig) *Inspector { } } +// SetManagedSchema records the logical schema this Inspector's BuildIR call +// represents, when it differs from the schema literally being introspected +// (see the managedSchema field comment). Must be called before BuildIR. +func (i *Inspector) SetManagedSchema(schema string) { + i.managedSchema = schema +} + // BuildIR builds the schema IR from the database for a specific schema func (i *Inspector) BuildIR(ctx context.Context, targetSchema string) (*IR, error) { + if i.managedSchema == "" { + i.managedSchema = targetSchema + } + schema := NewIR() // Sequential prerequisites @@ -1304,25 +1331,26 @@ func (i *Inspector) stripSameSchemaPrefix(typeName, routineSchema string) string } } - // Also strip a prefix matching a schema known to host an installed - // extension, but ONLY when routineSchema is pgschema's own temporary - // comparison schema (pgschema_tmp_*, see GenerateTempSchemaName) AND the - // specific type is a catalog-verified extension member (see - // populateExtensionSchemas) - never for a routine's genuine, permanent - // home schema, and never for an ordinary type that merely happens to - // live in a schema an extension also occupies. + // Also strip a prefix matching managedSchema specifically - never any + // other schema, even one that hosts an installed extension - and only + // when the specific type is a catalog-verified extension member (see + // populateExtensionSchemas). // - // The temp-schema gate matters because both the embedded and external - // plan-DB providers apply desired-state SQL into exactly one such temp - // schema per plan/apply (see cmd/plan/plan.go), so a function's - // routineSchema during that introspection is always the temp schema name - // rather than its real target schema, which would otherwise leave a - // genuinely same-extension type qualified there while the live database - // renders it bare (issue #518). Without this gate, a genuinely - // cross-schema type reference on a real routine (e.g. a function in - // "app" taking a "domain.vector" parameter) would also get stripped, - // even though it must stay qualified since neither generated DDL nor the - // real target's search_path would otherwise be able to resolve it. + // Scoping to managedSchema (rather than "any known extension schema", or + // gating on routineSchema looking like a temp schema) matters because + // when introspecting pgschema's own temporary comparison schema, every + // object's routineSchema is that temp schema's literal name regardless + // of which real schema it represents - so a routine genuinely declared + // in some other, unmanaged schema (e.g. "app") that merely takes a + // parameter of managedSchema's extension type (e.g. "domain.vector") + // would also have routineSchema equal to the temp schema string during + // that introspection. Checking only against managedSchema itself (never + // looping every known extension schema, and never keying off + // routineSchema's shape) is what correctly leaves that genuine + // cross-schema reference qualified on both the temp and real-target + // sides, while still stripping a type that's genuinely in managedSchema + // wherever it's introspected from (issue #518, PR #608 review + // feedback). // // The extension-membership check matters because a schema can host both // extension-owned and ordinary user-defined objects (e.g. an "exts" @@ -1330,12 +1358,8 @@ func (i *Inspector) stripSameSchemaPrefix(typeName, routineSchema string) string // "exts.status" type) - only the former is safe to treat as // automatically resolvable without qualification (PR #608 review // feedback). - if strings.HasPrefix(routineSchema, "pgschema_tmp_") { - for extSchema := range i.extensionSchemas { - stripped, ok := stripSchemaPrefixIfMatches(typeName, extSchema) - if !ok { - continue - } + if i.managedSchema != "" && i.extensionSchemas[i.managedSchema] { + if stripped, ok := stripSchemaPrefixIfMatches(typeName, i.managedSchema); ok { // extensionOwnedTypes is keyed by the raw, unquoted pg_type.typname // (see populateExtensionSchemas), so a quoted mixed-case type (e.g. // stripped == "\"Vector\"") must be unquoted before the membership @@ -1344,7 +1368,7 @@ func (i *Inspector) stripSameSchemaPrefix(typeName, routineSchema string) string // feedback). stripped itself (still quoted, if it was quoted) is what // gets returned, so a valid identifier is preserved either way. baseType := unquoteIdentifier(strings.TrimSuffix(stripped, "[]")) - if i.extensionOwnedTypes[extSchema+"."+baseType] { + if i.extensionOwnedTypes[i.managedSchema+"."+baseType] { return stripped } } @@ -1495,40 +1519,38 @@ func (i *Inspector) stripSameSchemaPrefixFromList(list, schema string) string { return StripSchemaQualifiers(list, schema) } -// stripExtensionMemberTypeQualifiers drops a "." qualifier from s -// only where the schema is a known extension schema AND the identifier -// immediately following it is a catalog-verified extension member (see -// populateExtensionSchemas / extensionOwnedTypes) - not merely because the -// schema hosts some extension. A schema can hold both extension-owned and -// ordinary user-defined objects (e.g. an "exts" schema with both pgvector's -// "vector" and an unrelated user-defined "exts.status" type); only the -// former is safe to treat as automatically resolvable without qualification -// (PR #608 review feedback). Callers gate this on the schema currently being -// introspected being pgschema's own temp comparison schema, same as -// stripSameSchemaPrefix. +// stripExtensionMemberTypeQualifiers drops a "." qualifier +// from s only where the identifier immediately following it is a +// catalog-verified extension member of managedSchema specifically (see +// populateExtensionSchemas / extensionOwnedTypes) - never any other schema, +// even one that hosts some other installed extension, and never an ordinary +// object that merely lives in managedSchema without being an extension +// member. Scoping to managedSchema alone (rather than looping every known +// extension schema) is what keeps a genuinely cross-schema reference to +// another schema's extension type qualified, on both the temp-schema and +// real-target sides (PR #608 review feedback - see stripSameSchemaPrefix's +// comment for the full rationale, which applies identically here). func (i *Inspector) stripExtensionMemberTypeQualifiers(s string) string { - if s == "" || len(i.extensionOwnedTypes) == 0 { + if s == "" || i.managedSchema == "" || !i.extensionSchemas[i.managedSchema] || len(i.extensionOwnedTypes) == 0 { return s } - for extSchema := range i.extensionSchemas { - // The identifier group matches either a quoted identifier (allowing - // "" escapes) or a bare one, so a quoted mixed-case member type (e.g. - // domain."Vector") is captured too - not just plain lowercase names - // (PR #608 review feedback). - re := regexp.MustCompile(`\b` + regexp.QuoteMeta(extSchema) + `\.("(?:[^"]|"")*"|[A-Za-z_][A-Za-z0-9_$]*)`) - s = re.ReplaceAllStringFunc(s, func(match string) string { - ident := match[len(extSchema)+1:] - // extensionOwnedTypes is keyed by the raw, unquoted pg_type.typname, - // so the membership check must unquote ident first; the returned - // value keeps ident as originally captured (quoted or not) so a - // valid identifier is preserved either way. - if i.extensionOwnedTypes[extSchema+"."+unquoteIdentifier(ident)] { - return ident - } - return match - }) - } - return s + extSchema := i.managedSchema + // The identifier group matches either a quoted identifier (allowing "" + // escapes) or a bare one, so a quoted mixed-case member type (e.g. + // domain."Vector") is captured too - not just plain lowercase names + // (PR #608 review feedback). + re := regexp.MustCompile(`\b` + regexp.QuoteMeta(extSchema) + `\.("(?:[^"]|"")*"|[A-Za-z_][A-Za-z0-9_$]*)`) + return re.ReplaceAllStringFunc(s, func(match string) string { + ident := match[len(extSchema)+1:] + // extensionOwnedTypes is keyed by the raw, unquoted pg_type.typname, + // so the membership check must unquote ident first; the returned + // value keeps ident as originally captured (quoted or not) so a + // valid identifier is preserved either way. + if i.extensionOwnedTypes[extSchema+"."+unquoteIdentifier(ident)] { + return ident + } + return match + }) } // oidToTypeName maps PostgreSQL type OIDs to standard SQL type names. @@ -2438,16 +2460,14 @@ func (i *Inspector) buildPrivileges(ctx context.Context, schema *IR, targetSchem // FUNCTION/PROCEDURE object_name is rendered by pg_get_function_identity_arguments // directly in SQL (see GetPrivilegesForSchema), so it never goes through // stripSameSchemaPrefix the way parseParametersFromSignature does. Strip it - // here too (routine's own schema always; any extension-member type only when - // targetSchema is pgschema's own temp comparison schema, matching - // stripSameSchemaPrefix's gating and its extension-membership check - see - // its comment for why both are required, or a genuinely cross-schema or - // non-member privilege signature would lose its qualifier). + // here too: routine's own schema always, plus managedSchema's extension-member + // types specifically (stripExtensionMemberTypeQualifiers is itself scoped to + // managedSchema - see its comment and stripSameSchemaPrefix's for why a + // genuinely cross-schema or non-member privilege signature must keep its + // qualifier). if objectType == "FUNCTION" || objectType == "PROCEDURE" { objectName = StripSchemaQualifiers(objectName, targetSchema) - if strings.HasPrefix(targetSchema, "pgschema_tmp_") { - objectName = i.stripExtensionMemberTypeQualifiers(objectName) - } + objectName = i.stripExtensionMemberTypeQualifiers(objectName) } owner := row.Owner.String isGrantable := row.IsGrantable.Valid && row.IsGrantable.Bool diff --git a/ir/inspector_extension_schema_test.go b/ir/inspector_extension_schema_test.go index ff2fbc5c..eac7bff9 100644 --- a/ir/inspector_extension_schema_test.go +++ b/ir/inspector_extension_schema_test.go @@ -3,45 +3,56 @@ package ir import "testing" // stripSameSchemaPrefix must strip a prefix matching either the routine's own -// schema, or (only when routineSchema is pgschema's own temp comparison -// schema, pgschema_tmp_*, AND the specific type is a catalog-verified -// extension member) a schema known to host an installed extension (issue -// #518): when introspecting that temp schema, a function's routineSchema *is* -// the temp schema name, so an extension-owned type's real schema qualifier -// (e.g. "domain.vector") never matches routineSchema and would otherwise -// survive unstripped - causing the temp-schema side and the real-target side -// of a diff to compare as different function signatures. +// schema, or (only for the schema this Inspector's managedSchema is set to, +// and only for a catalog-verified extension member of it) an +// extension-owned type (issue #518): when introspecting pgschema's own +// temporary comparison schema, every function's routineSchema is that temp +// schema's literal name regardless of which real schema it represents, so an +// extension-owned type's real schema qualifier (e.g. "domain.vector") never +// matches routineSchema and would otherwise survive unstripped - causing the +// temp-schema side and the real-target side of a diff to compare as +// different function signatures. // -// Critically, this must NOT fire for a routine's genuine, permanent schema: -// a function actually declared in "app" taking a "domain.vector" parameter -// is a real cross-schema reference and must stay qualified, or generated -// CREATE/DROP/GRANT DDL would reference an unresolvable bare "vector" (a -// regression caught in PR #608 review - see the temp-schema-prefix guard). +// Scoping the fallback to managedSchema specifically (never "any known +// extension schema") matters because a function that is itself part of the +// schema being managed can still take a parameter whose type genuinely lives +// in a different schema (e.g. a function declared in a managed "app" schema +// taking a "domain.vector" parameter, where pgvector lives in "domain", not +// "app"). Since every function's routineSchema during temp-schema +// introspection is the same temp-schema literal regardless of which real +// schema it represents, a check keyed off "is routineSchema a temp schema" +// alone can't distinguish that genuine cross-schema reference from a +// same-managed-schema one - only checking against managedSchema itself can +// (a regression caught in PR #608 review, HIGH severity: the original +// temp-schema-prefix-only gate stripped this case incorrectly). // -// It also must NOT fire for a type that merely lives in a schema an -// extension happens to occupy but isn't itself an extension member (a schema -// can host both) - another regression caught in PR #608 review, addressed by -// checking extensionOwnedTypes rather than extensionSchemas alone. +// It also must NOT fire for a type that merely lives in managedSchema +// without being an extension member (a schema can host both extension-owned +// and ordinary user-defined objects) - another regression caught in PR #608 +// review, addressed by checking extensionOwnedTypes rather than +// extensionSchemas alone. func TestStripSameSchemaPrefix_ExtensionSchemaAware(t *testing.T) { tests := []struct { name string typeName string routineSchema string + managedSchema string extensionSchemas map[string]bool extensionOwnedTypes map[string]bool want string }{ { - name: "strips routine's own schema, no extension schemas known", - typeName: "domain.mytype", - routineSchema: "domain", - extensionSchemas: nil, - want: "mytype", + name: "strips routine's own schema, no extension schemas known", + typeName: "domain.mytype", + routineSchema: "domain", + managedSchema: "domain", + want: "mytype", }, { - name: "routine schema is the temp schema, and type is a confirmed extension member", + name: "temp-schema introspection, type is a confirmed member of managedSchema", typeName: "domain.vector", routineSchema: "pgschema_tmp_20260101_000000_abcd1234", + managedSchema: "domain", extensionSchemas: map[string]bool{"domain": true}, extensionOwnedTypes: map[string]bool{"domain.vector": true}, want: "vector", @@ -50,6 +61,7 @@ func TestStripSameSchemaPrefix_ExtensionSchemaAware(t *testing.T) { name: "quoted extension schema qualifier", typeName: `"domain".vector`, routineSchema: "pgschema_tmp_xxx", + managedSchema: "domain", extensionSchemas: map[string]bool{"domain": true}, extensionOwnedTypes: map[string]bool{"domain.vector": true}, want: "vector", @@ -58,6 +70,7 @@ func TestStripSameSchemaPrefix_ExtensionSchemaAware(t *testing.T) { name: "array of a confirmed extension member type", typeName: "domain.vector[]", routineSchema: "pgschema_tmp_xxx", + managedSchema: "domain", extensionSchemas: map[string]bool{"domain": true}, extensionOwnedTypes: map[string]bool{"domain.vector": true}, want: "vector[]", @@ -66,6 +79,7 @@ func TestStripSameSchemaPrefix_ExtensionSchemaAware(t *testing.T) { name: "quoted mixed-case extension member type - membership check must unquote", typeName: `domain."Vector"`, routineSchema: "pgschema_tmp_xxx", + managedSchema: "domain", extensionSchemas: map[string]bool{"domain": true}, extensionOwnedTypes: map[string]bool{"domain.Vector": true}, want: `"Vector"`, @@ -74,30 +88,56 @@ func TestStripSameSchemaPrefix_ExtensionSchemaAware(t *testing.T) { name: "quoted mixed-case array of a confirmed extension member type", typeName: `domain."Vector"[]`, routineSchema: "pgschema_tmp_xxx", + managedSchema: "domain", extensionSchemas: map[string]bool{"domain": true}, extensionOwnedTypes: map[string]bool{"domain.Vector": true}, want: `"Vector"[]`, }, { - name: "genuine cross-schema reference in a real (non-temp) schema is preserved", + // A function declared in the managed "app" schema takes a parameter + // whose type genuinely lives in "domain" (a different schema, which + // hosts pgvector) - real-target-side introspection: routineSchema + // equals managedSchema ("app"), which does not itself host any + // extension, so the fallback never even considers "domain". + name: "genuine cross-schema reference within the managed schema's own function - real side", typeName: "domain.vector", routineSchema: "app", + managedSchema: "app", extensionSchemas: map[string]bool{"domain": true}, extensionOwnedTypes: map[string]bool{"domain.vector": true}, want: "domain.vector", }, { - name: "schema hosts an extension, but this specific type is not a member", + // Same scenario, but introspected via the temp schema - HIGH + // severity regression from PR #608 review: a gate based only on + // "is routineSchema a temp schema" would strip this incorrectly, + // since every function's routineSchema is the same temp-schema + // literal here regardless of which real schema it belongs to. + // Checking against managedSchema ("app", which hosts no + // extension) rather than looping every known extension schema is + // what keeps this qualified, matching the real side above. + name: "genuine cross-schema reference within the managed schema's own function - temp side", + typeName: "domain.vector", + routineSchema: "pgschema_tmp_xxx", + managedSchema: "app", + extensionSchemas: map[string]bool{"domain": true}, + extensionOwnedTypes: map[string]bool{"domain.vector": true}, + want: "domain.vector", + }, + { + name: "managed schema hosts an extension, but this specific type is not a member", typeName: "exts.status", routineSchema: "pgschema_tmp_xxx", + managedSchema: "exts", extensionSchemas: map[string]bool{"exts": true}, extensionOwnedTypes: map[string]bool{"exts.vector": true}, want: "exts.status", }, { - name: "cross-schema type unaffected - schema matches neither routine nor any extension", + name: "cross-schema type unaffected - schema matches neither routine nor managed schema", typeName: "utils.hstore", routineSchema: "pgschema_tmp_xxx", + managedSchema: "domain", extensionSchemas: map[string]bool{"domain": true}, extensionOwnedTypes: map[string]bool{"domain.vector": true}, want: "utils.hstore", @@ -106,6 +146,7 @@ func TestStripSameSchemaPrefix_ExtensionSchemaAware(t *testing.T) { name: "already-bare type unaffected", typeName: "vector", routineSchema: "pgschema_tmp_xxx", + managedSchema: "domain", extensionSchemas: map[string]bool{"domain": true}, extensionOwnedTypes: map[string]bool{"domain.vector": true}, want: "vector", @@ -114,6 +155,7 @@ func TestStripSameSchemaPrefix_ExtensionSchemaAware(t *testing.T) { name: "empty type name", typeName: "", routineSchema: "domain", + managedSchema: "domain", extensionSchemas: map[string]bool{"domain": true}, extensionOwnedTypes: map[string]bool{"domain.vector": true}, want: "", @@ -122,7 +164,7 @@ func TestStripSameSchemaPrefix_ExtensionSchemaAware(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - insp := &Inspector{extensionSchemas: tt.extensionSchemas, extensionOwnedTypes: tt.extensionOwnedTypes} + insp := &Inspector{managedSchema: tt.managedSchema, extensionSchemas: tt.extensionSchemas, extensionOwnedTypes: tt.extensionOwnedTypes} if got := insp.stripSameSchemaPrefix(tt.typeName, tt.routineSchema); got != tt.want { t.Errorf("stripSameSchemaPrefix(%q, %q) = %q, want %q", tt.typeName, tt.routineSchema, got, tt.want) } @@ -132,13 +174,14 @@ func TestStripSameSchemaPrefix_ExtensionSchemaAware(t *testing.T) { // stripSameSchemaPrefixFromReturnType must decompose SETOF and TABLE(...) // return types the same way ir/normalize.go's stripSchemaFromReturnType -// does, applying the extension-membership-aware stripSameSchemaPrefix to -// each contained type rather than a single top-level prefix check. Without -// this, "RETURNS vector" would compare as "domain.vector" (temp side) vs -// "vector" (real target side) and spuriously trigger a drop+recreate (PR -// #608 review feedback). +// does, applying the managedSchema-scoped stripSameSchemaPrefix to each +// contained type rather than a single top-level prefix check. Without this, +// "RETURNS vector" would compare as "domain.vector" (temp side) vs "vector" +// (real target side) and spuriously trigger a drop+recreate (PR #608 review +// feedback). func TestStripSameSchemaPrefixFromReturnType(t *testing.T) { insp := &Inspector{ + managedSchema: "domain", extensionSchemas: map[string]bool{"domain": true}, extensionOwnedTypes: map[string]bool{"domain.vector": true}, } @@ -168,10 +211,10 @@ func TestStripSameSchemaPrefixFromReturnType(t *testing.T) { want: "TABLE(id integer, embedding vector)", }, { - name: "direct extension-owned return type on a real, permanent schema is preserved", + name: "direct extension-owned return type on the managed schema's own real routine", returnType: "domain.vector", - routineSchema: "app", - want: "domain.vector", + routineSchema: "domain", + want: "vector", }, } @@ -182,13 +225,28 @@ func TestStripSameSchemaPrefixFromReturnType(t *testing.T) { } }) } + + t.Run("genuine cross-schema return type on a differently-managed schema is preserved", func(t *testing.T) { + appInsp := &Inspector{ + managedSchema: "app", + extensionSchemas: map[string]bool{"domain": true}, + extensionOwnedTypes: map[string]bool{"domain.vector": true}, + } + for _, routineSchema := range []string{"app", "pgschema_tmp_xxx"} { + if got := appInsp.stripSameSchemaPrefixFromReturnType("domain.vector", routineSchema); got != "domain.vector" { + t.Errorf("stripSameSchemaPrefixFromReturnType(%q, %q) = %q, want %q", "domain.vector", routineSchema, got, "domain.vector") + } + } + }) } // stripExtensionMemberTypeQualifiers is buildPrivileges' equivalent of // stripSameSchemaPrefix's extension-membership check, operating on a whole -// function/procedure identity-arguments string instead of a single type. +// function/procedure identity-arguments string instead of a single type, and +// scoped to managedSchema the same way. func TestStripExtensionMemberTypeQualifiers(t *testing.T) { insp := &Inspector{ + managedSchema: "domain", extensionSchemas: map[string]bool{"domain": true}, extensionOwnedTypes: map[string]bool{"domain.vector": true, "domain.Vector": true}, } @@ -227,4 +285,16 @@ func TestStripExtensionMemberTypeQualifiers(t *testing.T) { } }) } + + t.Run("genuine cross-schema privilege signature on a differently-managed schema is preserved", func(t *testing.T) { + appInsp := &Inspector{ + managedSchema: "app", + extensionSchemas: map[string]bool{"domain": true}, + extensionOwnedTypes: map[string]bool{"domain.vector": true}, + } + in := "vector_search(query_embedding domain.vector)" + if got := appInsp.stripExtensionMemberTypeQualifiers(in); got != in { + t.Errorf("stripExtensionMemberTypeQualifiers(%q) = %q, want unchanged %q", in, got, in) + } + }) } From 38496b46e6798f620e7c8a6b49def2572164ee12 Mon Sep 17 00:00:00 2001 From: ayusssmaan Date: Sun, 20 Sep 2026 19:07:03 +0530 Subject: [PATCH 5/5] fix: SQL-aware qualifier stripping, extend to aggregates, document search_path limitation Addresses a fifth round of review feedback: - ir/inspector.go (MEDIUM): stripExtensionMemberTypeQualifiers used a raw regex, which can't distinguish a genuine "." qualifier from the same bytes appearing inside an unrelated quoted identifier (e.g. a parameter literally named "domain.vector" - dots are valid inside quotes), and never matched a quoted schema ("Domain".vector) at all since it only checked for the unquoted schema name as literal text. Replaced with a single-pass, quoted-identifier-aware tokenizer (same scanning rules as the existing StripSchemaQualifiers) with one token of lookahead to check extension membership before stripping, rather than matching raw text. - ir/inspector.go (MEDIUM, "previously missed"): stripSameSchemaPrefixFromList (used for aggregate identity args and signatures via buildAggregates) only applied the basic same-schema strip, never the extension-membership fallback - so an aggregate over an extension-owned type keyed as "vector" on the real side but stayed "domain.vector" on the temp side, spuriously dropped/recreated. Now also applies stripExtensionMemberTypeQualifiers, matching the pattern already used for privileges. (AggregateReturnType already went through stripSameSchemaPrefix directly, which already covers this via the managedSchema fix - only the list-based helper was missing it.) - internal/postgres/external.go ("previously missed"): documented, rather than further "fixed", the fact that adding the managed schema to search_path exposes its whole namespace, not just the confirmed extension member. Considered and rejected two alternatives: rewriting desired-state SQL text to explicitly qualify bare extension-type references (reintroduces the exact type-vs-column-name ambiguity issue #354's design already avoided - a regex/text approach can't reliably tell a type reference from an identically-named column apart, so it risks silently qualifying the wrong token instead of just failing loudly), and rejecting any temp-schema dependency on a non-extension object in the managed schema (breaks the legitimate case this fix targets - ddms's domain schema intentionally co-locates the vector extension with ordinary tables/functions). The practical mitigation is operational: keep the plan database's copy of an extension-hosting schema free of objects absent from the real target. Added tests for the two exact SQL-parsing edge cases (quoted identifier containing dotted text; quoted schema qualifier) and for the aggregate list helper. go build/vet clean, full go test ./... green. Re-verified end-to-end against the real ddms model files (raw pgschema CLI and the actual deploydb orchestrator pipeline) - still "No changes detected." --- internal/postgres/external.go | 24 +++++++ ir/inspector.go | 90 +++++++++++++++++++++------ ir/inspector_extension_schema_test.go | 38 +++++++++++ 3 files changed, 132 insertions(+), 20 deletions(-) diff --git a/internal/postgres/external.go b/internal/postgres/external.go index ee6f253b..9c558a46 100644 --- a/internal/postgres/external.go +++ b/internal/postgres/external.go @@ -197,6 +197,30 @@ func (ed *ExternalDatabase) ApplySchema(ctx context.Context, schema string, sql // then resolve during planning that the real target, lacking the // extension entirely, could never resolve at apply time. Only trust an // extension confirmed present on both sides (PR #608 review feedback). + // + // Known limitation, deliberately not "fixed" further (PR #608 review + // feedback): adding the managed schema to search_path exposes every bare + // reference to that schema's whole namespace, not just the confirmed + // extension member - if the plan database's copy of that schema also had + // some other object the target lacks, a bare reference could resolve on + // the plan side and then fail to apply on the real target. There is no + // narrower alternative that's actually safer: + // - Postgres's search_path has no per-object granularity - it's + // schema-wide or nothing. + // - Rewriting the desired-state SQL text to explicitly qualify bare + // extension-type references instead of expanding search_path would + // reintroduce exactly the ambiguity issue #354's design already + // rejected: text can't reliably distinguish a type reference from an + // identically-named column/parameter, so a rewrite risks silently + // qualifying the wrong token rather than just failing loudly. + // - Rejecting any dependency from the temp schema on a non-extension + // object in the managed schema isn't viable either: this whole + // scenario (ddms's domain schema) legitimately co-locates the + // extension with ordinary user tables/functions in the same schema, + // which is exactly the case #518 needs to keep working. + // The practical mitigation is operational, not code: keep the plan + // database's copy of an extension-hosting schema free of objects that + // don't also exist on the real target. confirmedSchemas := filterConfirmedExtensionSchemas(extraSchemas, ed.targetExtensions) setSearchPathSQL := fmt.Sprintf("SET search_path TO %s", buildDesiredStateSearchPath(ed.tempSchema, schema, confirmedSchemas)) if _, err := util.ExecContextWithLogging(ctx, conn, setSearchPathSQL, "set search_path for desired state"); err != nil { diff --git a/ir/inspector.go b/ir/inspector.go index 1f8a338a..68f89858 100644 --- a/ir/inspector.go +++ b/ir/inspector.go @@ -5,7 +5,6 @@ import ( "database/sql" "encoding/json" "fmt" - "regexp" "sort" "strconv" "strings" @@ -1510,13 +1509,19 @@ func (i *Inspector) populateExtensionSchemas(ctx context.Context) error { } // stripSameSchemaPrefixFromList is stripSameSchemaPrefix for a comma-separated -// argument list such as pg_get_function_identity_arguments output. It strips a +// argument list such as pg_get_function_identity_arguments output (used for +// aggregate identity args/signatures - see buildAggregates). It strips a // schema qualifier only where an identifier token equal to the schema (bare or // quote_ident form) directly precedes a dot, so a schema that needs quoting // ("MySchema".v) normalizes like public.v while quoted names that merely -// contain the schema text ("public.foo") are left intact. +// contain the schema text ("public.foo") are left intact. Also applies +// managedSchema's extension-membership-aware stripping (same as +// stripExtensionMemberTypeQualifiers uses for privileges) - without this, an +// aggregate over an extension-owned type (e.g. "domain.vector") would key as +// "vector" on the real side but stay "domain.vector" on the temp side and be +// spuriously dropped/recreated (PR #608 review feedback). func (i *Inspector) stripSameSchemaPrefixFromList(list, schema string) string { - return StripSchemaQualifiers(list, schema) + return i.stripExtensionMemberTypeQualifiers(StripSchemaQualifiers(list, schema)) } // stripExtensionMemberTypeQualifiers drops a "." qualifier @@ -1535,22 +1540,67 @@ func (i *Inspector) stripExtensionMemberTypeQualifiers(s string) string { return s } extSchema := i.managedSchema - // The identifier group matches either a quoted identifier (allowing "" - // escapes) or a bare one, so a quoted mixed-case member type (e.g. - // domain."Vector") is captured too - not just plain lowercase names - // (PR #608 review feedback). - re := regexp.MustCompile(`\b` + regexp.QuoteMeta(extSchema) + `\.("(?:[^"]|"")*"|[A-Za-z_][A-Za-z0-9_$]*)`) - return re.ReplaceAllStringFunc(s, func(match string) string { - ident := match[len(extSchema)+1:] - // extensionOwnedTypes is keyed by the raw, unquoted pg_type.typname, - // so the membership check must unquote ident first; the returned - // value keeps ident as originally captured (quoted or not) so a - // valid identifier is preserved either way. - if i.extensionOwnedTypes[extSchema+"."+unquoteIdentifier(ident)] { - return ident - } - return match - }) + + // A regex over raw text can't tell a genuine "." qualifier + // from the same bytes appearing inside an unrelated quoted identifier + // (e.g. a parameter literally named "domain.vector"), and can't match a + // quoted schema ("Domain".vector) at all. Use the same single-pass, + // quoted-identifier-aware tokenizer as StripSchemaQualifiers instead, + // with one token of lookahead to check extension membership before + // deciding whether to strip (PR #608 review feedback). + var out strings.Builder + out.Grow(len(s)) + pos := 0 + for pos < len(s) { + token, end := scanIdentToken(s, pos) + if isIdentToken(token) && end < len(s) && s[end] == '.' && unquoteIdentifier(token) == extSchema { + nextTok, nextEnd := scanIdentToken(s, end+1) + if isIdentToken(nextTok) && i.extensionOwnedTypes[extSchema+"."+unquoteIdentifier(nextTok)] { + out.WriteString(nextTok) + pos = nextEnd + continue + } + } + out.WriteString(token) + pos = end + } + return out.String() +} + +// scanIdentToken extracts one token from s starting at pos, using the same +// rules as StripSchemaQualifiers: a quoted identifier (honoring "" escapes), +// a run of identifier characters, or a single other byte. Returns the token +// and the position right after it. +func scanIdentToken(s string, pos int) (token string, end int) { + start := pos + switch { + case s[pos] == '"': + pos++ + for pos < len(s) { + if s[pos] == '"' { + if pos+1 < len(s) && s[pos+1] == '"' { + pos += 2 + continue + } + pos++ + break + } + pos++ + } + case isIdentChar(s[pos]): + for pos < len(s) && isIdentChar(s[pos]) { + pos++ + } + default: + pos++ + } + return s[start:pos], pos +} + +// isIdentToken reports whether token (as returned by scanIdentToken) is an +// identifier - quoted or bare - rather than a single punctuation byte. +func isIdentToken(token string) bool { + return token != "" && (token[0] == '"' || isIdentChar(token[0])) } // oidToTypeName maps PostgreSQL type OIDs to standard SQL type names. diff --git a/ir/inspector_extension_schema_test.go b/ir/inspector_extension_schema_test.go index eac7bff9..820d41d6 100644 --- a/ir/inspector_extension_schema_test.go +++ b/ir/inspector_extension_schema_test.go @@ -276,6 +276,25 @@ func TestStripExtensionMemberTypeQualifiers(t *testing.T) { in: `h(x domain."Vector")`, want: `h(x "Vector")`, }, + { + // A parameter literally NAMED the quoted identifier "domain.vector" + // (a valid, if unusual, Postgres identifier - dots are permitted + // inside quotes). A raw-text regex can't tell this apart from a + // genuine schema.type qualifier since it doesn't track quoting + // context; the tokenizer treats the whole quoted string as one + // atomic token and never looks inside it (PR #608 review feedback). + name: "quoted identifier that merely contains dotted text is left untouched", + in: `f("domain.vector" integer)`, + want: `f("domain.vector" integer)`, + }, + { + // A quoted schema qualifier - the old regex only ever matched the + // bare, unquoted schema name literally, so this never stripped at + // all (PR #608 review feedback). + name: "quoted schema qualifier on a confirmed member type", + in: `k(x "domain".vector)`, + want: `k(x vector)`, + }, } for _, tt := range tests { @@ -298,3 +317,22 @@ func TestStripExtensionMemberTypeQualifiers(t *testing.T) { } }) } + +// stripSameSchemaPrefixFromList (used for aggregate identity args and +// signatures) must also apply managedSchema's extension-membership-aware +// stripping, not just the basic same-schema strip - otherwise an aggregate +// over an extension-owned type keys as "vector" on the real side but stays +// "domain.vector" on the temp side and is spuriously dropped/recreated (PR +// #608 review feedback). +func TestStripSameSchemaPrefixFromList(t *testing.T) { + insp := &Inspector{ + managedSchema: "domain", + extensionSchemas: map[string]bool{"domain": true}, + extensionOwnedTypes: map[string]bool{"domain.vector": true}, + } + in := "domain.vector" + want := "vector" + if got := insp.stripSameSchemaPrefixFromList(in, "pgschema_tmp_xxx"); got != want { + t.Errorf("stripSameSchemaPrefixFromList(%q, %q) = %q, want %q", in, "pgschema_tmp_xxx", got, want) + } +}