diff --git a/internal/diff/diff.go b/internal/diff/diff.go index 080845b0..48f8b5e9 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -1603,14 +1603,14 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto // See https://github.com/pgplex/pgschema/issues/253 generateDropPrivilegesSQL(d.revokedDefaultGrantsOnNewTables, targetSchema, collector) - // Create explicit object privileges - generateCreatePrivilegesSQL(d.addedPrivileges, targetSchema, collector) - - // Create column-level privileges - generateCreateColumnPrivilegesSQL(d.addedColumnPrivileges, targetSchema, collector) - // Revoke default PUBLIC privileges (new revokes) generateRevokeDefaultPrivilegesSQL(d.addedRevokedDefaultPrivs, targetSchema, collector) + + // Note: Explicit privilege creates and modifications are handled in generateModifySQL + // (after object modifications/recreations) to ensure: + // 1. DROP+CREATE'd objects (e.g., materialized views) don't wipe out privilege changes + // 2. REVOKEs from modifications execute before new GRANTs + // See https://github.com/pgplex/pgschema/issues/324 } // generateModifySQL generates ALTER statements @@ -1653,11 +1653,15 @@ func (d *ddlDiff) generateModifySQL(targetSchema string, collector *diffCollecto // Modify default privileges generateModifyDefaultPrivilegesSQL(d.modifiedDefaultPrivileges, targetSchema, collector) - // Modify explicit object privileges + // All explicit privilege operations run AFTER object modifications/recreations + // to avoid DROP+CREATE'd objects (e.g., materialized views) wiping out privilege changes. + // Modifications (which contain REVOKEs) run before creates (which contain GRANTs) + // to prevent table-level REVOKEs from undoing column-level GRANTs. + // See https://github.com/pgplex/pgschema/issues/324 generateModifyPrivilegesSQL(d.modifiedPrivileges, targetSchema, collector) - - // Modify column-level privileges generateModifyColumnPrivilegesSQL(d.modifiedColumnPrivileges, targetSchema, collector) + generateCreatePrivilegesSQL(d.addedPrivileges, targetSchema, collector) + generateCreateColumnPrivilegesSQL(d.addedColumnPrivileges, targetSchema, collector) } // generateDropSQL generates DROP statements in reverse dependency order diff --git a/testdata/diff/privilege/issue_324_grant_revoke_order/diff.sql b/testdata/diff/privilege/issue_324_grant_revoke_order/diff.sql new file mode 100644 index 00000000..83414f97 --- /dev/null +++ b/testdata/diff/privilege/issue_324_grant_revoke_order/diff.sql @@ -0,0 +1,3 @@ +REVOKE UPDATE ON TABLE sometable FROM app_user; + +GRANT UPDATE (somecolumn) ON TABLE sometable TO app_user; diff --git a/testdata/diff/privilege/issue_324_grant_revoke_order/new.sql b/testdata/diff/privilege/issue_324_grant_revoke_order/new.sql new file mode 100644 index 00000000..1c12601f --- /dev/null +++ b/testdata/diff/privilege/issue_324_grant_revoke_order/new.sql @@ -0,0 +1,10 @@ +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'app_user') THEN + CREATE ROLE app_user; + END IF; +END $$; + +CREATE TABLE sometable (somecolumn text); + +GRANT UPDATE (somecolumn) ON sometable TO app_user; diff --git a/testdata/diff/privilege/issue_324_grant_revoke_order/old.sql b/testdata/diff/privilege/issue_324_grant_revoke_order/old.sql new file mode 100644 index 00000000..72808b90 --- /dev/null +++ b/testdata/diff/privilege/issue_324_grant_revoke_order/old.sql @@ -0,0 +1,10 @@ +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'app_user') THEN + CREATE ROLE app_user; + END IF; +END $$; + +CREATE TABLE sometable (somecolumn text); + +GRANT UPDATE ON sometable TO app_user; diff --git a/testdata/diff/privilege/issue_324_grant_revoke_order/plan.json b/testdata/diff/privilege/issue_324_grant_revoke_order/plan.json new file mode 100644 index 00000000..78de0b2d --- /dev/null +++ b/testdata/diff/privilege/issue_324_grant_revoke_order/plan.json @@ -0,0 +1,26 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.7.2", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "2f50f8550c48f6ac58cde417097cdd7c293b3ae0d9bd8861b197edb73e80b148" + }, + "groups": [ + { + "steps": [ + { + "sql": "REVOKE UPDATE ON TABLE sometable FROM app_user;", + "type": "privilege", + "operation": "drop", + "path": "privileges.TABLE.sometable.app_user" + }, + { + "sql": "GRANT UPDATE (somecolumn) ON TABLE sometable TO app_user;", + "type": "column_privilege", + "operation": "create", + "path": "column_privileges.TABLE.sometable.somecolumn.app_user" + } + ] + } + ] +} diff --git a/testdata/diff/privilege/issue_324_grant_revoke_order/plan.sql b/testdata/diff/privilege/issue_324_grant_revoke_order/plan.sql new file mode 100644 index 00000000..83414f97 --- /dev/null +++ b/testdata/diff/privilege/issue_324_grant_revoke_order/plan.sql @@ -0,0 +1,3 @@ +REVOKE UPDATE ON TABLE sometable FROM app_user; + +GRANT UPDATE (somecolumn) ON TABLE sometable TO app_user; diff --git a/testdata/diff/privilege/issue_324_grant_revoke_order/plan.txt b/testdata/diff/privilege/issue_324_grant_revoke_order/plan.txt new file mode 100644 index 00000000..7c3bd47f --- /dev/null +++ b/testdata/diff/privilege/issue_324_grant_revoke_order/plan.txt @@ -0,0 +1,18 @@ +Plan: 1 to add, 1 to drop. + +Summary by type: + privileges: 1 to drop + column privileges: 1 to add + +Privileges: + - app_user + +Column privileges: + + app_user + +DDL to be executed: +-------------------------------------------------- + +REVOKE UPDATE ON TABLE sometable FROM app_user; + +GRANT UPDATE (somecolumn) ON TABLE sometable TO app_user; diff --git a/testdata/diff/privilege/issue_324_modify_grant_to_column/diff.sql b/testdata/diff/privilege/issue_324_modify_grant_to_column/diff.sql new file mode 100644 index 00000000..83414f97 --- /dev/null +++ b/testdata/diff/privilege/issue_324_modify_grant_to_column/diff.sql @@ -0,0 +1,3 @@ +REVOKE UPDATE ON TABLE sometable FROM app_user; + +GRANT UPDATE (somecolumn) ON TABLE sometable TO app_user; diff --git a/testdata/diff/privilege/issue_324_modify_grant_to_column/new.sql b/testdata/diff/privilege/issue_324_modify_grant_to_column/new.sql new file mode 100644 index 00000000..138f2744 --- /dev/null +++ b/testdata/diff/privilege/issue_324_modify_grant_to_column/new.sql @@ -0,0 +1,11 @@ +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'app_user') THEN + CREATE ROLE app_user; + END IF; +END $$; + +CREATE TABLE sometable (somecolumn text); + +GRANT SELECT ON sometable TO app_user; +GRANT UPDATE (somecolumn) ON sometable TO app_user; diff --git a/testdata/diff/privilege/issue_324_modify_grant_to_column/old.sql b/testdata/diff/privilege/issue_324_modify_grant_to_column/old.sql new file mode 100644 index 00000000..485c2984 --- /dev/null +++ b/testdata/diff/privilege/issue_324_modify_grant_to_column/old.sql @@ -0,0 +1,10 @@ +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'app_user') THEN + CREATE ROLE app_user; + END IF; +END $$; + +CREATE TABLE sometable (somecolumn text); + +GRANT SELECT, UPDATE ON sometable TO app_user; diff --git a/testdata/diff/privilege/issue_324_modify_grant_to_column/plan.json b/testdata/diff/privilege/issue_324_modify_grant_to_column/plan.json new file mode 100644 index 00000000..a4b780d4 --- /dev/null +++ b/testdata/diff/privilege/issue_324_modify_grant_to_column/plan.json @@ -0,0 +1,26 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.7.2", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "1dec24426019fcb5e34cd89432fe55c1f3128e871f5b770263874c4ada106d6f" + }, + "groups": [ + { + "steps": [ + { + "sql": "REVOKE UPDATE ON TABLE sometable FROM app_user;", + "type": "privilege", + "operation": "alter", + "path": "privileges.TABLE.sometable.app_user" + }, + { + "sql": "GRANT UPDATE (somecolumn) ON TABLE sometable TO app_user;", + "type": "column_privilege", + "operation": "create", + "path": "column_privileges.TABLE.sometable.somecolumn.app_user" + } + ] + } + ] +} diff --git a/testdata/diff/privilege/issue_324_modify_grant_to_column/plan.sql b/testdata/diff/privilege/issue_324_modify_grant_to_column/plan.sql new file mode 100644 index 00000000..83414f97 --- /dev/null +++ b/testdata/diff/privilege/issue_324_modify_grant_to_column/plan.sql @@ -0,0 +1,3 @@ +REVOKE UPDATE ON TABLE sometable FROM app_user; + +GRANT UPDATE (somecolumn) ON TABLE sometable TO app_user; diff --git a/testdata/diff/privilege/issue_324_modify_grant_to_column/plan.txt b/testdata/diff/privilege/issue_324_modify_grant_to_column/plan.txt new file mode 100644 index 00000000..cc6e4f2a --- /dev/null +++ b/testdata/diff/privilege/issue_324_modify_grant_to_column/plan.txt @@ -0,0 +1,18 @@ +Plan: 1 to add, 1 to modify. + +Summary by type: + privileges: 1 to modify + column privileges: 1 to add + +Privileges: + ~ app_user + +Column privileges: + + app_user + +DDL to be executed: +-------------------------------------------------- + +REVOKE UPDATE ON TABLE sometable FROM app_user; + +GRANT UPDATE (somecolumn) ON TABLE sometable TO app_user;