feat: add support for column-level GRANT privileges #238
Conversation
Column-level privileges like `GRANT SELECT (col1, col2) ON TABLE t TO role` were previously silently ignored during schema inspection. This meant that column grants defined in schema files would not be detected or included in migration plans, causing privilege drift between desired and actual state. This change adds full support for column-level privileges: - Add ColumnPrivilege struct to IR with proper identity (table + columns + grantee) - Add GetColumnPrivilegesForSchema query to extract from pg_attribute.attacl - Add buildColumnPrivileges to inspector to resolve grantee OIDs and group columns - Add DiffTypeColumnPrivilege and comparison logic in diff package - Add column_privilege.go with GRANT/REVOKE DDL generation - Update formatter and plan output to display column privileges Column privileges are now properly detected, diffed, and included in migration plans alongside table-level privileges.
There was a problem hiding this comment.
Pull request overview
This PR adds full support for column-level privilege grants (e.g., GRANT SELECT (col1, col2) ON TABLE t TO role), which were previously silently ignored during schema inspection. This caused privilege drift between the desired state in schema files and the actual database state.
Changes:
- Introduced
ColumnPrivilegestruct in IR with proper identity handling (table + columns + grantee) - Added SQL query
GetColumnPrivilegesForSchemato extract column privileges frompg_attribute.attacl - Implemented diff comparison and DDL generation for column privilege GRANT/REVOKE statements
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
testdata/diff/privilege/grant_table_select/plan.txt |
Updated test expectations to include column privileges in plan output |
testdata/diff/privilege/grant_table_select/plan.sql |
Added expected SQL for column-level GRANT statement |
testdata/diff/privilege/grant_table_select/plan.json |
Added column_privilege entry to JSON migration plan |
testdata/diff/privilege/grant_table_select/old.sql |
Added column_reader role setup for testing |
testdata/diff/privilege/grant_table_select/new.sql |
Added column-level GRANT test case alongside table-level GRANT |
testdata/diff/privilege/grant_table_select/diff.sql |
Added expected diff SQL for column privilege |
ir/queries/queries.sql.go |
Added generated query function for column privileges |
ir/queries/queries.sql |
Added GetColumnPrivilegesForSchema query to extract from pg_attribute.attacl |
ir/ir.go |
Added ColumnPrivilege struct with GetObjectKey and GetFullKey methods |
ir/inspector.go |
Added buildColumnPrivileges to resolve OIDs and group columns by privilege set |
internal/plan/plan.go |
Added TypeColumnPrivilege constant and ordering |
internal/dump/formatter.go |
Added column_privilege to privileges directory mapping and TABLE grouping |
internal/diff/diff.go |
Added column privilege comparison logic and integration into migration flow |
internal/diff/column_privilege.go |
Implemented GRANT/REVOKE DDL generation for column privileges |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| quotedCols := make([]string, len(cp.Columns)) | ||
| for i, col := range cp.Columns { | ||
| quotedCols[i] = ir.QuoteIdentifier(col) | ||
| } | ||
| sort.Strings(quotedCols) | ||
| colStr := strings.Join(quotedCols, ", ") |
There was a problem hiding this comment.
The columns are being sorted AFTER quoting, which could lead to incorrect sort order. Since cp.Columns is already sorted (as documented in the ColumnPrivilege struct), the quotedCols should maintain that order. Sorting quoted identifiers can produce a different order than sorting unquoted identifiers, especially with case-sensitive or special characters.
The sort.Strings(quotedCols) on line 94 should be removed. The columns should be quoted in the order they appear in cp.Columns, which is already sorted.
| quotedCols := make([]string, len(cp.Columns)) | ||
| for i, col := range cp.Columns { | ||
| quotedCols[i] = ir.QuoteIdentifier(col) | ||
| } | ||
| sort.Strings(quotedCols) | ||
| colStr := strings.Join(quotedCols, ", ") |
There was a problem hiding this comment.
The columns are being sorted AFTER quoting, which could lead to incorrect sort order. Since cp.Columns is already sorted (as documented in the ColumnPrivilege struct), the quotedCols should maintain that order. Sorting quoted identifiers can produce a different order than sorting unquoted identifiers, especially with case-sensitive or special characters.
The sort.Strings(quotedCols) on line 123 should be removed. The columns should be quoted in the order they appear in cp.Columns, which is already sorted.
| quotedCols := make([]string, len(d.New.Columns)) | ||
| for i, col := range d.New.Columns { | ||
| quotedCols[i] = ir.QuoteIdentifier(col) | ||
| } | ||
| sort.Strings(quotedCols) | ||
| colStr := strings.Join(quotedCols, ", ") |
There was a problem hiding this comment.
The columns are being sorted AFTER quoting, which could lead to incorrect sort order. Since d.New.Columns is already sorted (as documented in the ColumnPrivilege struct), the quotedCols should maintain that order. Sorting quoted identifiers can produce a different order than sorting unquoted identifiers, especially with case-sensitive or special characters.
The sort.Strings(quotedCols) on line 168 should be removed. The columns should be quoted in the order they appear in d.New.Columns, which is already sorted.
tianzhou
left a comment
tianzhou
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the contribution
Column-level privileges like `GRANT SELECT (col1, col2) ON TABLE t TO role` were previously silently ignored during schema inspection. This meant that column grants defined in schema files would not be detected or included in migration plans, causing privilege drift between desired and actual state. This change adds full support for column-level privileges: - Add ColumnPrivilege struct to IR with proper identity (table + columns + grantee) - Add GetColumnPrivilegesForSchema query to extract from pg_attribute.attacl - Add buildColumnPrivileges to inspector to resolve grantee OIDs and group columns - Add DiffTypeColumnPrivilege and comparison logic in diff package - Add column_privilege.go with GRANT/REVOKE DDL generation - Update formatter and plan output to display column privileges Column privileges are now properly detected, diffed, and included in migration plans alongside table-level privileges.
2 participants
Summary
GRANT SELECT (col1, col2) ON TABLE t TO rolewere previously silently ignored during schema inspectionChanges
ColumnPrivilegestruct to IR with proper identity (table + columns + grantee)GetColumnPrivilegesForSchemaquery to extract frompg_attribute.attaclbuildColumnPrivilegesto inspector to resolve grantee OIDs and group columnsDiffTypeColumnPrivilegeand comparison logic in diff packagecolumn_privilege.gowith GRANT/REVOKE DDL generationTest plan
grant_table_selecttest case with column-level GRANTTestDiffFromFiles) and integration tests (TestPlanAndApply) pass