Skip to content

fix: use DROP+CREATE for views with column changes instead of CREATE OR REPLACE (#308) #311

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tianzhou merged 2 commits into from
Feb 22, 2026
Merged

fix: use DROP+CREATE for views with column changes instead of CREATE OR REPLACE (#308) #311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b30275a
fix: use DROP+CREATE for views with column changes instead of CREATE …
tianzhou Feb 22, 2026
8c9297c
fix: update outdated field comment in dependentViewsContext
tianzhou Feb 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions internal/diff/diff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,11 @@ func GenerateMigration(oldIR, newIR *ir.IR, targetSchema string) []Diff {

if structurallyDifferent || commentChanged || indexesChanged || triggersChanged {
// For materialized views with structural changes, mark for recreation
if newView.Materialized && structurallyDifferent {
// For regular views with column changes incompatible with CREATE OR REPLACE VIEW,
// also mark for recreation (issue #308)
needsRecreate := structurallyDifferent && (newView.Materialized || viewColumnsRequireRecreate(oldView, newView))

if needsRecreate {
diff.modifiedViews = append(diff.modifiedViews, &viewDiff{
Old: oldView,
New: newView,
Expand Down Expand Up @@ -1624,9 +1628,10 @@ 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)
// Exclude newly added views - they will be created in CREATE phase after mat views
dependentViewsCtx := findDependentViewsForMatViews(d.allNewViews, d.modifiedViews, d.addedViews)
// Find views that depend on views being recreated (issue #268, #308)
// Handles both materialized views and regular views with RequiresRecreate
// Exclude newly added views - they will be created in CREATE phase after recreated views
dependentViewsCtx := findDependentViewsForRecreatedViews(d.allNewViews, d.modifiedViews, d.addedViews)

// Track views recreated as dependencies to avoid duplicate processing
recreatedViews := make(map[string]bool)
Expand Down
146 changes: 93 additions & 53 deletions internal/diff/view.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,22 @@ func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *d
var allDependentViewsToRecreate []*ir.View
seenDependentViews := make(map[string]bool)

// Phase 1: Drop all dependent views and drop/recreate all materialized views
// Phase 1: Drop all dependent views and drop/recreate views that require recreation
for _, diff := range diffs {
// Handle materialized views that require recreation (DROP + CREATE)
// Handle views that require recreation (DROP + CREATE)
// This applies to materialized views with structural changes and
// regular views with column changes incompatible with CREATE OR REPLACE (issue #308)
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
// Determine types based on whether view is materialized
diffType := DiffTypeView
if diff.New.Materialized {
diffType = DiffTypeMaterializedView
}

// Get dependent views for this view
var dependentViews []*ir.View
if dependentViewsCtx != nil {
dependentViews = dependentViewsCtx.GetDependents(viewKey)
Expand All @@ -123,12 +131,12 @@ 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).
// Skip views that have already been dropped (when a view depends on multiple 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)
// Skip if already dropped (view depends on multiple views being recreated)
if droppedDependentViews[depViewKey] {
continue
}
Expand Down Expand Up @@ -162,16 +170,21 @@ func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *d
createSQL := generateViewSQL(diff.New, targetSchema)

context := &diffContext{
Type: DiffTypeMaterializedView,
Type: diffType,
Operation: DiffOperationCreate,
Path: fmt.Sprintf("%s.%s", diff.New.Schema, diff.New.Name),
Source: diff,
CanRunInTransaction: true,
}
collector.collect(context, createSQL)
} else {
// DROP the old materialized view
dropSQL := fmt.Sprintf("DROP MATERIALIZED VIEW %s RESTRICT;", viewName)
// DROP the old view
var dropSQL string
if diff.New.Materialized {
dropSQL = fmt.Sprintf("DROP MATERIALIZED VIEW %s RESTRICT;", viewName)
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency and safety, consider using DROP MATERIALIZED VIEW IF EXISTS instead of DROP MATERIALIZED VIEW when recreating materialized views. Regular views use IF EXISTS at line 186, and this prevents potential errors if the view doesn't exist (e.g., in edge cases or when replaying migrations).

Suggested change
dropSQL = fmt.Sprintf("DROP MATERIALIZED VIEW %s RESTRICT;", viewName)
dropSQL = fmt.Sprintf("DROP MATERIALIZED VIEW IF EXISTS %s RESTRICT;", viewName)

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@tianzhou tianzhou Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing codebase deliberately uses DROP MATERIALIZED VIEW (without IF EXISTS) in all occurrences — see lines 466, 1428, and 1463. This is intentional: materialized views in the recreation path are known to exist (they appear in both old and new states), and pre-dropped views are explicitly checked via the preDroppedViews map and skipped. Adding IF EXISTS here would be inconsistent with the established pattern. Regular views use IF EXISTS because they can be cascade-dropped by other operations, which doesn't apply to materialized views (which always use RESTRICT).

} else {
dropSQL = fmt.Sprintf("DROP VIEW IF EXISTS %s RESTRICT;", viewName)
}
createSQL := generateViewSQL(diff.New, targetSchema)

statements := []SQLStatement{
Expand All @@ -187,7 +200,7 @@ func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *d

// Use DiffOperationAlter to categorize as a modification
context := &diffContext{
Type: DiffTypeMaterializedView,
Type: diffType,
Operation: DiffOperationAlter,
Path: fmt.Sprintf("%s.%s", diff.New.Schema, diff.New.Name),
Source: diff,
Expand All @@ -198,15 +211,23 @@ func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *d

// Add view comment if present
if diff.New.Comment != "" {
sql := fmt.Sprintf("COMMENT ON MATERIALIZED VIEW %s IS %s;", viewName, quoteString(diff.New.Comment))
var commentSQL string
var commentType DiffType
if diff.New.Materialized {
commentSQL = fmt.Sprintf("COMMENT ON MATERIALIZED VIEW %s IS %s;", viewName, quoteString(diff.New.Comment))
commentType = DiffTypeMaterializedViewComment
} else {
commentSQL = fmt.Sprintf("COMMENT ON VIEW %s IS %s;", viewName, quoteString(diff.New.Comment))
commentType = DiffTypeViewComment
}
commentContext := &diffContext{
Type: DiffTypeMaterializedViewComment,
Type: commentType,
Operation: DiffOperationCreate,
Path: fmt.Sprintf("%s.%s", diff.New.Schema, diff.New.Name),
Source: diff.New,
CanRunInTransaction: true,
}
collector.collect(commentContext, sql)
collector.collect(commentContext, commentSQL)
}

// Recreate indexes for materialized views
Expand Down Expand Up @@ -555,6 +576,39 @@ func viewsEqual(old, new *ir.View) bool {
return old.Definition == new.Definition
}

// viewColumnsRequireRecreate checks whether the view's column set has changed
// in a way that requires DROP + CREATE instead of CREATE OR REPLACE.
// PostgreSQL's CREATE OR REPLACE VIEW only allows adding new columns at the end;
// it rejects any changes to existing column names or positions.
func viewColumnsRequireRecreate(old, new *ir.View) bool {
oldCols := old.Columns
newCols := new.Columns

// If column info is not available, fall back to safe behavior (no recreate needed;
// CREATE OR REPLACE will fail at apply time if columns are incompatible)
if len(oldCols) == 0 || len(newCols) == 0 {
return false
}

// If old columns are a prefix of new columns, CREATE OR REPLACE VIEW is safe
// (only new columns added at end)
if len(newCols) >= len(oldCols) {
prefixMatch := true
for i, col := range oldCols {
if newCols[i] != col {
prefixMatch = false
break
}
}
if prefixMatch {
return false
}
}

// Columns were reordered, renamed, or removed — requires DROP + CREATE
return true
}

// viewDependsOnView checks if viewA depends on viewB
func viewDependsOnView(viewA *ir.View, viewBName string) bool {
if viewA == nil || viewA.Definition == "" {
Expand Down Expand Up @@ -617,29 +671,9 @@ 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
}

// Check for unqualified name using word boundary matching
if containsIdentifier(view.Definition, matViewName) {
return true
}

// Check for qualified name (schema.matview)
qualifiedName := matViewSchema + "." + matViewName
if containsIdentifier(view.Definition, qualifiedName) {
return true
}

return false
}

// dependentViewsContext tracks views that depend on materialized views being recreated
// dependentViewsContext tracks views that depend on views being recreated
type dependentViewsContext struct {
// dependents maps materialized view key (schema.name) to list of dependent regular views
// dependents maps view key (schema.name) to list of dependent views
dependents map[string][]*ir.View
}

Expand All @@ -650,21 +684,22 @@ func newDependentViewsContext() *dependentViewsContext {
}
}

// GetDependents returns the list of views that depend on the given materialized view
func (ctx *dependentViewsContext) GetDependents(matViewKey string) []*ir.View {
// GetDependents returns the list of views that depend on the given view being recreated
func (ctx *dependentViewsContext) GetDependents(viewKey string) []*ir.View {
if ctx == nil || ctx.dependents == nil {
return nil
}
return ctx.dependents[matViewKey]
return ctx.dependents[viewKey]
}

// 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).
// findDependentViewsForRecreatedViews finds all views that depend on views being recreated.
// This includes transitive dependencies (views that depend on views that depend on the recreated view).
// Handles both materialized views and regular views with RequiresRecreate (issue #308).
// allViews contains all views from the new state (used for dependency analysis and recreation)
// modifiedViews contains the materialized views being recreated
// modifiedViews contains the views being recreated
// 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 {
// because they will be created in the CREATE phase after recreated views
func findDependentViewsForRecreatedViews(allViews map[string]*ir.View, modifiedViews []*viewDiff, addedViews []*ir.View) *dependentViewsContext {
ctx := newDependentViewsContext()

// Build a set of added view keys to exclude from dependents
Expand All @@ -674,21 +709,26 @@ func findDependentViewsForMatViews(allViews map[string]*ir.View, modifiedViews [
}

for _, viewDiff := range modifiedViews {
if !viewDiff.RequiresRecreate || !viewDiff.New.Materialized {
if !viewDiff.RequiresRecreate {
continue
}

matViewKey := viewDiff.New.Schema + "." + viewDiff.New.Name
recreatedViewKey := viewDiff.New.Schema + "." + viewDiff.New.Name

// Find all regular views that directly depend on this materialized view
// Find all views that directly depend on this view being recreated
// 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) {
// Skip the view being recreated itself
if viewKey == recreatedViewKey {
continue
}
if viewDependsOnView(view, viewDiff.New.Name) ||
viewDependsOnView(view, viewDiff.New.Schema+"."+viewDiff.New.Name) {
directDependents = append(directDependents, view)
}
}
Expand All @@ -700,7 +740,7 @@ func findDependentViewsForMatViews(allViews map[string]*ir.View, modifiedViews [
// Topologically sort the dependents so they can be dropped/recreated in correct order
sortedDependents := topologicallySortViews(allDependents)

ctx.dependents[matViewKey] = sortedDependents
ctx.dependents[recreatedViewKey] = sortedDependents
}

return ctx
Expand Down Expand Up @@ -760,19 +800,19 @@ func findTransitiveDependents(initialViews []*ir.View, allViews map[string]*ir.V
return result
}

// sortModifiedViewsForProcessing sorts modifiedViews to ensure materialized views
// sortModifiedViewsForProcessing sorts modifiedViews to ensure 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
// Views with RequiresRecreate should come first
iRecreate := views[i].RequiresRecreate
jRecreate := views[j].RequiresRecreate

if iMatRecreate && !jMatRecreate {
if iRecreate && !jRecreate {
return true
}
if !iMatRecreate && jMatRecreate {
if !iRecreate && jRecreate {
return false
}

Expand Down
38 changes: 38 additions & 0 deletions ir/inspector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1357,10 +1357,17 @@ func (i *Inspector) buildViews(ctx context.Context, schema *IR, targetSchema str
definition = strings.TrimSuffix(definition, ";")
}

// Fetch view column names from pg_attribute (ordered by attnum)
columns, err := i.getViewColumns(ctx, schemaName, viewName)
if err != nil {
return fmt.Errorf("failed to get columns for view %s.%s: %w", schemaName, viewName, err)
}

v := &View{
Schema: schemaName,
Name: viewName,
Definition: definition,
Columns: columns,
Comment: comment,
Materialized: view.IsMaterialized.Valid && view.IsMaterialized.Bool,
}
Expand All @@ -1371,6 +1378,37 @@ func (i *Inspector) buildViews(ctx context.Context, schema *IR, targetSchema str
return nil
}

// getViewColumns returns the ordered list of column names for a view or materialized view.
// Uses pg_attribute to get column names ordered by their position (attnum).
func (i *Inspector) getViewColumns(ctx context.Context, schemaName, viewName string) ([]string, error) {
query := `
SELECT a.attname
FROM pg_attribute a
JOIN pg_class c ON a.attrelid = c.oid
JOIN pg_namespace n ON c.relnamespace = n.oid
WHERE n.nspname = $1
AND c.relname = $2
AND a.attnum > 0
AND NOT a.attisdropped
ORDER BY a.attnum`

rows, err := i.db.QueryContext(ctx, query, schemaName, viewName)
if err != nil {
return nil, err
}
defer rows.Close()

var columns []string
for rows.Next() {
var colName string
if err := rows.Scan(&colName); err != nil {
return nil, err
}
columns = append(columns, colName)
}
return columns, rows.Err()
}

// extractWhenClauseFromTriggerDef extracts the WHEN clause from a trigger definition
// returned by pg_get_triggerdef(). The format is:
// "CREATE TRIGGER name ... WHEN (condition) EXECUTE FUNCTION ..."
Expand Down
1 change: 1 addition & 0 deletions ir/ir.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ type View struct {
Schema string `json:"schema"`
Name string `json:"name"`
Definition string `json:"definition"`
Columns []string `json:"columns,omitempty"` // Ordered list of output column names
Comment string `json:"comment,omitempty"`
Materialized bool `json:"materialized,omitempty"`
Indexes map[string]*Index `json:"indexes,omitempty"` // For materialized views only
Expand Down
2 changes: 1 addition & 1 deletion testdata/diff/comment/add_index_comment/plan.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"pgschema_version": "1.7.2",
"created_at": "1970-01-01T00:00:00Z",
"source_fingerprint": {
"hash": "4f1ff7accdf71ea376888abac067affdc0307d15f9b6fc44b17603c20f63a39b"
"hash": "d5424ad774c4a220725ab96071453230e0817f6ed77df1c3d427be4c28d1d3e5"
},
"groups": [
{
Expand Down
2 changes: 1 addition & 1 deletion testdata/diff/comment/add_view_comment/plan.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"pgschema_version": "1.7.2",
"created_at": "1970-01-01T00:00:00Z",
"source_fingerprint": {
"hash": "c90334f48d6ac1978ac04366373e4611a43d8027dca3e849d191772d87f0373d"
"hash": "5f72cd96b40f589c3c326b1677cfe598fd18d335c67824a10a730f7179a7e79d"
},
"groups": [
{
Expand Down
2 changes: 1 addition & 1 deletion testdata/diff/create_index/drop_index/plan.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"pgschema_version": "1.7.2",
"created_at": "1970-01-01T00:00:00Z",
"source_fingerprint": {
"hash": "1001a06e3e83694102fd12e599753c2cdc6f10fe057fb695e9a4feb5f8e280d2"
"hash": "f2873843c4de053af739e6d0641037ede53c6c78a5c6b7887f30826a4f6dfe34"
},
"groups": [
{
Expand Down
2 changes: 1 addition & 1 deletion testdata/diff/create_materialized_view/alter_materialized_view/plan.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"pgschema_version": "1.7.2",
"created_at": "1970-01-01T00:00:00Z",
"source_fingerprint": {
"hash": "731eb3ecfccfda8ea697e2def6e8a6ef1e3a5abd0707741f23aa416f83db54ab"
"hash": "f0ac3bb964fd207775142c2f9119442b29f04e10c74de4162cf6e468335e5835"
},
"groups": [
{
Expand Down
Loading