feat: support GRANT/REVOKE for schema objects #233
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add declarative GRANT/REVOKE support for schema-level objects: - Tables, views, sequences, functions, procedures, types Key features: - Schema-level privilege storage (like DefaultPrivileges) - Aggregated privileges per object+grantee - Explicit REVOKE GRANT OPTION FOR tracking - Default PUBLIC grants excluded, explicit revokes tracked - Full function signature support for privilege identification Implementation: - ir/ir.go: Privilege and RevokedDefaultPrivilege structs - ir/inspector.go: ACL parsing from pg_class, pg_proc, pg_type - internal/diff/privilege.go: Diff logic and DDL generation - internal/dump/formatter.go: Privilege output formatting Test coverage: 10 test cases covering grants, revokes, grant option changes, and privilege modifications. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds declarative GRANT/REVOKE support for schema-level database objects (tables, views, sequences, functions, procedures, and types), addressing issue #227. The implementation provides comprehensive privilege tracking and diff generation capabilities.
Key changes:
- New IR structures for storing explicit privilege grants and revoked default PUBLIC privileges
- ACL parsing from PostgreSQL system catalogs (pg_class, pg_proc, pg_type) to extract privilege information
- Diff generation logic to produce GRANT/REVOKE DDL statements with support for privilege modifications and grant option handling
Reviewed changes
Copilot reviewed 65 out of 65 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| ir/ir.go | Adds Privilege and RevokedDefaultPrivilege structs to schema model for tracking explicit grants and revoked defaults |
| ir/inspector.go | Implements buildPrivileges() and buildRevokedDefaultPrivileges() to parse ACLs from PostgreSQL system catalogs |
| internal/diff/privilege.go | Contains diff logic and DDL generation for GRANT/REVOKE statements with proper privilege comparison |
| internal/diff/diff.go | Integrates privilege diffing into main diff workflow with new DiffType constants |
| internal/dump/formatter.go | Adds privilege directory and grouping support for multi-file output format |
| testdata/diff/privilege/* | Comprehensive test cases covering grants, revokes, grant options, and privilege modifications across object types |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Remove unnecessary columns and comments from test fixtures. Use minimal table/domain definitions to focus on privilege testing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 65 out of 65 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add TypePrivilege and TypeRevokedDefaultPrivilege to plan.go - Add them to getObjectOrder() so they appear in plan summaries - Move embedded SQL queries to ir/queries/queries.sql for consistency - Update buildPrivileges() and buildRevokedDefaultPrivileges() to use sqlc-generated query methods - Regenerate test expectations with proper summary output 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 68 out of 68 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add GetFullKey() method to Privilege for map storage that includes WithGrantOption, preventing key collisions when same (object, grantee) has different grant options - Refactor diff logic to use GetFullKey() for storage while using GetObjectKey() for modification detection (preserves REVOKE GRANT OPTION FOR behavior) - Improve error handling in role lookup: distinguish sql.ErrNoRows (role deleted) from other errors that should be reported - Remove unused objectOwners variable 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 68 out of 68 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The comment incorrectly stated that objects with ACL are selected, when in fact all objects are selected and ACL filtering happens in the public_grants CTE. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
alecthomas
pushed a commit
to alecthomas/pgschema
that referenced
this pull request
Jan 26, 2026
* feat: support GRANT/REVOKE for schema objects Add declarative GRANT/REVOKE support for schema-level objects: - Tables, views, sequences, functions, procedures, types Key features: - Schema-level privilege storage (like DefaultPrivileges) - Aggregated privileges per object+grantee - Explicit REVOKE GRANT OPTION FOR tracking - Default PUBLIC grants excluded, explicit revokes tracked - Full function signature support for privilege identification Implementation: - ir/ir.go: Privilege and RevokedDefaultPrivilege structs - ir/inspector.go: ACL parsing from pg_class, pg_proc, pg_type - internal/diff/privilege.go: Diff logic and DDL generation - internal/dump/formatter.go: Privilege output formatting Test coverage: 10 test cases covering grants, revokes, grant option changes, and privilege modifications. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore: simplify privilege test cases Remove unnecessary columns and comments from test fixtures. Use minimal table/domain definitions to focus on privilege testing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: add privilege types to plan summary and refactor to sqlc - Add TypePrivilege and TypeRevokedDefaultPrivilege to plan.go - Add them to getObjectOrder() so they appear in plan summaries - Move embedded SQL queries to ir/queries/queries.sql for consistency - Update buildPrivileges() and buildRevokedDefaultPrivileges() to use sqlc-generated query methods - Regenerate test expectations with proper summary output 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address PR review issues for privilege handling - Add GetFullKey() method to Privilege for map storage that includes WithGrantOption, preventing key collisions when same (object, grantee) has different grant options - Refactor diff logic to use GetFullKey() for storage while using GetObjectKey() for modification detection (preserves REVOKE GRANT OPTION FOR behavior) - Improve error handling in role lookup: distinguish sql.ErrNoRows (role deleted) from other errors that should be reported - Remove unused objectOwners variable 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: fix misleading comment about ACL filtering The comment incorrectly stated that objects with ACL are selected, when in fact all objects are selected and ACL filtering happens in the public_grants CTE. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Address part of #227
Add declarative GRANT/REVOKE support for schema-level objects:
Key features:
Implementation:
Test coverage: 10 test cases covering grants, revokes, grant option changes, and privilege modifications.
🤖 Generated with Claude Code