Skip to content

fix: strip quotes from type names in view dependency detection#337

Merged
tianzhou merged 2 commits intopgplex:mainfrom
FFX3:fix/quoted-type-name-view-dependency
Mar 13, 2026
Merged

fix: strip quotes from type names in view dependency detection#337
tianzhou merged 2 commits intopgplex:mainfrom
FFX3:fix/quoted-type-name-view-dependency

Conversation

@FFX3
Copy link
Contributor

@FFX3 FFX3 commented Mar 4, 2026

When a function returns a quoted type like SETOF public."ViewName", the extractBaseTypeName function was not stripping the double quotes. This caused the lookup in functionReferencesNewView to fail because the view lookup map stores unquoted names (e.g., public.viewname).

As a result, functions with quoted return types were not detected as having view dependencies, causing them to be created before the views they depend on, leading to "type does not exist" errors.

When a function returns a quoted type like SETOF public."ViewName",
the extractBaseTypeName function was not stripping the double quotes.
This caused the lookup in functionReferencesNewView to fail because
the view lookup map stores unquoted names (e.g., public.viewname).

As a result, functions with quoted return types were not detected as
having view dependencies, causing them to be created before the views
they depend on, leading to "type does not exist" errors.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@greptile-apps
Copy link

greptile-apps bot commented Mar 4, 2026

Greptile Summary

This PR fixes a bug in extractBaseTypeName where double-quoted PostgreSQL identifiers (e.g. SETOF public."ViewName") were not having their quotes stripped before view-dependency lookup, causing functions that return quoted view types to be ordered before their dependent views — leading to type does not exist errors at runtime.

Key changes:

  • Added strings.ReplaceAll(t, "\"", "") in extractBaseTypeName to strip SQL delimited-identifier quotes, so the unquoted name can be matched against the lowercase view lookup map built by buildSchemaNameLookup.
  • Updated the function's doc comment to document the new stripping behavior.

Notes:

  • The fix is minimal and correct for all realistic PostgreSQL type expressions (quoted schema, quoted type name, quoted SETOF types, quoted array types).
  • No unit tests were added for the quoted-identifier path in extractBaseTypeName or functionReferencesNewView, leaving the fix without a regression guard.

Confidence Score: 4/5

  • Safe to merge — the one-line fix is correct and non-breaking, but lacks regression tests for the new behavior.
  • The change is minimal (a single strings.ReplaceAll call added after existing stripping steps), targets a well-understood code path, and introduces no new dependencies. The logic is correct: stripping SQL delimiter quotes before case-folded lookup is the right approach, and the existing downstream strings.ToLower in typeMatchesLookup handles the rest. Score is 4 rather than 5 only because the fix has no accompanying test coverage, leaving the quoted-identifier path un-guarded against future regressions.
  • No files require special attention, but consider adding unit tests for extractBaseTypeName in diff_test.go.

Important Files Changed

Filename Overview
internal/diff/diff.go Adds strings.ReplaceAll to strip double quotes from SQL identifiers in extractBaseTypeName; fix is correct and low-risk but no unit tests were added for the new behavior.

Sequence Diagram

sequenceDiagram
    participant Diff as diff.go (orderDDLStatements)
    participant FRV as functionReferencesNewView
    participant EBT as extractBaseTypeName
    participant TML as typeMatchesLookup
    participant VL as newViewLookup (map)

    Diff->>FRV: fn, newViewLookup
    FRV->>EBT: fn.ReturnType (e.g. SETOF public."ViewName")
    EBT-->>EBT: strip SETOF → public."ViewName"
    EBT-->>EBT: strip [] → public."ViewName"
    EBT-->>EBT: strip quotes → public.ViewName  [NEW]
    EBT-->>FRV: "public.ViewName"
    FRV->>TML: typeName="public.ViewName", schema, newViewLookup
    TML-->>TML: ToLower → "public.viewname"
    TML->>VL: lookup["public.viewname"]
    VL-->>TML: found ✓
    TML-->>FRV: true
    FRV-->>Diff: true → function depends on view, order accordingly
Loading

Last reviewed commit: b1f3b10

Copy link
Contributor

@tianzhou tianzhou left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. please add an unit test

@FFX3
Copy link
Contributor Author

FFX3 commented Mar 4, 2026

I'll take a look at adding the tests

Add tests for the extractBaseTypeName function to verify correct
handling of quoted identifiers in type expressions. Tests cover:
- Simple types
- SETOF prefix with schema-qualified types
- Quoted type names (e.g., SETOF public."ViewName")
- Quoted schema and type names
- Array notation
- Combined cases (SETOF + quoted + array)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@FFX3
Copy link
Contributor Author

FFX3 commented Mar 12, 2026

Thanks for the contribution. please add an unit test

Tests added

@FFX3 FFX3 requested a review from tianzhou March 12, 2026 19:10
Copy link
Contributor

@tianzhou tianzhou left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the contribution.

@tianzhou tianzhou merged commit d609e87 into pgplex:main Mar 13, 2026
1 check passed
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.

2 participants