From 9c8ec279fd05ce9e90019c64930b81b4256d0e1e Mon Sep 17 00:00:00 2001 From: Azim Sonawalla Date: Wed, 14 Jan 2026 19:50:14 -0500 Subject: [PATCH] fix: emit REVOKE statements before DROP statements When dropping objects (functions, tables, etc.) that have explicit privileges granted, REVOKE statements must execute before DROP statements. Previously, REVOKEs were emitted after DROPs, causing failures because the object no longer exists. Reorder generateDropSQL() to emit privilege revocations first, then object drops. --- internal/diff/diff.go | 18 ++++++------------ .../create_function/drop_function/diff.sql | 1 + .../diff/create_function/drop_function/old.sql | 12 +++++++++++- .../create_function/drop_function/plan.json | 8 +++++++- .../create_function/drop_function/plan.sql | 2 ++ .../create_function/drop_function/plan.txt | 8 +++++++- 6 files changed, 34 insertions(+), 15 deletions(-) diff --git a/internal/diff/diff.go b/internal/diff/diff.go index b34d5c52..07c05465 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -1513,6 +1513,12 @@ func (d *ddlDiff) generateModifySQL(targetSchema string, collector *diffCollecto // preDroppedViews contains views that were already dropped in the pre-drop phase func (d *ddlDiff) generateDropSQL(targetSchema string, collector *diffCollector, preDroppedViews map[string]bool) { + // REVOKE privileges BEFORE dropping objects (objects must exist for REVOKE to succeed) + generateRestoreDefaultPrivilegesSQL(d.droppedRevokedDefaultPrivs, targetSchema, collector) + generateDropColumnPrivilegesSQL(d.droppedColumnPrivileges, targetSchema, collector) + generateDropPrivilegesSQL(d.droppedPrivileges, targetSchema, collector) + generateDropDefaultPrivilegesSQL(d.droppedDefaultPrivileges, targetSchema, collector) + // Drop triggers from modified tables first (triggers depend on functions) generateDropTriggersFromModifiedTables(d.modifiedTables, targetSchema, collector) @@ -1535,18 +1541,6 @@ func (d *ddlDiff) generateDropSQL(targetSchema string, collector *diffCollector, // Drop types generateDropTypesSQL(d.droppedTypes, targetSchema, collector) - // Restore default PUBLIC privileges (dropped revokes = restore defaults) - generateRestoreDefaultPrivilegesSQL(d.droppedRevokedDefaultPrivs, targetSchema, collector) - - // Drop column-level privileges - generateDropColumnPrivilegesSQL(d.droppedColumnPrivileges, targetSchema, collector) - - // Drop explicit object privileges - generateDropPrivilegesSQL(d.droppedPrivileges, targetSchema, collector) - - // Drop default privileges - generateDropDefaultPrivilegesSQL(d.droppedDefaultPrivileges, targetSchema, collector) - // Drop schemas // Note: Schema deletion is out of scope for schema-level comparisons } diff --git a/testdata/diff/create_function/drop_function/diff.sql b/testdata/diff/create_function/drop_function/diff.sql index ecdbb813..827c3d35 100644 --- a/testdata/diff/create_function/drop_function/diff.sql +++ b/testdata/diff/create_function/drop_function/diff.sql @@ -1,3 +1,4 @@ +REVOKE EXECUTE ON FUNCTION process_order(order_id integer, discount_percent numeric) FROM api_role; DROP FUNCTION IF EXISTS get_user_stats(integer); DROP FUNCTION IF EXISTS process_order(integer, numeric); DROP FUNCTION IF EXISTS process_payment(integer, text); diff --git a/testdata/diff/create_function/drop_function/old.sql b/testdata/diff/create_function/drop_function/old.sql index a5b0db11..f24ec9a3 100644 --- a/testdata/diff/create_function/drop_function/old.sql +++ b/testdata/diff/create_function/drop_function/old.sql @@ -54,4 +54,14 @@ BEGIN status := 'SUCCESS'; processed_at := NOW(); END; -$$; \ No newline at end of file +$$; + +-- Role and grant for testing REVOKE ordering +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'api_role') THEN + CREATE ROLE api_role; + END IF; +END $$; + +GRANT EXECUTE ON FUNCTION process_order(integer, numeric) TO api_role; \ No newline at end of file diff --git a/testdata/diff/create_function/drop_function/plan.json b/testdata/diff/create_function/drop_function/plan.json index a01e9166..1421134f 100644 --- a/testdata/diff/create_function/drop_function/plan.json +++ b/testdata/diff/create_function/drop_function/plan.json @@ -3,11 +3,17 @@ "pgschema_version": "1.6.0", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { - "hash": "1228242f86da4b19cafc006075668cd3e73588c01012241f6e72ed02432a158b" + "hash": "755b64b40a0ad2e6918bb3ff23f9b9ac6936c38ff588b5ad7ddc98bbf36315de" }, "groups": [ { "steps": [ + { + "sql": "REVOKE EXECUTE ON FUNCTION process_order(order_id integer, discount_percent numeric) FROM api_role;", + "type": "privilege", + "operation": "drop", + "path": "privileges.FUNCTION.process_order(order_id integer, discount_percent numeric).api_role" + }, { "sql": "DROP FUNCTION IF EXISTS get_user_stats(integer);", "type": "function", diff --git a/testdata/diff/create_function/drop_function/plan.sql b/testdata/diff/create_function/drop_function/plan.sql index 6c482a5a..7d2aa9cc 100644 --- a/testdata/diff/create_function/drop_function/plan.sql +++ b/testdata/diff/create_function/drop_function/plan.sql @@ -1,3 +1,5 @@ +REVOKE EXECUTE ON FUNCTION process_order(order_id integer, discount_percent numeric) FROM api_role; + DROP FUNCTION IF EXISTS get_user_stats(integer); DROP FUNCTION IF EXISTS process_order(integer, numeric); diff --git a/testdata/diff/create_function/drop_function/plan.txt b/testdata/diff/create_function/drop_function/plan.txt index b5f5e5b6..814100ab 100644 --- a/testdata/diff/create_function/drop_function/plan.txt +++ b/testdata/diff/create_function/drop_function/plan.txt @@ -1,16 +1,22 @@ -Plan: 3 to drop. +Plan: 4 to drop. Summary by type: functions: 3 to drop + privileges: 1 to drop Functions: - get_user_stats - process_order - process_payment +Privileges: + - api_role + DDL to be executed: -------------------------------------------------- +REVOKE EXECUTE ON FUNCTION process_order(order_id integer, discount_percent numeric) FROM api_role; + DROP FUNCTION IF EXISTS get_user_stats(integer); DROP FUNCTION IF EXISTS process_order(integer, numeric);