fix: support view options (security_invoker, security_barrier) in dump and plan

[tianzhou]

Summary

• Add support for PostgreSQL view reloptions (security_invoker, security_barrier, check_option) across the entire pipeline
• pgschema dump now includes WITH (option=value) in CREATE VIEW output
• pgschema plan detects option changes and generates ALTER VIEW SET/RESET migration DDL
• Both CREATE VIEW ... WITH (security_invoker = true) and ALTER VIEW ... SET (security_invoker = true) are handled correctly

Fixes #350

Changes

File	Change
ir/ir.go	Added Options []string to View struct
ir/queries/queries.sql	Added c.reloptions to view query
ir/queries/queries.sql.go	Updated generated code to scan reloptions
ir/inspector.go	Copy and sort reloptions into Options slice
internal/diff/view.go	WITH clause in CREATE VIEW, ALTER VIEW SET/RESET for option changes
internal/diff/diff.go	Added OptionsChanged to viewDiff struct

Test plan

• 1 diff test covering: add option (via WITH), add option (via ALTER VIEW SET), and remove option
• All existing view and materialized view tests pass (no regressions)

# Run new test
PGSCHEMA_TEST_FILTER="create_view/issue_350_view_options" go test -v ./internal/diff -run TestDiffFromFiles
PGSCHEMA_TEST_FILTER="create_view/issue_350_view_options" go test -v ./cmd -run TestPlanAndApply

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR adds first-class support for PostgreSQL view reloptions (security_invoker, security_barrier, check_option) across the dump and plan/diff pipelines. The query layer now fetches pg_class.reloptions, the IR carries an Options map, CREATE [OR REPLACE] VIEW … WITH (…) is emitted in dump output, and ALTER VIEW SET/RESET DDL is generated for option changes. The feature is well-scoped and the regular-view path works correctly.

Key finding:

• Logic regression — materialized views: viewsEqual was extended to call viewOptionsEqual (line 682, view.go). Because viewsEqual feeds structurallyDifferent in diff.go (line 783), and needsRecreate is structurallyDifferent && newView.Materialized, any option change on a materialized view (e.g. an autovacuum_enabled storage parameter) now triggers a full DROP + CREATE instead of the correct ALTER MATERIALIZED VIEW … SET (…). The OptionsChanged flag is only ever set in the non-needsRecreate branch (line 850), so this path is silently unreachable for materialized views.

Confidence Score: 2/5

• Safe for common regular-view use case, but carries a latent logic regression for materialized views with storage parameters.
• The regular-view pipeline (dump, plan, apply) is well-tested and correct. However, the materialized-view code path has a logic flaw introduced by including options in viewsEqual: an options-only change now incorrectly evaluates needsRecreate = true, causing an unnecessary and potentially data-destroying DROP+CREATE instead of an ALTER MATERIALIZED VIEW SET statement. While the test suite doesn't exercise this scenario (no tests for materialized views with storage parameter changes), this is a realistic use case in production databases.
• internal/diff/diff.go (needsRecreate logic must exclude pure options changes for materialized views) and internal/diff/view.go (generateViewSQL WITH clause is correct for both view types).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[GenerateMigration] --> B{viewsEqual?}
    B -- "false (structurallyDifferent)" --> C{needsRecreate?}
    B -- "true" --> D{commentChanged or indexesChanged or triggersChanged?}
    D -- "yes" --> E[Add viewDiff with OptionsChanged]
    D -- "no" --> F[No diff generated]
    C -- "true\n(mat. view OR column mismatch)" --> G["RequiresRecreate=true\n⚠️ OptionsChanged NOT set"]
    C -- "false\n(regular view)" --> E
    E --> H[generateModifyViewsSQL]
    G --> I["DROP + CREATE OR REPLACE VIEW\n(includes WITH clause via generateViewSQL)"]
    H --> J{optionsOnlyChange?}
    J -- "yes" --> K["ALTER VIEW SET/RESET\n✓ Correct for regular views"]
    J -- "no (def changed too)" --> L["CREATE OR REPLACE VIEW WITH (...)\n✓ Both definition and options updated"]
    style G fill:#f99,stroke:#c00
    style I fill:#f99,stroke:#c00

Loading

Comments Outside Diff (1)

1. internal/diff/diff.go, line 834-841 (link)

   Options-only change on materialized views triggers unnecessary DROP+CREATE

   viewsEqual now calls viewOptionsEqual (line 682, view.go), so any option change causes structurallyDifferent = true. For a materialized view, needsRecreate then evaluates to true && true, triggering a full DROP + CREATE instead of ALTER MATERIALIZED VIEW … SET (…).

   This means if a materialized view has storage parameters (e.g., autovacuum_enabled) stored in pg_class.reloptions and those parameters change, the migration drops and recreates the entire materialized view—including all its data—instead of emitting an ALTER MATERIALIZED VIEW … SET (…) statement.

   The fix is to exclude a pure options change from the needsRecreate condition, analogous to how comment changes were excluded before this PR:

   needsRecreate := structurallyDifferent && !optionsChanged && (newView.Materialized || viewColumnsRequireRecreate(oldView, newView))

   Alternatively, the options check can be removed from viewsEqual and kept only in the optionsChanged variable (which is already computed independently on line 785), so structurallyDifferent only reflects schema/definition/materialization changes as it did before.

_{Last reviewed commit: 87823cf}

[Copilot]

Pull request overview

Adds end-to-end support for PostgreSQL view reloptions (e.g., security_invoker, security_barrier) so they are preserved in dumps and correctly planned/migrated via ALTER VIEW ... SET/RESET.

Changes:

• Extend IR + inspector to capture pg_class.reloptions for views and parse them into a structured Options map.
• Render view options in CREATE [OR REPLACE] VIEW ... WITH (...) and emit ALTER VIEW ... SET/RESET when options change.
• Add dump and diff test fixtures + an integration test covering issue #350 scenarios.

Reviewed changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
ir/ir.go	Adds View.Options map[string]string to represent view reloptions in IR.
ir/queries/queries.sql	Extends view inspection query to include c.reloptions for views.
ir/queries/queries.sql.go	Updates generated scanning logic to read reloptions into ViewOptions.
ir/inspector.go	Parses reloptions entries (key=value) into the IR View.Options map.
internal/diff/diff.go	Tracks OptionsChanged in viewDiff to drive option-only migrations.
internal/diff/view.go	Emits WITH (...) in CREATE VIEW and generates ALTER VIEW ... SET/RESET for option diffs.
cmd/dump/dump_integration_test.go	Adds an integration test for dumping views with security_invoker / security_barrier.
testdata/dump/issue_350_view_security_invoker/*	New dump fixture validating options are preserved in dump output.
testdata/diff/create_view/issue_350_*/*	New diff fixtures validating SET/RESET behavior for option add/change/remove.

---

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

[Copilot]

Pull request overview

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

---

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

[Copilot]

optionsOnlyChange is named as if only options changed, but the condition doesn’t enforce that (e.g., comment+options changes also satisfy it). Consider renaming it to reflect what it actually checks (options changed and definitions are equal) to avoid misleading future maintainers.

Suggested change

	// Check if only options changed (e.g., security_invoker, security_barrier)
	optionsOnlyChange := diff.OptionsChanged && definitionsEqual && diff.Old.Materialized == diff.New.Materialized
	// Handle non-structural changes (comment-only, index-only, trigger-only, or options-only)
	if commentOnlyChange || indexOnlyChange || triggerOnlyChange || optionsOnlyChange {
	// Check if options changed while definition and materialization are unchanged
	optionsChangeWithEqualDefinitionAndMaterialization := diff.OptionsChanged && definitionsEqual && diff.Old.Materialized == diff.New.Materialized
	// Handle non-structural changes (comment-only, index-only, trigger-only, or options change with equal definition/materialization)
	if commentOnlyChange || indexOnlyChange || triggerOnlyChange || optionsChangeWithEqualDefinitionAndMaterialization {

[Copilot]

The PR description states View.Options is a map[string]string, but in code View.Options is a []string (and this helper/doc comment also describes it as a slice). Please update the PR description (or the code) so reviewers/users aren’t misled about the IR shape and JSON format.

[Copilot]

Pull request overview

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

---

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

[Copilot]

Because structurallyDifferent is derived from !viewsEqual(oldView, newView) (and viewsEqual now includes Options), a materialized view reloptions-only change will be considered structural. Given the later needsRecreate := structurallyDifferent && newView.Materialized ... logic, this likely forces DROP+CREATE for matview option changes even though ALTER MATERIALIZED VIEW SET/RESET would suffice (and DROP may fail under RESTRICT). Consider basing the structural/recreate decision on definition/column changes only, and handle OptionsChanged separately via the ALTER SET/RESET path.

[Copilot]

There doesn’t appear to be a diff fixture covering reloptions changes on materialized views (e.g., asserting ALTER MATERIALIZED VIEW ... SET/RESET and avoiding DROP+CREATE). Adding a targeted test case would help prevent regressions, especially given the new option diff logic shared between views and matviews.