fix: handle dependent views when recreating materialized views #272
Conversation
When a materialized view needs to be dropped and recreated (e.g., adding a column), pgschema now properly handles dependent regular views by: 1. Dropping dependent views before dropping the materialized view 2. Recreating dependent views after recreating the materialized view This fixes the error "cannot drop materialized view because other objects depend on it" that occurred when views depended on a materialized view being modified. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This pull request addresses issue #268 by implementing support for handling dependent regular views when a materialized view needs to be dropped and recreated (e.g., when adding a column). The implementation detects views that directly depend on materialized views being modified and drops them before dropping the materialized view, then recreates them afterward.
Changes:
- Added dependency detection logic to find regular views that depend on materialized views being recreated
- Modified view recreation logic to drop dependent views with CASCADE before dropping the materialized view, then recreate them afterward
- Updated test case to verify dependent view handling
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/diff/view.go | Added dependentViewsContext type and methods for tracking dependent views; added viewDependsOnMaterializedView for detecting dependencies; modified generateModifyViewsSQL to drop and recreate dependent views around materialized view recreation |
| internal/diff/diff.go | Added allNewViews field to ddlDiff to store all views from new state; modified generateModifySQL to find and pass dependent views context to view generation |
| testdata/diff/create_materialized_view/drop_materialized_view/* | Updated test to include a regular view that depends on the materialized view being modified; updated expected outputs to show dependent view being dropped and recreated |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1. Use RESTRICT instead of CASCADE when dropping dependent views to fail safely if there are transitive dependencies we haven't tracked 2. Track recreated views to avoid duplicate processing when a dependent view is also independently modified 3. Fix misleading comment about allViews parameter Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1. Use word boundary regex for identifier matching to avoid false positives (e.g., "user" no longer matches "users" or "user_id") 2. Implement transitive dependency tracking - now finds views that depend on views that depend on the materialized view 3. Topologically sort dependent views before dropping/recreating to ensure correct order when views have nested dependencies 4. Sort modifiedViews to process materialized views with RequiresRecreate first, ensuring dependent views are tracked before their own modifications would be processed Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Drop dependent views first (in reverse order to handle nested dependencies). | ||
| // We use RESTRICT (not CASCADE) to fail safely if there are transitive | ||
| // dependencies that we haven't tracked. This prevents silently dropping | ||
| // views that wouldn't be recreated. | ||
| for i := len(dependentViews) - 1; i >= 0; i-- { | ||
| depView := dependentViews[i] | ||
| depViewName := qualifyEntityName(depView.Schema, depView.Name, targetSchema) | ||
| dropDepSQL := fmt.Sprintf("DROP VIEW IF EXISTS %s RESTRICT;", depViewName) | ||
|
|
||
| depContext := &diffContext{ | ||
| Type: DiffTypeView, | ||
| Operation: DiffOperationRecreate, | ||
| Path: fmt.Sprintf("%s.%s", depView.Schema, depView.Name), | ||
| Source: depView, | ||
| CanRunInTransaction: true, | ||
| } | ||
| collector.collect(depContext, dropDepSQL) | ||
| } |
There was a problem hiding this comment.
If a view depends on multiple materialized views that are all being recreated, the current implementation will attempt to drop and recreate the dependent view multiple times (once for each materialized view it depends on). This could cause failures when recreating the view after the first materialized view is processed, because the view may reference other materialized views that haven't been recreated yet. While this edge case isn't covered in issue #268, consider handling it in a future enhancement by collecting all dependent views across all materialized views and processing them together.
1. Check both unqualified and schema-qualified names when detecting transitive view dependencies (e.g., "view_a" and "public.view_a") 2. Log regexp errors instead of silently ignoring them in containsIdentifier() Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…views When a view depends on multiple materialized views that are all being recreated in the same migration, the view was being dropped and recreated multiple times (once per mat view). Now we track dropped dependent views and skip redundant operations. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
When a view depends on multiple materialized views that are all being recreated, the old approach would fail: 1. Drop view_a, drop/recreate mat_view_1, recreate view_a 2. Try to drop mat_view_2 → FAILS (view_a now depends on it) New two-phase approach: 1. Phase 1: Drop all dependent views, then drop/recreate all mat views 2. Phase 2: Recreate all dependent views AFTER all mat views are done This ensures views depending on multiple mat views are only recreated once all their dependencies are available. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
testdata/diff/create_materialized_view/drop_materialized_view/plan.txt
Outdated
Show resolved
Hide resolved
For schema-qualified identifiers like "public.active_employees", the previous pattern could incorrectly match "other.public.active_employees" because the dot before "public" satisfied the [^a-z0-9_] requirement. Now treats dots as part of the word boundary for qualified identifiers to avoid false positives in longer qualified paths. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Enhanced test case now covers: 1. Multi-matview dependency: employee_summary depends on BOTH active_employees AND dept_stats materialized views 2. Nested dependencies: employee_ids -> employee_names -> active_employees Expected behavior: - Drop nested deps first (employee_ids, employee_names) - Drop multi-matview dep (employee_summary) - Drop/recreate all materialized views - Recreate all dependent views AFTER all mat views are done NOTE: Expected output files are placeholders - need regeneration when embedded postgres is available. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When a NEW view V depends on a materialized view M being recreated, V was incorrectly included in the dependents list. This caused V to be: 1. Created in CREATE phase 2. Dropped and recreated in MODIFY phase (incorrectly) Now newly added views are excluded from the dependents list since they will be created in the CREATE phase after all mat views are recreated. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1. Regex pattern: Use [^a-zA-Z0-9_] instead of [^a-z0-9_] to properly exclude uppercase letters as word boundaries. Previously "FROMactive_employees" would incorrectly match "active_employees". 2. Topological sort: Re-sort allDependentViewsToRecreate in Phase 2 to ensure correct order when views from different mat view dependency lists have cross-dependencies (e.g., V3 from mat_B depends on V1 from mat_A). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 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.
1 participant
Fix #268
When a materialized view needs to be dropped and recreated (e.g., adding a column), pgschema now properly handles dependent regular views by:
This fixes the error "cannot drop materialized view because other objects depend on it" that occurred when views depended on a materialized view being modified.