Skip to content

fix(diff): include view reloptions changes#5500

Open
pjpjq wants to merge 1 commit into
supabase:developfrom
pjpjq:codex/fix-view-reloption-diff
Open

fix(diff): include view reloptions changes#5500
pjpjq wants to merge 1 commit into
supabase:developfrom
pjpjq:codex/fix-view-reloption-diff

Conversation

@pjpjq

@pjpjq pjpjq commented Jun 7, 2026

Copy link
Copy Markdown

Summary

  • Compare source and target view pg_class.reloptions for non-pg-delta db diff.
  • Generate ALTER VIEW ... SET/RESET (...) for reloption-only changes on existing views.
  • Add regression coverage for regular views, materialized views, schema filtering, and the connected diff path.

Fixes #5476.

Tests

  • cd apps/cli-go && go test ./internal/db/diff ./internal/db/pull ./cmd

@pjpjq pjpjq requested a review from a team as a code owner June 7, 2026 10:40
@pjpjq pjpjq changed the title fix(diff): 补充视图 reloptions 差异 fix(diff): include view reloptions changes Jun 9, 2026
@pjpjq pjpjq force-pushed the codex/fix-view-reloption-diff branch from 62b67d8 to 3f15dba Compare June 9, 2026 10:57
@SahilArate

Copy link
Copy Markdown

Nice approach the post-processor pattern is the right call here given migra's blind spot on pg_class.reloptions.
A few thoughts that might help before merge:

  1. No-op guard: if both source and target have reloptions = NULL, the diff should be a clean no-op. Worth adding a test case to confirm there's no spurious SET () emitted.
  2. Dropped view edge case: if a view exists in target but not source (i.e. it's being dropped), the post-processor shouldn't emit an ALTER VIEW SET for it — the drop statement from migra already handles it. A guard on view existence in both catalogs would be defensive here.
  3. Multiple reloptions at once: if security_invoker and another reloption both change in the same diff, the generated SQL should batch them: ALTER VIEW foo SET (security_invoker=false, check_option=cascaded) rather than two separate statements.
  4. RESET vs SET: when a reloption is removed entirely (not just changed), RESET is the correct SQL. Worth verifying the test covers the full remove case, not just value swap.
    Happy to help test or iterate on any of these!

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.

db diff misses reloption-only changes on existing views

2 participants