feat(dump): add --qualify-schema to force full schema qualification (#321) by tianzhou · Pull Request #492 · pgplex/pgschema

tianzhou · 2026-06-29T10:53:49Z

Fixes #321.

What

Adds a pgschema dump --qualify-schema flag. By default, dump uses "smart qualification" — it omits the schema prefix for objects in the target schema. With --qualify-schema, dump always emits fully schema-qualified names (e.g. public.users, account public.user_kind), which:

avoids ambiguous-identifier problems (e.g. reserved-word table names like user), per Respect quoted identifiers in plpgsql functions when dumping schema #320, and
simplifies static analysis of the dumped SQL (no need to track schema context from directory/file).

How

The dump builds its SQL via diff.GenerateMigration(empty → target), so qualification lives in the shared internal/diff package (also used by plan/apply). To keep this dump-scoped and opt-in:

GenerateMigrationWithOptions(old, new, targetSchema, qualifySchema) carries the option on diffCollector (the seam every generator already receives). GenerateMigration remains a thin wrapper passing false, so plan/apply behavior is unchanged.
Three qualify helpers gained mode variants — QualifyEntityNameWithQuotesMode, qualifyEntityNameMode, stripSchemaPrefixMode — with the originals delegating with false.
Create-path builders thread qualifySchema from the collector down to those helpers.
dump adds the --qualify-schema flag → DumpConfig.QualifySchema, calls GenerateMigrationWithOptions, and passes it to DumpFormatter so comment headers keep the schema name (instead of -) when forced.

Coverage

Forced qualification applies to the structural names/types pgschema generates: tables/columns (+LIKE), functions (name/params/return), sequences (+OWNED BY), FK references, types/domains/composite attrs, aggregates (+STYPE/MSTYPE), views, index/view COMMENTs, CREATE TRIGGER target table, and ALTER TABLE … ROW LEVEL SECURITY.

Out of scope (intentional)

Free-form expression text — view bodies, policy USING/CHECK, trigger function bodies — is left as-is (that SQL may already contain qualified identifiers; rewriting it would require full parsing). Privilege generators don't use the qualify helpers.

Tests

New internal/diff/qualify_schema_test.go: builder-level tests for table+column type, FK reference, enum type, and sequence (+OWNED BY) — each asserting forced-qualification output and that the default (false) output is unchanged.
Full go test ./internal/diff/... passes — existing fixtures unchanged, confirming no default-behavior drift.

🤖 Generated with Claude Code

Scope & known limitation

--qualify-schema forces full schema qualification for object identifiers — table / view / index / sequence / type / function / procedure / policy names, and ON / REFERENCES / OWNED BY / COMMENT targets. This covers the reserved-word / ambiguous-identifier class from #320.

It does not qualify same-schema type references (column types, composite attribute types, function/procedure params + returns, aggregate state types, domain base types). The IR stores same-schema user-defined types as bare strings, so the render-side helper can only strip a target-schema prefix, not add one. Cross-schema type references already arrive qualified and are preserved. Inspector/IR work to enable type-reference qualification is tracked in #493.

Free-form expression text (view bodies, policy USING/CHECK, trigger function bodies) is intentionally left as-is.

…WIP] Foundation for `dump --qualify-schema` (issue #321), default-false so plan/apply and existing dump output are unchanged: - cmd/dump: `--qualify-schema` flag + `DumpConfig.QualifySchema`; dump uses `diff.GenerateMigrationWithOptions(...)` and passes the option to the formatter. - internal/diff: `GenerateMigrationWithOptions` carries the option on `diffCollector` (the seam reaching every generator). - ir/quote.go, internal/diff: mode variants of the three qualify helpers (`QualifyEntityNameWithQuotesMode`, `qualifyEntityNameMode`, `stripSchemaPrefixMode`) that always qualify / preserve the prefix when enabled. - internal/dump/formatter: comment headers keep the target schema name (not "-") when qualifySchema is on. TODO (next commit): thread collector.qualifySchema from the create-path generators into the leaf builders and swap their qualify-helper calls to the *Mode variants so the flag actually forces qualification; then tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

generateCreateTablesSQL now passes collector.qualifySchema into generateTableSQL, which threads it to writeColumnDefinitionToBuilder. Table names, LIKE-clause source tables, and column type names now stay schema-qualified when --qualify-schema is set (via the *Mode helper variants). Default (false) output is unchanged. Still TODO: thread the remaining create generators (constraints/FK refs, functions, sequences, views, types, indexes, triggers, policies, privileges) + tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

generateFunctionSQL / formatFunctionParameter take qualifySchema (threaded from collector at all call sites); generateFunctionComment reads collector.qualifySchema. Function name, parameter types/defaults, and return type now stay schema-qualified under --qualify-schema. Covers issue #321's CREATE FUNCTION example. Default unchanged. Remaining create generators: sequence, type, view, constraint(FK refs), index, trigger, policy, privilege, aggregate. Then tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…e paths [WIP] - generateSequenceSQL: sequence name and OWNED BY owner table qualify under the flag. - generateConstraintSQL: FK REFERENCES target table qualifies under the flag (threaded from generateTableSQL's inline-constraint loop). - Updated test callers to pass qualifySchema=false (default behavior). Remaining: type/domain attrs, aggregate state types, views, indexes, triggers, policies, privileges. Then tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…hs [WIP] - generateTypeSQL: type/domain/composite name and composite attribute types qualify. - generateAggregateSQL: aggregate name and STYPE/MSTYPE state types qualify. - generateAggregateComment: name qualifies (reads collector.qualifySchema). Covered now: tables/columns, functions, sequences, constraints(FK), types, aggregates. Remaining: views, indexes, triggers, policies, privileges; then tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…reate paths [WIP] - generateViewSQL: view name qualifies under the flag (the targetSchema!="" branch now honors qualifySchema; the targetSchema=="" branch already always-qualified). - index/view COMMENT ON statements qualify the object name. - generateTriggerSQLWithMode: CREATE TRIGGER ON <table> qualifies the table. - generateRLSChangesSQL: ALTER TABLE ... ROW LEVEL SECURITY qualifies the table. - Updated all builder callers to pass collector.qualifySchema; test callers pass false. Structural entity names/types now qualify consistently across object types. Per design, free-form expression text (view bodies, policy USING/CHECK, trigger function bodies) is left as-is. Privilege generators don't use the qualify helpers (out of scope). Next: positive + negative byte-identical tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Fast builder-level tests for --qualify-schema: table name + column type, FK referenced table, enum type, and sequence name + OWNED BY owner. Each asserts the forced-qualification output AND that the default (qualifySchema=false) output is unchanged — the negative cases guard against any default-behavior drift. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

greptile-apps · 2026-06-29T10:55:54Z

Greptile Summary

This PR adds an opt-in dump mode for fully schema-qualified SQL. The main changes are:

New --qualify-schema flag on pgschema dump.
New diff option plumbing through GenerateMigrationWithOptions and diffCollector.
Mode-aware qualification helpers for many create-path builders.
Formatter support for schema names in dump comment headers.
Builder-level tests for several qualified dump outputs.

Confidence Score: 4/5

Forced qualification is incomplete across several dump SQL paths.

The flag reaches the collector and many create builders.
Procedure, domain, table alteration, and index DDL paths can still emit unqualified target-schema names.
These paths can keep the ambiguity that the new flag is meant to remove.

internal/diff/procedure.go, internal/diff/type.go, internal/diff/table.go, internal/diff/index.go

Important Files Changed

Filename	Overview
cmd/dump/dump.go	Adds the CLI flag and passes it into diff generation and dump formatting.
internal/diff/diff.go	Adds the option-bearing migration entry point and collector state, but downstream generators are not all wired to it.
internal/diff/table.go	Create-table output is updated, while related ALTER and deferred reference paths still use legacy schema stripping.
internal/diff/type.go	Type names and composite attributes are updated, but domain base types can remain unqualified.
internal/diff/index.go	Index comments honor the new mode, but index DDL can still use unqualified target table names.
internal/dump/formatter.go	Stores the formatter option and keeps target schema names in comment headers when forced qualification is enabled.
ir/quote.go	Adds a mode-aware quoted-name helper while preserving default smart qualification.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[dump --qualify-schema] --> B[DumpConfig.QualifySchema]
  B --> C[GenerateMigrationWithOptions]
  C --> D[diffCollector.qualifySchema]
  D --> E[Updated create-path generators]
  D -. not fully threaded .-> F[Procedures]
  D -. not fully threaded .-> G[ALTER/deferred table paths]
  D -. partial .-> H[Index DDL]
  E --> I[Fully qualified SQL]
  F --> J[Unqualified SQL under the flag]
  G --> J
  H --> J

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
  A[dump --qualify-schema] --> B[DumpConfig.QualifySchema]
  B --> C[GenerateMigrationWithOptions]
  C --> D[diffCollector.qualifySchema]
  D --> E[Updated create-path generators]
  D -. not fully threaded .-> F[Procedures]
  D -. not fully threaded .-> G[ALTER/deferred table paths]
  D -. partial .-> H[Index DDL]
  E --> I[Fully qualified SQL]
  F --> J[Unqualified SQL under the flag]
  G --> J
  H --> J

_{Reviews (1): Last reviewed commit: "style(dump): gofmt struct alignment for ..." | Re-trigger Greptile}

Copilot

Pull request overview

This PR adds an opt-in pgschema dump --qualify-schema flag intended to force fully schema-qualified DDL output (instead of the default “smart qualification” that omits the target-schema prefix), by threading a qualifySchema option through the shared internal/diff SQL generators and adjusting dump formatting to keep schema names in comment headers.

Changes:

Adds diff.GenerateMigrationWithOptions(..., qualifySchema) and stores the option on diffCollector so generators can opt into forced qualification without changing default plan/apply behavior.
Introduces mode-aware qualification helpers (e.g., QualifyEntityNameWithQuotesMode, qualifyEntityNameMode, stripSchemaPrefixMode) and threads the option through multiple SQL builders.
Adds dump CLI/config plumbing for --qualify-schema, updates dump formatter behavior, and adds targeted builder-level tests for qualification behavior.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
ir/quote.go	Adds `QualifyEntityNameWithQuotesMode` to support forced qualification with proper quoting.
internal/dump/formatter.go	Adds `qualifySchema` to formatter and preserves schema name in comment headers when forced qualification is enabled.
internal/diff/view.go	Threads `qualifySchema` into view SQL generation and qualification for view comments/triggers.
internal/diff/type.go	Threads `qualifySchema` into type SQL generation and type-attribute datatype qualification.
internal/diff/trigger.go	Threads `qualifySchema` into trigger SQL generation (target table qualification).
internal/diff/table.go	Adds `stripSchemaPrefixMode` and threads `qualifySchema` through CREATE TABLE + column/type handling.
internal/diff/sequence.go	Threads `qualifySchema` into sequence naming and OWNED BY qualification.
internal/diff/qualify_schema_test.go	Adds new builder-level tests for forced qualification vs default behavior.
internal/diff/policy.go	Applies forced qualification to RLS ALTER TABLE statements.
internal/diff/index.go	Applies forced qualification to index comment targets.
internal/diff/identifier_quote_test.go	Updates tests to reflect new builder signatures/options plumbing.
internal/diff/function.go	Threads `qualifySchema` into function SQL generation including params/return types/defaults and comments.
internal/diff/diff.go	Adds `GenerateMigrationWithOptions` and `qualifyEntityNameMode`; stores the option on the collector.
internal/diff/constraint.go	Threads `qualifySchema` into FK referenced-table qualification.
internal/diff/collector.go	Stores `qualifySchema` on the collector as shared generator configuration.
internal/diff/aggregate.go	Threads `qualifySchema` into aggregate naming and state-type qualification.
cmd/dump/multifile_test.go	Updates formatter construction for the new `NewDumpFormatter` signature.
cmd/dump/dump.go	Adds `--qualify-schema` flag, wires it into config, calls `GenerateMigrationWithOptions`, and passes through to formatter.
cmd/dump/dump_test.go	Updates formatter construction for the new `NewDumpFormatter` signature.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

# Conflicts: # internal/diff/table.go # internal/diff/view.go

pg-codex local review found create-path qualification sites outside the original three-helper inventory. A full re-audit of all four qualify helpers (getTableNameWithSchema, qualifyEntityName, stripSchemaPrefix, QualifyEntityNameWithQuotes) against the empty->target dump path turned up these unthreaded create-path emitters: - CREATE INDEX ... ON <table> (generateIndexSQLWithName) - CREATE POLICY ... ON <table> (generatePolicySQL) - CREATE PROCEDURE name + param type (generateProcedureSQL / formatParameterString) - COMMENT ON PROCEDURE/SEQUENCE/TABLE/COLUMN - deferred FK ALTER TABLE + REFERENCES (generateDeferredConstraintsSQL / generateForeignKeyClause) Adds getTableNameWithSchemaMode and Mode variants for the index/policy/ FK-clause helpers; threads qualifySchema through the procedure path mirroring the function path. Modify/drop callers keep the delegating defaults, so plan/apply output is unchanged. Tests: extends qualify_schema_test.go with index ON-table, policy ON-table, procedure (name + same-schema param type + DEFAULT), and deferred FK REFERENCES coverage (positive + bare-default negative). Full ./internal/diff/... suite passes with all existing fixtures unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Addresses greptile P1: the domain BaseType branch emitted typeObj.BaseType directly while composite attribute types already went through stripSchemaPrefixMode. Route BaseType through the same helper so a cross-schema (or target-schema-prefixed) base type is preserved under forced qualification and the target-schema prefix is stripped otherwise. Default output is unchanged (full ./internal/diff/... suite passes with all fixtures intact). Note: same-schema user-defined base types are stored bare by the inspector, so this cannot add a prefix to them — that limitation is shared by all type-reference positions and is tracked separately from object-identifier qualification. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Clarify that the flag forces qualification of object identifiers and notes the known limitation that same-schema type references are not affected (the IR stores them without schema identity). Tracked for a follow-up inspector/IR change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 6 comments.

- procedure.go: the recreate (DROP+CREATE) path now passes collector.qualifySchema to the CREATE half for coherence — a CREATE statement should honor forced qualification. Byte-identical in practice (only `dump` sets the flag, and dump is empty->target / create-only, so the modify path never runs with qualifySchema=true). - qualify_schema_test.go: the table-column and procedure-parameter tests used pre-qualified type strings (public.user_kind / public.currency) that the inspector never produces for same-schema types. Switched to bare type refs and inverted the assertions to guard the #493 boundary: the object *name* is qualified under the flag, while the same-schema type *reference* stays bare (forced qualification cannot invent a schema for a bare ref). - formatter_test.go (new): table-driven coverage for getCommentSchemaName, exercising qualifySchema=true (target schema preserved vs collapsed to "-"), cross-schema, and unqualified paths. go build/vet, full ./internal/diff/... + ./internal/dump/... suites pass; gofmt clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…f limitation The file-level comment still claimed names/types are always qualified; the tests now document that bare same-schema type references stay bare under forced qualification (#493). Comment-only change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.

Addresses Copilot: the docstring claimed qualifySchema=true makes the emitted DDL "always" schema-qualify entity names, but DROP/ALTER-in-place paths keep smart qualification. Reword to scope the guarantee to the create-path (empty->target) object identifiers that dump actually emits, and note the #493 same-schema type-reference limitation. Comment-only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

tianzhou and others added 8 commits June 28, 2026 21:46

style(dump): gofmt struct alignment for new QualifySchema field

c8a0989

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings June 29, 2026 10:53

Copilot started reviewing on behalf of tianzhou June 29, 2026 10:54 View session

greptile-apps Bot reviewed Jun 29, 2026

View reviewed changes

Comment thread internal/diff/diff.go

Comment thread internal/diff/type.go

Comment thread internal/diff/table.go

Comment thread internal/diff/index.go

Copilot AI reviewed Jun 29, 2026

View reviewed changes

Comment thread internal/diff/index.go

Comment thread internal/diff/policy.go

Comment thread internal/diff/diff.go

tianzhou and others added 4 commits June 29, 2026 04:05

Merge remote-tracking branch 'origin/main' into feat/dump-qualify-schema

aefae55

# Conflicts: # internal/diff/table.go # internal/diff/view.go

tianzhou mentioned this pull request Jun 29, 2026

Inspector/IR should preserve schema identity for type references (enable --qualify-schema for types) #493

Open

tianzhou requested a review from Copilot June 29, 2026 11:46

Copilot started reviewing on behalf of tianzhou June 29, 2026 11:46 View session

Copilot AI reviewed Jun 29, 2026

View reviewed changes

tianzhou and others added 2 commits June 29, 2026 19:25

tianzhou requested a review from Copilot June 30, 2026 02:47

Copilot started reviewing on behalf of tianzhou June 30, 2026 02:47 View session

Copilot AI reviewed Jun 30, 2026

View reviewed changes

Comment thread internal/diff/diff.go

tianzhou and others added 2 commits June 29, 2026 20:50

docs(diff): clarify qualify-schema option scope

bc75f46

tianzhou requested a review from Copilot June 30, 2026 04:00

Copilot started reviewing on behalf of tianzhou June 30, 2026 04:01 View session

Copilot AI reviewed Jun 30, 2026

View reviewed changes

Comment thread internal/diff/diff.go Outdated

Potential fix for pull request finding

4943f7b

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

tianzhou merged commit dc353de into main Jun 30, 2026
1 check passed

tianzhou deleted the feat/dump-qualify-schema branch June 30, 2026 04:18

tianzhou mentioned this pull request Jun 30, 2026

docs(dump): document --qualify-schema flag (#321) #494

Merged

Uh oh!

Conversation

tianzhou commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

How

Coverage

Out of scope (intentional)

Tests

Scope & known limitation

Uh oh!

greptile-apps Bot commented Jun 29, 2026

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Flowchart

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tianzhou commented Jun 29, 2026 •

edited

Loading