Skip to content

Commit 11678c5

Browse files
fix: exclude extension-owned objects and privileges from inspection (#595) (#604)
* fix: exclude extension members and privileges from inspection (#595) * fix: preserve partition overrides with unmanaged extension parents
1 parent 319b88c commit 11678c5

12 files changed

Lines changed: 1573 additions & 35 deletions

File tree

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
package dump
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"testing"
7+
8+
"github.com/pgplex/pgschema/ir"
9+
"github.com/pgplex/pgschema/testutil"
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
// Issue #595: pg_stat_statements' view was dumped while its required function
14+
// was omitted. Neither definition belongs in an application schema dump.
15+
func TestDumpExtensionMembers(t *testing.T) {
16+
if testing.Short() {
17+
t.Skip("Skipping integration test in short mode")
18+
}
19+
pg := testutil.SetupPostgres(t)
20+
defer pg.Stop()
21+
db, host, port, name, user, password := testutil.ConnectToPostgres(t, pg)
22+
defer db.Close()
23+
ctx := context.Background()
24+
// Catalog inspection and view creation do not execute pg_stat_statements,
25+
// so the bundled extension needs no shared_preload_libraries modification.
26+
_, err := db.ExecContext(ctx, `
27+
CREATE EXTENSION pg_stat_statements;
28+
CREATE TABLE app_requests (id integer PRIMARY KEY);
29+
CREATE VIEW app_stats AS SELECT queryid FROM pg_stat_statements;
30+
GRANT SELECT ON app_requests TO PUBLIC;
31+
GRANT SELECT (queryid) ON pg_stat_statements TO PUBLIC;
32+
`)
33+
require.NoError(t, err)
34+
config := &DumpConfig{
35+
Host: host, Port: port, DB: name, User: user, Password: password,
36+
Schema: "public", NoComments: true, ConfigDir: t.TempDir(),
37+
}
38+
dumped, err := ExecuteDump(config)
39+
require.NoError(t, err)
40+
require.NotContains(t, dumped, "VIEW pg_stat_statements")
41+
require.NotContains(t, dumped, "CREATE OR REPLACE FUNCTION pg_stat_statements")
42+
require.NotContains(t, dumped, "ON TABLE pg_stat_statements")
43+
require.Contains(t, dumped, "CREATE OR REPLACE VIEW app_stats")
44+
require.Contains(t, dumped, "FROM pg_stat_statements")
45+
require.Contains(t, dumped, "GRANT SELECT ON TABLE app_requests TO PUBLIC")
46+
// Reload the unmodified native dump while the extension is still installed.
47+
_, err = db.ExecContext(ctx, `DROP VIEW app_stats; DROP TABLE app_requests;`)
48+
require.NoError(t, err)
49+
_, err = db.ExecContext(ctx, dumped)
50+
require.NoError(t, err)
51+
roundtrip, err := ExecuteDump(config)
52+
require.NoError(t, err)
53+
require.Equal(t, dumped, roundtrip)
54+
}
55+
56+
// An application partition remains managed even when its parent is an
57+
// extension member. Replaying its dump must preserve its own column rules.
58+
func TestDumpExtensionPartitionOverrides(t *testing.T) {
59+
if testing.Short() {
60+
t.Skip("Skipping integration test in short mode")
61+
}
62+
pg := testutil.SetupPostgres(t)
63+
defer pg.Stop()
64+
db, host, port, name, user, password := testutil.ConnectToPostgres(t, pg)
65+
defer db.Close()
66+
ctx := context.Background()
67+
_, err := db.ExecContext(ctx, `CREATE SCHEMA extension595; CREATE EXTENSION hstore SCHEMA extension595;`)
68+
require.NoError(t, err)
69+
for _, tc := range []struct{ parentSchema, childSchema string }{
70+
{"public", "public"},
71+
{"Extension Space", "App Space"},
72+
} {
73+
t.Run(tc.childSchema, func(t *testing.T) {
74+
parent := ir.QuoteIdentifier(tc.parentSchema) + `."Member Parent"`
75+
child := ir.QuoteIdentifier(tc.childSchema) + `."App Child"`
76+
_, err := db.ExecContext(ctx, fmt.Sprintf(`
77+
CREATE SCHEMA IF NOT EXISTS %s;
78+
CREATE SCHEMA IF NOT EXISTS %s;
79+
CREATE TABLE %s (
80+
id integer NOT NULL,
81+
priority integer DEFAULT 0,
82+
notes text,
83+
inherited integer DEFAULT 42 NOT NULL,
84+
calculated integer GENERATED ALWAYS AS (id * 2) STORED
85+
) PARTITION BY RANGE (id);
86+
ALTER EXTENSION hstore ADD TABLE %s;
87+
CREATE TABLE %s PARTITION OF %s (
88+
priority DEFAULT 10, notes NOT NULL
89+
) FOR VALUES FROM (0) TO (100);
90+
`, ir.QuoteIdentifier(tc.parentSchema), ir.QuoteIdentifier(tc.childSchema), parent, parent, child, parent))
91+
require.NoError(t, err)
92+
config := &DumpConfig{Host: host, Port: port, DB: name, User: user, Password: password,
93+
Schema: tc.childSchema, NoComments: true, QualifySchema: true, ConfigDir: t.TempDir()}
94+
dumped, err := ExecuteDump(config)
95+
require.NoError(t, err)
96+
require.NotContains(t, dumped, `CREATE TABLE IF NOT EXISTS `+parent+` (`)
97+
_, err = db.ExecContext(ctx, `DROP TABLE `+child)
98+
require.NoError(t, err)
99+
_, err = db.ExecContext(ctx, dumped)
100+
require.NoError(t, err, dumped)
101+
var priority, inherited, calculated int
102+
err = db.QueryRowContext(ctx, `INSERT INTO `+child+` (id, notes) VALUES (1, 'kept') RETURNING priority, inherited, calculated`).Scan(&priority, &inherited, &calculated)
103+
require.NoError(t, err)
104+
require.Equal(t, 10, priority, "partition default must survive dump replay")
105+
require.Equal(t, 42, inherited)
106+
require.Equal(t, 2, calculated)
107+
_, err = db.ExecContext(ctx, `INSERT INTO `+child+` (id) VALUES (2)`)
108+
require.ErrorContains(t, err, "23502", "partition NOT NULL must survive dump replay")
109+
roundtrip, err := ExecuteDump(config)
110+
require.NoError(t, err)
111+
require.Equal(t, dumped, roundtrip)
112+
_, err = db.ExecContext(ctx, `DROP TABLE `+child+`; ALTER EXTENSION hstore DROP TABLE `+parent+`; DROP TABLE `+parent)
113+
require.NoError(t, err)
114+
})
115+
}
116+
}

‎cmd/plan/plan.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,13 @@ func normalizeSchemaNames(irData *ir.IR, fromSchema, toSchema string) {
584584
*column.GeneratedExpr = stripQualifiers(replaceString(*column.GeneratedExpr))
585585
}
586586
}
587+
// Unmanaged parent defaults must use the same schema context as
588+
// child defaults, otherwise inherited expressions look like overrides.
589+
for _, column := range table.PartitionParentColumns {
590+
if column.DefaultValue != nil {
591+
*column.DefaultValue = stripQualifiers(replaceString(*column.DefaultValue))
592+
}
593+
}
587594

588595
// Normalize schema names in indexes
589596
for _, index := range table.Indexes {

‎cmd/plan/plan_test.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@ import (
44
"fmt"
55
"os"
66
"path/filepath"
7+
"strings"
78
"testing"
89

10+
"github.com/pgplex/pgschema/internal/diff"
911
"github.com/pgplex/pgschema/ir"
1012
"github.com/spf13/cobra"
1113
)
@@ -237,6 +239,32 @@ func TestNormalizeSchemaNames_StripsSameSchemaQualifiersFromViewDefinitions(t *t
237239
}
238240
}
239241

242+
func TestNormalizeSchemaNames_PreservesInheritedPartitionDefault(t *testing.T) {
243+
const tempSchema = "pgschema_tmp_partition595"
244+
childDefault, parentDefault := "public.member_default()", "public.member_default()"
245+
child := &ir.Table{
246+
Schema: tempSchema, Name: "app_child", PartitionOf: "member_parent",
247+
PartitionOfSchema: "public", PartitionBound: "FOR VALUES FROM (0) TO (10)",
248+
Columns: []*ir.Column{{Name: "value", DefaultValue: &childDefault}},
249+
PartitionParentColumns: []*ir.Column{{Name: "value", DefaultValue: &parentDefault}},
250+
}
251+
desired := &ir.IR{Schemas: map[string]*ir.Schema{
252+
tempSchema: {Name: tempSchema, Tables: map[string]*ir.Table{"app_child": child}},
253+
}}
254+
normalizeSchemaNames(desired, tempSchema, "public")
255+
changes := diff.GenerateMigration(ir.NewIR(), desired, "public")
256+
var statements []string
257+
for _, change := range changes {
258+
for _, statement := range change.Statements {
259+
statements = append(statements, statement.SQL)
260+
}
261+
}
262+
sql := strings.Join(statements, "\n")
263+
if !strings.Contains(sql, "PARTITION OF member_parent") || strings.Contains(sql, "DEFAULT") {
264+
t.Fatalf("expected inherited default without a child override, got %s", sql)
265+
}
266+
}
267+
240268
func TestNormalizeSchemaNames_PreservesViewTableAliasesMatchingTargetSchema(t *testing.T) {
241269
irData := &ir.IR{
242270
Schemas: map[string]*ir.Schema{

‎docs/cli/plan-db.mdx‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,19 @@ pgschema apply \
6565

6666
pgschema does not manage extensions, so `dump` never emits `CREATE EXTENSION` and you should not add it to your schema files. Instead, the plan database must already have the extensions your schema uses, in the same schema as on the target.
6767

68+
Extension-owned objects are also unmanaged. `dump` omits their definitions and
69+
privileges, and `plan` leaves them alone. This includes privileges changed after
70+
installation, such as a custom grant on `spatial_ref_sys` or a revoked `PUBLIC`
71+
grant on an extension function. Manage those privileges separately; do not put
72+
extension-member `GRANT`/`REVOKE` statements in the desired schema. In an external
73+
plan database those statements can affect the preinstalled extension itself,
74+
outside the temporary schema, without representing a managed change.
75+
76+
Application objects that use extension types or functions remain managed,
77+
including their privileges. Membership is determined from PostgreSQL's dependency
78+
catalog, not object names or the schema containing the extension. Unlike
79+
`pg_dump`, pgschema does not export changes from an extension's initial privileges.
80+
6881
The default embedded plan database handles this for you: it installs every extension found on the target database before applying your schema. This covers all extensions bundled with PostgreSQL (`hstore`, `pg_trgm`, `citext`, `uuid-ossp`, `btree_gist`, ...). Third-party extensions such as `postgis` or `pgvector` are not bundled, so for those you need an external plan database.
6982

7083
An external plan database is not mirrored automatically: install every extension your schema uses yourself, bundled ones included, in the same schema as on the target:

‎docs/syntax/grant_revoke.mdx‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,11 @@ pgschema understands the following `GRANT`/`REVOKE` features:
4444
- **REVOKE GRANT OPTION FOR**: Revoke only the grant option while keeping the privilege
4545
- **PUBLIC**: Special grantee representing all roles
4646

47+
These features apply to application-owned objects. Extension-member privileges,
48+
including custom grants and revoked defaults, are excluded from dump and plan
49+
along with the extension-owned definitions. Manage them separately from desired
50+
schema SQL; see [Using PostgreSQL Extensions](/cli/plan-db#using-postgresql-extensions).
51+
4752
## Examples
4853

4954
### Grant table privileges

‎internal/diff/table.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -937,9 +937,13 @@ func generateTableSQL(table *ir.Table, targetSchema string, qualifySchema bool,
937937

938938
// Detect per-child column overrides (DEFAULT, NOT NULL) by comparing against the parent.
939939
parentKey := parentSchema + "." + table.PartitionOf
940+
parentColumns := table.PartitionParentColumns
940941
if parentTable, ok := allTables[parentKey]; ok {
941-
parentCols := make(map[string]*ir.Column, len(parentTable.Columns))
942-
for _, col := range parentTable.Columns {
942+
parentColumns = parentTable.Columns
943+
}
944+
if len(parentColumns) > 0 {
945+
parentCols := make(map[string]*ir.Column, len(parentColumns))
946+
for _, col := range parentColumns {
943947
parentCols[col.Name] = col
944948
}
945949
for _, col := range table.Columns {

‎ir/inspector.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,10 @@ func (i *Inspector) BuildIR(ctx context.Context, targetSchema string) (*IR, erro
140140
return nil, err
141141
}
142142

143+
if err := i.buildPartitionParentColumns(ctx, schema, targetSchema); err != nil {
144+
return nil, fmt.Errorf("failed to build partition parent columns: %w", err)
145+
}
146+
143147
// Load rows of data-managed tables now that columns, constraints, and
144148
// partitions are known.
145149
if err := i.buildRows(ctx, schema, targetSchema); err != nil {
@@ -760,6 +764,42 @@ func (i *Inspector) buildPartitionMapping(ctx context.Context, schema *IR, targe
760764
return partitionMapping
761765
}
762766

767+
// buildPartitionParentColumns keeps unmanaged parents available for comparing
768+
// inherited column properties, without adding their definitions to the IR.
769+
func (i *Inspector) buildPartitionParentColumns(ctx context.Context, schema *IR, targetSchema string) error {
770+
children := make(map[string]*Table)
771+
for name, table := range schema.Schemas[targetSchema].Tables {
772+
if table.PartitionOf == "" {
773+
continue
774+
}
775+
parentSchema := table.PartitionOfSchema
776+
if parentSchema == "" {
777+
parentSchema = targetSchema
778+
}
779+
if s := schema.Schemas[parentSchema]; s != nil && s.Tables[table.PartitionOf] != nil {
780+
continue
781+
}
782+
children[name] = table
783+
}
784+
if len(children) == 0 {
785+
return nil
786+
}
787+
columns, err := i.queries.GetPartitionParentColumnsForSchema(ctx, targetSchema)
788+
if err != nil {
789+
return err
790+
}
791+
for _, col := range columns {
792+
if table := children[col.ChildTable]; table != nil {
793+
column := &Column{Name: col.ColumnName, IsNullable: col.IsNullable.Bool}
794+
if col.ColumnDefault.Valid {
795+
column.DefaultValue = &col.ColumnDefault.String
796+
}
797+
table.PartitionParentColumns = append(table.PartitionParentColumns, column)
798+
}
799+
}
800+
return nil
801+
}
802+
763803
// sortPrimaryKeyColumnsForPartitionedTable sorts primary key constraint columns
764804
// to ensure partition key columns come first
765805
func (i *Inspector) sortPrimaryKeyColumnsForPartitionedTable(constraint *Constraint, partitionKey string) {

‎ir/ir.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@ type Table struct {
7575
// schema definition, so it is excluded from serialization (and thereby
7676
// from fingerprints and plan JSON).
7777
AllConstraintNames map[string]bool `json:"-"`
78+
// PartitionParentColumns holds comparison metadata when a partition's
79+
// parent is outside the managed IR, e.g. an extension member. It preserves
80+
// child DEFAULT/NOT NULL overrides without managing or fingerprinting the
81+
// parent itself.
82+
PartitionParentColumns []*Column `json:"-"`
7883
// DataManaged is true when the table matches [data] in pgschema.toml and
7984
// its rows are part of the desired state. Rows holds those rows, in
8085
// DataColumns() order. Both are excluded from serialization, and therefore

‎ir/normalize.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,11 @@ func normalizeTable(table *Table) {
161161
for _, column := range table.Columns {
162162
normalizeColumn(column, table.Schema)
163163
}
164+
// Compare parent expressions in the child's schema context, just like its
165+
// own defaults. The parent can live in a different (unmanaged) schema.
166+
for _, column := range table.PartitionParentColumns {
167+
normalizeColumn(column, table.Schema)
168+
}
164169

165170
// Normalize policies
166171
for _, policy := range table.Policies {
@@ -1598,6 +1603,7 @@ func IsTextLikeType(typeName string) bool {
15981603
// to avoid a perpetual spurious diff (issue #473):
15991604
// - array-level cast: "col::text = ANY ((ARRAY['a'::varchar])::text[])" (wrapping paren + array cast)
16001605
// - element-level: "col::text = ANY (ARRAY[('a'::varchar)::text])" (cast on each element)
1606+
//
16011607
// Both collapse to "col::text IN ('a'::varchar)".
16021608
func convertAnyArrayToIn(expr string) string {
16031609
const anyMarker = " = ANY ("

0 commit comments

Comments
 (0)