fix: exclude extension-owned objects and privileges from inspection (#595) - #604
Conversation
Greptile SummaryThis PR excludes extension-owned objects and their privileges from managed inspection while retaining application objects that depend on extensions. The follow-up repair preserves application partition behavior when its parent is an unmanaged extension member.
Confidence Score: 5/5The PR appears safe to merge; the previously reported partition override loss is fully addressed and no new actionable defect remains. Transient parent columns are populated before normalization, excluded from serialization and fingerprints, normalized with the same schema context as child columns, and used only when the managed parent is unavailable. Temporary-schema restoration now covers both sides of the default comparison, preventing inherited function defaults from producing redundant overrides while preserving child-specific DEFAULT and NOT NULL clauses. The previous partition finding was fully fixed, and its thread is resolved. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
DB[(PostgreSQL catalogs)] --> Inspect[Schema inspection]
Inspect --> Filter{Extension member?}
Filter -->|Yes| Omit[Exclude managed definition and ACLs]
Filter -->|No| IR[Managed application IR]
Parent[Unmanaged partition parent columns] --> Metadata[Transient comparison metadata]
Metadata --> Normalize[Normalize with child schema context]
IR --> Normalize
Normalize --> Diff[DEFAULT / NOT NULL override comparison]
Diff --> DDL[Application partition DDL]
Reviews (2): Last reviewed commit: "fix: preserve partition overrides with u..." | Re-trigger Greptile |
|
@greptileai Please re-review the repair in e207b9d, especially preservation of application partition DEFAULT/NOT NULL overrides and inherited function defaults. The original finding is addressed with transient parent metadata, runtime regressions and native saved-plan acceptance; details are in the reply on the original thread. |
There was a problem hiding this comment.
🔵 Needs a closer look
The changes span catalog inspection, planning, diffing, generated queries, and PostgreSQL extension behavior, requiring final human review.
Pull request overview
Updates PostgreSQL inspection and planning to exclude extension-owned objects and privileges while preserving application objects and partition overrides that depend on extensions.
Changes:
- Adds extension membership filtering across catalog and ACL queries.
- Preserves and normalizes application partition metadata.
- Adds regression coverage and documents planner and privilege limitations.
File summaries
| File | Description |
|---|---|
ir/queries/queries.sql.go |
Regenerated inspection queries. |
ir/queries/queries.sql |
Adds extension filters and partition metadata queries. |
ir/queries/extension_members_test.go |
Tests extension membership and ACL filtering. |
ir/normalize.go |
Normalizes inherited defaults. |
ir/ir.go |
Adds transient partition metadata. |
ir/inspector.go |
Loads unmanaged parent metadata. |
internal/diff/table.go |
Preserves partition overrides in generated DDL. |
docs/syntax/grant_revoke.mdx |
Documents unmanaged extension privileges. |
docs/cli/plan-db.mdx |
Documents extension planner requirements. |
cmd/plan/plan.go |
Remaps planner schema references. |
cmd/plan/plan_test.go |
Tests planner normalization. |
cmd/dump/extension_integration_test.go |
Covers extension dump and replay behavior. |
Review details
Files not reviewed (1)
- ir/queries/queries.sql.go: Generated file
- Files reviewed: 11/12 changed files
- Comments generated: 0
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
Broad PostgreSQL catalog, privilege, planning, and partition behavior changes require final human review.
Review details
Files not reviewed (1)
- ir/queries/queries.sql.go: Generated file
- Files reviewed: 11/12 changed files
- Comments generated: 0 new
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
tianzhou
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the contribution.
A dump with
pg_stat_statementscurrently includes its views but omits their required extension functions. With preinstalled PostGIS and an external planner, an application-only schema also produces extension-member drops and ACL changes; saved-plan apply fails with PostgreSQL2BP01. This patch leaves extension-owned definitions and privileges outside application schema management, while retaining application objects that reference extensions.Fixes #595. Reproduced on current main
319b88c83d62b2c9a62eff7a09ec6563954bcf4b, not only the v1.13.0 release.Change
pg_dependcatalog/subobject identity anddeptype = 'e'; ordinary dependencies andAUTO_EXTENSION(x) remain managed.Compatibility decision: member privileges are unmanaged even when deliberately changed after installation. Unlike
pg_dump, this does not exportpg_init_privsACL deltas. Ordinary application GRANT/REVOKE remains supported. This restriction and the external planner caveat are documented in the extension and privilege guides. Extension-member GRANT/REVOKE belongs outside desired application SQL. Extension installation/upgrades and roles are not added to the managed scope; external databases must still have matching preinstalled extensions.Validation
ALTER EXTENSION ... ADDmembership with bundled hstore: tables, views/materialized views, sequences, enum/composite/domain types, routines/aggregates and attached metadata/ACLs; application controls, quoted names, same-name objects across schemas, extension-owned schema,xdependency, and transactional OID-collision tests. They fail on baseline and pass with the fix.pg_stat_statementsdump/reload regression preserves an application view using the extension while omitting member definitions and column ACLs. It needs no shared preload change because it inspects definitions rather than querying statistics.-count=1because its existing TestMain reads the version environment before Go test logging.privilege/fixtures passed. The adjacent partition-override diff and plan/apply fixture also passed. Relevantgo vetand build passed. Independent Astra and Opus reviews completed; final verification reconciled executable evidence separately.Commands for the automated regressions:
Standalone reproduction
Use an otherwise unused local PostgreSQL server with PostGIS and pg_stat_statements available, the latter preloaded for querying statistics. Set
PGHOST,PGPORT,PGUSERand authentication for that disposable server. Use fresh database names and a directory with no ignore/config file.pgschemabelow is either the baseline or patched binary.Before: member views/tables/types leak into the dump/plan, with unwanted REVOKEs; apply rejects a member drop with
2BP01. After: extension-only dump contains no managed objects; plan creates onlyf(); apply succeeds and repeating plan is empty. PostgreSQL rejected the baseline member drop; no successful member deletion is claimed.Scope limits: this patch does not repair existing quoted-schema view
search_patherrors, qualification-only churn for application views over extension relations, or non-public vector schema/opclass rendering. The separate public-schema spatial/vector/function-view workflow and quoted application schema without views passed; these unrelated cases are not counted as passing.Application partitions of extension-owned parents remain managed and their DEFAULT/NOT NULL overrides are covered by native replay and saved-plan acceptance. Cross-schema partition-stub behavior in external planning is unchanged; quoted cross-schema coverage here is direct native dump/replay. The direct trigger-membership predicate is defensive; regressions cover triggers on member tables, not a separately extension-owned trigger on an application table.