fix: emit COMMENT and DISABLE for newly added table triggers

[tianzhou]

Fixes #515

Summary

• The added-trigger branch in table.go emitted CREATE TRIGGER but skipped COMMENT ON TRIGGER and ALTER TABLE DISABLE TRIGGER, causing trigger comments (and disabled state) to be silently dropped on first apply.
• Added the same comment/disabled-state handling that the modified-trigger and view-trigger paths already had.
• Amended the existing create_trigger/add_trigger test fixture to include a trigger comment, verifying the fix end-to-end.

Test plan

• PGSCHEMA_TEST_FILTER="create_trigger/add_trigger" go test ./internal/diff/ -run TestDiffFromFiles passes
• PGSCHEMA_TEST_FILTER="create_trigger/add_trigger" go test ./cmd/ -run TestPlanAndApply passes
• Full TestDiffFromFiles suite passes with no regressions

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR fixes a silent data-loss bug where COMMENT ON TRIGGER and ALTER TABLE DISABLE TRIGGER were not emitted for newly added table triggers. The added-trigger branch in table.go now mirrors the pattern already used by the modified-trigger path and the standalone trigger.go path.

• internal/diff/table.go: Adds generateTriggerComment and generateTriggerEnabledState calls after the CREATE TRIGGER statement in the AddedTriggers loop, consistent with the modify-trigger path (lines 1588/1594) and with trigger.go (lines 154/159).
• Test fixtures: The add_trigger fixture is extended with a COMMENT ON TRIGGER in new.sql, and all golden files (diff.sql, plan.sql, plan.json, plan.txt) are updated to reflect the expected output.

Confidence Score: 4/5

Safe to merge; the fix is a small, targeted addition that closes a silent omission in the add-trigger code path.

The code change is correct and directly mirrors three pre-existing patterns in the same codebase. The only gaps are a missing test for the disabled-trigger-on-add case and a dropped trailing newline in plan.txt. Neither affects correctness of the generated SQL.

testdata/diff/create_trigger/add_trigger/plan.txt — trailing newline was dropped during fixture regeneration.

Important Files Changed

Filename	Overview
internal/diff/table.go	Adds COMMENT ON TRIGGER and ALTER TABLE DISABLE TRIGGER emission for newly added triggers, matching the existing modified-trigger path and trigger.go patterns.
testdata/diff/create_trigger/add_trigger/plan.txt	Updated to show COMMENT statement; incidentally loses the trailing newline on the last line. Also exposes the pre-existing double-entry display for employees_last_modified_trigger (+ then ~).
testdata/diff/create_trigger/add_trigger/plan.json	Adds the expected COMMENT ON TRIGGER alter entry for employees_last_modified_trigger; structure is consistent with the rest of the plan.
testdata/diff/create_trigger/add_trigger/new.sql	Adds COMMENT ON TRIGGER to the new.sql fixture to drive the end-to-end comment test.
testdata/diff/create_trigger/add_trigger/diff.sql	Adds expected COMMENT ON TRIGGER line to the diff output fixture; correct.
testdata/diff/create_trigger/add_trigger/plan.sql	Adds expected COMMENT ON TRIGGER line to plan.sql; matches diff.sql, correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[AddedTriggers loop] --> B[generateTriggerSQLWithMode\nCREATE TRIGGER]
    B --> C[collector.collect\nDiffOperationCreate]
    C --> D{trigger.Comment != empty}
    D -- Yes --> E[generateTriggerComment\nCOMMENT ON TRIGGER\nDiffOperationAlter]
    D -- No --> F{trigger.Disabled}
    E --> F
    F -- Yes --> G[generateTriggerEnabledState\nALTER TABLE DISABLE TRIGGER\nDiffOperationAlter]
    F -- No --> H[Next trigger]
    G --> H

    style B fill:#90EE90
    style E fill:#90EE90
    style G fill:#90EE90

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[AddedTriggers loop] --> B[generateTriggerSQLWithMode\nCREATE TRIGGER]
    B --> C[collector.collect\nDiffOperationCreate]
    C --> D{trigger.Comment != empty}
    D -- Yes --> E[generateTriggerComment\nCOMMENT ON TRIGGER\nDiffOperationAlter]
    D -- No --> F{trigger.Disabled}
    E --> F
    F -- Yes --> G[generateTriggerEnabledState\nALTER TABLE DISABLE TRIGGER\nDiffOperationAlter]
    F -- No --> H[Next trigger]
    G --> H

    style B fill:#90EE90
    style E fill:#90EE90
    style G fill:#90EE90

Loading

_{Reviews (1): Last reviewed commit: "fix: emit COMMENT and DISABLE for newly ..." | Re-trigger Greptile}

[Copilot]

Pull request overview

This PR fixes a schema convergence bug where newly added table triggers were created but their associated COMMENT ON TRIGGER and disabled state (ALTER TABLE ... DISABLE TRIGGER) were not emitted on the first apply, causing immediate post-apply drift (Issue #515).

Changes:

• Emit COMMENT ON TRIGGER after CREATE TRIGGER for triggers in the AddedTriggers path in internal/diff/table.go.
• Emit ALTER TABLE ... DISABLE TRIGGER for newly added triggers that are marked disabled in the desired state.
• Update the existing create_trigger/add_trigger fixture to include a trigger comment and verify the end-to-end plan/apply output.

Reviewed changes

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

Show a summary per file

File	Description
internal/diff/table.go	Ensures newly added table triggers also emit comment and disabled-state DDL, matching the behavior of modified-trigger and view-trigger paths.
testdata/diff/create_trigger/add_trigger/new.sql	Adds a trigger comment to the desired-state fixture for coverage.
testdata/diff/create_trigger/add_trigger/diff.sql	Updates expected diff output to include COMMENT ON TRIGGER for the newly added trigger.
testdata/diff/create_trigger/add_trigger/plan.sql	Updates expected plan SQL to include the trigger comment statement.
testdata/diff/create_trigger/add_trigger/plan.txt	Updates expected human-readable plan output to reflect the additional trigger comment step.
testdata/diff/create_trigger/add_trigger/plan.json	Updates expected machine-readable plan output to include an alter step for the trigger comment.

---

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

[tianzhou]

Both Greptile (4/5) and Copilot reviewed — no generated comments, safe to merge.

Greptile noted two minor points:

1. Trailing newline in plan.txt — this matches the test runner's actual output (copied verbatim from plan_actual.txt), so it's consistent.
2. No test for disabled-trigger-on-add — fair point, but the code path is identical to the view-trigger path which is already exercised. Happy to add one if desired.