From 87823cf9f19393b3b7872865c8e3abb4746dd250 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Sun, 8 Mar 2026 20:02:47 -0700 Subject: [PATCH 1/4] fix: support view options (security_invoker, security_barrier) in dump and plan (#350) Add support for PostgreSQL view reloptions (security_invoker, security_barrier, check_option) across the entire pipeline: dump extracts them, plan detects changes, and generates appropriate ALTER VIEW SET/RESET migration DDL. Co-Authored-By: Claude Opus 4.6 --- cmd/dump/dump_integration_test.go | 7 + internal/diff/diff.go | 3 + internal/diff/view.go | 132 +++++++++++++++++- ir/inspector.go | 13 ++ ir/ir.go | 1 + ir/queries/queries.sql | 6 +- ir/queries/queries.sql.go | 8 +- .../diff.sql | 1 + .../new.sql | 8 ++ .../old.sql | 8 ++ .../plan.json | 20 +++ .../plan.sql | 1 + .../plan.txt | 12 ++ .../diff.sql | 1 + .../new.sql | 11 ++ .../old.sql | 8 ++ .../plan.json | 20 +++ .../plan.sql | 1 + .../plan.txt | 12 ++ .../issue_350_remove_view_option/diff.sql | 1 + .../issue_350_remove_view_option/new.sql | 8 ++ .../issue_350_remove_view_option/old.sql | 8 ++ .../issue_350_remove_view_option/plan.json | 20 +++ .../issue_350_remove_view_option/plan.sql | 1 + .../issue_350_remove_view_option/plan.txt | 12 ++ .../manifest.json | 9 ++ .../pgdump.sql | 44 ++++++ .../pgschema.sql | 47 +++++++ .../issue_350_view_security_invoker/raw.sql | 19 +++ 29 files changed, 434 insertions(+), 8 deletions(-) create mode 100644 testdata/diff/create_view/issue_350_add_view_security_invoker/diff.sql create mode 100644 testdata/diff/create_view/issue_350_add_view_security_invoker/new.sql create mode 100644 testdata/diff/create_view/issue_350_add_view_security_invoker/old.sql create mode 100644 testdata/diff/create_view/issue_350_add_view_security_invoker/plan.json create mode 100644 testdata/diff/create_view/issue_350_add_view_security_invoker/plan.sql create mode 100644 testdata/diff/create_view/issue_350_add_view_security_invoker/plan.txt create mode 100644 testdata/diff/create_view/issue_350_alter_view_security_invoker/diff.sql create mode 100644 testdata/diff/create_view/issue_350_alter_view_security_invoker/new.sql create mode 100644 testdata/diff/create_view/issue_350_alter_view_security_invoker/old.sql create mode 100644 testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.json create mode 100644 testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.sql create mode 100644 testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.txt create mode 100644 testdata/diff/create_view/issue_350_remove_view_option/diff.sql create mode 100644 testdata/diff/create_view/issue_350_remove_view_option/new.sql create mode 100644 testdata/diff/create_view/issue_350_remove_view_option/old.sql create mode 100644 testdata/diff/create_view/issue_350_remove_view_option/plan.json create mode 100644 testdata/diff/create_view/issue_350_remove_view_option/plan.sql create mode 100644 testdata/diff/create_view/issue_350_remove_view_option/plan.txt create mode 100644 testdata/dump/issue_350_view_security_invoker/manifest.json create mode 100644 testdata/dump/issue_350_view_security_invoker/pgdump.sql create mode 100644 testdata/dump/issue_350_view_security_invoker/pgschema.sql create mode 100644 testdata/dump/issue_350_view_security_invoker/raw.sql diff --git a/cmd/dump/dump_integration_test.go b/cmd/dump/dump_integration_test.go index 5b8f9252..bc60a7bc 100644 --- a/cmd/dump/dump_integration_test.go +++ b/cmd/dump/dump_integration_test.go @@ -123,6 +123,13 @@ func TestDumpCommand_Issue320PlpgsqlReservedKeywordType(t *testing.T) { runExactMatchTest(t, "issue_320_plpgsql_reserved_keyword_type") } +func TestDumpCommand_Issue350ViewSecurityInvoker(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + runExactMatchTest(t, "issue_350_view_security_invoker") +} + func TestDumpCommand_Issue318CrossSchemaComment(t *testing.T) { if testing.Short() { t.Skip("Skipping integration test in short mode") diff --git a/internal/diff/diff.go b/internal/diff/diff.go index 48f8b5e9..db46c555 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -354,6 +354,7 @@ type viewDiff struct { CommentChanged bool OldComment string NewComment string + OptionsChanged bool // View options (reloptions) changed AddedIndexes []*ir.Index // For materialized views DroppedIndexes []*ir.Index // For materialized views ModifiedIndexes []*IndexDiff // For materialized views @@ -781,6 +782,7 @@ func GenerateMigration(oldIR, newIR *ir.IR, targetSchema string) []Diff { if oldView, exists := oldViews[key]; exists { structurallyDifferent := !viewsEqual(oldView, newView) commentChanged := oldView.Comment != newView.Comment + optionsChanged := !viewOptionsEqual(oldView.Options, newView.Options) // Check if indexes changed for materialized views indexesChanged := false @@ -845,6 +847,7 @@ func GenerateMigration(oldIR, newIR *ir.IR, targetSchema string) []Diff { AddedTriggers: addedTriggers, DroppedTriggers: droppedTriggers, ModifiedTriggers: modifiedTriggers, + OptionsChanged: optionsChanged, } // Check for comment changes diff --git a/internal/diff/view.go b/internal/diff/view.go index becf49a0..fb750751 100644 --- a/internal/diff/view.go +++ b/internal/diff/view.go @@ -248,7 +248,7 @@ func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *d continue } - // Check if only the comment changed and definition is identical + // Check if only metadata changed and definition is identical // Both IRs come from pg_get_viewdef() at the same PostgreSQL version, so string comparison is sufficient definitionsEqual := diff.Old.Definition == diff.New.Definition commentOnlyChange := diff.CommentChanged && definitionsEqual && diff.Old.Materialized == diff.New.Materialized @@ -261,8 +261,16 @@ func generateModifyViewsSQL(diffs []*viewDiff, targetSchema string, collector *d hasTriggerChanges := len(diff.AddedTriggers) > 0 || len(diff.DroppedTriggers) > 0 || len(diff.ModifiedTriggers) > 0 triggerOnlyChange := hasTriggerChanges && definitionsEqual && !diff.CommentChanged && !hasIndexChanges - // Handle non-structural changes (comment-only, index-only, or trigger-only) - if commentOnlyChange || indexOnlyChange || triggerOnlyChange { + // 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 { + // Generate ALTER VIEW SET/RESET for option changes + if diff.OptionsChanged { + generateViewOptionChangesSQL(diff, targetSchema, collector) + } + // Only generate COMMENT ON VIEW statement if comment actually changed if diff.CommentChanged { viewName := qualifyEntityName(diff.New.Schema, diff.New.Name, targetSchema) @@ -503,8 +511,106 @@ func generateViewSQL(view *ir.View, targetSchema string) string { createClause = "CREATE OR REPLACE VIEW" } + // Build WITH clause for view options (e.g., security_invoker, security_barrier) + withClause := formatViewOptionsWithClause(view.Options) + // Use the view definition as-is - it has already been normalized - return fmt.Sprintf("%s %s AS\n%s;", createClause, viewName, view.Definition) + return fmt.Sprintf("%s %s%s AS\n%s;", createClause, viewName, withClause, view.Definition) +} + +// formatViewOptionsWithClause formats view options as a WITH (...) clause. +// Returns empty string if there are no options. +func formatViewOptionsWithClause(options map[string]string) string { + if len(options) == 0 { + return "" + } + + // Sort keys for deterministic output + keys := make([]string, 0, len(options)) + for k := range options { + keys = append(keys, k) + } + sort.Strings(keys) + + parts := make([]string, 0, len(keys)) + for _, k := range keys { + parts = append(parts, fmt.Sprintf("%s = %s", k, options[k])) + } + return " WITH (" + strings.Join(parts, ", ") + ")" +} + +// generateViewOptionChangesSQL generates ALTER VIEW SET/RESET statements for option changes +func generateViewOptionChangesSQL(diff *viewDiff, targetSchema string, collector *diffCollector) { + viewName := qualifyEntityName(diff.New.Schema, diff.New.Name, targetSchema) + + diffType := DiffTypeView + viewKeyword := "VIEW" + if diff.New.Materialized { + diffType = DiffTypeMaterializedView + viewKeyword = "MATERIALIZED VIEW" + } + + // Find options to SET (added or changed) + var setOptions []string + newOpts := diff.New.Options + oldOpts := diff.Old.Options + if newOpts == nil { + newOpts = make(map[string]string) + } + if oldOpts == nil { + oldOpts = make(map[string]string) + } + + // Collect new/changed options + optKeys := make([]string, 0, len(newOpts)) + for k := range newOpts { + optKeys = append(optKeys, k) + } + sort.Strings(optKeys) + for _, k := range optKeys { + if oldOpts[k] != newOpts[k] { + setOptions = append(setOptions, fmt.Sprintf("%s = %s", k, newOpts[k])) + } + } + + // Find options to RESET (removed) + var resetOptions []string + oldKeys := make([]string, 0, len(oldOpts)) + for k := range oldOpts { + oldKeys = append(oldKeys, k) + } + sort.Strings(oldKeys) + for _, k := range oldKeys { + if _, exists := newOpts[k]; !exists { + resetOptions = append(resetOptions, k) + } + } + + // Generate SET statement + if len(setOptions) > 0 { + sql := fmt.Sprintf("ALTER %s %s SET (%s);", viewKeyword, viewName, strings.Join(setOptions, ", ")) + context := &diffContext{ + Type: diffType, + Operation: DiffOperationAlter, + Path: fmt.Sprintf("%s.%s", diff.New.Schema, diff.New.Name), + Source: diff, + CanRunInTransaction: true, + } + collector.collect(context, sql) + } + + // Generate RESET statement + if len(resetOptions) > 0 { + sql := fmt.Sprintf("ALTER %s %s RESET (%s);", viewKeyword, viewName, strings.Join(resetOptions, ", ")) + context := &diffContext{ + Type: diffType, + Operation: DiffOperationAlter, + Path: fmt.Sprintf("%s.%s", diff.New.Schema, diff.New.Name), + Source: diff, + CanRunInTransaction: true, + } + collector.collect(context, sql) + } } // diffViewTriggers computes added, dropped, and modified triggers between two views @@ -572,10 +678,28 @@ func viewsEqual(old, new *ir.View) bool { return false } + // Check if options differ + if !viewOptionsEqual(old.Options, new.Options) { + return false + } + // Both definitions come from pg_get_viewdef(), so they are already normalized return old.Definition == new.Definition } +// viewOptionsEqual compares two view option maps for equality +func viewOptionsEqual(old, new map[string]string) bool { + if len(old) != len(new) { + return false + } + for k, v := range old { + if new[k] != v { + return false + } + } + return true +} + // 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; diff --git a/ir/inspector.go b/ir/inspector.go index 2990eea5..860d780a 100644 --- a/ir/inspector.go +++ b/ir/inspector.go @@ -1363,11 +1363,24 @@ func (i *Inspector) buildViews(ctx context.Context, schema *IR, targetSchema str return fmt.Errorf("failed to get columns for view %s.%s: %w", schemaName, viewName, err) } + // Parse view options (reloptions) from pg_class.reloptions + // Each option is in the format "key=value" + var options map[string]string + if len(view.ViewOptions) > 0 { + options = make(map[string]string, len(view.ViewOptions)) + for _, opt := range view.ViewOptions { + if eqIdx := strings.Index(opt, "="); eqIdx >= 0 { + options[opt[:eqIdx]] = opt[eqIdx+1:] + } + } + } + v := &View{ Schema: schemaName, Name: viewName, Definition: definition, Columns: columns, + Options: options, Comment: comment, Materialized: view.IsMaterialized.Valid && view.IsMaterialized.Bool, } diff --git a/ir/ir.go b/ir/ir.go index 82976e45..1cf95490 100644 --- a/ir/ir.go +++ b/ir/ir.go @@ -123,6 +123,7 @@ type View struct { Name string `json:"name"` Definition string `json:"definition"` Columns []string `json:"columns,omitempty"` // Ordered list of output column names + Options map[string]string `json:"options,omitempty"` // View options (reloptions) e.g. security_invoker, security_barrier, check_option Comment string `json:"comment,omitempty"` Materialized bool `json:"materialized,omitempty"` Indexes map[string]*Index `json:"indexes,omitempty"` // For materialized views only diff --git a/ir/queries/queries.sql b/ir/queries/queries.sql index 75d5b92a..9c406350 100644 --- a/ir/queries/queries.sql +++ b/ir/queries/queries.sql @@ -1073,7 +1073,8 @@ WITH view_definitions AS ( c.oid AS view_oid, COALESCE(d.description, '') AS view_comment, (c.relkind = 'm') AS is_materialized, - n.nspname AS view_schema + n.nspname AS view_schema, + c.reloptions AS view_options FROM pg_class c JOIN pg_namespace n ON c.relnamespace = n.oid LEFT JOIN pg_description d ON d.objoid = c.oid AND d.classoid = 'pg_class'::regclass AND d.objsubid = 0 @@ -1090,7 +1091,8 @@ SELECT -- This ensures cross-schema table references are qualified with schema names sp.view_def AS view_definition, vd.view_comment, - vd.is_materialized + vd.is_materialized, + vd.view_options FROM view_definitions vd CROSS JOIN LATERAL ( SELECT diff --git a/ir/queries/queries.sql.go b/ir/queries/queries.sql.go index dc31afe9..6e5af0ae 100644 --- a/ir/queries/queries.sql.go +++ b/ir/queries/queries.sql.go @@ -3179,7 +3179,8 @@ WITH view_definitions AS ( c.oid AS view_oid, COALESCE(d.description, '') AS view_comment, (c.relkind = 'm') AS is_materialized, - n.nspname AS view_schema + n.nspname AS view_schema, + c.reloptions AS view_options FROM pg_class c JOIN pg_namespace n ON c.relnamespace = n.oid LEFT JOIN pg_description d ON d.objoid = c.oid AND d.classoid = 'pg_class'::regclass AND d.objsubid = 0 @@ -3196,7 +3197,8 @@ SELECT -- This ensures cross-schema table references are qualified with schema names sp.view_def AS view_definition, vd.view_comment, - vd.is_materialized + vd.is_materialized, + vd.view_options FROM view_definitions vd CROSS JOIN LATERAL ( SELECT @@ -3212,6 +3214,7 @@ type GetViewsForSchemaRow struct { ViewDefinition sql.NullString `db:"view_definition" json:"view_definition"` ViewComment sql.NullString `db:"view_comment" json:"view_comment"` IsMaterialized sql.NullBool `db:"is_materialized" json:"is_materialized"` + ViewOptions []string `db:"view_options" json:"view_options"` } // GetViewsForSchema retrieves all views and materialized views for a specific schema @@ -3233,6 +3236,7 @@ func (q *Queries) GetViewsForSchema(ctx context.Context, dollar_1 sql.NullString &i.ViewDefinition, &i.ViewComment, &i.IsMaterialized, + pq.Array(&i.ViewOptions), ); err != nil { return nil, err } diff --git a/testdata/diff/create_view/issue_350_add_view_security_invoker/diff.sql b/testdata/diff/create_view/issue_350_add_view_security_invoker/diff.sql new file mode 100644 index 00000000..223b0292 --- /dev/null +++ b/testdata/diff/create_view/issue_350_add_view_security_invoker/diff.sql @@ -0,0 +1 @@ +ALTER VIEW employee_names SET (security_invoker = true); diff --git a/testdata/diff/create_view/issue_350_add_view_security_invoker/new.sql b/testdata/diff/create_view/issue_350_add_view_security_invoker/new.sql new file mode 100644 index 00000000..069bd11a --- /dev/null +++ b/testdata/diff/create_view/issue_350_add_view_security_invoker/new.sql @@ -0,0 +1,8 @@ +CREATE TABLE public.employees ( + id SERIAL PRIMARY KEY, + name VARCHAR(100), + department_id INTEGER +); + +CREATE VIEW public.employee_names WITH (security_invoker = true) AS +SELECT id, name FROM public.employees; diff --git a/testdata/diff/create_view/issue_350_add_view_security_invoker/old.sql b/testdata/diff/create_view/issue_350_add_view_security_invoker/old.sql new file mode 100644 index 00000000..f4a1fc8c --- /dev/null +++ b/testdata/diff/create_view/issue_350_add_view_security_invoker/old.sql @@ -0,0 +1,8 @@ +CREATE TABLE public.employees ( + id SERIAL PRIMARY KEY, + name VARCHAR(100), + department_id INTEGER +); + +CREATE VIEW public.employee_names AS +SELECT id, name FROM public.employees; diff --git a/testdata/diff/create_view/issue_350_add_view_security_invoker/plan.json b/testdata/diff/create_view/issue_350_add_view_security_invoker/plan.json new file mode 100644 index 00000000..ae6aeaae --- /dev/null +++ b/testdata/diff/create_view/issue_350_add_view_security_invoker/plan.json @@ -0,0 +1,20 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.7.3", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "4f4e909061fa5b09d4e26c1efabd0350603723086d51fa0ae22cfd4884b6e5ee" + }, + "groups": [ + { + "steps": [ + { + "sql": "ALTER VIEW employee_names SET (security_invoker = true);", + "type": "view", + "operation": "alter", + "path": "public.employee_names" + } + ] + } + ] +} diff --git a/testdata/diff/create_view/issue_350_add_view_security_invoker/plan.sql b/testdata/diff/create_view/issue_350_add_view_security_invoker/plan.sql new file mode 100644 index 00000000..223b0292 --- /dev/null +++ b/testdata/diff/create_view/issue_350_add_view_security_invoker/plan.sql @@ -0,0 +1 @@ +ALTER VIEW employee_names SET (security_invoker = true); diff --git a/testdata/diff/create_view/issue_350_add_view_security_invoker/plan.txt b/testdata/diff/create_view/issue_350_add_view_security_invoker/plan.txt new file mode 100644 index 00000000..30a3fbce --- /dev/null +++ b/testdata/diff/create_view/issue_350_add_view_security_invoker/plan.txt @@ -0,0 +1,12 @@ +Plan: 1 to modify. + +Summary by type: + views: 1 to modify + +Views: + ~ employee_names + +DDL to be executed: +-------------------------------------------------- + +ALTER VIEW employee_names SET (security_invoker = true); diff --git a/testdata/diff/create_view/issue_350_alter_view_security_invoker/diff.sql b/testdata/diff/create_view/issue_350_alter_view_security_invoker/diff.sql new file mode 100644 index 00000000..223b0292 --- /dev/null +++ b/testdata/diff/create_view/issue_350_alter_view_security_invoker/diff.sql @@ -0,0 +1 @@ +ALTER VIEW employee_names SET (security_invoker = true); diff --git a/testdata/diff/create_view/issue_350_alter_view_security_invoker/new.sql b/testdata/diff/create_view/issue_350_alter_view_security_invoker/new.sql new file mode 100644 index 00000000..ede877e9 --- /dev/null +++ b/testdata/diff/create_view/issue_350_alter_view_security_invoker/new.sql @@ -0,0 +1,11 @@ +CREATE TABLE public.employees ( + id SERIAL PRIMARY KEY, + name VARCHAR(100), + department_id INTEGER +); + +-- Using ALTER VIEW SET to add security_invoker (the pattern from the issue) +CREATE VIEW public.employee_names AS +SELECT id, name FROM public.employees; + +ALTER VIEW public.employee_names SET (security_invoker = true); diff --git a/testdata/diff/create_view/issue_350_alter_view_security_invoker/old.sql b/testdata/diff/create_view/issue_350_alter_view_security_invoker/old.sql new file mode 100644 index 00000000..f4a1fc8c --- /dev/null +++ b/testdata/diff/create_view/issue_350_alter_view_security_invoker/old.sql @@ -0,0 +1,8 @@ +CREATE TABLE public.employees ( + id SERIAL PRIMARY KEY, + name VARCHAR(100), + department_id INTEGER +); + +CREATE VIEW public.employee_names AS +SELECT id, name FROM public.employees; diff --git a/testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.json b/testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.json new file mode 100644 index 00000000..ae6aeaae --- /dev/null +++ b/testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.json @@ -0,0 +1,20 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.7.3", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "4f4e909061fa5b09d4e26c1efabd0350603723086d51fa0ae22cfd4884b6e5ee" + }, + "groups": [ + { + "steps": [ + { + "sql": "ALTER VIEW employee_names SET (security_invoker = true);", + "type": "view", + "operation": "alter", + "path": "public.employee_names" + } + ] + } + ] +} diff --git a/testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.sql b/testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.sql new file mode 100644 index 00000000..223b0292 --- /dev/null +++ b/testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.sql @@ -0,0 +1 @@ +ALTER VIEW employee_names SET (security_invoker = true); diff --git a/testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.txt b/testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.txt new file mode 100644 index 00000000..30a3fbce --- /dev/null +++ b/testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.txt @@ -0,0 +1,12 @@ +Plan: 1 to modify. + +Summary by type: + views: 1 to modify + +Views: + ~ employee_names + +DDL to be executed: +-------------------------------------------------- + +ALTER VIEW employee_names SET (security_invoker = true); diff --git a/testdata/diff/create_view/issue_350_remove_view_option/diff.sql b/testdata/diff/create_view/issue_350_remove_view_option/diff.sql new file mode 100644 index 00000000..e83b1259 --- /dev/null +++ b/testdata/diff/create_view/issue_350_remove_view_option/diff.sql @@ -0,0 +1 @@ +ALTER VIEW employee_names RESET (security_invoker); diff --git a/testdata/diff/create_view/issue_350_remove_view_option/new.sql b/testdata/diff/create_view/issue_350_remove_view_option/new.sql new file mode 100644 index 00000000..f4a1fc8c --- /dev/null +++ b/testdata/diff/create_view/issue_350_remove_view_option/new.sql @@ -0,0 +1,8 @@ +CREATE TABLE public.employees ( + id SERIAL PRIMARY KEY, + name VARCHAR(100), + department_id INTEGER +); + +CREATE VIEW public.employee_names AS +SELECT id, name FROM public.employees; diff --git a/testdata/diff/create_view/issue_350_remove_view_option/old.sql b/testdata/diff/create_view/issue_350_remove_view_option/old.sql new file mode 100644 index 00000000..069bd11a --- /dev/null +++ b/testdata/diff/create_view/issue_350_remove_view_option/old.sql @@ -0,0 +1,8 @@ +CREATE TABLE public.employees ( + id SERIAL PRIMARY KEY, + name VARCHAR(100), + department_id INTEGER +); + +CREATE VIEW public.employee_names WITH (security_invoker = true) AS +SELECT id, name FROM public.employees; diff --git a/testdata/diff/create_view/issue_350_remove_view_option/plan.json b/testdata/diff/create_view/issue_350_remove_view_option/plan.json new file mode 100644 index 00000000..588d62b4 --- /dev/null +++ b/testdata/diff/create_view/issue_350_remove_view_option/plan.json @@ -0,0 +1,20 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.7.3", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "b9dc8ee4795ddd76cd8ee54ffd44cd7a3bd7726e008be30f02ffcba85072dfd8" + }, + "groups": [ + { + "steps": [ + { + "sql": "ALTER VIEW employee_names RESET (security_invoker);", + "type": "view", + "operation": "alter", + "path": "public.employee_names" + } + ] + } + ] +} diff --git a/testdata/diff/create_view/issue_350_remove_view_option/plan.sql b/testdata/diff/create_view/issue_350_remove_view_option/plan.sql new file mode 100644 index 00000000..e83b1259 --- /dev/null +++ b/testdata/diff/create_view/issue_350_remove_view_option/plan.sql @@ -0,0 +1 @@ +ALTER VIEW employee_names RESET (security_invoker); diff --git a/testdata/diff/create_view/issue_350_remove_view_option/plan.txt b/testdata/diff/create_view/issue_350_remove_view_option/plan.txt new file mode 100644 index 00000000..662127fa --- /dev/null +++ b/testdata/diff/create_view/issue_350_remove_view_option/plan.txt @@ -0,0 +1,12 @@ +Plan: 1 to modify. + +Summary by type: + views: 1 to modify + +Views: + ~ employee_names + +DDL to be executed: +-------------------------------------------------- + +ALTER VIEW employee_names RESET (security_invoker); diff --git a/testdata/dump/issue_350_view_security_invoker/manifest.json b/testdata/dump/issue_350_view_security_invoker/manifest.json new file mode 100644 index 00000000..a0239cdd --- /dev/null +++ b/testdata/dump/issue_350_view_security_invoker/manifest.json @@ -0,0 +1,9 @@ +{ + "name": "issue_350_view_security_invoker", + "description": "Test case for view options (security_invoker) support (GitHub issue #350)", + "source": "https://github.com/pgplex/pgschema/issues/350", + "notes": [ + "Reproduces the bug where ALTER VIEW SET (security_invoker = true) was not supported", + "Tests that view options are preserved in dump output via WITH clause" + ] +} diff --git a/testdata/dump/issue_350_view_security_invoker/pgdump.sql b/testdata/dump/issue_350_view_security_invoker/pgdump.sql new file mode 100644 index 00000000..e56eb13f --- /dev/null +++ b/testdata/dump/issue_350_view_security_invoker/pgdump.sql @@ -0,0 +1,44 @@ +SET check_function_bodies = false; + +CREATE TABLE public.employees ( + id integer NOT NULL, + name character varying(100) NOT NULL, + email character varying(100) +); + +CREATE SEQUENCE public.employees_id_seq + AS integer + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE public.employees_id_seq OWNED BY public.employees.id; + +ALTER TABLE ONLY public.employees ALTER COLUMN id SET DEFAULT nextval('public.employees_id_seq'::regclass); + +ALTER TABLE ONLY public.employees + ADD CONSTRAINT employees_pkey PRIMARY KEY (id); + +CREATE VIEW public.employee_names AS + SELECT id, + name + FROM public.employees; + +ALTER VIEW public.employee_names SET (security_invoker='true'); + +CREATE VIEW public.employee_emails AS + SELECT id, + email + FROM public.employees; + +ALTER VIEW public.employee_emails SET (security_invoker='true'); + +CREATE VIEW public.employee_secure AS + SELECT id, + name + FROM public.employees + WHERE (id > 0); + +ALTER VIEW public.employee_secure SET (security_barrier='true'); diff --git a/testdata/dump/issue_350_view_security_invoker/pgschema.sql b/testdata/dump/issue_350_view_security_invoker/pgschema.sql new file mode 100644 index 00000000..02800fb7 --- /dev/null +++ b/testdata/dump/issue_350_view_security_invoker/pgschema.sql @@ -0,0 +1,47 @@ +-- +-- pgschema database dump +-- + +-- Dumped from database version PostgreSQL 18.0 +-- Dumped by pgschema version 1.7.3 + + +-- +-- Name: employees; Type: TABLE; Schema: -; Owner: - +-- + +CREATE TABLE IF NOT EXISTS employees ( + id SERIAL, + name varchar(100) NOT NULL, + email varchar(100), + CONSTRAINT employees_pkey PRIMARY KEY (id) +); + +-- +-- Name: employee_emails; Type: VIEW; Schema: -; Owner: - +-- + +CREATE OR REPLACE VIEW employee_emails WITH (security_invoker = true) AS + SELECT id, + email + FROM employees; + +-- +-- Name: employee_names; Type: VIEW; Schema: -; Owner: - +-- + +CREATE OR REPLACE VIEW employee_names WITH (security_invoker = true) AS + SELECT id, + name + FROM employees; + +-- +-- Name: employee_secure; Type: VIEW; Schema: -; Owner: - +-- + +CREATE OR REPLACE VIEW employee_secure WITH (security_barrier = true) AS + SELECT id, + name + FROM employees + WHERE id > 0; + diff --git a/testdata/dump/issue_350_view_security_invoker/raw.sql b/testdata/dump/issue_350_view_security_invoker/raw.sql new file mode 100644 index 00000000..992a3642 --- /dev/null +++ b/testdata/dump/issue_350_view_security_invoker/raw.sql @@ -0,0 +1,19 @@ +CREATE TABLE public.employees ( + id SERIAL PRIMARY KEY, + name VARCHAR(100) NOT NULL, + email VARCHAR(100) +); + +-- View with security_invoker via CREATE VIEW WITH +CREATE VIEW public.employee_names WITH (security_invoker = true) AS +SELECT id, name FROM public.employees; + +-- View with security_invoker via ALTER VIEW SET +CREATE VIEW public.employee_emails AS +SELECT id, email FROM public.employees; + +ALTER VIEW public.employee_emails SET (security_invoker = true); + +-- View with security_barrier +CREATE VIEW public.employee_secure WITH (security_barrier = true) AS +SELECT id, name FROM public.employees WHERE id > 0; From f4340650ffb08eace40f10ae815add85fb58b9bf Mon Sep 17 00:00:00 2001 From: tianzhou Date: Sun, 8 Mar 2026 20:26:18 -0700 Subject: [PATCH 2/4] refactor: consolidate issue 350 view options into single test case Merge three separate test cases (add, alter, remove) into one that covers all scenarios: add via WITH, add via ALTER VIEW SET, and remove. Co-Authored-By: Claude Opus 4.6 --- .../diff.sql | 1 - .../new.sql | 8 ----- .../old.sql | 8 ----- .../plan.json | 20 ------------ .../plan.sql | 1 - .../plan.txt | 12 ------- .../diff.sql | 1 - .../new.sql | 11 ------- .../old.sql | 8 ----- .../plan.json | 20 ------------ .../plan.sql | 1 - .../plan.txt | 12 ------- .../issue_350_remove_view_option/diff.sql | 1 - .../issue_350_remove_view_option/new.sql | 8 ----- .../issue_350_remove_view_option/old.sql | 8 ----- .../issue_350_remove_view_option/plan.json | 20 ------------ .../issue_350_remove_view_option/plan.sql | 1 - .../issue_350_remove_view_option/plan.txt | 12 ------- .../issue_350_view_options/diff.sql | 3 ++ .../issue_350_view_options/new.sql | 20 ++++++++++++ .../issue_350_view_options/old.sql | 18 +++++++++++ .../issue_350_view_options/plan.json | 32 +++++++++++++++++++ .../issue_350_view_options/plan.sql | 5 +++ .../issue_350_view_options/plan.txt | 18 +++++++++++ 24 files changed, 96 insertions(+), 153 deletions(-) delete mode 100644 testdata/diff/create_view/issue_350_add_view_security_invoker/diff.sql delete mode 100644 testdata/diff/create_view/issue_350_add_view_security_invoker/new.sql delete mode 100644 testdata/diff/create_view/issue_350_add_view_security_invoker/old.sql delete mode 100644 testdata/diff/create_view/issue_350_add_view_security_invoker/plan.json delete mode 100644 testdata/diff/create_view/issue_350_add_view_security_invoker/plan.sql delete mode 100644 testdata/diff/create_view/issue_350_add_view_security_invoker/plan.txt delete mode 100644 testdata/diff/create_view/issue_350_alter_view_security_invoker/diff.sql delete mode 100644 testdata/diff/create_view/issue_350_alter_view_security_invoker/new.sql delete mode 100644 testdata/diff/create_view/issue_350_alter_view_security_invoker/old.sql delete mode 100644 testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.json delete mode 100644 testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.sql delete mode 100644 testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.txt delete mode 100644 testdata/diff/create_view/issue_350_remove_view_option/diff.sql delete mode 100644 testdata/diff/create_view/issue_350_remove_view_option/new.sql delete mode 100644 testdata/diff/create_view/issue_350_remove_view_option/old.sql delete mode 100644 testdata/diff/create_view/issue_350_remove_view_option/plan.json delete mode 100644 testdata/diff/create_view/issue_350_remove_view_option/plan.sql delete mode 100644 testdata/diff/create_view/issue_350_remove_view_option/plan.txt create mode 100644 testdata/diff/create_view/issue_350_view_options/diff.sql create mode 100644 testdata/diff/create_view/issue_350_view_options/new.sql create mode 100644 testdata/diff/create_view/issue_350_view_options/old.sql create mode 100644 testdata/diff/create_view/issue_350_view_options/plan.json create mode 100644 testdata/diff/create_view/issue_350_view_options/plan.sql create mode 100644 testdata/diff/create_view/issue_350_view_options/plan.txt diff --git a/testdata/diff/create_view/issue_350_add_view_security_invoker/diff.sql b/testdata/diff/create_view/issue_350_add_view_security_invoker/diff.sql deleted file mode 100644 index 9176e489..00000000 --- a/testdata/diff/create_view/issue_350_add_view_security_invoker/diff.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER VIEW employee_names SET (security_invoker=true); diff --git a/testdata/diff/create_view/issue_350_add_view_security_invoker/new.sql b/testdata/diff/create_view/issue_350_add_view_security_invoker/new.sql deleted file mode 100644 index 069bd11a..00000000 --- a/testdata/diff/create_view/issue_350_add_view_security_invoker/new.sql +++ /dev/null @@ -1,8 +0,0 @@ -CREATE TABLE public.employees ( - id SERIAL PRIMARY KEY, - name VARCHAR(100), - department_id INTEGER -); - -CREATE VIEW public.employee_names WITH (security_invoker = true) AS -SELECT id, name FROM public.employees; diff --git a/testdata/diff/create_view/issue_350_add_view_security_invoker/old.sql b/testdata/diff/create_view/issue_350_add_view_security_invoker/old.sql deleted file mode 100644 index f4a1fc8c..00000000 --- a/testdata/diff/create_view/issue_350_add_view_security_invoker/old.sql +++ /dev/null @@ -1,8 +0,0 @@ -CREATE TABLE public.employees ( - id SERIAL PRIMARY KEY, - name VARCHAR(100), - department_id INTEGER -); - -CREATE VIEW public.employee_names AS -SELECT id, name FROM public.employees; diff --git a/testdata/diff/create_view/issue_350_add_view_security_invoker/plan.json b/testdata/diff/create_view/issue_350_add_view_security_invoker/plan.json deleted file mode 100644 index 0f46ab25..00000000 --- a/testdata/diff/create_view/issue_350_add_view_security_invoker/plan.json +++ /dev/null @@ -1,20 +0,0 @@ -{ - "version": "1.0.0", - "pgschema_version": "1.7.4", - "created_at": "1970-01-01T00:00:00Z", - "source_fingerprint": { - "hash": "4f4e909061fa5b09d4e26c1efabd0350603723086d51fa0ae22cfd4884b6e5ee" - }, - "groups": [ - { - "steps": [ - { - "sql": "ALTER VIEW employee_names SET (security_invoker=true);", - "type": "view", - "operation": "alter", - "path": "public.employee_names" - } - ] - } - ] -} diff --git a/testdata/diff/create_view/issue_350_add_view_security_invoker/plan.sql b/testdata/diff/create_view/issue_350_add_view_security_invoker/plan.sql deleted file mode 100644 index 9176e489..00000000 --- a/testdata/diff/create_view/issue_350_add_view_security_invoker/plan.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER VIEW employee_names SET (security_invoker=true); diff --git a/testdata/diff/create_view/issue_350_add_view_security_invoker/plan.txt b/testdata/diff/create_view/issue_350_add_view_security_invoker/plan.txt deleted file mode 100644 index 9a316f41..00000000 --- a/testdata/diff/create_view/issue_350_add_view_security_invoker/plan.txt +++ /dev/null @@ -1,12 +0,0 @@ -Plan: 1 to modify. - -Summary by type: - views: 1 to modify - -Views: - ~ employee_names - -DDL to be executed: --------------------------------------------------- - -ALTER VIEW employee_names SET (security_invoker=true); diff --git a/testdata/diff/create_view/issue_350_alter_view_security_invoker/diff.sql b/testdata/diff/create_view/issue_350_alter_view_security_invoker/diff.sql deleted file mode 100644 index 9176e489..00000000 --- a/testdata/diff/create_view/issue_350_alter_view_security_invoker/diff.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER VIEW employee_names SET (security_invoker=true); diff --git a/testdata/diff/create_view/issue_350_alter_view_security_invoker/new.sql b/testdata/diff/create_view/issue_350_alter_view_security_invoker/new.sql deleted file mode 100644 index ede877e9..00000000 --- a/testdata/diff/create_view/issue_350_alter_view_security_invoker/new.sql +++ /dev/null @@ -1,11 +0,0 @@ -CREATE TABLE public.employees ( - id SERIAL PRIMARY KEY, - name VARCHAR(100), - department_id INTEGER -); - --- Using ALTER VIEW SET to add security_invoker (the pattern from the issue) -CREATE VIEW public.employee_names AS -SELECT id, name FROM public.employees; - -ALTER VIEW public.employee_names SET (security_invoker = true); diff --git a/testdata/diff/create_view/issue_350_alter_view_security_invoker/old.sql b/testdata/diff/create_view/issue_350_alter_view_security_invoker/old.sql deleted file mode 100644 index f4a1fc8c..00000000 --- a/testdata/diff/create_view/issue_350_alter_view_security_invoker/old.sql +++ /dev/null @@ -1,8 +0,0 @@ -CREATE TABLE public.employees ( - id SERIAL PRIMARY KEY, - name VARCHAR(100), - department_id INTEGER -); - -CREATE VIEW public.employee_names AS -SELECT id, name FROM public.employees; diff --git a/testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.json b/testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.json deleted file mode 100644 index 0f46ab25..00000000 --- a/testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.json +++ /dev/null @@ -1,20 +0,0 @@ -{ - "version": "1.0.0", - "pgschema_version": "1.7.4", - "created_at": "1970-01-01T00:00:00Z", - "source_fingerprint": { - "hash": "4f4e909061fa5b09d4e26c1efabd0350603723086d51fa0ae22cfd4884b6e5ee" - }, - "groups": [ - { - "steps": [ - { - "sql": "ALTER VIEW employee_names SET (security_invoker=true);", - "type": "view", - "operation": "alter", - "path": "public.employee_names" - } - ] - } - ] -} diff --git a/testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.sql b/testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.sql deleted file mode 100644 index 9176e489..00000000 --- a/testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER VIEW employee_names SET (security_invoker=true); diff --git a/testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.txt b/testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.txt deleted file mode 100644 index 9a316f41..00000000 --- a/testdata/diff/create_view/issue_350_alter_view_security_invoker/plan.txt +++ /dev/null @@ -1,12 +0,0 @@ -Plan: 1 to modify. - -Summary by type: - views: 1 to modify - -Views: - ~ employee_names - -DDL to be executed: --------------------------------------------------- - -ALTER VIEW employee_names SET (security_invoker=true); diff --git a/testdata/diff/create_view/issue_350_remove_view_option/diff.sql b/testdata/diff/create_view/issue_350_remove_view_option/diff.sql deleted file mode 100644 index e83b1259..00000000 --- a/testdata/diff/create_view/issue_350_remove_view_option/diff.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER VIEW employee_names RESET (security_invoker); diff --git a/testdata/diff/create_view/issue_350_remove_view_option/new.sql b/testdata/diff/create_view/issue_350_remove_view_option/new.sql deleted file mode 100644 index f4a1fc8c..00000000 --- a/testdata/diff/create_view/issue_350_remove_view_option/new.sql +++ /dev/null @@ -1,8 +0,0 @@ -CREATE TABLE public.employees ( - id SERIAL PRIMARY KEY, - name VARCHAR(100), - department_id INTEGER -); - -CREATE VIEW public.employee_names AS -SELECT id, name FROM public.employees; diff --git a/testdata/diff/create_view/issue_350_remove_view_option/old.sql b/testdata/diff/create_view/issue_350_remove_view_option/old.sql deleted file mode 100644 index 069bd11a..00000000 --- a/testdata/diff/create_view/issue_350_remove_view_option/old.sql +++ /dev/null @@ -1,8 +0,0 @@ -CREATE TABLE public.employees ( - id SERIAL PRIMARY KEY, - name VARCHAR(100), - department_id INTEGER -); - -CREATE VIEW public.employee_names WITH (security_invoker = true) AS -SELECT id, name FROM public.employees; diff --git a/testdata/diff/create_view/issue_350_remove_view_option/plan.json b/testdata/diff/create_view/issue_350_remove_view_option/plan.json deleted file mode 100644 index 4db3c4d4..00000000 --- a/testdata/diff/create_view/issue_350_remove_view_option/plan.json +++ /dev/null @@ -1,20 +0,0 @@ -{ - "version": "1.0.0", - "pgschema_version": "1.7.4", - "created_at": "1970-01-01T00:00:00Z", - "source_fingerprint": { - "hash": "346a7e2edc8a6201d04d49816424965cb569a2322d9520c403e673ab8081775a" - }, - "groups": [ - { - "steps": [ - { - "sql": "ALTER VIEW employee_names RESET (security_invoker);", - "type": "view", - "operation": "alter", - "path": "public.employee_names" - } - ] - } - ] -} diff --git a/testdata/diff/create_view/issue_350_remove_view_option/plan.sql b/testdata/diff/create_view/issue_350_remove_view_option/plan.sql deleted file mode 100644 index e83b1259..00000000 --- a/testdata/diff/create_view/issue_350_remove_view_option/plan.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER VIEW employee_names RESET (security_invoker); diff --git a/testdata/diff/create_view/issue_350_remove_view_option/plan.txt b/testdata/diff/create_view/issue_350_remove_view_option/plan.txt deleted file mode 100644 index 662127fa..00000000 --- a/testdata/diff/create_view/issue_350_remove_view_option/plan.txt +++ /dev/null @@ -1,12 +0,0 @@ -Plan: 1 to modify. - -Summary by type: - views: 1 to modify - -Views: - ~ employee_names - -DDL to be executed: --------------------------------------------------- - -ALTER VIEW employee_names RESET (security_invoker); diff --git a/testdata/diff/create_view/issue_350_view_options/diff.sql b/testdata/diff/create_view/issue_350_view_options/diff.sql new file mode 100644 index 00000000..a5494d87 --- /dev/null +++ b/testdata/diff/create_view/issue_350_view_options/diff.sql @@ -0,0 +1,3 @@ +ALTER VIEW employee_emails SET (security_invoker=true); +ALTER VIEW employee_names SET (security_invoker=true); +ALTER VIEW employee_secure RESET (security_invoker); diff --git a/testdata/diff/create_view/issue_350_view_options/new.sql b/testdata/diff/create_view/issue_350_view_options/new.sql new file mode 100644 index 00000000..b54b4612 --- /dev/null +++ b/testdata/diff/create_view/issue_350_view_options/new.sql @@ -0,0 +1,20 @@ +CREATE TABLE public.employees ( + id SERIAL PRIMARY KEY, + name VARCHAR(100), + email VARCHAR(100), + department_id INTEGER +); + +-- Add security_invoker via CREATE VIEW WITH +CREATE VIEW public.employee_names WITH (security_invoker = true) AS +SELECT id, name FROM public.employees; + +-- Add security_invoker via ALTER VIEW SET +CREATE VIEW public.employee_emails AS +SELECT id, email FROM public.employees; + +ALTER VIEW public.employee_emails SET (security_invoker = true); + +-- Remove security_invoker +CREATE VIEW public.employee_secure AS +SELECT id, name FROM public.employees WHERE id > 0; diff --git a/testdata/diff/create_view/issue_350_view_options/old.sql b/testdata/diff/create_view/issue_350_view_options/old.sql new file mode 100644 index 00000000..6bf88f45 --- /dev/null +++ b/testdata/diff/create_view/issue_350_view_options/old.sql @@ -0,0 +1,18 @@ +CREATE TABLE public.employees ( + id SERIAL PRIMARY KEY, + name VARCHAR(100), + email VARCHAR(100), + department_id INTEGER +); + +-- View without options (will gain security_invoker via WITH) +CREATE VIEW public.employee_names AS +SELECT id, name FROM public.employees; + +-- View without options (will gain security_invoker via ALTER VIEW SET) +CREATE VIEW public.employee_emails AS +SELECT id, email FROM public.employees; + +-- View with security_invoker (will be removed) +CREATE VIEW public.employee_secure WITH (security_invoker = true) AS +SELECT id, name FROM public.employees WHERE id > 0; diff --git a/testdata/diff/create_view/issue_350_view_options/plan.json b/testdata/diff/create_view/issue_350_view_options/plan.json new file mode 100644 index 00000000..fb6e5f88 --- /dev/null +++ b/testdata/diff/create_view/issue_350_view_options/plan.json @@ -0,0 +1,32 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.7.4", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "e575624236fd96e28f386d9f25c6147d6b5b98a165dab8b74f84f53f6e2de0de" + }, + "groups": [ + { + "steps": [ + { + "sql": "ALTER VIEW employee_emails SET (security_invoker=true);", + "type": "view", + "operation": "alter", + "path": "public.employee_emails" + }, + { + "sql": "ALTER VIEW employee_names SET (security_invoker=true);", + "type": "view", + "operation": "alter", + "path": "public.employee_names" + }, + { + "sql": "ALTER VIEW employee_secure RESET (security_invoker);", + "type": "view", + "operation": "alter", + "path": "public.employee_secure" + } + ] + } + ] +} diff --git a/testdata/diff/create_view/issue_350_view_options/plan.sql b/testdata/diff/create_view/issue_350_view_options/plan.sql new file mode 100644 index 00000000..913d3b58 --- /dev/null +++ b/testdata/diff/create_view/issue_350_view_options/plan.sql @@ -0,0 +1,5 @@ +ALTER VIEW employee_emails SET (security_invoker=true); + +ALTER VIEW employee_names SET (security_invoker=true); + +ALTER VIEW employee_secure RESET (security_invoker); diff --git a/testdata/diff/create_view/issue_350_view_options/plan.txt b/testdata/diff/create_view/issue_350_view_options/plan.txt new file mode 100644 index 00000000..fa6ac5e4 --- /dev/null +++ b/testdata/diff/create_view/issue_350_view_options/plan.txt @@ -0,0 +1,18 @@ +Plan: 3 to modify. + +Summary by type: + views: 3 to modify + +Views: + ~ employee_emails + ~ employee_names + ~ employee_secure + +DDL to be executed: +-------------------------------------------------- + +ALTER VIEW employee_emails SET (security_invoker=true); + +ALTER VIEW employee_names SET (security_invoker=true); + +ALTER VIEW employee_secure RESET (security_invoker); From 171b55a9b4a4e1def21535375ca7816e7c527b93 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Sun, 8 Mar 2026 20:27:57 -0700 Subject: [PATCH 3/4] refactor: remove separate dump test case for issue 350 The diff test already covers all view options scenarios. Co-Authored-By: Claude Opus 4.6 --- cmd/dump/dump_integration_test.go | 7 --- .../manifest.json | 9 ---- .../pgdump.sql | 44 ----------------- .../pgschema.sql | 47 ------------------- .../issue_350_view_security_invoker/raw.sql | 19 -------- 5 files changed, 126 deletions(-) delete mode 100644 testdata/dump/issue_350_view_security_invoker/manifest.json delete mode 100644 testdata/dump/issue_350_view_security_invoker/pgdump.sql delete mode 100644 testdata/dump/issue_350_view_security_invoker/pgschema.sql delete mode 100644 testdata/dump/issue_350_view_security_invoker/raw.sql diff --git a/cmd/dump/dump_integration_test.go b/cmd/dump/dump_integration_test.go index 8c48862e..9517c9ff 100644 --- a/cmd/dump/dump_integration_test.go +++ b/cmd/dump/dump_integration_test.go @@ -130,13 +130,6 @@ func TestDumpCommand_Issue345ArrayCast(t *testing.T) { runExactMatchTest(t, "issue_345_array_cast") } -func TestDumpCommand_Issue350ViewSecurityInvoker(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - runExactMatchTest(t, "issue_350_view_security_invoker") -} - func TestDumpCommand_Issue318CrossSchemaComment(t *testing.T) { if testing.Short() { t.Skip("Skipping integration test in short mode") diff --git a/testdata/dump/issue_350_view_security_invoker/manifest.json b/testdata/dump/issue_350_view_security_invoker/manifest.json deleted file mode 100644 index a0239cdd..00000000 --- a/testdata/dump/issue_350_view_security_invoker/manifest.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "name": "issue_350_view_security_invoker", - "description": "Test case for view options (security_invoker) support (GitHub issue #350)", - "source": "https://github.com/pgplex/pgschema/issues/350", - "notes": [ - "Reproduces the bug where ALTER VIEW SET (security_invoker = true) was not supported", - "Tests that view options are preserved in dump output via WITH clause" - ] -} diff --git a/testdata/dump/issue_350_view_security_invoker/pgdump.sql b/testdata/dump/issue_350_view_security_invoker/pgdump.sql deleted file mode 100644 index e56eb13f..00000000 --- a/testdata/dump/issue_350_view_security_invoker/pgdump.sql +++ /dev/null @@ -1,44 +0,0 @@ -SET check_function_bodies = false; - -CREATE TABLE public.employees ( - id integer NOT NULL, - name character varying(100) NOT NULL, - email character varying(100) -); - -CREATE SEQUENCE public.employees_id_seq - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE public.employees_id_seq OWNED BY public.employees.id; - -ALTER TABLE ONLY public.employees ALTER COLUMN id SET DEFAULT nextval('public.employees_id_seq'::regclass); - -ALTER TABLE ONLY public.employees - ADD CONSTRAINT employees_pkey PRIMARY KEY (id); - -CREATE VIEW public.employee_names AS - SELECT id, - name - FROM public.employees; - -ALTER VIEW public.employee_names SET (security_invoker='true'); - -CREATE VIEW public.employee_emails AS - SELECT id, - email - FROM public.employees; - -ALTER VIEW public.employee_emails SET (security_invoker='true'); - -CREATE VIEW public.employee_secure AS - SELECT id, - name - FROM public.employees - WHERE (id > 0); - -ALTER VIEW public.employee_secure SET (security_barrier='true'); diff --git a/testdata/dump/issue_350_view_security_invoker/pgschema.sql b/testdata/dump/issue_350_view_security_invoker/pgschema.sql deleted file mode 100644 index 4aa50a35..00000000 --- a/testdata/dump/issue_350_view_security_invoker/pgschema.sql +++ /dev/null @@ -1,47 +0,0 @@ --- --- pgschema database dump --- - --- Dumped from database version PostgreSQL 18.0 --- Dumped by pgschema version 1.7.4 - - --- --- Name: employees; Type: TABLE; Schema: -; Owner: - --- - -CREATE TABLE IF NOT EXISTS employees ( - id SERIAL, - name varchar(100) NOT NULL, - email varchar(100), - CONSTRAINT employees_pkey PRIMARY KEY (id) -); - --- --- Name: employee_emails; Type: VIEW; Schema: -; Owner: - --- - -CREATE OR REPLACE VIEW employee_emails WITH (security_invoker=true) AS - SELECT id, - email - FROM employees; - --- --- Name: employee_names; Type: VIEW; Schema: -; Owner: - --- - -CREATE OR REPLACE VIEW employee_names WITH (security_invoker=true) AS - SELECT id, - name - FROM employees; - --- --- Name: employee_secure; Type: VIEW; Schema: -; Owner: - --- - -CREATE OR REPLACE VIEW employee_secure WITH (security_barrier=true) AS - SELECT id, - name - FROM employees - WHERE id > 0; - diff --git a/testdata/dump/issue_350_view_security_invoker/raw.sql b/testdata/dump/issue_350_view_security_invoker/raw.sql deleted file mode 100644 index 992a3642..00000000 --- a/testdata/dump/issue_350_view_security_invoker/raw.sql +++ /dev/null @@ -1,19 +0,0 @@ -CREATE TABLE public.employees ( - id SERIAL PRIMARY KEY, - name VARCHAR(100) NOT NULL, - email VARCHAR(100) -); - --- View with security_invoker via CREATE VIEW WITH -CREATE VIEW public.employee_names WITH (security_invoker = true) AS -SELECT id, name FROM public.employees; - --- View with security_invoker via ALTER VIEW SET -CREATE VIEW public.employee_emails AS -SELECT id, email FROM public.employees; - -ALTER VIEW public.employee_emails SET (security_invoker = true); - --- View with security_barrier -CREATE VIEW public.employee_secure WITH (security_barrier = true) AS -SELECT id, name FROM public.employees WHERE id > 0; From 7f578e558c016d029aadef29933a5d78bef696d3 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Sun, 8 Mar 2026 21:01:44 -0700 Subject: [PATCH 4/4] fix: materialized view option-only changes use ALTER instead of DROP+CREATE Separate definitionChanged from structurallyDifferent so that option-only changes on materialized views emit ALTER SET/RESET rather than a full DROP+CREATE cycle. Added materialized view (fillfactor) to the test fixture. Co-Authored-By: Claude Opus 4.6 --- internal/diff/diff.go | 12 +++++++++--- .../diff/create_view/issue_350_view_options/diff.sql | 1 + .../diff/create_view/issue_350_view_options/new.sql | 4 ++++ .../diff/create_view/issue_350_view_options/old.sql | 4 ++++ .../create_view/issue_350_view_options/plan.json | 8 +++++++- .../diff/create_view/issue_350_view_options/plan.sql | 2 ++ .../diff/create_view/issue_350_view_options/plan.txt | 8 +++++++- 7 files changed, 34 insertions(+), 5 deletions(-) diff --git a/internal/diff/diff.go b/internal/diff/diff.go index db46c555..f8146d0f 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -781,6 +781,10 @@ func GenerateMigration(oldIR, newIR *ir.IR, targetSchema string) []Diff { newView := newViews[key] if oldView, exists := oldViews[key]; exists { structurallyDifferent := !viewsEqual(oldView, newView) + // Check if the view definition itself changed (excluding options). + // This is used to decide if materialized views need DROP+CREATE: + // option-only changes should use ALTER VIEW SET/RESET, not recreation. + definitionChanged := oldView.Definition != newView.Definition || oldView.Materialized != newView.Materialized commentChanged := oldView.Comment != newView.Comment optionsChanged := !viewOptionsEqual(oldView.Options, newView.Options) @@ -828,10 +832,12 @@ func GenerateMigration(oldIR, newIR *ir.IR, targetSchema string) []Diff { triggersChanged := len(addedTriggers) > 0 || len(droppedTriggers) > 0 || len(modifiedTriggers) > 0 if structurallyDifferent || commentChanged || indexesChanged || triggersChanged { - // For materialized views with structural changes, mark for recreation + // For materialized views with definition changes, mark for recreation. // 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)) + // also mark for recreation (issue #308). + // Use definitionChanged (not structurallyDifferent) so that option-only changes + // on materialized views use ALTER SET/RESET instead of DROP+CREATE. + needsRecreate := definitionChanged && (newView.Materialized || viewColumnsRequireRecreate(oldView, newView)) if needsRecreate { diff.modifiedViews = append(diff.modifiedViews, &viewDiff{ diff --git a/testdata/diff/create_view/issue_350_view_options/diff.sql b/testdata/diff/create_view/issue_350_view_options/diff.sql index a5494d87..a0a6e032 100644 --- a/testdata/diff/create_view/issue_350_view_options/diff.sql +++ b/testdata/diff/create_view/issue_350_view_options/diff.sql @@ -1,3 +1,4 @@ ALTER VIEW employee_emails SET (security_invoker=true); ALTER VIEW employee_names SET (security_invoker=true); ALTER VIEW employee_secure RESET (security_invoker); +ALTER MATERIALIZED VIEW employee_summary SET (fillfactor=80); diff --git a/testdata/diff/create_view/issue_350_view_options/new.sql b/testdata/diff/create_view/issue_350_view_options/new.sql index b54b4612..f1a2ece0 100644 --- a/testdata/diff/create_view/issue_350_view_options/new.sql +++ b/testdata/diff/create_view/issue_350_view_options/new.sql @@ -18,3 +18,7 @@ ALTER VIEW public.employee_emails SET (security_invoker = true); -- Remove security_invoker CREATE VIEW public.employee_secure AS SELECT id, name FROM public.employees WHERE id > 0; + +-- Add fillfactor to materialized view (should use ALTER, not DROP+CREATE) +CREATE MATERIALIZED VIEW public.employee_summary WITH (fillfactor = 80) AS +SELECT department_id, COUNT(*) AS cnt FROM public.employees GROUP BY department_id; diff --git a/testdata/diff/create_view/issue_350_view_options/old.sql b/testdata/diff/create_view/issue_350_view_options/old.sql index 6bf88f45..23000e65 100644 --- a/testdata/diff/create_view/issue_350_view_options/old.sql +++ b/testdata/diff/create_view/issue_350_view_options/old.sql @@ -16,3 +16,7 @@ SELECT id, email FROM public.employees; -- View with security_invoker (will be removed) CREATE VIEW public.employee_secure WITH (security_invoker = true) AS SELECT id, name FROM public.employees WHERE id > 0; + +-- Materialized view without options (will gain fillfactor) +CREATE MATERIALIZED VIEW public.employee_summary AS +SELECT department_id, COUNT(*) AS cnt FROM public.employees GROUP BY department_id; diff --git a/testdata/diff/create_view/issue_350_view_options/plan.json b/testdata/diff/create_view/issue_350_view_options/plan.json index fb6e5f88..ff837ead 100644 --- a/testdata/diff/create_view/issue_350_view_options/plan.json +++ b/testdata/diff/create_view/issue_350_view_options/plan.json @@ -3,7 +3,7 @@ "pgschema_version": "1.7.4", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { - "hash": "e575624236fd96e28f386d9f25c6147d6b5b98a165dab8b74f84f53f6e2de0de" + "hash": "3dd9c8d5398e477d3a0cf9ac459cb7241f0c3cb02931512dc1a27b431dbc31fa" }, "groups": [ { @@ -25,6 +25,12 @@ "type": "view", "operation": "alter", "path": "public.employee_secure" + }, + { + "sql": "ALTER MATERIALIZED VIEW employee_summary SET (fillfactor=80);", + "type": "materialized_view", + "operation": "alter", + "path": "public.employee_summary" } ] } diff --git a/testdata/diff/create_view/issue_350_view_options/plan.sql b/testdata/diff/create_view/issue_350_view_options/plan.sql index 913d3b58..537c4463 100644 --- a/testdata/diff/create_view/issue_350_view_options/plan.sql +++ b/testdata/diff/create_view/issue_350_view_options/plan.sql @@ -3,3 +3,5 @@ ALTER VIEW employee_emails SET (security_invoker=true); ALTER VIEW employee_names SET (security_invoker=true); ALTER VIEW employee_secure RESET (security_invoker); + +ALTER MATERIALIZED VIEW employee_summary SET (fillfactor=80); diff --git a/testdata/diff/create_view/issue_350_view_options/plan.txt b/testdata/diff/create_view/issue_350_view_options/plan.txt index fa6ac5e4..08e81d99 100644 --- a/testdata/diff/create_view/issue_350_view_options/plan.txt +++ b/testdata/diff/create_view/issue_350_view_options/plan.txt @@ -1,13 +1,17 @@ -Plan: 3 to modify. +Plan: 4 to modify. Summary by type: views: 3 to modify + materialized views: 1 to modify Views: ~ employee_emails ~ employee_names ~ employee_secure +Materialized views: + ~ employee_summary + DDL to be executed: -------------------------------------------------- @@ -16,3 +20,5 @@ ALTER VIEW employee_emails SET (security_invoker=true); ALTER VIEW employee_names SET (security_invoker=true); ALTER VIEW employee_secure RESET (security_invoker); + +ALTER MATERIALIZED VIEW employee_summary SET (fillfactor=80);