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

[tianzhou]

Summary

• When a table gains a new column and a dependent view uses SELECT *, the view's column positions shift. PostgreSQL's CREATE OR REPLACE VIEW rejects this because it cannot rename or reorder existing columns.
• This fix adds view column tracking (via pg_attribute) to detect incompatible column changes, and uses DROP VIEW + CREATE VIEW instead of CREATE OR REPLACE VIEW when needed.
• Dependent views are automatically dropped and recreated in correct dependency order.

Fixes #308

Test plan

• Added test case dependency/issue_308_view_select_star_column_reorder that reproduces the exact scenario from the issue
• Run: PGSCHEMA_TEST_FILTER="dependency/issue_308" go test -v ./internal/diff -run TestDiffFromFiles
• Run: PGSCHEMA_TEST_FILTER="dependency/issue_308" go test -v ./cmd -run TestPlanAndApply
• All existing view, materialized view, and dependency tests pass
• Full integration test suite passes

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR fixes issue #308 by detecting when view column changes are incompatible with PostgreSQL's CREATE OR REPLACE VIEW and using DROP VIEW + CREATE VIEW instead.

Key Changes:

• Added Columns field to ir.View struct to track ordered column names from pg_attribute
• Implemented viewColumnsRequireRecreate() to detect when columns are reordered, renamed, or removed (incompatible with CREATE OR REPLACE)
• Generalized materialized view recreation logic to also handle regular views with incompatible column changes
• Added comprehensive test case issue_308_view_select_star_column_reorder that reproduces the exact scenario where a table gains a new column and a dependent view uses SELECT *

Technical Approach:
The fix correctly identifies that PostgreSQL's CREATE OR REPLACE VIEW only allows adding new columns at the end. When old columns are a prefix of new columns, CREATE OR REPLACE is safe. Otherwise, the view requires DROP + CREATE to handle reordering/renaming/removal.

The implementation handles dependent views correctly by reusing the existing dependency tracking system, ensuring views are dropped and recreated in proper dependency order.

Confidence Score: 5/5

• This PR is safe to merge with minimal risk
• The implementation is well-designed with proper fallback behavior (returns false when column info unavailable), comprehensive test coverage matching the exact issue scenario, and correctly reuses existing dependency tracking infrastructure. All test plan.json files regenerated with updated fingerprints.
• No files require special attention

Important Files Changed

Filename	Overview
ir/ir.go	Added Columns field to View struct to track ordered column names for detecting incompatible changes
ir/inspector.go	Added getViewColumns() to query pg_attribute for view column names in order; integrates with view building
internal/diff/view.go	Implemented viewColumnsRequireRecreate() to detect column changes; generalized mat view recreation logic for regular views; updated comments and function names to handle both view types
internal/diff/diff.go	Updated view diff logic to mark regular views for recreation when columns require it; renamed function to findDependentViewsForRecreatedViews
testdata/diff/dependency/issue_308_view_select_star_column_reorder/old.sql	Test case setup: tables and view with SELECT * before column addition
testdata/diff/dependency/issue_308_view_select_star_column_reorder/new.sql	Test case: same view after table gains new column, triggering column reordering in view
testdata/diff/dependency/issue_308_view_select_star_column_reorder/diff.sql	Expected migration: ALTER TABLE + DROP VIEW + CREATE VIEW to handle column reordering

Sequence Diagram

sequenceDiagram
    participant User
    participant Inspector as ir.Inspector
    participant Diff as diff.GenerateMigration
    participant ViewDiff as diff.viewColumnsRequireRecreate

    User->>Inspector: buildViews(schema)
    Inspector->>Inspector: getViewColumns(view)
    Note over Inspector: Query pg_attribute for<br/>column names ordered by attnum
    Inspector-->>User: View with Columns field populated

    User->>Diff: GenerateMigration(oldIR, newIR)
    Diff->>Diff: Compare old vs new views
    Note over Diff: Check if structurallyDifferent

    alt View definition changed
        Diff->>ViewDiff: viewColumnsRequireRecreate(old, new)
        ViewDiff->>ViewDiff: Compare old.Columns vs new.Columns
        
        alt Columns prefix match
            Note over ViewDiff: Safe for CREATE OR REPLACE
            ViewDiff-->>Diff: false
        else Columns reordered/renamed/removed
            Note over ViewDiff: Requires DROP + CREATE
            ViewDiff-->>Diff: true
        end

        alt Materialized OR columnsRequireRecreate
            Diff->>Diff: Mark RequiresRecreate = true
            Note over Diff: Generate DROP VIEW + CREATE VIEW
        else Regular view, columns compatible
            Diff->>Diff: Use CREATE OR REPLACE VIEW
        end
    end

    Diff->>Diff: findDependentViewsForRecreatedViews()
    Note over Diff: Handle dependent views,<br/>drop and recreate in order
    Diff-->>User: Migration DDL

Loading

_{Last reviewed commit: b30275a}

[Copilot]

Pull request overview

This PR fixes issue #308 where adding a column to a table causes dependent views using SELECT * to fail when pgschema attempts to update them using CREATE OR REPLACE VIEW. PostgreSQL rejects this because the expanded column list reorders existing columns, which CREATE OR REPLACE VIEW doesn't allow.

Changes:

• Added view column tracking via pg_attribute to detect incompatible column changes
• Implemented viewColumnsRequireRecreate logic to determine when DROP+CREATE is needed instead of CREATE OR REPLACE
• Extended existing materialized view recreation logic to also handle regular views with column changes

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
ir/ir.go	Added Columns field to View struct to track ordered column names
ir/inspector.go	Added getViewColumns function to query pg_attribute for view columns
internal/diff/view.go	Added viewColumnsRequireRecreate function and updated view recreation logic to handle both materialized and regular views
internal/diff/diff.go	Updated view diff logic to trigger recreation for regular views with incompatible column changes
testdata/diff/dependency/issue_308_view_select_star_column_reorder/*	New test case demonstrating the fix for the exact scenario from issue #308
testdata/diff/*/plan.json	Updated fingerprint hashes due to new Columns field in View IR

---

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

[Copilot]

The comment is outdated and should be updated to reflect that this field now tracks dependencies for both materialized views and regular views that require recreation (issue #308), not just materialized views. Consider updating to: "dependents maps view key (schema.name) to list of dependent views"

Suggested change

	// dependents maps materialized view key (schema.name) to list of dependent regular views
	// dependents maps view key (schema.name) to list of dependent views

[tianzhou]

Fixed. Updated the field comment to match the struct comment.

[Copilot]

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)

[tianzhou]

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).