Skip to content

fix: plan fails with schema-qualified references in function bodies (#399) #400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tianzhou merged 1 commit into from
Apr 11, 2026
Merged

fix: plan fails with schema-qualified references in function bodies (#399) #400

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions internal/postgres/desired_state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ func stripSchemaQualifications(sql string, schemaName string) string {
// Schema qualifiers inside function/procedure bodies (dollar-quoted blocks)
// must be preserved — the user may need them when search_path doesn't include
// the function's schema (e.g., SET search_path = ''). (Issue #354)
//
// To avoid type-identity mismatches between stripped parameter types and
// unstripped body references (Issue #399), callers should disable function
// body validation with SET check_function_bodies = off before executing
// the resulting SQL.
segments := splitDollarQuotedSegments(sql)
var result strings.Builder
result.Grow(len(sql))
Expand Down
9 changes: 9 additions & 0 deletions internal/postgres/embedded.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,15 @@ func (ep *EmbeddedPostgres) ApplySchema(ctx context.Context, schema string, sql
return fmt.Errorf("failed to set search_path: %w", err)
}

// Disable function body validation to avoid type-identity mismatches (issue #399).
// Schema qualifications inside dollar-quoted function bodies are preserved (issue #354),
// but parameter types are stripped. For SQL-language functions, PostgreSQL validates the
// body at creation time, which can fail when body references use the original schema's
// types while parameters reference the temporary schema's types.
if _, err := util.ExecContextWithLogging(ctx, conn, "SET check_function_bodies = off", "disable function body validation for desired state"); err != nil {
return fmt.Errorf("failed to disable check_function_bodies: %w", err)
}
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SET check_function_bodies = off persists on the session. Because this uses db.Conn() (connection pool) and then conn.Close(), that connection may be reused later with check_function_bodies still disabled. Consider using BEGIN; SET LOCAL check_function_bodies = off; ... COMMIT; around the desired-state SQL, or resetting the GUC (RESET check_function_bodies) in a defer so pooled connections are returned to a clean state even on failures.

Suggested change
}
}
defer func() {
_, _ = conn.ExecContext(context.Background(), "RESET check_function_bodies")
}()

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@tianzhou tianzhou Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the other comment — the EmbeddedPostgres instance owns its own private *sql.DB connected to a temporary embedded PostgreSQL instance that gets destroyed after plan generation. The connection pool is never shared. The existing SET search_path two lines above follows the same pattern with no cleanup, so adding a defer only for this setting would be inconsistent.


// Strip schema qualifications from SQL before applying to temporary schema
// This ensures that objects are created in the temporary schema via search_path
// rather than being explicitly qualified with the original schema name
Expand Down
9 changes: 9 additions & 0 deletions internal/postgres/external.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,15 @@ func (ed *ExternalDatabase) ApplySchema(ctx context.Context, schema string, sql
return fmt.Errorf("failed to set search_path: %w", err)
}

// Disable function body validation to avoid type-identity mismatches (issue #399).
// Schema qualifications inside dollar-quoted function bodies are preserved (issue #354),
// but parameter types are stripped. For SQL-language functions, PostgreSQL validates the
// body at creation time, which can fail when body references use the original schema's
// types while parameters reference the temporary schema's types.
if _, err := util.ExecContextWithLogging(ctx, conn, "SET check_function_bodies = off", "disable function body validation for desired state"); err != nil {
return fmt.Errorf("failed to disable check_function_bodies: %w", err)
}
Comment on lines +134 to +141
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SET check_function_bodies = off is session-scoped. Since this uses db.Conn() (a pooled connection) and conn.Close() returns it to the pool, the setting can leak into later uses of the same underlying connection. To avoid cross-call side effects, wrap the desired-state application in an explicit transaction and use SET LOCAL check_function_bodies = off, or ensure the setting is reset (e.g., RESET check_function_bodies) via defer, including on error paths.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@tianzhou tianzhou Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The *sql.DB pool here is private to the ExternalDatabase instance, which is short-lived (created for plan generation, then Stop()'d). No other code shares this pool. The same session-scoped SET search_path on the line right above also has no cleanup/reset — adding a defer for check_function_bodies but not for search_path would be inconsistent. Both are fine as-is given the instance lifecycle.


// Strip schema qualifications from SQL before applying to temporary schema
// This ensures that objects are created in the temporary schema via search_path
// rather than being explicitly qualified with the original schema name
Expand Down
15 changes: 15 additions & 0 deletions testdata/diff/create_function/issue_399_schema_qualified_body/diff.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
CREATE OR REPLACE FUNCTION role_has_cap(
p_role role_type,
p_cap text
)
RETURNS boolean
LANGUAGE sql
STABLE
AS $$
SELECT EXISTS (
SELECT 1
FROM public.role_caps rc
WHERE rc.role = p_role
AND rc.capability = p_cap
);
$$;
22 changes: 22 additions & 0 deletions testdata/diff/create_function/issue_399_schema_qualified_body/new.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
CREATE TYPE public.role_type AS ENUM ('OWNER', 'MEMBER');

CREATE TABLE public.role_caps (
role public.role_type NOT NULL,
capability text NOT NULL,
PRIMARY KEY (role, capability)
);

CREATE OR REPLACE FUNCTION public.role_has_cap(
p_role public.role_type,
p_cap text
) RETURNS boolean
LANGUAGE sql
STABLE
AS $$
SELECT EXISTS (
SELECT 1
FROM public.role_caps rc
WHERE rc.role = p_role
AND rc.capability = p_cap
);
$$;
7 changes: 7 additions & 0 deletions testdata/diff/create_function/issue_399_schema_qualified_body/old.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
CREATE TYPE role_type AS ENUM ('OWNER', 'MEMBER');

CREATE TABLE role_caps (
role role_type NOT NULL,
capability text NOT NULL,
PRIMARY KEY (role, capability)
);
20 changes: 20 additions & 0 deletions testdata/diff/create_function/issue_399_schema_qualified_body/plan.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"version": "1.0.0",
"pgschema_version": "1.9.0",
"created_at": "1970-01-01T00:00:00Z",
"source_fingerprint": {
"hash": "eb148b37b7b6325bdd5f0c1c120dfe0bd71a062ce69951aa946c452aff2dc662"
},
"groups": [
{
"steps": [
{
"sql": "CREATE OR REPLACE FUNCTION role_has_cap(\n p_role role_type,\n p_cap text\n)\nRETURNS boolean\nLANGUAGE sql\nSTABLE\nAS $$\n SELECT EXISTS (\n SELECT 1\n FROM public.role_caps rc\n WHERE rc.role = p_role\n AND rc.capability = p_cap\n );\n$$;",
"type": "function",
"operation": "create",
"path": "public.role_has_cap"
}
]
}
]
}
15 changes: 15 additions & 0 deletions testdata/diff/create_function/issue_399_schema_qualified_body/plan.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
CREATE OR REPLACE FUNCTION role_has_cap(
p_role role_type,
p_cap text
)
RETURNS boolean
LANGUAGE sql
STABLE
AS $$
SELECT EXISTS (
SELECT 1
FROM public.role_caps rc
WHERE rc.role = p_role
AND rc.capability = p_cap
);
$$;
26 changes: 26 additions & 0 deletions testdata/diff/create_function/issue_399_schema_qualified_body/plan.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
Plan: 1 to add.

Summary by type:
functions: 1 to add

Functions:
+ role_has_cap

DDL to be executed:
--------------------------------------------------

CREATE OR REPLACE FUNCTION role_has_cap(
p_role role_type,
p_cap text
)
RETURNS boolean
LANGUAGE sql
STABLE
AS $$
SELECT EXISTS (
SELECT 1
FROM public.role_caps rc
WHERE rc.role = p_role
AND rc.capability = p_cap
);
$$;