feat(dump): qualify same-schema type references under --qualify-schema (#493) by tianzhou · Pull Request #514 · pgplex/pgschema

tianzhou · 2026-07-20T16:27:41Z

Follow-up to #492. Implements Mechanism A of #493: preserve the schema of same-schema user-defined type references so dump --qualify-schema emits them fully qualified, while the default (smart-qualification) dump stays byte-identical.

What

--qualify-schema already forced qualification for object identifiers. It could not qualify same-schema type references because the inspector/IR stored them bare — the render helper (stripSchemaPrefixMode) can only strip a target-schema prefix, never add one. This teaches the inspector to preserve the schema for those references; the existing render seams then qualify under the flag and strip by default.

Type references now qualified:

column types (domain / enum / composite)
domain base types
aggregate state types (STYPE / MSTYPE)
composite-type attributes

How

Column CASE branches (regular + partitioned column queries): flip the same-schema domain/enum/composite branch from bare typname to schema.typname.
format_type-based sites (domain base type, aggregate state/mstate type, composite attribute): rewritten to an explicit pg_type/pg_namespace join + CASE that keeps format_type's canonical spelling and typmod for built-in and array types, and qualifies user-defined scalar types (splicing the typmod back where relevant). Built-ins are never pg_catalog-qualified.
Dependency sort (internal/diff/topological.go): because type references now arrive qualified and possibly quote_ident-quoted, extractTypeName unquotes each identifier component and splits on the last unquoted dot, so edges still match the bare typeMap keys (Gate-3 fix). Regression tests added.

Default output is unchanged

The render seams (stripSchemaPrefixMode, type.go, aggregate.go, table.go) were already qualifySchema-aware; only the IR changed. Verified byte-identical:

internal/diff TestDiffFromFiles (150+ fixtures) — unchanged
cmd/dump TestDumpCommand_* (18 suites × PG 14–18) — unchanged

Tests

New cmd/dump/qualify_schema_integration_test.go — drives the full pipeline (SQL → embedded DB → inspector → IR → dump) and asserts all four slices qualify under the flag and stay bare by default, and that built-ins are never pg_catalog-qualified.
internal/diff/topological_test.go — TestExtractTypeNameNormalizesQuoting and TestTopologicallySortTypesQualifiedQuotedRefs (the latter fails without the unquote fix).
Updated the stale builder test TestQualifySchema_TableAndColumnType to the new end state; refreshed the file header and the --qualify-schema flag help.

Out of scope (follow-up)

Function/procedure parameter and return types — these come from PostgreSQL's format-functions as composite signature strings and the inspector deliberately strips their schema during signature parsing (inspector.go), so they need a different mechanism. Left bare and documented in the flag help and test comments.

🤖 Generated with Claude Code

#493) `dump --qualify-schema` previously forced qualification only for object identifiers; same-schema user-defined type references stayed bare because the inspector/IR discarded their schema. This preserves the schema for the "Mechanism A" type references so the existing render seams (stripSchemaPrefixMode) qualify them under the flag and strip them by default. Inspector queries now emit qualified user-defined types for: - column types (domain/enum/composite) - domain base types - aggregate state types (STYPE/MSTYPE) - composite-type attributes format_type-based sites are rewritten to an explicit pg_type/pg_namespace join + CASE that keeps format_type's canonical spelling and typmod for built-in/array types while qualifying user-defined scalar types. Because the inspector now emits qualified (and possibly quote_ident-quoted) type references, extractTypeName in the type dependency sort is normalized to unquote each identifier component and split on the last *unquoted* dot, so edges still match the bare typeMap keys. Default (smart-qualification) output is byte-identical: the full diff and dump fixture suites pass unchanged across PG 14-18. A new dump integration test drives the whole pipeline and asserts both directions plus that built-ins are never pg_catalog-qualified. Function/procedure parameter and return types are intentionally out of scope (the inspector strips their schema during signature parsing) and tracked as follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

greptile-apps · 2026-07-20T16:30:57Z

Greptile Summary

This PR preserves schema information for user-defined type references in dumps. The main changes are:

Qualify same-schema type references in inspector queries.
Keep built-in and array type formatting through format_type.
Normalize quoted type references for dependency sorting.
Add dump and topology regression coverage.

Confidence Score: 4/5

Quoted identifiers containing dots can produce incorrect type dependency ordering.

The new normalization flattens distinct schema/type pairs into the same lookup key.
The primary qualification paths are covered by focused integration tests.

internal/diff/topological.go

Important Files Changed

Filename	Overview
ir/queries/queries.sql	Preserves schema-qualified user-defined references while retaining canonical formatting for built-ins and arrays.
ir/queries/queries.sql.go	Regenerated query constants mirror the SQL source changes.
internal/diff/topological.go	Normalizes quoted qualified references, but its flattened lookup key is ambiguous for identifiers containing dots.
cmd/dump/qualify_schema_integration_test.go	Adds end-to-end coverage for qualified and default rendering of the intended type-reference categories.
internal/diff/topological_test.go	Covers quoted names and dots in type names, but not collisions between dotted schema and type components.

_{Reviews (1): Last reviewed commit: "feat(dump): qualify same-schema type ref..." | Re-trigger Greptile}

Copilot

Pull request overview

This PR extends pgschema dump --qualify-schema so that same-schema user-defined type references are preserved by the inspector/IR and can therefore be emitted fully schema-qualified under the flag, while keeping the default (smart-qualification) dump output unchanged for existing fixtures.

Changes:

Updates inspector SQL to preserve schema qualification for same-schema type references (columns; domains’ base types; composite-type attributes; aggregates’ STYPE/MSTYPE), using pg_type/pg_namespace joins and quote_ident(...)-based qualification where needed.
Fixes type dependency sorting to tolerate qualified and quote_ident-quoted type references by normalizing quoted identifiers and splitting on the last unquoted dot.
Adds/updates tests: new end-to-end dump integration test for --qualify-schema type references, plus unit tests for the dependency-sort normalization behavior and refreshed qualify-schema builder expectations.

Reviewed changes

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

Show a summary per file

File	Description
ir/queries/queries.sql	Inspector query changes to preserve schema-qualified type references for same-schema UDTs.
ir/queries/queries.sql.go	Regenerated sqlc output reflecting the updated queries.
internal/diff/topological.go	Normalizes quoted/qualified type references so dependency edges still match `typeMap` keys.
internal/diff/topological_test.go	Adds regression coverage for quoted/qualified type reference normalization and sorting.
internal/diff/qualify_schema_test.go	Updates qualify-schema builder test expectations for qualified IR type refs.
cmd/dump/qualify_schema_integration_test.go	New end-to-end test asserting `--qualify-schema` qualifies key type-reference slices while defaults remain bare.
cmd/dump/dump.go	Updates `--qualify-schema` help text to reflect expanded type-reference qualification scope and remaining limitations.

Files not reviewed (1)

ir/queries/queries.sql.go: Generated file

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

…#493) Addresses CI failures and review feedback on the same-schema type-reference qualification (Mechanism A). - stripSchemaPrefixMode: the inspector emits schema prefixes via quote_ident, so for a target schema whose name requires quoting (e.g. "My Schema") the raw-prefix strip missed the quoted form and default output leaked the qualification. Strip both the raw and quote_ident forms (and the ::cast form). Regression test added. - topological type sort: replace the ambiguous "schema.name" graph key with a NUL-delimited typeGraphKey so legal-but-dotted identifiers (schema "a.b" type "c" vs schema "a" type "b.c") never collide. Collision test added. - Regenerate three plan.json fixtures whose source_fingerprint changed: the fingerprint hashes the serialized IR, which now stores qualified type strings, so schemas containing same-schema user-defined type references get a new hash. The plan DDL is byte-identical; only the hash line changed. Verified: internal/diff and TestPlanAndApply pass; dump fixtures pass (one CI/local flake was embedded-postgres startup port contention, unrelated). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.

Files not reviewed (1)

ir/queries/queries.sql.go: Generated file

Comments suppressed due to low confidence (1)

ir/queries/queries.sql:80

The inspector now emits same-schema type references as quote_ident(schema)||'.'||quote_ident(type). In default (smart-qualification) mode, stripSchemaPrefixMode only strips an unquoted prefix (targetSchema + "."). If the target schema requires quoting (e.g. "MySchema"), the default dump will fail to strip the prefix ("MySchema".type), changing output unexpectedly.

        CASE
            WHEN dt.typtype = 'd' THEN
                quote_ident(dn.nspname) || '.' || quote_ident(dt.typname)
            WHEN dt.typtype = 'e' OR dt.typtype = 'c' THEN
                quote_ident(dn.nspname) || '.' || quote_ident(dt.typname)
            WHEN dt.typtype = 'b' AND dt.typcategory = 'A' THEN

Follow-up review hardening for extractTypeName: - Strip a trailing numeric typmod suffix (e.g. "(384)", "(10,2)") before building the dependency-graph key, so a qualified reference the inspector emits with typmod still matches the bare typeMap key. Conservative: only a digits/comma/space typmod is stripped, so a quoted identifier that legally contains parentheses (ends in '"', not ')') is left intact. In practice typmod is only appended to built-in/extension base types (never enum/ composite/domain typeMap entries), but this makes the matching robust. - Add test coverage for the typmod cases and for an escaped-quote ("") inside a quoted identifier, confirming findLastUnquotedDot's parity handling treats a dot inside such an identifier as non-separating. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.

Files not reviewed (1)

ir/queries/queries.sql.go: Generated file

The column resolved_type array branch still emitted same-schema user-defined element types unqualified (WHEN en.nspname = c.table_schema THEN quote_ident(et.typname)), so a column like color[] (enum/composite/ domain array in the same schema) arrived in the IR without schema identity and dump --qualify-schema could not qualify it. Remove the same-schema case so non-pg_catalog element types are always qualified, mirroring the scalar domain/enum/composite branches; pg_catalog element types stay bare. Default output is unchanged because stripSchemaPrefixMode strips the target schema. Extend the dump integration test with a same-schema array column asserting "shades public.color[]" under --qualify-schema and "shades color[]" by default. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.

Files not reviewed (1)

ir/queries/queries.sql.go: Generated file

+    CASE
+        WHEN bn.nspname IS NULL OR bn.nspname = 'pg_catalog' OR bt.typcategory = 'A' THEN
+            format_type(t.typbasetype, t.typtypmod)
+        ELSE


+    CASE
+        WHEN atn.nspname IS NULL OR atn.nspname = 'pg_catalog' OR at.typcategory = 'A' THEN
+            format_type(a.atttypid, a.atttypmod)
+        ELSE


+    CASE
+        WHEN stn.nspname IS NULL OR stn.nspname = 'pg_catalog' OR stt.typcategory = 'A' THEN
+            format_type(a.aggtranstype, NULL)
+        ELSE quote_ident(stn.nspname) || '.' || quote_ident(stt.typname)
+    END AS state_type,


+    CASE WHEN a.aggmtransfn = 0 THEN ''
+         WHEN mstn.nspname IS NULL OR mstn.nspname = 'pg_catalog' OR mstt.typcategory = 'A' THEN
+            format_type(a.aggmtranstype, NULL)
+         ELSE quote_ident(mstn.nspname) || '.' || quote_ident(mstt.typname)
+    END AS mstate_type,


Copilot AI review requested due to automatic review settings July 20, 2026 16:27

Copilot started reviewing on behalf of tianzhou July 20, 2026 16:28 View session

greptile-apps Bot reviewed Jul 20, 2026

View reviewed changes

Comment thread internal/diff/topological.go Outdated

Copilot AI reviewed Jul 20, 2026

View reviewed changes

Comment thread ir/queries/queries.sql

Comment thread ir/queries/queries.sql

tianzhou requested a review from Copilot July 20, 2026 17:36

Copilot started reviewing on behalf of tianzhou July 20, 2026 17:37 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

Comment thread internal/diff/topological.go Outdated

Comment thread internal/diff/topological.go

tianzhou requested a review from Copilot July 20, 2026 18:12

Copilot started reviewing on behalf of tianzhou July 20, 2026 18:12 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

Comment thread ir/queries/queries.sql

Comment thread ir/queries/queries.sql

Comment thread ir/queries/queries.sql.go

Comment thread ir/queries/queries.sql.go

tianzhou requested a review from Copilot July 20, 2026 18:30

Copilot started reviewing on behalf of tianzhou July 20, 2026 18:30 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(dump): qualify same-schema type references under --qualify-schema (#493)#514

feat(dump): qualify same-schema type references under --qualify-schema (#493)#514
tianzhou wants to merge 4 commits into
mainfrom
feat/qualify-schema-type-refs-493

tianzhou commented Jul 20, 2026

Uh oh!

greptile-apps Bot commented Jul 20, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

tianzhou commented Jul 20, 2026

What

How

Default output is unchanged

Tests

Out of scope (follow-up)

Uh oh!

greptile-apps Bot commented Jul 20, 2026

Greptile Summary

Confidence Score: 4/5

Important Files Changed

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants