Skip to content

fix: recreate policy when removing WITH CHECK clause (#551) - #554

Merged
tianzhou merged 2 commits into
mainfrom
fix/issue-551-policy-with-check-removal
Aug 21, 2026
Merged

fix: recreate policy when removing WITH CHECK clause (#551)#554
tianzhou merged 2 commits into
mainfrom
fix/issue-551-policy-with-check-removal

Conversation

@tianzhou

Copy link
Copy Markdown
Contributor

Summary

  • When a policy's WITH CHECK clause is removed, needsRecreate now returns true, triggering DROP + CREATE instead of ALTER POLICY
  • Previously emitted invalid SQL WITH CHECK ; because ALTER POLICY cannot clear an existing WITH CHECK expression
  • Adds regression test issue_551_remove_with_check verifying DROP + CREATE output

Closes #551

Test plan

  • Diff test for the exact repro scenario passes
  • Integration test applies successfully against real Postgres
  • Full diff test suite passes
  • Full integration test suite (running)

🤖 Generated with Claude Code

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 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 20, 2026 06:21
@greptile-apps

greptile-apps Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR changes removal of a policy WITH CHECK clause from an invalid ALTER POLICY operation to transactional DROP-and-CREATE recreation.

  • Adds the recreation condition in policy diffing.
  • Adds a regression fixture covering the generated plan and SQL.
  • The recreation path does not preserve an existing policy comment.

Confidence Score: 4/5

The policy recreation behavior should preserve existing policy comments before this PR is merged.

Removing WITH CHECK now drops and recreates the policy, but the recreation generator restores only structural clauses, so any existing COMMENT ON POLICY metadata is silently deleted.

Files Needing Attention: internal/diff/policy.go and the policy recreation branch in internal/diff/table.go

Important Files Changed

Filename Overview
internal/diff/policy.go Correctly detects that removing WITH CHECK requires recreation, but the selected recreation path drops unrelated policy comments.
testdata/diff/create_policy/issue_551_remove_with_check/old.sql Defines the source policy with WITH CHECK but does not exercise preservation of attached policy metadata.
testdata/diff/create_policy/issue_551_remove_with_check/new.sql Defines the intended policy state without WITH CHECK.
testdata/diff/create_policy/issue_551_remove_with_check/diff.sql Correctly captures the expected DROP-and-CREATE SQL for the basic regression scenario.

Reviews (1): Last reviewed commit: "fix: recreate policy when removing WITH ..." | Re-trigger Greptile

Comment thread internal/diff/policy.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes invalid SQL generation when removing an explicit RLS policy WITH CHECK clause by forcing policy recreation (DROP + CREATE) instead of attempting ALTER POLICY, and adds a regression diff test case for the reported scenario (#551).

Changes:

  • Update needsRecreate to return true when a policy’s WITH CHECK clause is removed, avoiding invalid WITH CHECK ; output.
  • Add a new diff regression fixture issue_551_remove_with_check validating DROP + CREATE plan output.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

File Description
internal/diff/policy.go Changes policy recreation decision logic to handle WITH CHECK removal correctly.
testdata/diff/create_policy/issue_551_remove_with_check/* Adds regression fixture asserting DROP + CREATE behavior in plan/diff outputs.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread internal/diff/policy.go Outdated
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 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

@tianzhou
tianzhou merged commit e739ca8 into main Aug 21, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pgSchema generates invalid WITH CHECK ; when removing an explicit RLS WITH CHECK clause

2 participants