From 8ef04f9ee93c6727462622e32c7e21378df9ff10 Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Fri, 30 Jan 2026 07:09:43 -0800 Subject: [PATCH 01/11] fix: handle dependent views when recreating materialized views (#268) 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 --- internal/diff/diff.go | 9 +- internal/diff/view.go | 121 +++++++++++++++++- .../drop_materialized_view/diff.sql | 12 ++ .../drop_materialized_view/new.sql | 19 ++- .../drop_materialized_view/old.sql | 11 +- .../drop_materialized_view/plan.json | 22 +++- .../drop_materialized_view/plan.sql | 15 +++ .../drop_materialized_view/plan.txt | 21 ++- 8 files changed, 220 insertions(+), 10 deletions(-) diff --git a/internal/diff/diff.go b/internal/diff/diff.go index 2c5b6559..7df1cdac 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -259,6 +259,7 @@ type ddlDiff struct { addedViews []*ir.View droppedViews []*ir.View modifiedViews []*viewDiff + allNewViews map[string]*ir.View // All views from new state (for dependent view handling) addedFunctions []*ir.Function droppedFunctions []*ir.Function modifiedFunctions []*functionDiff @@ -882,6 +883,9 @@ func GenerateMigration(oldIR, newIR *ir.IR, targetSchema string) []Diff { } } + // Store all new views for dependent view handling (issue #268) + diff.allNewViews = newViews + // Compare sequences across all schemas oldSequences := make(map[string]*ir.Sequence) newSequences := make(map[string]*ir.Sequence) @@ -1581,8 +1585,11 @@ func (d *ddlDiff) generateModifySQL(targetSchema string, collector *diffCollecto // Modify tables generateModifyTablesSQL(d.modifiedTables, targetSchema, collector) + // Find views that depend on materialized views being recreated (issue #268) + dependentViewsCtx := findDependentViewsForMatViews(d.allNewViews, d.modifiedViews) + // Modify views - pass preDroppedViews to skip DROP for already-dropped views - generateModifyViewsSQL(d.modifiedViews, targetSchema, collector, preDroppedViews) + generateModifyViewsSQL(d.modifiedViews, targetSchema, collector, preDroppedViews, dependentViewsCtx) // Modify functions generateModifyFunctionsSQL(d.modifiedFunctions, targetSchema, collector) diff --git a/internal/diff/view.go b/internal/diff/view.go index 949a7326..b4f3a622 100644 --- a/internal/diff/view.go +++ b/internal/diff/view.go @@ -80,13 +80,36 @@ func generateCreateViewsSQL(views []*ir.View, targetSchema string, collector *di // generateModifyViewsSQL generates CREATE OR REPLACE VIEW statements or comment changes // preDroppedViews contains views that were already dropped in the pre-drop phase -func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *diffCollector, preDroppedViews map[string]bool) { +// dependentViewsCtx contains views that depend on materialized views being recreated +func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *diffCollector, preDroppedViews map[string]bool, dependentViewsCtx *dependentViewsContext) { for _, diff := range diffs { // Handle materialized views that require recreation (DROP + CREATE) if diff.RequiresRecreate { viewKey := diff.New.Schema + "." + diff.New.Name viewName := qualifyEntityName(diff.New.Schema, diff.New.Name, targetSchema) + // Get dependent views for this materialized view + var dependentViews []*ir.View + if dependentViewsCtx != nil { + dependentViews = dependentViewsCtx.GetDependents(viewKey) + } + + // Drop dependent views first (in reverse order to handle nested dependencies) + 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 CASCADE;", depViewName) + + depContext := &diffContext{ + Type: DiffTypeView, + Operation: DiffOperationRecreate, + Path: fmt.Sprintf("%s.%s", depView.Schema, depView.Name), + Source: depView, + CanRunInTransaction: true, + } + collector.collect(depContext, dropDepSQL) + } + // Check if already pre-dropped if preDroppedViews != nil && preDroppedViews[viewKey] { // Skip DROP, only CREATE since view was already dropped in pre-drop phase @@ -127,6 +150,34 @@ func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *d collector.collectStatements(context, statements) } + // Recreate dependent views (in original order) + for _, depView := range dependentViews { + createDepSQL := generateViewSQL(depView, targetSchema) + + depContext := &diffContext{ + Type: DiffTypeView, + Operation: DiffOperationRecreate, + Path: fmt.Sprintf("%s.%s", depView.Schema, depView.Name), + Source: depView, + CanRunInTransaction: true, + } + collector.collect(depContext, createDepSQL) + + // Recreate view comment if present + if depView.Comment != "" { + depViewName := qualifyEntityName(depView.Schema, depView.Name, targetSchema) + commentSQL := fmt.Sprintf("COMMENT ON VIEW %s IS %s;", depViewName, quoteString(depView.Comment)) + commentContext := &diffContext{ + Type: DiffTypeViewComment, + Operation: DiffOperationCreate, + Path: fmt.Sprintf("%s.%s", depView.Schema, depView.Name), + Source: depView, + CanRunInTransaction: true, + } + collector.collect(commentContext, commentSQL) + } + } + // Add view comment if present if diff.New.Comment != "" { sql := fmt.Sprintf("COMMENT ON MATERIALIZED VIEW %s IS %s;", viewName, quoteString(diff.New.Comment)) @@ -371,3 +422,71 @@ func viewDependsOnTable(view *ir.View, tableSchema, tableName string) bool { return false } + +// viewDependsOnMaterializedView checks if a regular view depends on a materialized view +func viewDependsOnMaterializedView(view *ir.View, matViewSchema, matViewName string) bool { + if view == nil || view.Definition == "" || view.Materialized { + return false + } + + def := strings.ToLower(view.Definition) + matViewNameLower := strings.ToLower(matViewName) + + // Check for unqualified name + if strings.Contains(def, matViewNameLower) { + return true + } + + // Check for qualified name (schema.matview) + qualifiedName := strings.ToLower(matViewSchema + "." + matViewName) + if strings.Contains(def, qualifiedName) { + return true + } + + return false +} + +// dependentViewsContext tracks views that depend on materialized views being recreated +type dependentViewsContext struct { + // dependents maps materialized view key (schema.name) to list of dependent regular views + dependents map[string][]*ir.View +} + +// newDependentViewsContext creates a new context for tracking dependent views +func newDependentViewsContext() *dependentViewsContext { + return &dependentViewsContext{ + dependents: make(map[string][]*ir.View), + } +} + +// GetDependents returns the list of views that depend on the given materialized view +func (ctx *dependentViewsContext) GetDependents(matViewKey string) []*ir.View { + if ctx == nil || ctx.dependents == nil { + return nil + } + return ctx.dependents[matViewKey] +} + +// findDependentViewsForMatViews finds all regular views that depend on materialized views being recreated +// allViews contains all views (both old and new state - we use new state for recreation) +// modifiedViews contains the materialized views being recreated +func findDependentViewsForMatViews(allViews map[string]*ir.View, modifiedViews []*viewDiff) *dependentViewsContext { + ctx := newDependentViewsContext() + + for _, viewDiff := range modifiedViews { + if !viewDiff.RequiresRecreate || !viewDiff.New.Materialized { + continue + } + + matViewKey := viewDiff.New.Schema + "." + viewDiff.New.Name + + // Find all regular views that depend on this materialized view + for _, view := range allViews { + if viewDependsOnMaterializedView(view, viewDiff.New.Schema, viewDiff.New.Name) { + ctx.dependents[matViewKey] = append(ctx.dependents[matViewKey], view) + } + } + } + + return ctx +} diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/diff.sql b/testdata/diff/create_materialized_view/drop_materialized_view/diff.sql index 2f333438..496406e3 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/diff.sql +++ b/testdata/diff/create_materialized_view/drop_materialized_view/diff.sql @@ -1 +1,13 @@ +DROP VIEW IF EXISTS employee_summary CASCADE; DROP MATERIALIZED VIEW active_employees RESTRICT; +CREATE MATERIALIZED VIEW IF NOT EXISTS active_employees AS + SELECT id, + name, + salary, + 'active'::text AS status_label + FROM employees + WHERE status::text = 'active'::text; +CREATE OR REPLACE VIEW employee_summary AS + SELECT id, + name + FROM active_employees; diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/new.sql b/testdata/diff/create_materialized_view/drop_materialized_view/new.sql index 1c48eaf3..a8840a22 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/new.sql +++ b/testdata/diff/create_materialized_view/drop_materialized_view/new.sql @@ -3,4 +3,21 @@ CREATE TABLE public.employees ( name VARCHAR(100) NOT NULL, salary DECIMAL(10,2) NOT NULL, status VARCHAR(20) NOT NULL -); \ No newline at end of file +); + +-- Modified materialized view with new column (requires drop and recreate) +CREATE MATERIALIZED VIEW public.active_employees AS +SELECT + id, + name, + salary, + 'active' AS status_label -- New column forces drop/recreate +FROM employees +WHERE status = 'active'; + +-- View that depends on the materialized view (reproduces issue #268) +CREATE VIEW public.employee_summary AS +SELECT + id, + name +FROM active_employees; diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/old.sql b/testdata/diff/create_materialized_view/drop_materialized_view/old.sql index 03fa883b..b04f63d6 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/old.sql +++ b/testdata/diff/create_materialized_view/drop_materialized_view/old.sql @@ -6,9 +6,16 @@ CREATE TABLE public.employees ( ); CREATE MATERIALIZED VIEW public.active_employees AS -SELECT +SELECT id, name, salary FROM employees -WHERE status = 'active'; \ No newline at end of file +WHERE status = 'active'; + +-- View that depends on the materialized view (reproduces issue #268) +CREATE VIEW public.employee_summary AS +SELECT + id, + name +FROM active_employees; diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/plan.json b/testdata/diff/create_materialized_view/drop_materialized_view/plan.json index b7fd2465..5c9e9568 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/plan.json +++ b/testdata/diff/create_materialized_view/drop_materialized_view/plan.json @@ -3,16 +3,34 @@ "pgschema_version": "1.6.1", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { - "hash": "731eb3ecfccfda8ea697e2def6e8a6ef1e3a5abd0707741f23aa416f83db54ab" + "hash": "10f8c720df8d669c175bf74e48ef73b6aa302b4416fc331e47236b6beb995cf9" }, "groups": [ { "steps": [ + { + "sql": "DROP VIEW IF EXISTS employee_summary CASCADE;", + "type": "view", + "operation": "recreate", + "path": "public.employee_summary" + }, { "sql": "DROP MATERIALIZED VIEW active_employees RESTRICT;", "type": "materialized_view", - "operation": "drop", + "operation": "alter", "path": "public.active_employees" + }, + { + "sql": "CREATE MATERIALIZED VIEW IF NOT EXISTS active_employees AS\n SELECT id,\n name,\n salary,\n 'active'::text AS status_label\n FROM employees\n WHERE status::text = 'active'::text;", + "type": "materialized_view", + "operation": "alter", + "path": "public.active_employees" + }, + { + "sql": "CREATE OR REPLACE VIEW employee_summary AS\n SELECT id,\n name\n FROM active_employees;", + "type": "view", + "operation": "recreate", + "path": "public.employee_summary" } ] } diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/plan.sql b/testdata/diff/create_materialized_view/drop_materialized_view/plan.sql index 2f333438..81554569 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/plan.sql +++ b/testdata/diff/create_materialized_view/drop_materialized_view/plan.sql @@ -1 +1,16 @@ +DROP VIEW IF EXISTS employee_summary CASCADE; + DROP MATERIALIZED VIEW active_employees RESTRICT; + +CREATE MATERIALIZED VIEW IF NOT EXISTS active_employees AS + SELECT id, + name, + salary, + 'active'::text AS status_label + FROM employees + WHERE status::text = 'active'::text; + +CREATE OR REPLACE VIEW employee_summary AS + SELECT id, + name + FROM active_employees; diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/plan.txt b/testdata/diff/create_materialized_view/drop_materialized_view/plan.txt index 1cdbe82c..ca0515e5 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/plan.txt +++ b/testdata/diff/create_materialized_view/drop_materialized_view/plan.txt @@ -1,12 +1,27 @@ -Plan: 1 to drop. +Plan: 1 to modify. Summary by type: - materialized views: 1 to drop + materialized views: 1 to modify Materialized views: - - active_employees + ~ active_employees DDL to be executed: -------------------------------------------------- +DROP VIEW IF EXISTS employee_summary CASCADE; + DROP MATERIALIZED VIEW active_employees RESTRICT; + +CREATE MATERIALIZED VIEW IF NOT EXISTS active_employees AS + SELECT id, + name, + salary, + 'active'::text AS status_label + FROM employees + WHERE status::text = 'active'::text; + +CREATE OR REPLACE VIEW employee_summary AS + SELECT id, + name + FROM active_employees; From facb2fbdbb0a67e28297f6ebcf53219cbd291827 Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Fri, 30 Jan 2026 07:24:30 -0800 Subject: [PATCH 02/11] fix: address PR review feedback for issue #268 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 --- internal/diff/diff.go | 5 +++- internal/diff/view.go | 24 +++++++++++++++---- .../drop_materialized_view/diff.sql | 2 +- .../drop_materialized_view/plan.json | 2 +- .../drop_materialized_view/plan.sql | 2 +- .../drop_materialized_view/plan.txt | 2 +- 6 files changed, 28 insertions(+), 9 deletions(-) diff --git a/internal/diff/diff.go b/internal/diff/diff.go index 7df1cdac..32ab5f7b 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -1588,8 +1588,11 @@ func (d *ddlDiff) generateModifySQL(targetSchema string, collector *diffCollecto // Find views that depend on materialized views being recreated (issue #268) dependentViewsCtx := findDependentViewsForMatViews(d.allNewViews, d.modifiedViews) + // Track views recreated as dependencies to avoid duplicate processing + recreatedViews := make(map[string]bool) + // Modify views - pass preDroppedViews to skip DROP for already-dropped views - generateModifyViewsSQL(d.modifiedViews, targetSchema, collector, preDroppedViews, dependentViewsCtx) + generateModifyViewsSQL(d.modifiedViews, targetSchema, collector, preDroppedViews, dependentViewsCtx, recreatedViews) // Modify functions generateModifyFunctionsSQL(d.modifiedFunctions, targetSchema, collector) diff --git a/internal/diff/view.go b/internal/diff/view.go index b4f3a622..993f0c02 100644 --- a/internal/diff/view.go +++ b/internal/diff/view.go @@ -81,7 +81,8 @@ func generateCreateViewsSQL(views []*ir.View, targetSchema string, collector *di // generateModifyViewsSQL generates CREATE OR REPLACE VIEW statements or comment changes // preDroppedViews contains views that were already dropped in the pre-drop phase // dependentViewsCtx contains views that depend on materialized views being recreated -func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *diffCollector, preDroppedViews map[string]bool, dependentViewsCtx *dependentViewsContext) { +// recreatedViews tracks views that were recreated as dependencies (to avoid duplicate processing) +func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *diffCollector, preDroppedViews map[string]bool, dependentViewsCtx *dependentViewsContext, recreatedViews map[string]bool) { for _, diff := range diffs { // Handle materialized views that require recreation (DROP + CREATE) if diff.RequiresRecreate { @@ -94,11 +95,14 @@ func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *d dependentViews = dependentViewsCtx.GetDependents(viewKey) } - // Drop dependent views first (in reverse order to handle nested dependencies) + // 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 CASCADE;", depViewName) + dropDepSQL := fmt.Sprintf("DROP VIEW IF EXISTS %s RESTRICT;", depViewName) depContext := &diffContext{ Type: DiffTypeView, @@ -152,6 +156,7 @@ func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *d // Recreate dependent views (in original order) for _, depView := range dependentViews { + depViewKey := depView.Schema + "." + depView.Name createDepSQL := generateViewSQL(depView, targetSchema) depContext := &diffContext{ @@ -163,6 +168,11 @@ func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *d } collector.collect(depContext, createDepSQL) + // Track this view as recreated to avoid duplicate processing + if recreatedViews != nil { + recreatedViews[depViewKey] = true + } + // Recreate view comment if present if depView.Comment != "" { depViewName := qualifyEntityName(depView.Schema, depView.Name, targetSchema) @@ -203,6 +213,12 @@ func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *d continue // Skip the normal processing for this view } + // Skip views that were already recreated as dependencies of a materialized view + viewKey := diff.New.Schema + "." + diff.New.Name + if recreatedViews != nil && recreatedViews[viewKey] { + continue + } + // Check if only the comment changed and definition is identical // Both IRs come from pg_get_viewdef() at the same PostgreSQL version, so string comparison is sufficient definitionsEqual := diff.Old.Definition == diff.New.Definition @@ -468,7 +484,7 @@ func (ctx *dependentViewsContext) GetDependents(matViewKey string) []*ir.View { } // findDependentViewsForMatViews finds all regular views that depend on materialized views being recreated -// allViews contains all views (both old and new state - we use new state for recreation) +// allViews contains all views from the new state (used for dependency analysis and recreation) // modifiedViews contains the materialized views being recreated func findDependentViewsForMatViews(allViews map[string]*ir.View, modifiedViews []*viewDiff) *dependentViewsContext { ctx := newDependentViewsContext() diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/diff.sql b/testdata/diff/create_materialized_view/drop_materialized_view/diff.sql index 496406e3..ae0cf785 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/diff.sql +++ b/testdata/diff/create_materialized_view/drop_materialized_view/diff.sql @@ -1,4 +1,4 @@ -DROP VIEW IF EXISTS employee_summary CASCADE; +DROP VIEW IF EXISTS employee_summary RESTRICT; DROP MATERIALIZED VIEW active_employees RESTRICT; CREATE MATERIALIZED VIEW IF NOT EXISTS active_employees AS SELECT id, diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/plan.json b/testdata/diff/create_materialized_view/drop_materialized_view/plan.json index 5c9e9568..92573637 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/plan.json +++ b/testdata/diff/create_materialized_view/drop_materialized_view/plan.json @@ -9,7 +9,7 @@ { "steps": [ { - "sql": "DROP VIEW IF EXISTS employee_summary CASCADE;", + "sql": "DROP VIEW IF EXISTS employee_summary RESTRICT;", "type": "view", "operation": "recreate", "path": "public.employee_summary" diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/plan.sql b/testdata/diff/create_materialized_view/drop_materialized_view/plan.sql index 81554569..58bb95a8 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/plan.sql +++ b/testdata/diff/create_materialized_view/drop_materialized_view/plan.sql @@ -1,4 +1,4 @@ -DROP VIEW IF EXISTS employee_summary CASCADE; +DROP VIEW IF EXISTS employee_summary RESTRICT; DROP MATERIALIZED VIEW active_employees RESTRICT; diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/plan.txt b/testdata/diff/create_materialized_view/drop_materialized_view/plan.txt index ca0515e5..4d810b3a 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/plan.txt +++ b/testdata/diff/create_materialized_view/drop_materialized_view/plan.txt @@ -9,7 +9,7 @@ Materialized views: DDL to be executed: -------------------------------------------------- -DROP VIEW IF EXISTS employee_summary CASCADE; +DROP VIEW IF EXISTS employee_summary RESTRICT; DROP MATERIALIZED VIEW active_employees RESTRICT; From a26a1f34730ab3cc1f928840380872ddd4da4e2e Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Fri, 30 Jan 2026 07:37:44 -0800 Subject: [PATCH 03/11] fix: address additional PR review feedback for issue #268 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 --- internal/diff/diff.go | 5 ++ internal/diff/view.go | 121 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 113 insertions(+), 13 deletions(-) diff --git a/internal/diff/diff.go b/internal/diff/diff.go index 32ab5f7b..0e1e398c 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -1591,6 +1591,11 @@ func (d *ddlDiff) generateModifySQL(targetSchema string, collector *diffCollecto // Track views recreated as dependencies to avoid duplicate processing recreatedViews := make(map[string]bool) + // Sort modifiedViews to process materialized views with RequiresRecreate first. + // This ensures dependent views are added to recreatedViews before their own + // modifications would be processed (and correctly skipped). + sortModifiedViewsForProcessing(d.modifiedViews) + // Modify views - pass preDroppedViews to skip DROP for already-dropped views generateModifyViewsSQL(d.modifiedViews, targetSchema, collector, preDroppedViews, dependentViewsCtx, recreatedViews) diff --git a/internal/diff/view.go b/internal/diff/view.go index 993f0c02..51e1069d 100644 --- a/internal/diff/view.go +++ b/internal/diff/view.go @@ -2,6 +2,8 @@ package diff import ( "fmt" + "regexp" + "sort" "strings" "github.com/pgschema/pgschema/ir" @@ -410,9 +412,25 @@ func viewsEqual(old, new *ir.View) bool { // viewDependsOnView checks if viewA depends on viewB func viewDependsOnView(viewA *ir.View, viewBName string) bool { - // Simple heuristic: check if viewB name appears in viewA definition - // This can be enhanced with proper dependency parsing later - return strings.Contains(strings.ToLower(viewA.Definition), strings.ToLower(viewBName)) + if viewA == nil || viewA.Definition == "" { + return false + } + return containsIdentifier(viewA.Definition, viewBName) +} + +// containsIdentifier checks if the given SQL text contains the identifier as a whole word. +// This uses word boundary matching to avoid false positives (e.g., "user" matching "users"). +func containsIdentifier(sqlText, identifier string) bool { + if sqlText == "" || identifier == "" { + return false + } + + // Build a regex pattern that matches the identifier as a whole word. + // Word boundaries in SQL are: start/end of string, whitespace, punctuation, operators. + // We use a pattern that matches the identifier not preceded/followed by word characters. + pattern := `(?i)(?:^|[^a-z0-9_])` + regexp.QuoteMeta(identifier) + `(?:[^a-z0-9_]|$)` + matched, _ := regexp.MatchString(pattern, sqlText) + return matched } // viewDependsOnTable checks if a view depends on a specific table @@ -445,17 +463,14 @@ func viewDependsOnMaterializedView(view *ir.View, matViewSchema, matViewName str return false } - def := strings.ToLower(view.Definition) - matViewNameLower := strings.ToLower(matViewName) - - // Check for unqualified name - if strings.Contains(def, matViewNameLower) { + // Check for unqualified name using word boundary matching + if containsIdentifier(view.Definition, matViewName) { return true } // Check for qualified name (schema.matview) - qualifiedName := strings.ToLower(matViewSchema + "." + matViewName) - if strings.Contains(def, qualifiedName) { + qualifiedName := matViewSchema + "." + matViewName + if containsIdentifier(view.Definition, qualifiedName) { return true } @@ -483,7 +498,8 @@ func (ctx *dependentViewsContext) GetDependents(matViewKey string) []*ir.View { return ctx.dependents[matViewKey] } -// findDependentViewsForMatViews finds all regular views that depend on materialized views being recreated +// findDependentViewsForMatViews finds all regular views that depend on materialized views being recreated. +// This includes transitive dependencies (views that depend on views that depend on the mat view). // allViews contains all views from the new state (used for dependency analysis and recreation) // modifiedViews contains the materialized views being recreated func findDependentViewsForMatViews(allViews map[string]*ir.View, modifiedViews []*viewDiff) *dependentViewsContext { @@ -496,13 +512,92 @@ func findDependentViewsForMatViews(allViews map[string]*ir.View, modifiedViews [ matViewKey := viewDiff.New.Schema + "." + viewDiff.New.Name - // Find all regular views that depend on this materialized view + // Find all regular views that directly depend on this materialized view + directDependents := make([]*ir.View, 0) for _, view := range allViews { if viewDependsOnMaterializedView(view, viewDiff.New.Schema, viewDiff.New.Name) { - ctx.dependents[matViewKey] = append(ctx.dependents[matViewKey], view) + directDependents = append(directDependents, view) } } + + // Find transitive dependencies (views that depend on the direct dependents) + allDependents := findTransitiveDependents(directDependents, allViews) + + // Topologically sort the dependents so they can be dropped/recreated in correct order + sortedDependents := topologicallySortViews(allDependents) + + ctx.dependents[matViewKey] = sortedDependents } return ctx } + +// findTransitiveDependents finds all views that transitively depend on the given views. +// Returns all dependents including the initial views, with no duplicates. +func findTransitiveDependents(initialViews []*ir.View, allViews map[string]*ir.View) []*ir.View { + if len(initialViews) == 0 { + return nil + } + + // Track visited views to avoid duplicates and cycles + visited := make(map[string]bool) + var result []*ir.View + + // Queue for BFS traversal + queue := make([]*ir.View, 0, len(initialViews)) + for _, v := range initialViews { + key := v.Schema + "." + v.Name + if !visited[key] { + visited[key] = true + queue = append(queue, v) + result = append(result, v) + } + } + + // BFS to find all transitive dependents + for len(queue) > 0 { + current := queue[0] + queue = queue[1:] + + // Find views that depend on the current view + for _, view := range allViews { + if view.Materialized { + continue // Skip materialized views + } + viewKey := view.Schema + "." + view.Name + if visited[viewKey] { + continue + } + + // Check if this view depends on the current view + if viewDependsOnView(view, current.Name) { + visited[viewKey] = true + queue = append(queue, view) + result = append(result, view) + } + } + } + + return result +} + +// sortModifiedViewsForProcessing sorts modifiedViews to ensure materialized views +// with RequiresRecreate are processed first. This ensures dependent views are +// added to recreatedViews before their own modifications would be processed. +func sortModifiedViewsForProcessing(views []*viewDiff) { + sort.SliceStable(views, func(i, j int) bool { + // Materialized views with RequiresRecreate should come first + iMatRecreate := views[i].RequiresRecreate && views[i].New.Materialized + jMatRecreate := views[j].RequiresRecreate && views[j].New.Materialized + + if iMatRecreate && !jMatRecreate { + return true + } + if !iMatRecreate && jMatRecreate { + return false + } + + // Otherwise maintain relative order (stable sort) + return false + }) +} From 9c2eeb2f6400131f18769106917d117d2f7a6add Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Fri, 30 Jan 2026 07:47:29 -0800 Subject: [PATCH 04/11] fix: address final PR review feedback for issue #268 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 --- internal/diff/view.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/internal/diff/view.go b/internal/diff/view.go index 51e1069d..2e9c446d 100644 --- a/internal/diff/view.go +++ b/internal/diff/view.go @@ -429,7 +429,13 @@ func containsIdentifier(sqlText, identifier string) bool { // Word boundaries in SQL are: start/end of string, whitespace, punctuation, operators. // We use a pattern that matches the identifier not preceded/followed by word characters. pattern := `(?i)(?:^|[^a-z0-9_])` + regexp.QuoteMeta(identifier) + `(?:[^a-z0-9_]|$)` - matched, _ := regexp.MatchString(pattern, sqlText) + matched, err := regexp.MatchString(pattern, sqlText) + if err != nil { + // This should never happen since regexp.QuoteMeta ensures valid pattern, + // but log it rather than silently ignoring + fmt.Printf("containsIdentifier: regexp error for pattern %q: %v\n", pattern, err) + return false + } return matched } @@ -569,8 +575,9 @@ func findTransitiveDependents(initialViews []*ir.View, allViews map[string]*ir.V continue } - // Check if this view depends on the current view - if viewDependsOnView(view, current.Name) { + // Check if this view depends on the current view (unqualified or schema-qualified) + if viewDependsOnView(view, current.Name) || + viewDependsOnView(view, current.Schema+"."+current.Name) { visited[viewKey] = true queue = append(queue, view) result = append(result, view) From a710fab89b6daafae0df9fc17f6fe77986f65a01 Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Fri, 30 Jan 2026 07:59:37 -0800 Subject: [PATCH 05/11] fix: avoid redundant drop/recreate when view depends on multiple mat 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 --- internal/diff/view.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/internal/diff/view.go b/internal/diff/view.go index 2e9c446d..f10fd326 100644 --- a/internal/diff/view.go +++ b/internal/diff/view.go @@ -85,6 +85,10 @@ func generateCreateViewsSQL(views []*ir.View, targetSchema string, collector *di // dependentViewsCtx contains views that depend on materialized views being recreated // recreatedViews tracks views that were recreated as dependencies (to avoid duplicate processing) func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *diffCollector, preDroppedViews map[string]bool, dependentViewsCtx *dependentViewsContext, recreatedViews map[string]bool) { + // Track dependent views that have already been dropped to avoid redundant operations + // when a view depends on multiple materialized views being recreated + droppedDependentViews := make(map[string]bool) + for _, diff := range diffs { // Handle materialized views that require recreation (DROP + CREATE) if diff.RequiresRecreate { @@ -101,8 +105,17 @@ func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *d // 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. + // Skip views that have already been dropped (when a view depends on multiple mat views). for i := len(dependentViews) - 1; i >= 0; i-- { depView := dependentViews[i] + depViewKey := depView.Schema + "." + depView.Name + + // Skip if already dropped (view depends on multiple mat views being recreated) + if droppedDependentViews[depViewKey] { + continue + } + droppedDependentViews[depViewKey] = true + depViewName := qualifyEntityName(depView.Schema, depView.Name, targetSchema) dropDepSQL := fmt.Sprintf("DROP VIEW IF EXISTS %s RESTRICT;", depViewName) @@ -157,8 +170,15 @@ func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *d } // Recreate dependent views (in original order) + // Skip views that have already been recreated (when a view depends on multiple mat views). for _, depView := range dependentViews { depViewKey := depView.Schema + "." + depView.Name + + // Skip if already recreated (view depends on multiple mat views being recreated) + if recreatedViews != nil && recreatedViews[depViewKey] { + continue + } + createDepSQL := generateViewSQL(depView, targetSchema) depContext := &diffContext{ From 1415d5f85b88e5c5bfadedbed8443f19003d1e42 Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Fri, 30 Jan 2026 08:16:05 -0800 Subject: [PATCH 06/11] fix: use two-phase approach for dependent view handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- internal/diff/view.go | 101 +++++++++++++++++++++++++----------------- 1 file changed, 60 insertions(+), 41 deletions(-) diff --git a/internal/diff/view.go b/internal/diff/view.go index f10fd326..c5466e36 100644 --- a/internal/diff/view.go +++ b/internal/diff/view.go @@ -89,6 +89,15 @@ func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *d // when a view depends on multiple materialized views being recreated droppedDependentViews := make(map[string]bool) + // Collect all dependent views that need to be recreated after ALL mat views are processed. + // This is critical: if a view depends on multiple mat views being recreated, we must + // wait until ALL mat views are recreated before recreating the dependent view. + // Otherwise, recreating the view after the first mat view would cause the second + // mat view's DROP to fail because the recreated view depends on it. + var allDependentViewsToRecreate []*ir.View + seenDependentViews := make(map[string]bool) + + // Phase 1: Drop all dependent views and drop/recreate all materialized views for _, diff := range diffs { // Handle materialized views that require recreation (DROP + CREATE) if diff.RequiresRecreate { @@ -129,6 +138,15 @@ func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *d collector.collect(depContext, dropDepSQL) } + // Collect dependent views for later recreation (deduplicated) + for _, depView := range dependentViews { + depViewKey := depView.Schema + "." + depView.Name + if !seenDependentViews[depViewKey] { + seenDependentViews[depViewKey] = true + allDependentViewsToRecreate = append(allDependentViewsToRecreate, depView) + } + } + // Check if already pre-dropped if preDroppedViews != nil && preDroppedViews[viewKey] { // Skip DROP, only CREATE since view was already dropped in pre-drop phase @@ -169,47 +187,6 @@ func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *d collector.collectStatements(context, statements) } - // Recreate dependent views (in original order) - // Skip views that have already been recreated (when a view depends on multiple mat views). - for _, depView := range dependentViews { - depViewKey := depView.Schema + "." + depView.Name - - // Skip if already recreated (view depends on multiple mat views being recreated) - if recreatedViews != nil && recreatedViews[depViewKey] { - continue - } - - createDepSQL := generateViewSQL(depView, targetSchema) - - depContext := &diffContext{ - Type: DiffTypeView, - Operation: DiffOperationRecreate, - Path: fmt.Sprintf("%s.%s", depView.Schema, depView.Name), - Source: depView, - CanRunInTransaction: true, - } - collector.collect(depContext, createDepSQL) - - // Track this view as recreated to avoid duplicate processing - if recreatedViews != nil { - recreatedViews[depViewKey] = true - } - - // Recreate view comment if present - if depView.Comment != "" { - depViewName := qualifyEntityName(depView.Schema, depView.Name, targetSchema) - commentSQL := fmt.Sprintf("COMMENT ON VIEW %s IS %s;", depViewName, quoteString(depView.Comment)) - commentContext := &diffContext{ - Type: DiffTypeViewComment, - Operation: DiffOperationCreate, - Path: fmt.Sprintf("%s.%s", depView.Schema, depView.Name), - Source: depView, - CanRunInTransaction: true, - } - collector.collect(commentContext, commentSQL) - } - } - // Add view comment if present if diff.New.Comment != "" { sql := fmt.Sprintf("COMMENT ON MATERIALIZED VIEW %s IS %s;", viewName, quoteString(diff.New.Comment)) @@ -356,6 +333,48 @@ func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *d } } } + + // Phase 2: Recreate all dependent views AFTER all materialized views have been processed. + // This is critical for views that depend on multiple mat views being recreated. + // The views are already topologically sorted, so recreating in order handles nested deps. + for _, depView := range allDependentViewsToRecreate { + depViewKey := depView.Schema + "." + depView.Name + + // Skip if already recreated by other means + if recreatedViews != nil && recreatedViews[depViewKey] { + continue + } + + createDepSQL := generateViewSQL(depView, targetSchema) + + depContext := &diffContext{ + Type: DiffTypeView, + Operation: DiffOperationRecreate, + Path: fmt.Sprintf("%s.%s", depView.Schema, depView.Name), + Source: depView, + CanRunInTransaction: true, + } + collector.collect(depContext, createDepSQL) + + // Track this view as recreated to avoid duplicate processing + if recreatedViews != nil { + recreatedViews[depViewKey] = true + } + + // Recreate view comment if present + if depView.Comment != "" { + depViewName := qualifyEntityName(depView.Schema, depView.Name, targetSchema) + commentSQL := fmt.Sprintf("COMMENT ON VIEW %s IS %s;", depViewName, quoteString(depView.Comment)) + commentContext := &diffContext{ + Type: DiffTypeViewComment, + Operation: DiffOperationCreate, + Path: fmt.Sprintf("%s.%s", depView.Schema, depView.Name), + Source: depView, + CanRunInTransaction: true, + } + collector.collect(commentContext, commentSQL) + } + } } // generateDropViewsSQL generates DROP [MATERIALIZED] VIEW statements From 73c58f6ddb0c404740c933e022e235aa62c009b9 Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Fri, 30 Jan 2026 08:34:35 -0800 Subject: [PATCH 07/11] fix: handle schema-qualified identifiers in regex pattern 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 --- internal/diff/view.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/internal/diff/view.go b/internal/diff/view.go index c5466e36..006db624 100644 --- a/internal/diff/view.go +++ b/internal/diff/view.go @@ -467,7 +467,15 @@ func containsIdentifier(sqlText, identifier string) bool { // Build a regex pattern that matches the identifier as a whole word. // Word boundaries in SQL are: start/end of string, whitespace, punctuation, operators. // We use a pattern that matches the identifier not preceded/followed by word characters. - pattern := `(?i)(?:^|[^a-z0-9_])` + regexp.QuoteMeta(identifier) + `(?:[^a-z0-9_]|$)` + // + // For schema-qualified identifiers (containing a dot), treat '.' as part of the word + // to avoid matching inside longer qualified paths like "other.schema.name". + var pattern string + if strings.Contains(identifier, ".") { + pattern = `(?i)(?:^|[^a-z0-9_.])` + regexp.QuoteMeta(identifier) + `(?:[^a-z0-9_.]|$)` + } else { + pattern = `(?i)(?:^|[^a-z0-9_])` + regexp.QuoteMeta(identifier) + `(?:[^a-z0-9_]|$)` + } matched, err := regexp.MatchString(pattern, sqlText) if err != nil { // This should never happen since regexp.QuoteMeta ensures valid pattern, From 39e1c2101f44e5b4506d08a2a29404530b3e4c44 Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Fri, 30 Jan 2026 08:41:54 -0800 Subject: [PATCH 08/11] test: enhance drop_materialized_view test case for complex scenarios 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 --- .../drop_materialized_view/diff.sql | 29 +++++++++++++- .../drop_materialized_view/new.sql | 32 +++++++++++++-- .../drop_materialized_view/old.sql | 30 +++++++++++++- .../drop_materialized_view/plan.json | 40 ++++++++++++++++++- .../drop_materialized_view/plan.sql | 29 +++++++++++++- .../drop_materialized_view/plan.txt | 33 +++++++++++++-- 6 files changed, 181 insertions(+), 12 deletions(-) diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/diff.sql b/testdata/diff/create_materialized_view/drop_materialized_view/diff.sql index ae0cf785..5ecfd8eb 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/diff.sql +++ b/testdata/diff/create_materialized_view/drop_materialized_view/diff.sql @@ -1,3 +1,13 @@ +-- Expected order of operations: +-- 1. Drop nested dependency chain: employee_ids -> employee_names -> active_employees +-- 2. Drop multi-matview dependent: employee_summary (depends on active_employees AND dept_stats) +-- 3. Drop and recreate both materialized views +-- 4. Recreate all dependent views AFTER all mat views are done +-- +-- NOTE: This file needs regeneration via tests. The structure below is approximate. + +DROP VIEW IF EXISTS employee_ids RESTRICT; +DROP VIEW IF EXISTS employee_names RESTRICT; DROP VIEW IF EXISTS employee_summary RESTRICT; DROP MATERIALIZED VIEW active_employees RESTRICT; CREATE MATERIALIZED VIEW IF NOT EXISTS active_employees AS @@ -7,7 +17,24 @@ CREATE MATERIALIZED VIEW IF NOT EXISTS active_employees AS 'active'::text AS status_label FROM employees WHERE status::text = 'active'::text; -CREATE OR REPLACE VIEW employee_summary AS +DROP MATERIALIZED VIEW dept_stats RESTRICT; +CREATE MATERIALIZED VIEW IF NOT EXISTS dept_stats AS + SELECT department, + count(*) AS employee_count, + avg(salary) AS avg_salary + FROM employees + GROUP BY department; +CREATE OR REPLACE VIEW employee_names AS SELECT id, name FROM active_employees; +CREATE OR REPLACE VIEW employee_ids AS + SELECT id + FROM employee_names; +CREATE OR REPLACE VIEW employee_summary AS + SELECT ae.id, + ae.name, + ds.employee_count AS dept_size + FROM active_employees ae + CROSS JOIN dept_stats ds + LIMIT 10; diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/new.sql b/testdata/diff/create_materialized_view/drop_materialized_view/new.sql index a8840a22..968179fb 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/new.sql +++ b/testdata/diff/create_materialized_view/drop_materialized_view/new.sql @@ -2,10 +2,11 @@ CREATE TABLE public.employees ( id SERIAL PRIMARY KEY, name VARCHAR(100) NOT NULL, salary DECIMAL(10,2) NOT NULL, + department VARCHAR(50) NOT NULL, status VARCHAR(20) NOT NULL ); --- Modified materialized view with new column (requires drop and recreate) +-- First materialized view - modified with new column (requires drop/recreate) CREATE MATERIALIZED VIEW public.active_employees AS SELECT id, @@ -15,9 +16,34 @@ SELECT FROM employees WHERE status = 'active'; --- View that depends on the materialized view (reproduces issue #268) -CREATE VIEW public.employee_summary AS +-- Second materialized view - modified with new column (requires drop/recreate) +CREATE MATERIALIZED VIEW public.dept_stats AS +SELECT + department, + COUNT(*) AS employee_count, + AVG(salary) AS avg_salary -- New column forces drop/recreate +FROM employees +GROUP BY department; + +-- View V1: depends on materialized view (for nested dependency test) +CREATE VIEW public.employee_names AS SELECT id, name FROM active_employees; + +-- View V2: nested dependency - depends on V1 which depends on mat view +CREATE VIEW public.employee_ids AS +SELECT + id +FROM employee_names; + +-- View that depends on BOTH materialized views (multi-matview dependency test) +CREATE VIEW public.employee_summary AS +SELECT + ae.id, + ae.name, + ds.employee_count AS dept_size +FROM active_employees ae +CROSS JOIN dept_stats ds +LIMIT 10; diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/old.sql b/testdata/diff/create_materialized_view/drop_materialized_view/old.sql index b04f63d6..5817b9bd 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/old.sql +++ b/testdata/diff/create_materialized_view/drop_materialized_view/old.sql @@ -2,9 +2,11 @@ CREATE TABLE public.employees ( id SERIAL PRIMARY KEY, name VARCHAR(100) NOT NULL, salary DECIMAL(10,2) NOT NULL, + department VARCHAR(50) NOT NULL, status VARCHAR(20) NOT NULL ); +-- First materialized view CREATE MATERIALIZED VIEW public.active_employees AS SELECT id, @@ -13,9 +15,33 @@ SELECT FROM employees WHERE status = 'active'; --- View that depends on the materialized view (reproduces issue #268) -CREATE VIEW public.employee_summary AS +-- Second materialized view +CREATE MATERIALIZED VIEW public.dept_stats AS +SELECT + department, + COUNT(*) AS employee_count +FROM employees +GROUP BY department; + +-- View V1: depends on materialized view (for nested dependency test) +CREATE VIEW public.employee_names AS SELECT id, name FROM active_employees; + +-- View V2: nested dependency - depends on V1 which depends on mat view +CREATE VIEW public.employee_ids AS +SELECT + id +FROM employee_names; + +-- View that depends on BOTH materialized views (multi-matview dependency test) +CREATE VIEW public.employee_summary AS +SELECT + ae.id, + ae.name, + ds.employee_count AS dept_size +FROM active_employees ae +CROSS JOIN dept_stats ds +LIMIT 10; diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/plan.json b/testdata/diff/create_materialized_view/drop_materialized_view/plan.json index 92573637..6a989771 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/plan.json +++ b/testdata/diff/create_materialized_view/drop_materialized_view/plan.json @@ -3,11 +3,23 @@ "pgschema_version": "1.6.1", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { - "hash": "10f8c720df8d669c175bf74e48ef73b6aa302b4416fc331e47236b6beb995cf9" + "hash": "PLACEHOLDER_NEEDS_REGENERATION" }, "groups": [ { "steps": [ + { + "sql": "DROP VIEW IF EXISTS employee_ids RESTRICT;", + "type": "view", + "operation": "recreate", + "path": "public.employee_ids" + }, + { + "sql": "DROP VIEW IF EXISTS employee_names RESTRICT;", + "type": "view", + "operation": "recreate", + "path": "public.employee_names" + }, { "sql": "DROP VIEW IF EXISTS employee_summary RESTRICT;", "type": "view", @@ -27,7 +39,31 @@ "path": "public.active_employees" }, { - "sql": "CREATE OR REPLACE VIEW employee_summary AS\n SELECT id,\n name\n FROM active_employees;", + "sql": "DROP MATERIALIZED VIEW dept_stats RESTRICT;", + "type": "materialized_view", + "operation": "alter", + "path": "public.dept_stats" + }, + { + "sql": "CREATE MATERIALIZED VIEW IF NOT EXISTS dept_stats AS\n SELECT department,\n count(*) AS employee_count,\n avg(salary) AS avg_salary\n FROM employees\n GROUP BY department;", + "type": "materialized_view", + "operation": "alter", + "path": "public.dept_stats" + }, + { + "sql": "CREATE OR REPLACE VIEW employee_names AS\n SELECT id,\n name\n FROM active_employees;", + "type": "view", + "operation": "recreate", + "path": "public.employee_names" + }, + { + "sql": "CREATE OR REPLACE VIEW employee_ids AS\n SELECT id\n FROM employee_names;", + "type": "view", + "operation": "recreate", + "path": "public.employee_ids" + }, + { + "sql": "CREATE OR REPLACE VIEW employee_summary AS\n SELECT ae.id,\n ae.name,\n ds.employee_count AS dept_size\n FROM active_employees ae\n CROSS JOIN dept_stats ds\n LIMIT 10;", "type": "view", "operation": "recreate", "path": "public.employee_summary" diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/plan.sql b/testdata/diff/create_materialized_view/drop_materialized_view/plan.sql index 58bb95a8..7258b2d3 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/plan.sql +++ b/testdata/diff/create_materialized_view/drop_materialized_view/plan.sql @@ -1,3 +1,9 @@ +-- NOTE: This file needs regeneration via tests. The structure below is approximate. + +DROP VIEW IF EXISTS employee_ids RESTRICT; + +DROP VIEW IF EXISTS employee_names RESTRICT; + DROP VIEW IF EXISTS employee_summary RESTRICT; DROP MATERIALIZED VIEW active_employees RESTRICT; @@ -10,7 +16,28 @@ CREATE MATERIALIZED VIEW IF NOT EXISTS active_employees AS FROM employees WHERE status::text = 'active'::text; -CREATE OR REPLACE VIEW employee_summary AS +DROP MATERIALIZED VIEW dept_stats RESTRICT; + +CREATE MATERIALIZED VIEW IF NOT EXISTS dept_stats AS + SELECT department, + count(*) AS employee_count, + avg(salary) AS avg_salary + FROM employees + GROUP BY department; + +CREATE OR REPLACE VIEW employee_names AS SELECT id, name FROM active_employees; + +CREATE OR REPLACE VIEW employee_ids AS + SELECT id + FROM employee_names; + +CREATE OR REPLACE VIEW employee_summary AS + SELECT ae.id, + ae.name, + ds.employee_count AS dept_size + FROM active_employees ae + CROSS JOIN dept_stats ds + LIMIT 10; diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/plan.txt b/testdata/diff/create_materialized_view/drop_materialized_view/plan.txt index 4d810b3a..c73d1864 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/plan.txt +++ b/testdata/diff/create_materialized_view/drop_materialized_view/plan.txt @@ -1,13 +1,19 @@ -Plan: 1 to modify. +Plan: 2 to modify. Summary by type: - materialized views: 1 to modify + materialized views: 2 to modify Materialized views: ~ active_employees + ~ dept_stats DDL to be executed: -------------------------------------------------- +-- NOTE: This file needs regeneration via tests. The structure below is approximate. + +DROP VIEW IF EXISTS employee_ids RESTRICT; + +DROP VIEW IF EXISTS employee_names RESTRICT; DROP VIEW IF EXISTS employee_summary RESTRICT; @@ -21,7 +27,28 @@ CREATE MATERIALIZED VIEW IF NOT EXISTS active_employees AS FROM employees WHERE status::text = 'active'::text; -CREATE OR REPLACE VIEW employee_summary AS +DROP MATERIALIZED VIEW dept_stats RESTRICT; + +CREATE MATERIALIZED VIEW IF NOT EXISTS dept_stats AS + SELECT department, + count(*) AS employee_count, + avg(salary) AS avg_salary + FROM employees + GROUP BY department; + +CREATE OR REPLACE VIEW employee_names AS SELECT id, name FROM active_employees; + +CREATE OR REPLACE VIEW employee_ids AS + SELECT id + FROM employee_names; + +CREATE OR REPLACE VIEW employee_summary AS + SELECT ae.id, + ae.name, + ds.employee_count AS dept_size + FROM active_employees ae + CROSS JOIN dept_stats ds + LIMIT 10; From fc8690f521e135c8f10df41c81a5e3a6105917da Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Fri, 30 Jan 2026 08:52:05 -0800 Subject: [PATCH 09/11] fix: update expected test output files with actual generated SQL Co-Authored-By: Claude Opus 4.5 --- .../drop_materialized_view/diff.sql | 14 +++----------- .../drop_materialized_view/plan.json | 16 ++++++++-------- .../drop_materialized_view/plan.sql | 8 +++----- .../drop_materialized_view/plan.txt | 9 ++++----- 4 files changed, 18 insertions(+), 29 deletions(-) diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/diff.sql b/testdata/diff/create_materialized_view/drop_materialized_view/diff.sql index 5ecfd8eb..9f305863 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/diff.sql +++ b/testdata/diff/create_materialized_view/drop_materialized_view/diff.sql @@ -1,14 +1,6 @@ --- Expected order of operations: --- 1. Drop nested dependency chain: employee_ids -> employee_names -> active_employees --- 2. Drop multi-matview dependent: employee_summary (depends on active_employees AND dept_stats) --- 3. Drop and recreate both materialized views --- 4. Recreate all dependent views AFTER all mat views are done --- --- NOTE: This file needs regeneration via tests. The structure below is approximate. - +DROP VIEW IF EXISTS employee_summary RESTRICT; DROP VIEW IF EXISTS employee_ids RESTRICT; DROP VIEW IF EXISTS employee_names RESTRICT; -DROP VIEW IF EXISTS employee_summary RESTRICT; DROP MATERIALIZED VIEW active_employees RESTRICT; CREATE MATERIALIZED VIEW IF NOT EXISTS active_employees AS SELECT id, @@ -36,5 +28,5 @@ CREATE OR REPLACE VIEW employee_summary AS ae.name, ds.employee_count AS dept_size FROM active_employees ae - CROSS JOIN dept_stats ds - LIMIT 10; + CROSS JOIN dept_stats ds + LIMIT 10; diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/plan.json b/testdata/diff/create_materialized_view/drop_materialized_view/plan.json index 6a989771..a2928b8b 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/plan.json +++ b/testdata/diff/create_materialized_view/drop_materialized_view/plan.json @@ -3,28 +3,28 @@ "pgschema_version": "1.6.1", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { - "hash": "PLACEHOLDER_NEEDS_REGENERATION" + "hash": "11bf597bede0110c90a6788ecf4c5c0a9191c5b762d76812e543b0eb55ee0daf" }, "groups": [ { "steps": [ { - "sql": "DROP VIEW IF EXISTS employee_ids RESTRICT;", + "sql": "DROP VIEW IF EXISTS employee_summary RESTRICT;", "type": "view", "operation": "recreate", - "path": "public.employee_ids" + "path": "public.employee_summary" }, { - "sql": "DROP VIEW IF EXISTS employee_names RESTRICT;", + "sql": "DROP VIEW IF EXISTS employee_ids RESTRICT;", "type": "view", "operation": "recreate", - "path": "public.employee_names" + "path": "public.employee_ids" }, { - "sql": "DROP VIEW IF EXISTS employee_summary RESTRICT;", + "sql": "DROP VIEW IF EXISTS employee_names RESTRICT;", "type": "view", "operation": "recreate", - "path": "public.employee_summary" + "path": "public.employee_names" }, { "sql": "DROP MATERIALIZED VIEW active_employees RESTRICT;", @@ -63,7 +63,7 @@ "path": "public.employee_ids" }, { - "sql": "CREATE OR REPLACE VIEW employee_summary AS\n SELECT ae.id,\n ae.name,\n ds.employee_count AS dept_size\n FROM active_employees ae\n CROSS JOIN dept_stats ds\n LIMIT 10;", + "sql": "CREATE OR REPLACE VIEW employee_summary AS\n SELECT ae.id,\n ae.name,\n ds.employee_count AS dept_size\n FROM active_employees ae\n CROSS JOIN dept_stats ds\n LIMIT 10;", "type": "view", "operation": "recreate", "path": "public.employee_summary" diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/plan.sql b/testdata/diff/create_materialized_view/drop_materialized_view/plan.sql index 7258b2d3..6e293631 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/plan.sql +++ b/testdata/diff/create_materialized_view/drop_materialized_view/plan.sql @@ -1,11 +1,9 @@ --- NOTE: This file needs regeneration via tests. The structure below is approximate. +DROP VIEW IF EXISTS employee_summary RESTRICT; DROP VIEW IF EXISTS employee_ids RESTRICT; DROP VIEW IF EXISTS employee_names RESTRICT; -DROP VIEW IF EXISTS employee_summary RESTRICT; - DROP MATERIALIZED VIEW active_employees RESTRICT; CREATE MATERIALIZED VIEW IF NOT EXISTS active_employees AS @@ -39,5 +37,5 @@ CREATE OR REPLACE VIEW employee_summary AS ae.name, ds.employee_count AS dept_size FROM active_employees ae - CROSS JOIN dept_stats ds - LIMIT 10; + CROSS JOIN dept_stats ds + LIMIT 10; diff --git a/testdata/diff/create_materialized_view/drop_materialized_view/plan.txt b/testdata/diff/create_materialized_view/drop_materialized_view/plan.txt index c73d1864..aa218356 100644 --- a/testdata/diff/create_materialized_view/drop_materialized_view/plan.txt +++ b/testdata/diff/create_materialized_view/drop_materialized_view/plan.txt @@ -9,14 +9,13 @@ Materialized views: DDL to be executed: -------------------------------------------------- --- NOTE: This file needs regeneration via tests. The structure below is approximate. + +DROP VIEW IF EXISTS employee_summary RESTRICT; DROP VIEW IF EXISTS employee_ids RESTRICT; DROP VIEW IF EXISTS employee_names RESTRICT; -DROP VIEW IF EXISTS employee_summary RESTRICT; - DROP MATERIALIZED VIEW active_employees RESTRICT; CREATE MATERIALIZED VIEW IF NOT EXISTS active_employees AS @@ -50,5 +49,5 @@ CREATE OR REPLACE VIEW employee_summary AS ae.name, ds.employee_count AS dept_size FROM active_employees ae - CROSS JOIN dept_stats ds - LIMIT 10; + CROSS JOIN dept_stats ds + LIMIT 10; From 3d7734dabdfc1aa7c4c96af30aafc2f517a71610 Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Fri, 30 Jan 2026 08:56:46 -0800 Subject: [PATCH 10/11] fix: exclude newly added views from dependent view handling 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 --- internal/diff/diff.go | 3 ++- internal/diff/view.go | 24 +++++++++++++++++++++--- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/internal/diff/diff.go b/internal/diff/diff.go index 0e1e398c..01fd298e 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -1586,7 +1586,8 @@ func (d *ddlDiff) generateModifySQL(targetSchema string, collector *diffCollecto generateModifyTablesSQL(d.modifiedTables, targetSchema, collector) // Find views that depend on materialized views being recreated (issue #268) - dependentViewsCtx := findDependentViewsForMatViews(d.allNewViews, d.modifiedViews) + // Exclude newly added views - they will be created in CREATE phase after mat views + dependentViewsCtx := findDependentViewsForMatViews(d.allNewViews, d.modifiedViews, d.addedViews) // Track views recreated as dependencies to avoid duplicate processing recreatedViews := make(map[string]bool) diff --git a/internal/diff/view.go b/internal/diff/view.go index 006db624..843a67e8 100644 --- a/internal/diff/view.go +++ b/internal/diff/view.go @@ -555,9 +555,17 @@ func (ctx *dependentViewsContext) GetDependents(matViewKey string) []*ir.View { // This includes transitive dependencies (views that depend on views that depend on the mat view). // allViews contains all views from the new state (used for dependency analysis and recreation) // modifiedViews contains the materialized views being recreated -func findDependentViewsForMatViews(allViews map[string]*ir.View, modifiedViews []*viewDiff) *dependentViewsContext { +// addedViews contains views that are newly added (not in old schema) - these should be excluded +// because they will be created in the CREATE phase after mat views are recreated +func findDependentViewsForMatViews(allViews map[string]*ir.View, modifiedViews []*viewDiff, addedViews []*ir.View) *dependentViewsContext { ctx := newDependentViewsContext() + // Build a set of added view keys to exclude from dependents + addedViewKeys := make(map[string]bool) + for _, view := range addedViews { + addedViewKeys[view.Schema+"."+view.Name] = true + } + for _, viewDiff := range modifiedViews { if !viewDiff.RequiresRecreate || !viewDiff.New.Materialized { continue @@ -566,15 +574,21 @@ func findDependentViewsForMatViews(allViews map[string]*ir.View, modifiedViews [ matViewKey := viewDiff.New.Schema + "." + viewDiff.New.Name // Find all regular views that directly depend on this materialized view + // Exclude newly added views - they will be created in CREATE phase directDependents := make([]*ir.View, 0) for _, view := range allViews { + viewKey := view.Schema + "." + view.Name + if addedViewKeys[viewKey] { + continue // Skip newly added views + } if viewDependsOnMaterializedView(view, viewDiff.New.Schema, viewDiff.New.Name) { directDependents = append(directDependents, view) } } // Find transitive dependencies (views that depend on the direct dependents) - allDependents := findTransitiveDependents(directDependents, allViews) + // Also exclude added views from transitive search + allDependents := findTransitiveDependents(directDependents, allViews, addedViewKeys) // Topologically sort the dependents so they can be dropped/recreated in correct order sortedDependents := topologicallySortViews(allDependents) @@ -587,7 +601,8 @@ func findDependentViewsForMatViews(allViews map[string]*ir.View, modifiedViews [ // findTransitiveDependents finds all views that transitively depend on the given views. // Returns all dependents including the initial views, with no duplicates. -func findTransitiveDependents(initialViews []*ir.View, allViews map[string]*ir.View) []*ir.View { +// excludeKeys contains view keys to exclude (e.g., newly added views) +func findTransitiveDependents(initialViews []*ir.View, allViews map[string]*ir.View, excludeKeys map[string]bool) []*ir.View { if len(initialViews) == 0 { return nil } @@ -621,6 +636,9 @@ func findTransitiveDependents(initialViews []*ir.View, allViews map[string]*ir.V if visited[viewKey] { continue } + if excludeKeys != nil && excludeKeys[viewKey] { + continue // Skip excluded views (e.g., newly added views) + } // Check if this view depends on the current view (unqualified or schema-qualified) if viewDependsOnView(view, current.Name) || From 626a577cf8846f19e1212cec1d2d6ee9206a6527 Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Fri, 30 Jan 2026 09:10:35 -0800 Subject: [PATCH 11/11] fix: regex word boundary and topological sort issues 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 --- internal/diff/view.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/internal/diff/view.go b/internal/diff/view.go index 843a67e8..27e68a16 100644 --- a/internal/diff/view.go +++ b/internal/diff/view.go @@ -336,8 +336,10 @@ func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *d // Phase 2: Recreate all dependent views AFTER all materialized views have been processed. // This is critical for views that depend on multiple mat views being recreated. - // The views are already topologically sorted, so recreating in order handles nested deps. - for _, depView := range allDependentViewsToRecreate { + // Re-sort topologically 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). + sortedDependentViews := topologicallySortViews(allDependentViewsToRecreate) + for _, depView := range sortedDependentViews { depViewKey := depView.Schema + "." + depView.Name // Skip if already recreated by other means @@ -470,11 +472,12 @@ func containsIdentifier(sqlText, identifier string) bool { // // For schema-qualified identifiers (containing a dot), treat '.' as part of the word // to avoid matching inside longer qualified paths like "other.schema.name". + // Use [^a-zA-Z0-9_] to exclude both upper and lowercase letters as word boundaries. var pattern string if strings.Contains(identifier, ".") { - pattern = `(?i)(?:^|[^a-z0-9_.])` + regexp.QuoteMeta(identifier) + `(?:[^a-z0-9_.]|$)` + pattern = `(?i)(?:^|[^a-zA-Z0-9_.])` + regexp.QuoteMeta(identifier) + `(?:[^a-zA-Z0-9_.]|$)` } else { - pattern = `(?i)(?:^|[^a-z0-9_])` + regexp.QuoteMeta(identifier) + `(?:[^a-z0-9_]|$)` + pattern = `(?i)(?:^|[^a-zA-Z0-9_])` + regexp.QuoteMeta(identifier) + `(?:[^a-zA-Z0-9_]|$)` } matched, err := regexp.MatchString(pattern, sqlText) if err != nil {