fix: order REVOKE before GRANT when transitioning between table and column privileges (#324)

[tianzhou]

Summary

• When changing from table-level GRANT to column-level GRANT (e.g., GRANT UPDATE → GRANT UPDATE (col)), the migration was emitting GRANT before REVOKE, leaving the user with no permissions
• Root cause: privilege modifications (containing REVOKEs) were in Phase 3 (Modify) while new column grants were in Phase 2 (Create)
• Fix: moved privilege/column-privilege modifications to execute before new privilege creates in the Create phase

Fixes #324

Test plan

• PGSCHEMA_TEST_FILTER="privilege/issue_324" go test -v ./internal/diff -run TestDiffFromFiles — two new test cases
• PGSCHEMA_TEST_FILTER="privilege/" go test -v ./internal/diff -run TestDiffFromFiles — all 13 privilege diff tests pass
• PGSCHEMA_TEST_FILTER="privilege/" go test -v ./cmd -run TestPlanAndApply — all 13 privilege integration tests pass
• PGSCHEMA_TEST_FILTER="default_privilege/" go test -v ./cmd -run TestPlanAndApply — all 9 default privilege tests pass

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

Fixed critical privilege ordering bug where transitioning from table-level to column-level privileges would leave users with no permissions.

Root Cause

• Table-level REVOKE operations (from privilege modifications) were executing in Phase 3 (Modify)
• Column-level GRANT operations (from new privilege creates) were executing in Phase 2 (Create)
• Result: GRANT executed first, then REVOKE undid it, leaving no permissions

Solution

• Moved generateModifyPrivilegesSQL and generateModifyColumnPrivilegesSQL from Modify phase to Create phase
• Positioned them before generateCreatePrivilegesSQL and generateCreateColumnPrivilegesSQL
• Ensures correct execution order: REVOKE table-level privilege → GRANT column-level privilege

Testing

• Two new test cases cover both scenarios (simple transition and mixed privilege types)
• All 13 privilege diff tests pass
• All 13 privilege integration tests pass
• All 9 default privilege tests pass

Confidence Score: 5/5

• Safe to merge - targeted fix with comprehensive test coverage and no side effects
• The fix is minimal, well-documented, and solves a critical bug. Moving privilege modification operations from Modify to Create phase ensures proper REVOKE-before-GRANT ordering without affecting other operations. All existing and new tests pass.
• No files require special attention

Important Files Changed

Filename	Overview
internal/diff/diff.go	moved privilege modification operations from Modify phase to Create phase to ensure REVOKE executes before GRANT
testdata/diff/privilege/issue_324_grant_revoke_order/diff.sql	expected migration SQL with REVOKE before GRANT ordering
testdata/diff/privilege/issue_324_modify_grant_to_column/diff.sql	expected migration SQL revoking table UPDATE before granting column UPDATE

_{Last reviewed commit: a9a9bb8}

[Copilot]

Pull request overview

Fixes privilege migration ordering so that when transitioning between table-level and column-level privileges, any necessary REVOKE executes before new GRANT statements (issue #324), preventing permissions from being unintentionally removed.

Changes:

• Reordered SQL generation so explicit privilege/column-privilege modifications are emitted before new privilege creates.
• Added two new regression test cases covering table→column privilege transitions and GRANT/REVOKE ordering in plan outputs.

Reviewed changes

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

Show a summary per file

File	Description
internal/diff/diff.go	Reorders privilege/column-privilege modification emission relative to new grants to address #324.
testdata/diff/privilege/issue_324_modify_grant_to_column/plan.txt	Adds expected human plan output for the modify→column-grant transition case.
testdata/diff/privilege/issue_324_modify_grant_to_column/plan.sql	Adds expected SQL plan output (REVOKE before GRANT).
testdata/diff/privilege/issue_324_modify_grant_to_column/plan.json	Adds expected JSON plan output reflecting corrected step ordering.
testdata/diff/privilege/issue_324_modify_grant_to_column/old.sql	Adds old schema input for regression test.
testdata/diff/privilege/issue_324_modify_grant_to_column/new.sql	Adds new schema input for regression test.
testdata/diff/privilege/issue_324_modify_grant_to_column/diff.sql	Adds expected diff SQL for file-based diff tests.
testdata/diff/privilege/issue_324_grant_revoke_order/plan.txt	Adds expected human plan output for GRANT/REVOKE ordering case.
testdata/diff/privilege/issue_324_grant_revoke_order/plan.sql	Adds expected SQL plan output (REVOKE before GRANT).
testdata/diff/privilege/issue_324_grant_revoke_order/plan.json	Adds expected JSON plan output reflecting corrected step ordering.
testdata/diff/privilege/issue_324_grant_revoke_order/old.sql	Adds old schema input for regression test.
testdata/diff/privilege/issue_324_grant_revoke_order/new.sql	Adds new schema input for regression test.
testdata/diff/privilege/issue_324_grant_revoke_order/diff.sql	Adds expected diff SQL for file-based diff tests.

---

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

[Copilot]

Pull request overview

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

---

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