Skip to content

Add support for extensions in the IR (#436)#443

Closed
guillaume86 wants to merge 2 commits into
pgplex:mainfrom
guillaume86:probe/extension-ir-entity
Closed

Add support for extensions in the IR (#436)#443
guillaume86 wants to merge 2 commits into
pgplex:mainfrom
guillaume86:probe/extension-ir-entity

Conversation

@guillaume86
Copy link
Copy Markdown

Summary

  • Adds ir.Extension as a cluster-level IR entity (lives at the IR root, not per-schema). Mirrors the per-entity pattern (struct + GetObjectName + thread-safe accessors).
  • Inspector.buildExtensions reads pg_extension via a new sqlc query and skips the always-present plpgsql built-in.
  • Diff emits CREATE EXTENSION IF NOT EXISTS <name>; at the head of the create-phase output and DROP EXTENSION IF EXISTS <name>; at the tail of the drop-phase output. Dumps now include extensions, so pgschema dump's output is replayable on a fresh cluster (the headline ask of Include Required Extensions in Dump Output #436).
  • New DiffTypeExtension with MarshalJSON/UnmarshalJSON symmetry, golden fixture under testdata/diff/create_extension/add_extension/.

Fixes #436

Decisions worth flagging for review

  • WITH SCHEMA is intentionally omitted from emission. pg_extension shows the actual installed schema, but during plan generation that's pgschema's temporary pgschema_tmp_* schema, which would leak into golden plans and be non-deterministic. Preserving the user-declared install schema requires either pinning it from the SQL source (currently unavailable to the inspector) or filtering out transient schemas. Tracked as a follow-up.
  • pg_depend filtering for "only-used" extensions deferred. Include Required Extensions in Dump Output #436 mentions detecting extensions actually referenced by schema objects (via pg_depend deptype='e'). This PR emits all installed extensions except plpgsql. The common cases (btree_gist, hstore, pgvector, citext, etc. — explicitly installed by users) are addressed cleanly; selective emission is the natural follow-up PR.
  • Version-bump diff (ALTER EXTENSION ... UPDATE) not implemented. Currently addedExtensions / droppedExtensions are tracked; modifiedExtensions is out of scope to keep this PR small. The fingerprinting would still detect a version change on the source side; emitting the upgrade DDL is a follow-up.

Test infrastructure note

Extensions are cluster-level state and persist across test cases on the shared embedded postgres instance. Without cleanup, a setup.sql that installs (say) btree_gist would leak into every subsequent test's inspector view. This PR adds a t.Cleanup in runPlanAndApplyTest that drops non-default extensions when each case exits. Touches cmd/migrate_integration_test.go — happy to split that out if preferred.

Test plan

  • New golden fixture testdata/diff/create_extension/add_extension/ — declares CREATE EXTENSION IF NOT EXISTS btree_gist; in new.sql, expects matching DDL emission and round-trip via TestPlanAndApply.
  • Full suite green locally on PG 17 (go test ./..., ~8 min including cleanup).
  • Existing dump-golden tests unaffected (extensions don't appear in dump output unless installed; the new cleanup prevents accumulation).

Run just the new fixture:

PGSCHEMA_TEST_FILTER="create_extension/" go test -v ./cmd -run TestPlanAndApply

🤖 Generated with Claude Code



Note (2026-05-23): pgproj (a downstream DACFX-shaped fork at https://github.com/guillaume86/pgproj) has adopted a hard-fork posture and will continue independently. This contribution remains open without expectation of merge; happy to address review feedback if a maintainer engages.

Extensions installed in the database are now read into the IR via a new
`Extension` entity and emitted by the diff/dump path so dumps remain
replayable on a fresh cluster. Without this, schemas that rely on
extension-provided types or operator classes (e.g., a GIST index on
UUID via `btree_gist`) couldn't be reproduced from `pgschema dump`
output.

Implementation:
- `ir.Extension` lives at the IR root (cluster-level, not per-schema)
- `Inspector.buildExtensions` queries `pg_extension` via a new sqlc
  query, excluding the always-present `plpgsql`
- `internal/diff/extension.go` emits `CREATE EXTENSION IF NOT EXISTS`
  at the head of the create-phase output and `DROP EXTENSION IF EXISTS`
  at the tail of the drop-phase output
- `WITH SCHEMA` is intentionally omitted from emission. Preserving the
  user-declared install schema requires more care than this PR takes
  on (`pg_extension` shows the *actual* install schema, which becomes
  pgschema's temporary schema during plan generation). Tracked as a
  follow-up

Note on tests: extensions are cluster-level state and persisted across
test cases on the shared embedded postgres instance. Without cleanup
between cases, any setup.sql that installs an extension would leak
into later tests' inspections. `TestPlanAndApply` now registers a
`t.Cleanup` that drops non-default extensions when each case exits.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 23, 2026

Greptile Summary

This PR adds extension support to the schema IR and migration output. The main changes are:

  • A root-level ir.Extension entity with inspector population from pg_extension.
  • A new extension diff type and create/drop SQL generation.
  • Extension DDL ordering before object creation and after object drops.
  • A golden fixture for creating btree_gist.
  • Test cleanup for non-default extensions on the shared embedded database.

Confidence Score: 2/5

These issues should be fixed before merging.

  • Common extension names like uuid-ossp can generate invalid SQL.

  • Extensions installed into a non-default schema can replay into the wrong schema.

  • Schema-scoped plans can include drops for unrelated database-level extensions.

  • Extension metadata changes can be missed entirely.

  • internal/diff/extension.go

  • internal/diff/diff.go

  • cmd/migrate_integration_test.go

Important Files Changed

Filename Overview
internal/diff/extension.go Adds extension DDL rendering, with identifier quoting and install-schema issues.
internal/diff/diff.go Adds root-level extension diffing, including global drop behavior and name-only comparison.
cmd/migrate_integration_test.go Adds shared extension cleanup, but only for fixtures with setup SQL.

Reviews (1): Last reviewed commit: "Add support for extensions in the IR (#4..." | Re-trigger Greptile

Comment on lines +58 to +60
func generateExtensionSQL(ext *ir.Extension) string {
return fmt.Sprintf("CREATE EXTENSION IF NOT EXISTS %s;", ext.Name)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Quote extension identifiers

ext.Name is emitted directly into the CREATE EXTENSION statement. Valid PostgreSQL extension names can require quoting, such as uuid-ossp; this renders as CREATE EXTENSION IF NOT EXISTS uuid-ossp;, which PostgreSQL does not parse as that extension name. A dump or plan containing that common extension will fail to apply.

Suggested change
func generateExtensionSQL(ext *ir.Extension) string {
return fmt.Sprintf("CREATE EXTENSION IF NOT EXISTS %s;", ext.Name)
}
func generateExtensionSQL(ext *ir.Extension) string {
return fmt.Sprintf("CREATE EXTENSION IF NOT EXISTS %s;", ir.QuoteIdentifier(ext.Name))
}

Context Used: CLAUDE.md (source)

Comment thread internal/diff/extension.go Outdated
Source: ext,
CanRunInTransaction: true,
}
collector.collect(context, fmt.Sprintf("DROP EXTENSION IF EXISTS %s;", ext.Name))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Quote dropped extensions

The drop path has the same raw identifier interpolation as the create path. If an installed extension name needs quoting, such as uuid-ossp or a mixed-case custom extension, the generated DROP EXTENSION statement fails or targets the wrong identifier.

Suggested change
collector.collect(context, fmt.Sprintf("DROP EXTENSION IF EXISTS %s;", ext.Name))
collector.collect(context, fmt.Sprintf("DROP EXTENSION IF EXISTS %s;", ir.QuoteIdentifier(ext.Name)))

Context Used: CLAUDE.md (source)

Comment thread internal/diff/extension.go Outdated
Comment on lines +58 to +59
func generateExtensionSQL(ext *ir.Extension) string {
return fmt.Sprintf("CREATE EXTENSION IF NOT EXISTS %s;", ext.Name)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Preserve extension schema

The inspector records Extension.Schema, but this renderer always emits CREATE EXTENSION without WITH SCHEMA. For a database with CREATE EXTENSION hstore SCHEMA utils and tables using utils.hstore, the replay installs hstore into the current search path instead of utils, so later DDL that references utils.hstore can fail with a missing type or create a different schema layout.

Context Used: CLAUDE.md (source)

Comment thread internal/diff/diff.go
Comment on lines +486 to +490
oldExtNames := sortedKeys(oldIR.Extensions)
for _, name := range oldExtNames {
if _, exists := newIR.Extensions[name]; !exists {
diff.droppedExtensions = append(diff.droppedExtensions, oldIR.Extensions[name])
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Avoid global extension drops

This compares all database extensions and drops any extension missing from the desired IR, even when the command is scoped to one target schema. If the current database has an extension used by another schema or application and the desired SQL for public does not declare it, a schema-scoped plan can emit DROP EXTENSION for unrelated database state.

Context Used: CLAUDE.md (source)

Comment thread internal/diff/diff.go
Comment on lines +478 to +490
{
extNames := sortedKeys(newIR.Extensions)
for _, name := range extNames {
newExt := newIR.Extensions[name]
if _, exists := oldIR.Extensions[name]; !exists {
diff.addedExtensions = append(diff.addedExtensions, newExt)
}
}
oldExtNames := sortedKeys(oldIR.Extensions)
for _, name := range oldExtNames {
if _, exists := newIR.Extensions[name]; !exists {
diff.droppedExtensions = append(diff.droppedExtensions, oldIR.Extensions[name])
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Detect extension changes

Extension diffing only checks whether the name exists in both IRs. The IR now carries Version, Schema, and Comment, but changes to those fields produce no diff, so pgschema plan can report no changes while the installed extension metadata still differs from the desired state.

Context Used: CLAUDE.md (source)

Comment thread cmd/migrate_integration_test.go Outdated
Comment on lines +251 to +257
// Extensions are cluster-level and persist across tests on the shared
// embedded postgres instance. Reset any non-default extensions when
// this test case finishes so the next case (or the next top-level test)
// inherits a clean cluster.
t.Cleanup(func() {
cleanupSharedClusterExtensions(t)
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Clean new-sql extensions

The cleanup is registered only when a fixture has a non-empty setup.sql. The new extension fixture installs btree_gist from new.sql through the shared embedded provider, and ApplySchema drops only the temporary schema, not database-level extensions. Because this fixture has no setup.sql, btree_gist can remain installed in the shared embedded database and contaminate later plan tests.

Context Used: CLAUDE.md (source)

@guillaume86 guillaume86 marked this pull request as draft May 23, 2026 21:24
Address Greptile review on pgplex#443:

- CREATE/DROP EXTENSION now go through ir.QuoteIdentifier, so names
  like uuid-ossp render as a properly quoted identifier instead of
  invalid SQL.
- The shared-cluster extension cleanup in runPlanAndApplyTest used to
  register only when setup.sql was non-empty. Extensions installed via
  new.sql (the create_extension fixture does exactly this) could leak
  across cases on the shared embedded postgres. Register the cleanup
  unconditionally.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

Include Required Extensions in Dump Output

1 participant