From 4d1fe88cb8ed4f8fb5158db6787529cefb3c590b Mon Sep 17 00:00:00 2001 From: tianzhou Date: Tue, 7 Jul 2026 01:04:56 -0700 Subject: [PATCH 1/3] fix: drop triggers on dropped tables before their functions (#505) When a table with a trigger and the trigger's function are both being dropped, pgschema emitted DROP FUNCTION before the trigger was removed, causing SQLSTATE 2BP01 ("cannot drop function because other objects depend on it"). Emit DROP TRIGGER for triggers on dropped tables and views before the function drop phase, mirroring what already existed for modified tables. Co-Authored-By: Claude Opus 4.6 --- internal/diff/diff.go | 6 ++ internal/diff/trigger.go | 65 +++++++++++++++++++ .../diff.sql | 5 ++ .../new.sql | 1 + .../old.sql | 20 ++++++ .../plan.json | 32 +++++++++ .../plan.sql | 5 ++ .../plan.txt | 21 ++++++ 8 files changed, 155 insertions(+) create mode 100644 testdata/diff/dependency/issue_505_drop_trigger_function_with_table/diff.sql create mode 100644 testdata/diff/dependency/issue_505_drop_trigger_function_with_table/new.sql create mode 100644 testdata/diff/dependency/issue_505_drop_trigger_function_with_table/old.sql create mode 100644 testdata/diff/dependency/issue_505_drop_trigger_function_with_table/plan.json create mode 100644 testdata/diff/dependency/issue_505_drop_trigger_function_with_table/plan.sql create mode 100644 testdata/diff/dependency/issue_505_drop_trigger_function_with_table/plan.txt diff --git a/internal/diff/diff.go b/internal/diff/diff.go index 8f9e9821..50a04215 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -2101,6 +2101,12 @@ func (d *ddlDiff) generateDropSQL(targetSchema string, collector *diffCollector, generateDropTriggersFromModifiedTables(d.modifiedTables, targetSchema, collector) generateDropTriggersFromModifiedViews(d.modifiedViews, targetSchema, collector, preDroppedViews) + // Drop triggers from tables and views that are themselves being dropped. + // This must happen before DROP FUNCTION so that trigger→function + // dependencies are removed first (#505). + generateDropTriggersFromDroppedTables(d.droppedTables, targetSchema, collector) + generateDropTriggersFromDroppedViews(d.droppedViews, targetSchema, collector, preDroppedViews) + // Drop aggregates before functions, since an aggregate depends on its // transition/final functions (issue #416). generateDropAggregatesSQL(d.droppedAggregates, targetSchema, collector) diff --git a/internal/diff/trigger.go b/internal/diff/trigger.go index c17763cc..c1105c11 100644 --- a/internal/diff/trigger.go +++ b/internal/diff/trigger.go @@ -234,6 +234,71 @@ func generateDropTriggersFromModifiedViews(views []*viewDiff, targetSchema strin } } +// generateDropTriggersFromDroppedTables emits DROP TRIGGER for all triggers on +// tables that are being dropped. This must run before DROP FUNCTION so that +// trigger→function dependencies are removed first (#505). +func generateDropTriggersFromDroppedTables(tables []*ir.Table, targetSchema string, collector *diffCollector) { + var allTriggers []*ir.Trigger + + for _, table := range tables { + for _, trigger := range table.Triggers { + allTriggers = append(allTriggers, trigger) + } + } + + sort.Slice(allTriggers, func(i, j int) bool { + return allTriggers[i].Name < allTriggers[j].Name + }) + + for _, trigger := range allTriggers { + tableName := getTableNameWithSchema(trigger.Schema, trigger.Table, targetSchema) + sql := fmt.Sprintf("DROP TRIGGER IF EXISTS %s ON %s;", ir.QuoteIdentifier(trigger.Name), tableName) + + context := &diffContext{ + Type: DiffTypeTableTrigger, + Operation: DiffOperationDrop, + Path: fmt.Sprintf("%s.%s.%s", trigger.Schema, trigger.Table, trigger.Name), + Source: trigger, + CanRunInTransaction: true, + } + collector.collect(context, sql) + } +} + +// generateDropTriggersFromDroppedViews emits DROP TRIGGER for all triggers on +// views that are being dropped (same rationale as dropped tables, #505). +func generateDropTriggersFromDroppedViews(views []*ir.View, targetSchema string, collector *diffCollector, preDroppedViews map[string]bool) { + var allTriggers []*ir.Trigger + + for _, view := range views { + viewKey := view.Schema + "." + view.Name + if preDroppedViews != nil && preDroppedViews[viewKey] { + continue + } + for _, trigger := range view.Triggers { + allTriggers = append(allTriggers, trigger) + } + } + + sort.Slice(allTriggers, func(i, j int) bool { + return allTriggers[i].Name < allTriggers[j].Name + }) + + for _, trigger := range allTriggers { + tableName := getTableNameWithSchema(trigger.Schema, trigger.Table, targetSchema) + sql := fmt.Sprintf("DROP TRIGGER IF EXISTS %s ON %s;", ir.QuoteIdentifier(trigger.Name), tableName) + + context := &diffContext{ + Type: DiffTypeViewTrigger, + Operation: DiffOperationDrop, + Path: fmt.Sprintf("%s.%s.%s", trigger.Schema, trigger.Table, trigger.Name), + Source: trigger, + CanRunInTransaction: true, + } + collector.collect(context, sql) + } +} + // generateTriggerSQLWithMode generates CREATE [OR REPLACE] TRIGGER or CREATE CONSTRAINT TRIGGER statement func generateTriggerSQLWithMode(trigger *ir.Trigger, targetSchema string, qualifySchema bool) string { // Build event list in standard order: INSERT, UPDATE, DELETE, TRUNCATE diff --git a/testdata/diff/dependency/issue_505_drop_trigger_function_with_table/diff.sql b/testdata/diff/dependency/issue_505_drop_trigger_function_with_table/diff.sql new file mode 100644 index 00000000..093b19d9 --- /dev/null +++ b/testdata/diff/dependency/issue_505_drop_trigger_function_with_table/diff.sql @@ -0,0 +1,5 @@ +DROP TRIGGER IF EXISTS events_notify ON events; + +DROP FUNCTION IF EXISTS notify_event(); + +DROP TABLE IF EXISTS events CASCADE; diff --git a/testdata/diff/dependency/issue_505_drop_trigger_function_with_table/new.sql b/testdata/diff/dependency/issue_505_drop_trigger_function_with_table/new.sql new file mode 100644 index 00000000..db77e35a --- /dev/null +++ b/testdata/diff/dependency/issue_505_drop_trigger_function_with_table/new.sql @@ -0,0 +1 @@ +-- empty: drop everything diff --git a/testdata/diff/dependency/issue_505_drop_trigger_function_with_table/old.sql b/testdata/diff/dependency/issue_505_drop_trigger_function_with_table/old.sql new file mode 100644 index 00000000..9d593316 --- /dev/null +++ b/testdata/diff/dependency/issue_505_drop_trigger_function_with_table/old.sql @@ -0,0 +1,20 @@ +CREATE TABLE events ( + id serial PRIMARY KEY, + payload jsonb NOT NULL, + created_at timestamp DEFAULT CURRENT_TIMESTAMP +); + +CREATE FUNCTION notify_event() +RETURNS trigger +LANGUAGE plpgsql +AS $$ +BEGIN + PERFORM pg_notify('events', NEW.id::text); + RETURN NEW; +END; +$$; + +CREATE TRIGGER events_notify + AFTER INSERT ON events + FOR EACH ROW + EXECUTE FUNCTION notify_event(); diff --git a/testdata/diff/dependency/issue_505_drop_trigger_function_with_table/plan.json b/testdata/diff/dependency/issue_505_drop_trigger_function_with_table/plan.json new file mode 100644 index 00000000..921d0894 --- /dev/null +++ b/testdata/diff/dependency/issue_505_drop_trigger_function_with_table/plan.json @@ -0,0 +1,32 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.12.0", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "b830414412e5c92476a85d494c5017028c5adcc5f7ead7b1322e85d7514349bc" + }, + "groups": [ + { + "steps": [ + { + "sql": "DROP TRIGGER IF EXISTS events_notify ON events;", + "type": "table.trigger", + "operation": "drop", + "path": "public.events.events_notify" + }, + { + "sql": "DROP FUNCTION IF EXISTS notify_event();", + "type": "function", + "operation": "drop", + "path": "public.notify_event" + }, + { + "sql": "DROP TABLE IF EXISTS events CASCADE;", + "type": "table", + "operation": "drop", + "path": "public.events" + } + ] + } + ] +} diff --git a/testdata/diff/dependency/issue_505_drop_trigger_function_with_table/plan.sql b/testdata/diff/dependency/issue_505_drop_trigger_function_with_table/plan.sql new file mode 100644 index 00000000..093b19d9 --- /dev/null +++ b/testdata/diff/dependency/issue_505_drop_trigger_function_with_table/plan.sql @@ -0,0 +1,5 @@ +DROP TRIGGER IF EXISTS events_notify ON events; + +DROP FUNCTION IF EXISTS notify_event(); + +DROP TABLE IF EXISTS events CASCADE; diff --git a/testdata/diff/dependency/issue_505_drop_trigger_function_with_table/plan.txt b/testdata/diff/dependency/issue_505_drop_trigger_function_with_table/plan.txt new file mode 100644 index 00000000..60bddc0a --- /dev/null +++ b/testdata/diff/dependency/issue_505_drop_trigger_function_with_table/plan.txt @@ -0,0 +1,21 @@ +Plan: 2 to drop. + +Summary by type: + functions: 1 to drop + tables: 1 to drop + +Functions: + - notify_event + +Tables: + - events + - events_notify (trigger) + +DDL to be executed: +-------------------------------------------------- + +DROP TRIGGER IF EXISTS events_notify ON events; + +DROP FUNCTION IF EXISTS notify_event(); + +DROP TABLE IF EXISTS events CASCADE; \ No newline at end of file From bdffa9a1a51293c377fa4c84943d03ba4b774ca1 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Tue, 7 Jul 2026 01:16:46 -0700 Subject: [PATCH 2/3] fix: use composite sort key for deterministic trigger drop ordering Sort dropped triggers by table name then trigger name, so output is stable when multiple dropped tables have same-named triggers. Co-Authored-By: Claude Opus 4.6 --- internal/diff/trigger.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/diff/trigger.go b/internal/diff/trigger.go index c1105c11..bc4882de 100644 --- a/internal/diff/trigger.go +++ b/internal/diff/trigger.go @@ -247,6 +247,9 @@ func generateDropTriggersFromDroppedTables(tables []*ir.Table, targetSchema stri } sort.Slice(allTriggers, func(i, j int) bool { + if allTriggers[i].Table != allTriggers[j].Table { + return allTriggers[i].Table < allTriggers[j].Table + } return allTriggers[i].Name < allTriggers[j].Name }) @@ -281,6 +284,9 @@ func generateDropTriggersFromDroppedViews(views []*ir.View, targetSchema string, } sort.Slice(allTriggers, func(i, j int) bool { + if allTriggers[i].Table != allTriggers[j].Table { + return allTriggers[i].Table < allTriggers[j].Table + } return allTriggers[i].Name < allTriggers[j].Name }) From a5791748302203ba9ce04c4df2d8b0951abc4438 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Tue, 7 Jul 2026 03:40:57 -0700 Subject: [PATCH 3/3] fix: include schema in trigger sort key for cross-schema determinism Co-Authored-By: Claude Opus 4.6 --- internal/diff/trigger.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/diff/trigger.go b/internal/diff/trigger.go index bc4882de..4a64ed13 100644 --- a/internal/diff/trigger.go +++ b/internal/diff/trigger.go @@ -247,6 +247,9 @@ func generateDropTriggersFromDroppedTables(tables []*ir.Table, targetSchema stri } sort.Slice(allTriggers, func(i, j int) bool { + if allTriggers[i].Schema != allTriggers[j].Schema { + return allTriggers[i].Schema < allTriggers[j].Schema + } if allTriggers[i].Table != allTriggers[j].Table { return allTriggers[i].Table < allTriggers[j].Table } @@ -284,6 +287,9 @@ func generateDropTriggersFromDroppedViews(views []*ir.View, targetSchema string, } sort.Slice(allTriggers, func(i, j int) bool { + if allTriggers[i].Schema != allTriggers[j].Schema { + return allTriggers[i].Schema < allTriggers[j].Schema + } if allTriggers[i].Table != allTriggers[j].Table { return allTriggers[i].Table < allTriggers[j].Table }