Skip to content

feat: add support for view reloptions (security_invoker, security_barrier) (#343)#347

Merged
tianzhou merged 3 commits intomainfrom
feat/issue-343-view-security-invoker
Mar 8, 2026
Merged

feat: add support for view reloptions (security_invoker, security_barrier) (#343)#347
tianzhou merged 3 commits intomainfrom
feat/issue-343-view-security-invoker

Conversation

@tianzhou
Copy link
Contributor

@tianzhou tianzhou commented Mar 8, 2026

Summary

  • Track pg_class.reloptions for views during schema inspection
  • Include WITH (...) clause in generated CREATE VIEW DDL when options are present
  • Compare view options in diff logic to detect option-only changes
  • Supports security_invoker, security_barrier, and any other view reloptions

Fixes #343

Test plan

  • Added security_invoker view to existing create_view/add_view test case
  • Verified diff, plan, and apply integration tests pass
  • PGSCHEMA_TEST_FILTER="create_view/" go test -v ./internal/diff -run TestDiffFromFiles
  • PGSCHEMA_TEST_FILTER="create_view/" go test -v ./cmd -run TestPlanAndApply

🤖 Generated with Claude Code

…rier) (#343)

Track pg_class.reloptions for views and include WITH clause in generated DDL.
This preserves security_invoker=true and other view options during plan/apply.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 8, 2026 12:49
@greptile-apps
Copy link

greptile-apps bot commented Mar 8, 2026

Greptile Summary

This PR adds support for tracking and generating view reloptions (security_invoker, security_barrier, and others) by reading pg_class.reloptions during inspection, storing them in the IR View.Options field, emitting a WITH (...) clause in generated DDL, and including options in the view equality/diff checks.

Key changes:

  • ir/ir.go: New Options []string field on View struct.
  • ir/queries/queries.sql + queries.sql.go: c.reloptions added to the views query and scanned via pq.Array.
  • ir/inspector.go: view.Reloptions mapped into View.Options.
  • internal/diff/view.go: generateViewSQL emits WITH (...), viewsEqual / generateModifyViewsSQL updated to factor in options, and a new viewOptionsEqual helper is introduced.
  • Test data: add_view fixture extended with a security_invoker view.

Issues identified:

  1. viewsEqual inlines an identical option-comparison loop instead of calling the newly-added viewOptionsEqual helper — creates a maintenance risk.
  2. viewOptionsEqual uses positional comparison rather than semantic comparison: ["security_barrier=true","security_invoker=true"] and ["security_invoker=true","security_barrier=true"] are treated as unequal even though they represent the same view state. Since pg_class.reloptions preserves insertion order (options set via successive ALTER VIEW … SET (…) calls can accumulate in arbitrary order), this can produce spurious diffs and unnecessary view recreations.

Confidence Score: 3/5

  • Functionally correct for the common case, but order-sensitive options comparison can trigger spurious view recreations when reloptions are set incrementally on the target database.
  • The core flow (inspect → store → emit WITH clause) is correct and well-tested for the create path. However, the positional comparison in viewOptionsEqual is a logic fragility: if pg_class.reloptions element order differs between the embedded plan instance and the target database, a no-op diff will be reported as a modification and an unnecessary CREATE OR REPLACE VIEW will be emitted. The code duplication between viewsEqual and viewOptionsEqual is a secondary maintenance risk — if the logic is ever updated in one place, it will silently diverge from the other.
  • internal/diff/view.go — viewOptionsEqual and the inline copy in viewsEqual both need an order-insensitive (sort-then-compare) approach, and viewsEqual should delegate to viewOptionsEqual rather than duplicate the logic.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[View in pg_class] -->|c.reloptions| B[GetViewsForSchema query]
    B -->|pq.Array scan| C[GetViewsForSchemaRow.Reloptions]
    C --> D[Inspector.buildViews]
    D -->|View.Options| E[IR View struct]

    E --> F{Diff: viewsEqual?}
    F -->|definition + options + materialized match| G[No change]
    F -->|differs| H[viewDiff created]

    H --> I{generateModifyViewsSQL}
    I -->|optionsEqual + definitionsEqual| J[commentOnly / indexOnly / triggerOnly path]
    I -->|options or definition changed| K[generateViewSQL]

    K --> L{view.Materialized?}
    L -->|yes| M[CREATE MATERIALIZED VIEW IF NOT EXISTS viewname WITH options AS ...]
    L -->|no| N[CREATE OR REPLACE VIEW viewname WITH options AS ...]
Loading

Last reviewed commit: f11efce

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds end-to-end support for PostgreSQL view reloptions so that options like security_invoker / security_barrier are preserved during inspection, included in generated CREATE VIEW DDL, and considered during diffing.

Changes:

  • Extend view inspection query to return pg_class.reloptions and store it in IR (View.Options).
  • Render view options via CREATE ... VIEW ... WITH (...) AS ... when options are present.
  • Include view options in view equality / modify-view logic so option-only changes are detected.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
ir/queries/queries.sql Adds c.reloptions to the GetViewsForSchema query output.
ir/queries/queries.sql.go Plumbs reloptions into generated sqlc row struct + scan (pq.Array).
ir/ir.go Adds View.Options to IR to represent view reloptions.
ir/inspector.go Populates View.Options from inspected reloptions.
internal/diff/view.go Compares options during diffs and renders WITH (...) in generated view DDL.
testdata/diff/create_view/add_view/* Updates fixture SQL/plan outputs to include a security_invoker view.

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

Address review feedback:
- Sort reloptions at ingestion (inspector) so comparison and DDL rendering
  are order-insensitive and deterministic
- Deduplicate: viewsEqual now delegates to viewOptionsEqual

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.


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

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@tianzhou tianzhou merged commit 23c2f56 into main Mar 8, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Add support for tracking security_invoker on views

2 participants