fix: drop triggers on dropped tables before their functions (#505)

[tianzhou]

Summary

Fixes #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").

• generateDropSQL already emitted DROP TRIGGER for triggers on modified tables/views before dropping functions
• But for dropped tables/views, triggers were never explicitly dropped — they relied on the later DROP TABLE CASCADE, which comes after DROP FUNCTION
• Added generateDropTriggersFromDroppedTables and generateDropTriggersFromDroppedViews to emit DROP TRIGGER for all triggers on dropped tables/views before the function drop phase

Test plan

• New fixture dependency/issue_505_drop_trigger_function_with_table — drops a table with trigger + its trigger function
• Existing dependency/function_to_trigger still passes (modified-table path unchanged)
• Diff test passes
• Integration test (plan-and-apply) passes

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes dependency-ordering during drops by ensuring triggers on relations that are being dropped are explicitly dropped before the function-drop phase, preventing SQLSTATE 2BP01 when a trigger function would otherwise still be referenced.

Changes:

• Emit DROP TRIGGER statements for triggers on dropped tables before DROP FUNCTION.
• Emit DROP TRIGGER statements for triggers on dropped views (skipping pre-dropped views) before DROP FUNCTION.
• Add a regression fixture for dropping a table with a trigger + its trigger function.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
internal/diff/trigger.go	Adds trigger-drop generation for dropped tables/views (pre-function drop ordering).
internal/diff/diff.go	Wires the new trigger-drop generation into the drop phase ordering.
testdata/diff/dependency/issue_505_drop_trigger_function_with_table/plan.txt	New expected plan output showing trigger dropped before function/table.
testdata/diff/dependency/issue_505_drop_trigger_function_with_table/plan.sql	New expected SQL plan statements for the regression scenario.
testdata/diff/dependency/issue_505_drop_trigger_function_with_table/plan.json	New expected JSON plan output including trigger-drop step ordering.
testdata/diff/dependency/issue_505_drop_trigger_function_with_table/old.sql	Schema setup with table + trigger + trigger function.
testdata/diff/dependency/issue_505_drop_trigger_function_with_table/new.sql	Target schema (empty) to force drops.
testdata/diff/dependency/issue_505_drop_trigger_function_with_table/diff.sql	Expected DDL diff showing trigger dropped before function/table.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[greptile-apps]

Greptile Summary

This PR fixes issue #505 by explicitly emitting DROP TRIGGER statements for triggers on dropped tables/views before DROP FUNCTION, closing the gap where DROP TABLE … CASCADE (which came after DROP FUNCTION) was relied upon to remove the trigger dependency.

• Adds generateDropTriggersFromDroppedTables and generateDropTriggersFromDroppedViews in trigger.go, following the same pattern as the existing generateDropTriggersFromModifiedTables/Views helpers, and calls them in generateDropSQL at the correct position (after modified-table trigger drops, before aggregate/function drops).
• Adds a new fixture dependency/issue_505_drop_trigger_function_with_table covering the table + trigger + trigger-function simultaneous drop case; plan.txt is missing a trailing newline that all sibling fixtures have.

Confidence Score: 4/5

Safe to merge — the fix is minimal, correctly positioned, and verified by a passing integration test; the only concerns are a non-stable sort for the rare case of same-named triggers across multiple dropped tables, and a missing trailing newline in one test fixture.

The core logic change is straightforward and well-tested. The sort-by-name-only ordering in both new functions is technically non-deterministic when multiple dropped tables share a trigger name, which could surface as flaky plan output in that edge case. The missing trailing newline in plan.txt is inconsistent with all other plan.txt fixtures and could cause test comparison failures if the fixture is regenerated.

internal/diff/trigger.go — sort comparator in both new functions; testdata/diff/dependency/issue_505_drop_trigger_function_with_table/plan.txt — missing trailing newline

Important Files Changed

Filename	Overview
internal/diff/trigger.go	Adds two new functions (generateDropTriggersFromDroppedTables, generateDropTriggersFromDroppedViews) that emit DROP TRIGGER before DROP FUNCTION for dropped relations; logic is correct and mirrors the existing modified-table helpers, but the sort key (name only) is not fully stable when multiple dropped tables/views share a trigger name.
internal/diff/diff.go	Two new calls inserted at the right position in generateDropSQL — after existing modified-table trigger drops and before aggregate/function drops — correctly fixing the dependency ordering for dropped tables/views.
testdata/diff/dependency/issue_505_drop_trigger_function_with_table/plan.txt	New fixture correctly documents expected plan output; missing trailing newline is inconsistent with other plan.txt files.
testdata/diff/dependency/issue_505_drop_trigger_function_with_table/diff.sql	Correctly captures the expected migration DDL: DROP TRIGGER → DROP FUNCTION → DROP TABLE.
testdata/diff/dependency/issue_505_drop_trigger_function_with_table/old.sql	Defines the starting schema (table + trigger function + trigger) that exercises the bug.
testdata/diff/dependency/issue_505_drop_trigger_function_with_table/new.sql	Empty desired state that drives all three objects being dropped.
testdata/diff/dependency/issue_505_drop_trigger_function_with_table/plan.json	Machine-readable plan fixture with correct step order: drop trigger, drop function, drop table.
testdata/diff/dependency/issue_505_drop_trigger_function_with_table/plan.sql	SQL plan fixture matching expected migration DDL.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[generateDropSQL] --> B[REVOKE privileges]
    B --> C[generateDropTriggersFromModifiedTables]
    C --> D[generateDropTriggersFromModifiedViews]
    D --> E["generateDropTriggersFromDroppedTables NEW"]
    E --> F["generateDropTriggersFromDroppedViews NEW"]
    F --> G[generateDropAggregatesSQL]
    G --> H[generateDropFunctionsSQL]
    H --> I[generateDropProceduresSQL]
    I --> J[generateDropViewsSQL]
    J --> K[generateDropTablesSQL]

    style E fill:#d4edda,stroke:#28a745
    style F fill:#d4edda,stroke:#28a745

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[generateDropSQL] --> B[REVOKE privileges]
    B --> C[generateDropTriggersFromModifiedTables]
    C --> D[generateDropTriggersFromModifiedViews]
    D --> E["generateDropTriggersFromDroppedTables NEW"]
    E --> F["generateDropTriggersFromDroppedViews NEW"]
    F --> G[generateDropAggregatesSQL]
    G --> H[generateDropFunctionsSQL]
    H --> I[generateDropProceduresSQL]
    I --> J[generateDropViewsSQL]
    J --> K[generateDropTablesSQL]

    style E fill:#d4edda,stroke:#28a745
    style F fill:#d4edda,stroke:#28a745

Loading

_{Reviews (1): Last reviewed commit: "fix: drop triggers on dropped tables bef..." | Re-trigger Greptile}

[tianzhou]

Re-checked the current head with green CI.

Verdict: ready to merge.

The remaining Greptile notes are non-blocking polish, not correctness issues in the shipped fix:

• the trigger-drop ordering bug from issue Can't drop trigger function #505 is fixed and covered by the new regression fixture
• the sort-by-name-only point is an edge-case determinism concern, not a demonstrated failure here
• the missing trailing newline in plan.txt is fixture hygiene only

From the reviewer side, I would merge this as-is and treat any sort-key tightening or newline cleanup as optional follow-up.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.