fix: strip schema qualifiers from custom types in RETURNS TABLE (#360)

[tianzhou]

Summary

When a function uses RETURNS TABLE(...) with custom types (domains, composite types, enums), pg_get_function_result may schema-qualify those types depending on search_path. The stripSchemaFromReturnType normalization was skipping TABLE return types entirely (early return with comment "TABLE types are already handled by normalizeFunctionReturnType"), but normalizeFunctionReturnType only normalizes type aliases — it never strips schema qualifiers.

This caused a false diff between desired state (TABLE(... public.datetimeoffset ...)) and current state (TABLE(... datetimeoffset ...)), producing unnecessary DROP FUNCTION + CREATE OR REPLACE FUNCTION in every plan run.

Fix: Parse TABLE return type column definitions and strip schema prefixes from each column's type, matching the existing behavior for SETOF and direct return types.

Fixes #360

Test plan

• Added testdata/diff/create_function/issue_360_returns_table_custom_type/ with a function using RETURNS TABLE containing a custom composite type
• Verified test fails before fix (produces unnecessary ALTER) and passes after fix (empty plan)
• All 11 create_function/ tests pass (diff + integration)

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR fixes two related issues with schema-qualifier normalisation for PostgreSQL functions and procedures:

1. Issue plan on same db that was dumped isn't empty #360 — stripSchemaFromReturnType was returning TABLE(...) return types unchanged (early-return with "already handled" comment) even though the TABLE column types could be schema-qualified by pg_get_function_result. The fix adds proper column-level parsing to strip the schema prefix from each column's type.

2. Issue SET search_path not showing correct on plan or apply if you use '' #354 follow-up — Schema qualifiers inside dollar-quoted function/procedure bodies are now preserved verbatim instead of being stripped at normalisation time. A new splitDollarQuotedSegments helper skips $$…$$ / $tag$…$tag$ blocks, and the diff comparison uses a new definitionsEqualIgnoringSchema helper that strips qualifiers at comparison time only.

The changes are well-structured and cover the main regression path. Two concerns worth addressing:

• Fragile comma-split for parameterized TABLE column types: Both the existing normalizeFunctionReturnType and the new stripSchemaFromReturnType TABLE handling use strings.Split(inner, ","). This will mis-parse column types that contain commas in their precision/scale specification (e.g., numeric(10, 2)), silently producing malformed column lists. A parenthesis-aware split should be used in both functions.
• Test coverage gap: The integration test for issue plan on same db that was dumped isn't empty #360 uses identical old.sql / new.sql files, validating idempotency but not the specific schema-stripping path. A fixture where one side has public.datetimeoffset and the other has datetimeoffset would directly exercise the fix.
• Quoting inconsistency in golden file: testdata/dump/issue_320_plpgsql_reserved_keyword_type/pgschema.sql now shows public."user" (quoted) in one function but public.user (unquoted) in another, for the same reserved-keyword table name. This likely reflects genuine pg_get_functiondef output, but should be verified to ensure it round-trips safely.

Confidence Score: 3/5

• This PR is generally safe to merge but contains a logic issue with comma-splitting of parameterized TABLE column types that could silently corrupt return-type comparisons for affected schemas.
• The core fix for issue plan on same db that was dumped isn't empty #360 is correct and well-tested for simple custom types. The dollar-quote preservation logic for function bodies is well-implemented with good unit tests. However, the strings.Split(inner, ",") used in both stripSchemaFromReturnType and the pre-existing normalizeFunctionReturnType will produce incorrect results for TABLE return types containing parameterized column types (e.g., numeric(10, 2)), which is a P1 logic bug. Additionally, the test fixture for the primary fix only validates idempotency rather than the schema-stripping path itself, reducing confidence that the exact scenario from the bug report is covered.
• ir/normalize.go — the TABLE comma-split logic in both normalizeFunctionReturnType and stripSchemaFromReturnType; testdata/dump/issue_320_plpgsql_reserved_keyword_type/pgschema.sql — inconsistent quoting of the user keyword.

Important Files Changed

Filename	Overview
ir/normalize.go	Core fix: stripSchemaFromReturnType now parses TABLE column definitions to strip schema prefixes. The stripSchemaPrefixFromBody function is exported (renamed to StripSchemaPrefixFromBody). Schema stripping is removed from normalizeFunction/normalizeProcedure bodies (moved to diff-time comparison). The comma-split logic for TABLE types is fragile for parameterized types like numeric(10, 2) — a pre-existing issue in normalizeFunctionReturnType that this change perpetuates.
internal/diff/function.go	Adds definitionsEqualIgnoringSchema helper used in all three function-comparison paths (functionsEqual, functionsEqualExceptAttributes, functionsEqualExceptComment). Correctly uses old.Schema (which has already been asserted equal to new.Schema before this call). Clean, low-risk change.
internal/diff/procedure.go	Mirrors the function.go change for procedures: both proceduresEqual and proceduresEqualExceptComment now use definitionsEqualIgnoringSchema. Straightforward and correct.
internal/postgres/desired_state.go	Adds splitDollarQuotedSegments and dollarQuoteRe to preserve dollar-quoted function/procedure bodies verbatim when stripping schema qualifiers. The regex correctly excludes $1/$2 parameter references. Unterminated dollar-quote blocks are treated as quoted (conservative). Solid implementation with good test coverage.
internal/postgres/desired_state_test.go	Adds thorough unit tests for splitDollarQuotedSegments: covers no quotes, simple $$, named tags, parameter references ($1/$2), multiple blocks, unterminated blocks, and empty input. Good coverage.
testdata/diff/create_function/issue_360_returns_table_custom_type/old.sql	old.sql and new.sql are identical, so the test only validates idempotency — not that stripSchemaFromReturnType actually strips public.datetimeoffset to datetimeoffset. A fixture that exercises the actual schema-stripping path would strengthen confidence in the fix.
testdata/dump/issue_320_plpgsql_reserved_keyword_type/pgschema.sql	Golden-file update that now preserves public. schema qualifiers inside dollar-quoted bodies. Shows inconsistent quoting of the user reserved keyword: quoted (public."user") in count_users FROM clause but unquoted (public.user) in get_first_user DECLARE and FROM, which may reflect genuine pg_get_functiondef differences but should be verified.
ir/normalize_test.go	Minimal change: updates call sites from the renamed stripSchemaPrefixFromBody to the exported StripSchemaPrefixFromBody. No logic changes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ReturnType from pg_get_function_result] --> B{SETOF prefix?}
    B -- Yes --> C[Strip schema from SETOF base type]
    B -- No --> D{TABLE prefix and suffix?}
    D -- Yes --> E["Split inner by comma (strings.Split)"]
    E --> F[Strip schema from each column type]
    F --> G["Rejoin → TABLE(col type, ...)"]
    D -- No --> H[Strip schema from direct type name]

    I[Function Body from pg_get_functiondef] --> J[splitDollarQuotedSegments]
    J --> K{Inside dollar-quoted block?}
    K -- Yes --> L[Preserve verbatim]
    K -- No --> M[stripSchemaQualificationsFromText]
    M --> N[Apply regex patterns 1-4]

    O[Diff comparison] --> P[definitionsEqualIgnoringSchema]
    P --> Q[StripSchemaPrefixFromBody on both sides]
    Q --> R{Stripped forms equal?}
    R -- Yes --> S[No diff generated]
    R -- No --> T[CREATE OR REPLACE or DROP + CREATE]

Loading

_{Last reviewed commit: "fix: strip schema qu..."}

[Copilot]

Pull request overview

Fixes false diffs/recreates for Postgres functions whose RETURNS TABLE(...) return types include schema-qualified custom types (domains/composites/enums), and adjusts schema-qualification handling for function/procedure bodies to avoid breaking cases involving SET search_path.

Changes:

• Strip same-schema qualifiers from custom types inside RETURNS TABLE(...) return type strings during IR normalization.
• Preserve schema qualifiers in function/procedure bodies in IR and instead compare definitions while ignoring same-schema qualification in the diff layer (Issue #354).
• Update desired-state SQL rewriting to avoid stripping qualifiers inside dollar-quoted blocks; refresh related fixtures and add a regression test case for Issue #360.

Reviewed changes

Copilot reviewed 21 out of 23 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
ir/normalize.go	Adds RETURNS TABLE(...) column-type schema stripping; stops stripping schema qualifiers from function/procedure bodies during normalization.
ir/normalize_test.go	Updates unit test to use exported StripSchemaPrefixFromBody.
internal/diff/function.go	Compares function definitions via schema-stripped normalization at diff time (definitionsEqualIgnoringSchema).
internal/diff/procedure.go	Uses the same schema-ignoring definition comparison for procedures.
internal/postgres/desired_state.go	Splits SQL into dollar-quoted vs non-quoted segments so schema stripping avoids function/procedure bodies.
internal/postgres/desired_state_test.go	Adds unit tests for dollar-quote segmentation helper.
testdata/diff/create_function/issue_360_returns_table_custom_type/*	New regression fixture proving empty plan when TABLE return types include custom types.
testdata/diff/create_function/issue_354_empty_search_path/*	Updates fixtures to reflect preserved schema qualification in function bodies.
testdata/diff/dependency/table_to_function/*	Updates dependency fixtures to reflect preserved schema qualification in function bodies.
testdata/dump/issue_252_function_schema_qualifier/pgschema.sql	Updates dump fixture to keep schema-qualified references in routine bodies.
testdata/dump/issue_320_plpgsql_reserved_keyword_type/pgschema.sql	Updates dump fixture to keep schema-qualified references in routine bodies.
testdata/diff/create_trigger/add_trigger/plan.json	Updates expected plan fingerprint/hash due to upstream SQL text changes.

---

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

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

Pull request overview

This PR fixes false diffs caused by schema-qualified custom types inside RETURNS TABLE(...) by normalizing/stripping same-schema type qualifiers per TABLE column type, and adjusts routine body handling so schema qualifiers are preserved in IR but ignored during diff comparisons.

Changes:

• Strip same-schema qualifiers from custom types inside RETURNS TABLE(...) (and improve TABLE column splitting to respect nested parentheses).
• Preserve schema qualifiers in function/procedure bodies in IR; compare definitions with a same-schema-stripped view during diffing to avoid search_path-related false diffs.
• Update/add test fixtures and unit tests for the new normalization behavior (including issue #360).

Reviewed changes

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

Show a summary per file

File	Description
testdata/dump/issue_320_plpgsql_reserved_keyword_type/pgschema.sql	Updates fixture to reflect preserved schema qualifiers in routine bodies.
testdata/dump/issue_252_function_schema_qualifier/pgschema.sql	Updates fixture to reflect schema-qualified table refs in routine bodies.
testdata/diff/dependency/table_to_function/plan.txt	Updates expected plan output with preserved schema qualification in function body.
testdata/diff/dependency/table_to_function/plan.sql	Updates expected SQL plan output with preserved schema qualification.
testdata/diff/dependency/table_to_function/plan.json	Updates expected JSON plan output with preserved schema qualification.
testdata/diff/dependency/table_to_function/diff.sql	Updates expected diff output with preserved schema qualification.
testdata/diff/create_trigger/add_trigger/plan.json	Updates fingerprint hash due to fixture changes.
testdata/diff/create_function/issue_360_returns_table_custom_type/plan.txt	Adds regression fixture asserting “No changes detected” for issue #360.
testdata/diff/create_function/issue_360_returns_table_custom_type/plan.json	Adds JSON fixture for the empty plan case.
testdata/diff/create_function/issue_360_returns_table_custom_type/old.sql	Adds old schema SQL fixture reproducing RETURNS TABLE custom-type scenario.
testdata/diff/create_function/issue_360_returns_table_custom_type/new.sql	Adds new schema SQL fixture (same as old; should diff to empty).
testdata/diff/create_function/issue_354_empty_search_path/plan.txt	Updates expected plan output for search_path edge case.
testdata/diff/create_function/issue_354_empty_search_path/plan.sql	Updates expected SQL plan output for search_path edge case.
testdata/diff/create_function/issue_354_empty_search_path/plan.json	Updates expected JSON plan output for search_path edge case.
testdata/diff/create_function/issue_354_empty_search_path/diff.sql	Updates expected diff output for search_path edge case.
ir/normalize_test.go	Updates/extends normalization unit tests (TABLE splitting + stripSchemaFromReturnType coverage).
ir/normalize.go	Implements TABLE column splitting and schema stripping within TABLE return types; exports StripSchemaPrefixFromBody; stops stripping body qualifiers during IR normalization.
internal/postgres/desired_state_test.go	Adds unit tests for dollar-quote segmentation helper.
internal/postgres/desired_state.go	Changes schema-qualification stripping to avoid touching dollar-quoted blocks (preserve routine bodies).
internal/diff/procedure.go	Compares procedure bodies via schema-stripped equality helper.
internal/diff/function.go	Compares function bodies via schema-stripped equality helper; adds definitionsEqualIgnoringSchema.

---

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

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

Pull request overview

Fixes false diffs for functions using RETURNS TABLE(...) where pg_get_function_result may schema-qualify custom types depending on search_path, by stripping same-schema qualifiers from each TABLE column’s type during IR normalization.

Changes:

• Add parsing helpers to split TABLE return column lists by top-level commas and split column name vs type (with quoted identifier support).
• Update return-type normalization/stripping to process RETURNS TABLE(...) column types (including types with nested parentheses like numeric(10, 2)).
• Add regression fixture for issue #360 and unit tests for the new parsing helpers + TABLE return-type stripping.

Reviewed changes

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

File	Description
ir/normalize.go	Adds TABLE column parsing helpers and strips same-schema qualifiers within RETURNS TABLE(...) types.
ir/normalize_test.go	Adds unit tests for the new parsing helpers and TABLE return-type stripping behavior.
testdata/diff/create_function/issue_360_returns_table_custom_type/*	New regression fixture demonstrating “empty plan” after normalization fix.

---

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

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

Pull request overview

Fixes a normalization gap in ir/normalize.go where RETURNS TABLE(...) return types could retain same-schema qualifiers (depending on search_path), causing false function diffs and unnecessary drop/recreate steps during planning.

Changes:

• Parse TABLE(...) return-type column definitions and strip same-schema prefixes from each column’s type.
• Improve TABLE column splitting to respect nested parentheses (e.g., numeric(10, 2)) and quoted identifiers.
• Add unit tests for the new parsing helpers and schema-stripping behavior, plus a regression fixture for issue #360.

Reviewed changes

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

File	Description
ir/normalize.go	Adds robust parsing for TABLE(...) return types and strips same-schema qualifiers from column types during normalization.
ir/normalize_test.go	Adds focused unit tests for splitTableColumns, splitColumnNameAndType, and TABLE-aware schema stripping.
testdata/diff/create_function/issue_360_returns_table_custom_type/*	Adds a regression test fixture ensuring plans are empty when only TABLE column type qualification differs.

---

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

You can also share your feedback on Copilot code review. Take the survey.