fix: INSTEAD OF triggers on views missing from dump (#287)#292
Merged
fix: INSTEAD OF triggers on views missing from dump (#287)#292
Conversation
INSTEAD OF triggers on views were not being extracted during schema inspection because buildTriggers() only checked the Tables map, not the Views map. This also fixes the latent INSTEAD_OF formatting bug (should be INSTEAD OF with a space) and adds proper schema normalization for view triggers during plan generation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes schema dumping/diffing gaps for INSTEAD OF triggers on views by carrying view triggers through the IR, inspector, diff engine, and dump formatter so they appear in dumps and migration plans.
Changes:
- Extend IR + inspector to associate triggers with views (not just tables) and avoid a view/trigger load race.
- Add
view.triggerdiff type and generate create/modify/drop SQL for view triggers (including correctINSTEAD OFformatting). - Add testdata coverage for issue #287 and normalize view-trigger schema references during plan generation.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
ir/ir.go |
Adds View.Triggers and fixes INSTEAD OF timing string. |
ir/inspector.go |
Ensures views load before triggers; stores triggers on either tables or views. |
internal/diff/diff.go |
Adds DiffTypeViewTrigger and diffs view triggers as part of view modifications. |
internal/diff/view.go |
Emits create/modify SQL for view triggers and computes trigger diffs between views. |
internal/diff/trigger.go |
Adds helpers to create/drop view-trigger diffs. |
internal/dump/formatter.go |
Places view.trigger diffs under views/ in multi-file dump mode. |
cmd/plan/plan.go |
Normalizes schema references inside view triggers during plan generation. |
testdata/diff/create_trigger/issue_287_instead_of_view_trigger/* |
Adds regression fixtures validating trigger diff/plan output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Normalize view triggers in normalizeView() for consistent dump output - Sort diffViewTriggers output deterministically to avoid flaky diffs - Move view trigger drops to early drop phase before function drops Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
buildTriggers()only checked theTablesmap, not theViewsmapTriggersfield to theViewIR struct and updated the inspector to associate triggers with views when the relation is a viewINSTEAD_OFformatting bug (should beINSTEAD OFwith a space) in theTriggerTimingInsteadOfconstantbuildTriggerscould run beforebuildViewspopulated the Views mapDiffTypeViewTriggertype for proper directory placement in multi-file dump mode and trigger diffing for view modificationsFixes #287
Test plan
testdata/diff/create_trigger/issue_287_instead_of_view_trigger/with old.sql (view without trigger) and new.sql (view with INSTEAD OF trigger)PGSCHEMA_TEST_FILTER="create_trigger/issue_287" go test -v ./internal/diff -run TestDiffFromFilesPGSCHEMA_TEST_FILTER="create_trigger/issue_287" go test -v ./cmd -run TestPlanAndApplygo test ./...🤖 Generated with Claude Code