From f4020600d04c8f09921374595e3ebf0f1892530b Mon Sep 17 00:00:00 2001 From: tianzhou Date: Wed, 19 Aug 2026 23:20:57 -0700 Subject: [PATCH 1/2] fix: recreate policy when removing WITH CHECK clause (#551) PostgreSQL ALTER POLICY cannot clear an existing WITH CHECK expression, so removing it requires DROP + CREATE. Previously this emitted invalid SQL `WITH CHECK ;`. Co-Authored-By: Claude Opus 4.6 --- internal/diff/policy.go | 5 ++++ .../issue_551_remove_with_check/diff.sql | 3 +++ .../issue_551_remove_with_check/new.sql | 11 ++++++++ .../issue_551_remove_with_check/old.sql | 12 +++++++++ .../issue_551_remove_with_check/plan.json | 26 +++++++++++++++++++ .../issue_551_remove_with_check/plan.sql | 3 +++ .../issue_551_remove_with_check/plan.txt | 16 ++++++++++++ 7 files changed, 76 insertions(+) create mode 100644 testdata/diff/create_policy/issue_551_remove_with_check/diff.sql create mode 100644 testdata/diff/create_policy/issue_551_remove_with_check/new.sql create mode 100644 testdata/diff/create_policy/issue_551_remove_with_check/old.sql create mode 100644 testdata/diff/create_policy/issue_551_remove_with_check/plan.json create mode 100644 testdata/diff/create_policy/issue_551_remove_with_check/plan.sql create mode 100644 testdata/diff/create_policy/issue_551_remove_with_check/plan.txt diff --git a/internal/diff/policy.go b/internal/diff/policy.go index 3e37c7b1..ef671332 100644 --- a/internal/diff/policy.go +++ b/internal/diff/policy.go @@ -254,6 +254,11 @@ func needsRecreate(old, new *ir.RLSPolicy) bool { if old.Permissive != new.Permissive { return true } + // PostgreSQL ALTER POLICY cannot clear an existing WITH CHECK clause, + // so removing it requires DROP + CREATE. + if old.WithCheck != "" && new.WithCheck == "" { + return true + } // All other changes (roles, using, with_check) can use ALTER POLICY return false } diff --git a/testdata/diff/create_policy/issue_551_remove_with_check/diff.sql b/testdata/diff/create_policy/issue_551_remove_with_check/diff.sql new file mode 100644 index 00000000..8a91ed6f --- /dev/null +++ b/testdata/diff/create_policy/issue_551_remove_with_check/diff.sql @@ -0,0 +1,3 @@ +DROP POLICY IF EXISTS agents_app_tenant ON agents; + +CREATE POLICY agents_app_tenant ON agents TO PUBLIC USING (organization_id = '00000000-0000-0000-0000-000000000001'::uuid); diff --git a/testdata/diff/create_policy/issue_551_remove_with_check/new.sql b/testdata/diff/create_policy/issue_551_remove_with_check/new.sql new file mode 100644 index 00000000..91fc1863 --- /dev/null +++ b/testdata/diff/create_policy/issue_551_remove_with_check/new.sql @@ -0,0 +1,11 @@ +CREATE TABLE public.agents ( + id integer PRIMARY KEY, + organization_id uuid NOT NULL +); + +ALTER TABLE public.agents ENABLE ROW LEVEL SECURITY; + +CREATE POLICY agents_app_tenant ON public.agents + FOR ALL + TO PUBLIC + USING (organization_id = '00000000-0000-0000-0000-000000000001'::uuid); diff --git a/testdata/diff/create_policy/issue_551_remove_with_check/old.sql b/testdata/diff/create_policy/issue_551_remove_with_check/old.sql new file mode 100644 index 00000000..088f5697 --- /dev/null +++ b/testdata/diff/create_policy/issue_551_remove_with_check/old.sql @@ -0,0 +1,12 @@ +CREATE TABLE public.agents ( + id integer PRIMARY KEY, + organization_id uuid NOT NULL +); + +ALTER TABLE public.agents ENABLE ROW LEVEL SECURITY; + +CREATE POLICY agents_app_tenant ON public.agents + FOR ALL + TO PUBLIC + USING (organization_id = '00000000-0000-0000-0000-000000000001'::uuid) + WITH CHECK (organization_id = '00000000-0000-0000-0000-000000000001'::uuid); diff --git a/testdata/diff/create_policy/issue_551_remove_with_check/plan.json b/testdata/diff/create_policy/issue_551_remove_with_check/plan.json new file mode 100644 index 00000000..77ad75ab --- /dev/null +++ b/testdata/diff/create_policy/issue_551_remove_with_check/plan.json @@ -0,0 +1,26 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.12.3", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "7c89eec1e4bb170bbbca9ed7028bf15211414531ee74b300818acb7766d84b45" + }, + "groups": [ + { + "steps": [ + { + "sql": "DROP POLICY IF EXISTS agents_app_tenant ON agents;", + "type": "table.policy", + "operation": "drop", + "path": "public.agents.agents_app_tenant" + }, + { + "sql": "CREATE POLICY agents_app_tenant ON agents TO PUBLIC USING (organization_id = '00000000-0000-0000-0000-000000000001'::uuid);", + "type": "table.policy", + "operation": "create", + "path": "public.agents.agents_app_tenant" + } + ] + } + ] +} diff --git a/testdata/diff/create_policy/issue_551_remove_with_check/plan.sql b/testdata/diff/create_policy/issue_551_remove_with_check/plan.sql new file mode 100644 index 00000000..8a91ed6f --- /dev/null +++ b/testdata/diff/create_policy/issue_551_remove_with_check/plan.sql @@ -0,0 +1,3 @@ +DROP POLICY IF EXISTS agents_app_tenant ON agents; + +CREATE POLICY agents_app_tenant ON agents TO PUBLIC USING (organization_id = '00000000-0000-0000-0000-000000000001'::uuid); diff --git a/testdata/diff/create_policy/issue_551_remove_with_check/plan.txt b/testdata/diff/create_policy/issue_551_remove_with_check/plan.txt new file mode 100644 index 00000000..16032f40 --- /dev/null +++ b/testdata/diff/create_policy/issue_551_remove_with_check/plan.txt @@ -0,0 +1,16 @@ +Plan: 1 to modify. + +Summary by type: + tables: 1 to modify + +Tables: + ~ agents + - agents_app_tenant (policy) + + agents_app_tenant (policy) + +DDL to be executed: +-------------------------------------------------- + +DROP POLICY IF EXISTS agents_app_tenant ON agents; + +CREATE POLICY agents_app_tenant ON agents TO PUBLIC USING (organization_id = '00000000-0000-0000-0000-000000000001'::uuid); From ecb72b6876e4f84a81d57701112e7cf834c30fb4 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Thu, 20 Aug 2026 21:03:43 -0700 Subject: [PATCH 2/2] fix: also recreate policy when removing USING clause, preserve comments Address review feedback: - USING removal has the same invalid SQL problem as WITH CHECK removal; add it to needsRecreate so it also triggers DROP + CREATE. - Emit COMMENT ON POLICY after recreation to preserve existing comments. Co-Authored-By: Claude Opus 4.6 --- internal/diff/policy.go | 7 +++++-- internal/diff/table.go | 13 +++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/internal/diff/policy.go b/internal/diff/policy.go index ef671332..bc9c9f16 100644 --- a/internal/diff/policy.go +++ b/internal/diff/policy.go @@ -254,8 +254,11 @@ func needsRecreate(old, new *ir.RLSPolicy) bool { if old.Permissive != new.Permissive { return true } - // PostgreSQL ALTER POLICY cannot clear an existing WITH CHECK clause, - // so removing it requires DROP + CREATE. + // PostgreSQL ALTER POLICY cannot clear an existing USING or WITH CHECK + // clause, so removing either requires DROP + CREATE. + if old.Using != "" && new.Using == "" { + return true + } if old.WithCheck != "" && new.WithCheck == "" { return true } diff --git a/internal/diff/table.go b/internal/diff/table.go index eb64fb93..3c7b60f7 100644 --- a/internal/diff/table.go +++ b/internal/diff/table.go @@ -1661,6 +1661,19 @@ func (td *tableDiff) generateAlterTableStatements(targetSchema string, collector CanRunInTransaction: true, } collector.collect(context, sql) + + if policyDiff.New.Comment != "" { + sql = fmt.Sprintf("COMMENT ON POLICY %s ON %s IS %s;", + ir.QuoteIdentifier(policyDiff.New.Name), tableName, quoteString(policyDiff.New.Comment)) + context = &diffContext{ + Type: DiffTypeTablePolicy, + Operation: DiffOperationAlter, + Path: fmt.Sprintf("%s.%s.%s", td.Table.Schema, td.Table.Name, policyDiff.New.Name), + Source: policyDiff, + CanRunInTransaction: true, + } + collector.collect(context, sql) + } } else { // Use ALTER POLICY for simple changes sql := generateAlterPolicySQL(policyDiff.Old, policyDiff.New, targetSchema)