fix: render policy and constraint expressions with table schema search_path (#449)

[tianzhou]

Summary

A pgschema plan generated immediately after a successful pgschema apply reported spurious changes for RLS policies and CHECK constraints referencing same-schema objects (#449). The apply succeeded, but the plan never converged to "No changes detected", causing perpetual ALTER POLICY rewrites and DROP CONSTRAINT / ADD ... NOT VALID / VALIDATE CONSTRAINT churn on every deploy.

Two distinct root causes, both rendering-context asymmetries between the current state (target database) and the desired state (temporary plan schema):

1. CHECK constraints ('disabled'::profiles.profile_status vs 'disabled'::profile_status): pg_get_constraintdef() ran under the inspector session's default search_path, so any non-public managed schema rendered same-schema type casts qualified on the current side, while normalizeSchemaNames stripped them on the desired side. This is the same drift reported in Unresolvable drift on CHECK constraints referencing functions/types in same schema #445, which reproduces on main despite being closed.

2. Policy expressions when the schema name equals a table name (profiles.profiles): expressions were rendered with search_path forced to pg_catalog and same-schema qualifiers regex-stripped afterwards. The regex could not distinguish schema qualifiers from table qualifiers, so it mangled table-qualified column references (profiles.id → id) on the current side only.

Fix

Render both expression kinds with search_path set to the table's own schema via a per-row LATERAL set_config(...) (the pattern already used for column defaults). Same-schema references then come out unqualified on both sides natively, while cross-schema references (e.g. Supabase's auth.uid(), #427) stay qualified. The now-obsolete and unsafe regex stripping in normalizePolicyExpression is removed; generated DDL still applies cleanly because apply sets search_path to the target schema.

Fixes #449
Also fixes the still-reproducible drift from #445.

Test plan

• New TestIssue449RepeatPlanIdempotency integration test (cmd/issue_449_integration_test.go) covering both drift sources via apply-then-replan; red before the fix, green after:

  go test -v ./cmd -run TestIssue449RepeatPlanIdempotency

• Verified the exact reproduction from v1.10.0: repeat plan drift for same-schema policy and CHECK expressions after apply #449 end to end against PostgreSQL 17 (external plan database): repeat plan now reports "No changes detected", and the stored policy keeps auth.uid() schema-qualified (Schema-qualified functions in RLS policy expressions are dumped/planned as unqualified functions #427 preserved).
• Regression suites, all passing: full TestDiffFromFiles (150+ cases), full dump suite (go test ./cmd/dump), TestPlanAndApply for create_policy/, create_table/, online/, TestNonPublicSchemaOperations, and ./ir unit tests.

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR changes how policy and constraint expressions are rendered for schema comparison. The main changes are:

• RLS policy normalization no longer regex-strips same-schema qualifiers.
• Constraint inspection now tries to render definitions under the table schema search path.
• RLS policy inspection now tries to render expressions under the table schema search path.
• A new integration test covers repeat-plan idempotency for issue 449.

Confidence Score: 3/5

These issues should be fixed before merging.

• The core query change depends on an unsafe ordering side effect.
• Policy and constraint rendering can still use the wrong search path.
• Non-public schemas that rely on public helper objects can still produce repeat-plan churn.

ir/queries/queries.sql and the generated ir/queries/queries.sql.go mirror need matching fixes.

Important Files Changed

Filename	Overview
ir/queries/queries.sql	Core expression-rendering queries now depend on per-row search_path changes.
ir/normalize.go	Policy expression normalization now preserves qualified references.
cmd/issue_449_integration_test.go	Adds integration coverage for apply-then-replan convergence.

_{Reviews (1): Last reviewed commit: "fix: render policy and constraint expres..." | Re-trigger Greptile}

[greptile-apps]

Search path ordering is unsafe

This lateral subquery relies on the unreferenced dummy target column to run before pg_get_constraintdef, but the outer query never reads dummy and SQL target-list evaluation is not a safe ordering dependency for changing a GUC. When pg_get_constraintdef runs under the previous session or previous row's search_path, CHECK and EXCLUDE definitions can still be rendered with the wrong qualification, leaving the repeat-plan drift this change is meant to remove.

[tianzhou]

The ordering here is the established pattern in this file: the column-default and index-predicate queries have used the same LATERAL set_config(...) + pg_get_expr arrangement since issue #218 — see the comment block on GetColumnsForSchema ("Use LATERAL join to guarantee execution order") which also documents the rejected alternative of a wrapper function. Within a row's target list PostgreSQL evaluates entries in order, and set_config is volatile, so the planner neither reorders nor elides it; it also re-executes for every row, so there is no stale previous-row state. Empirically: the new TestIssue449RepeatPlanIdempotency was red before this change and green after, and the full diff (150+ cases) and dump suites pass — those inspect many constraint rows across schemas. If this ordering were unreliable, the pre-existing column-default query would have exhibited the same nondeterminism long ago.

[greptile-apps]

You're right. My concern was overstated. PostgreSQL does guarantee that volatile functions in a target list are evaluated in order and not elided — the planner cannot skip set_config because it has side effects, regardless of whether dummy is referenced upstream. The pre-existing column-default queries using this exact pattern would have surfaced nondeterminism long ago if the ordering were unreliable.

I withdraw the comment. The implementation is sound.

[greptile-apps]

Policy rendering can drift

This has the same ordering problem as the constraint query: set_config is selected as an unused dummy column, so pg_get_expr is not structurally forced to run after the table-schema search_path is installed. If the expression is rendered under the old session state, a policy can still alternate between qualified and unqualified references, producing repeated ALTER POLICY output after an apply.

[tianzhou]

Same as the constraint-query thread: this is the pre-existing LATERAL set_config pattern from the column-default and index queries (issue #218), relying on in-order evaluation of volatile target-list entries, re-executed per row. Empirically validated by the red-to-green TestIssue449RepeatPlanIdempotency plus the full diff and dump suites.

[greptile-apps]

Fair point. You're right that this is the pre-existing LATERAL set_config pattern established in #218 for column defaults and indexes, and it's consistent across all expression-rendering queries in the file. My concern about ordering was based on SQL standard semantics, but PostgreSQL's implementation guarantees left-to-right evaluation of volatile target-list entries within a SELECT — and this codebase already relies on that guarantee in multiple places with empirical validation.

Given the red-to-green test coverage on TestIssue449RepeatPlanIdempotency plus the full diff/dump regression suite, the concern is resolved. Happy to withdraw it.

[greptile-apps]

Public fallback is missing

The desired-state database is loaded with search_path set to the temporary schema plus public, but current-state policy rendering now uses only the table schema. For a non-public managed schema with a policy that calls a helper function resolved from public, the desired side can render has_scope(...) while the current side renders public.has_scope(...), so plans can keep reporting policy changes even after a successful apply.

[tianzhou]

The apply-time search_path (tmp, public) only affects name resolution at CREATE time — PostgreSQL stores the resolved OID in polqual. Rendering happens at inspection time, and both the current and desired states are inspected through this same query, so both render under search_path = the inspected table's own schema (temp schema on the desired side, managed schema on the current side). A helper resolved from public therefore renders as public.has_scope(...) on both sides — symmetric, no drift. Verified empirically on PG 17 with exactly this scenario (non-public schema, policy + CHECK calling a public helper and casting to a public enum, with both qualified and unqualified references in the source file): apply followed by replan reports "No changes detected" in all variants.

[greptile-apps]

Constraint context differs

The desired-state path applies SQL with public in the fallback search path, but this current-state constraint query renders with only the table schema. A CHECK constraint in a non-public schema that calls a public helper or casts to a public extension type can therefore render unqualified on the desired side and public.-qualified on the current side, causing repeat plans to keep dropping and re-adding the constraint.

[tianzhou]

Same as the policy-query thread: apply-time search_path only affects resolution, not rendering. Both sides are inspected through this same query, so a CHECK constraint referencing a public helper or casting to a public type renders public.-qualified on both sides — symmetric, no drift. Verified empirically on PG 17 (non-public schema, CHECK (public.has_scope(scope)) and 'bad'::public.pub_status, qualified and unqualified source variants): repeat plan after apply reports "No changes detected".

[greptile-apps]

You're right — I was wrong. Since both the current state and desired state are read through the same query (with the same set_config lateral), cross-schema references like public.has_scope(...) or 'bad'::public.pub_status render public.-qualified on both sides, keeping them symmetric. My concern about asymmetric rendering was unfounded. Thanks for the empirical verification.

[Copilot]

Pull request overview

Fixes non-converging pgschema plan output after a successful pgschema apply when CHECK constraints and/or RLS policy expressions reference same-schema objects, by making catalog deparsing deterministic via a per-row search_path set to the table’s schema (and removing unsafe regex-based qualifier stripping).

Changes:

• Render CHECK / EXCLUDE constraint definitions with pg_get_constraintdef() under search_path = <table schema> via a LATERAL set_config(...).
• Render RLS policy USING / WITH CHECK expressions with pg_get_expr() under search_path = <table schema>, and remove regex stripping from normalizePolicyExpression.
• Add an integration test that applies a schema then immediately replans to assert idempotency for the reported drift cases (#449 / #445).

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
ir/queries/queries.sql	Adjusts constraint/policy expression rendering by setting per-row search_path to the owning table schema.
ir/queries/queries.sql.go	Regenerated SQLC output reflecting the updated SQL queries.
ir/normalize.go	Removes policy qualifier regex stripping; relies on deterministic rendering instead.
ir/normalize_test.go	Updates unit tests to reflect new normalization behavior (qualifiers preserved as-rendered).
cmd/issue_449_integration_test.go	Adds apply-then-replan idempotency integration test covering the regression.

Files not reviewed (1)

• ir/queries/queries.sql.go: Language not supported

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[tianzhou]

Keeping ON true deliberately: it matches the existing LATERAL set_config pattern used by the column-default and index queries in this file, which also execute for every row (columns are far more numerous than constraints). Inspection is a cold path that runs once per plan/dump, and set_config costs microseconds per row, so filtering on contype would save negligible work while diverging from the established pattern.

[tianzhou]

Triaged all review comments — no code changes needed. Summary:

Greptile P1 "search path ordering is unsafe" (2 threads) — dismissed. The LATERAL set_config + pg_get_* arrangement is the pattern this file has used for column defaults and index predicates since #218 (with the rationale and rejected alternatives documented inline). set_config is volatile so it is neither reordered nor elided, it re-executes per row, and the red→green regression test plus the full diff/dump suites validate the behavior across many-row inspections.

Greptile P1 "public fallback is missing" (2 threads) — dismissed; the claim conflates apply-time name resolution with render-time deparsing. Both current and desired states are inspected through the same queries, so public helpers render public.-qualified on both sides. Verified empirically on PG 17: a non-public schema with a policy + CHECK referencing public.has_scope() and a public enum cast (qualified and unqualified source variants) converges to "No changes detected" after apply.

Copilot "restrict LATERAL ON true to contype c/x" (4 threads, 1 unique point) — acknowledged but not changed: inspection is a cold path, set_config costs microseconds over typically tens of constraint rows, and ON true keeps the query consistent with the existing LATERAL pattern (which already runs per-row for all columns).