diff --git a/apps/cli-go/docs/supabase/db/diff.md b/apps/cli-go/docs/supabase/db/diff.md index a046707ec2..59ea20ab03 100644 --- a/apps/cli-go/docs/supabase/db/diff.md +++ b/apps/cli-go/docs/supabase/db/diff.md @@ -12,4 +12,3 @@ While the diff command is able to capture most schema changes, there are cases w - Changes to publication - Changes to storage buckets -- Views with `security_invoker` attributes diff --git a/apps/cli-go/internal/db/diff/diff.go b/apps/cli-go/internal/db/diff/diff.go index 46d465aff2..73e8411ac3 100644 --- a/apps/cli-go/internal/db/diff/diff.go +++ b/apps/cli-go/internal/db/diff/diff.go @@ -211,7 +211,13 @@ func DiffDatabase(ctx context.Context, schema []string, config pgconn.Config, w } else { fmt.Fprintln(w, "Diffing schemas...") } - return differ(ctx, shadowConfig, config, schema, options...) + out, err := differ(ctx, shadowConfig, config, schema, options...) + if err != nil { + return out, err + } + // Restore view reloptions that the underlying diff engine dropped + // (e.g. WITH (security_invoker=true)). See issue #3973. + return applyViewReloptionsFromTarget(ctx, out, config, options...), nil } func migrateBaseDatabase(ctx context.Context, config pgconn.Config, migrations []string, fsys afero.Fs, options ...func(*pgx.ConnConfig)) error { diff --git a/apps/cli-go/internal/db/diff/view_options.go b/apps/cli-go/internal/db/diff/view_options.go new file mode 100644 index 0000000000..1634653b77 --- /dev/null +++ b/apps/cli-go/internal/db/diff/view_options.go @@ -0,0 +1,117 @@ +package diff + +import ( + "context" + "fmt" + "os" + "regexp" + "strings" + + "github.com/jackc/pgconn" + "github.com/jackc/pgx/v4" + "github.com/supabase/cli/internal/utils" + "github.com/supabase/cli/pkg/pgxv5" +) + +// viewDefinitionPattern matches a CREATE [OR REPLACE] [MATERIALIZED] VIEW +// "schema"."name" AS prefix. Capture groups: 1 = schema, 2 = view name, 3 = +// the trailing "AS" token, whose start offset is used to splice a WITH (...) +// clause in front of it without touching the rest of the statement. +var viewDefinitionPattern = regexp.MustCompile(`(?i)create\s+(?:or\s+replace\s+)?(?:materialized\s+)?view\s+(?:if\s+not\s+exists\s+)?"([^"]+)"\."([^"]+)"\s+(as\b)`) + +// SELECT_VIEW_RELOPTIONS reads reloptions for every view and materialized view +// in the target database. djrobstep/migra, pg-schema-diff and @pgkit/migra +// emit CREATE VIEW statements without the WITH (...) clause, so the CLI +// reattaches it from this query after the diff is produced. +// +// See https://github.com/supabase/cli/issues/3973. +const SELECT_VIEW_RELOPTIONS = `SELECT n.nspname AS nspname, + c.relname AS relname, + c.reloptions AS reloptions + FROM pg_class c + JOIN pg_namespace n ON c.relnamespace = n.oid + WHERE c.relkind IN ('v','m') + AND c.reloptions IS NOT NULL + AND array_length(c.reloptions, 1) > 0` + +type viewKey struct { + schema string + name string +} + +type viewReloptionsRow struct { + Nspname string `db:"nspname"` + Relname string `db:"relname"` + Reloptions []string `db:"reloptions"` +} + +// PatchViewReloptions rewrites CREATE [OR REPLACE] [MATERIALIZED] VIEW +// statements in sql to include a WITH (...) clause for any view present in +// reloptions. Statements that do not match an entry in reloptions are left +// untouched, so the function is safe to call unconditionally on every diff. +func PatchViewReloptions(sql string, reloptions map[viewKey][]string) string { + if sql == "" || len(reloptions) == 0 { + return sql + } + return viewDefinitionPattern.ReplaceAllStringFunc(sql, func(match string) string { + indexes := viewDefinitionPattern.FindStringSubmatchIndex(match) + if len(indexes) < 8 { + return match + } + key := viewKey{ + schema: match[indexes[2]:indexes[3]], + name: match[indexes[4]:indexes[5]], + } + opts, ok := reloptions[key] + if !ok || len(opts) == 0 { + return match + } + asStart := indexes[6] + return match[:asStart] + "with (" + strings.Join(opts, ", ") + ") " + match[asStart:] + }) +} + +// SelectViewReloptions queries conn for every view and materialized view with +// non-empty reloptions, keyed by (schema, name). +func SelectViewReloptions(ctx context.Context, conn *pgx.Conn) (map[viewKey][]string, error) { + rows, err := conn.Query(ctx, SELECT_VIEW_RELOPTIONS) + if err != nil { + return nil, err + } + collected, err := pgxv5.CollectRows[viewReloptionsRow](rows) + if err != nil { + return nil, err + } + out := make(map[viewKey][]string, len(collected)) + for _, r := range collected { + out[viewKey{schema: r.Nspname, name: r.Relname}] = r.Reloptions + } + return out, nil +} + +// applyViewReloptionsFromTarget restores WITH (...) clauses on view +// definitions by querying target for the live reloptions and patching sql in +// place. Any failure to connect or query is treated as a soft error so the +// diff is preserved as-is; the caller is responsible for showing the original +// output to the user. A short warning is logged to stderr to make the +// degradation visible. +func applyViewReloptionsFromTarget(ctx context.Context, sql string, target pgconn.Config, options ...func(*pgx.ConnConfig)) string { + if !viewDefinitionPattern.MatchString(sql) { + return sql + } + conn, err := utils.ConnectByConfig(ctx, target, options...) + if err != nil { + fmt.Fprintln(os.Stderr, utils.Yellow("WARNING:"), "could not connect to target database to restore view reloptions:", err) + return sql + } + defer conn.Close(context.Background()) + reloptions, err := SelectViewReloptions(ctx, conn) + if err != nil { + fmt.Fprintln(os.Stderr, utils.Yellow("WARNING:"), "could not read view reloptions from target database:", err) + return sql + } + if len(reloptions) == 0 { + return sql + } + return PatchViewReloptions(sql, reloptions) +} diff --git a/apps/cli-go/internal/db/diff/view_options_test.go b/apps/cli-go/internal/db/diff/view_options_test.go new file mode 100644 index 0000000000..0160d4ca49 --- /dev/null +++ b/apps/cli-go/internal/db/diff/view_options_test.go @@ -0,0 +1,102 @@ +package diff + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestPatchViewReloptions(t *testing.T) { + t.Run("inserts WITH clause for create or replace view", func(t *testing.T) { + in := `create or replace view "public"."user_details" as SELECT id FROM users;` + out := PatchViewReloptions(in, map[viewKey][]string{ + {schema: "public", name: "user_details"}: {"security_invoker=true"}, + }) + assert.Equal(t, + `create or replace view "public"."user_details" with (security_invoker=true) as SELECT id FROM users;`, + out, + ) + }) + + t.Run("preserves multiple reloptions in order", func(t *testing.T) { + in := `create or replace view "public"."v" as select 1;` + out := PatchViewReloptions(in, map[viewKey][]string{ + {schema: "public", name: "v"}: {"security_invoker=true", "check_option=local"}, + }) + assert.Equal(t, + `create or replace view "public"."v" with (security_invoker=true, check_option=local) as select 1;`, + out, + ) + }) + + t.Run("handles materialized view", func(t *testing.T) { + in := `CREATE MATERIALIZED VIEW "public"."mv" AS SELECT 1;` + out := PatchViewReloptions(in, map[viewKey][]string{ + {schema: "public", name: "mv"}: {"fillfactor=70"}, + }) + assert.Equal(t, + `CREATE MATERIALIZED VIEW "public"."mv" with (fillfactor=70) AS SELECT 1;`, + out, + ) + }) + + t.Run("patches multiple views in one diff", func(t *testing.T) { + in := `create or replace view "public"."a" as select 1; +create or replace view "public"."b" as select 2; +create or replace view "public"."c" as select 3;` + out := PatchViewReloptions(in, map[viewKey][]string{ + {schema: "public", name: "a"}: {"security_invoker=true"}, + {schema: "public", name: "c"}: {"security_invoker=true"}, + }) + assert.Equal(t, + `create or replace view "public"."a" with (security_invoker=true) as select 1; +create or replace view "public"."b" as select 2; +create or replace view "public"."c" with (security_invoker=true) as select 3;`, + out, + ) + }) + + t.Run("leaves diff unchanged when no matching view is present", func(t *testing.T) { + in := `alter table "public"."users" add column "email" text;` + out := PatchViewReloptions(in, map[viewKey][]string{ + {schema: "public", name: "user_details"}: {"security_invoker=true"}, + }) + assert.Equal(t, in, out) + }) + + t.Run("leaves view unchanged when reloptions map has no entry", func(t *testing.T) { + in := `create or replace view "public"."v" as select 1;` + out := PatchViewReloptions(in, map[viewKey][]string{ + {schema: "other", name: "v"}: {"security_invoker=true"}, + }) + assert.Equal(t, in, out) + }) + + t.Run("returns input unchanged when reloptions map is empty", func(t *testing.T) { + in := `create or replace view "public"."v" as select 1;` + assert.Equal(t, in, PatchViewReloptions(in, nil)) + assert.Equal(t, in, PatchViewReloptions(in, map[viewKey][]string{})) + }) + + t.Run("matches schemas with uppercase keywords", func(t *testing.T) { + in := `CREATE OR REPLACE VIEW "public"."v" AS SELECT 1;` + out := PatchViewReloptions(in, map[viewKey][]string{ + {schema: "public", name: "v"}: {"security_invoker=true"}, + }) + assert.Equal(t, + `CREATE OR REPLACE VIEW "public"."v" with (security_invoker=true) AS SELECT 1;`, + out, + ) + }) + + t.Run("matches view name containing the word as", func(t *testing.T) { + in := `create or replace view "public"."alias" as select 1;` + out := PatchViewReloptions(in, map[viewKey][]string{ + {schema: "public", name: "alias"}: {"security_invoker=true"}, + }) + assert.Equal(t, + `create or replace view "public"."alias" with (security_invoker=true) as select 1;`, + out, + ) + }) +}