Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
FFX3 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion internal/diff/diff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1929,7 +1929,7 @@ func functionReferencesNewView(fn *ir.Function, newViews map[string]struct{}) bo
}

// extractBaseTypeName extracts the base type name from a type expression,
// stripping SETOF prefix and array notation.
// stripping SETOF prefix, array notation, and double quotes from identifiers.
func extractBaseTypeName(typeExpr string) string {
t := strings.TrimSpace(typeExpr)
// Strip SETOF prefix (case-insensitive)
Expand All @@ -1940,6 +1940,8 @@ func extractBaseTypeName(typeExpr string) string {
for len(t) > 2 && t[len(t)-2:] == "[]" {
t = t[:len(t)-2]
}
// Strip double quotes from identifiers (e.g., public."ViewName" -> public.ViewName)
t = strings.ReplaceAll(t, "\"", "")
Copy link

@greptile-apps greptile-apps bot Mar 4, 2026

Choose a reason for hiding this comment

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

No tests added for the new quoted-identifier behavior

The fix is correct, but there are no unit tests in the diff covering the new quoted-identifier stripping logic. Without tests, future refactors could silently regress this case. Consider adding a test for extractBaseTypeName (or an integration-style test for functionReferencesNewView) that covers at minimum:

  • SETOF public."ViewName" → public.ViewName
  • "Schema"."View" → Schema.View
  • "MyType"[] → MyType
  • public.unquoted → public.unquoted (no-op, regression guard)

Example:

func TestExtractBaseTypeName_QuotedIdentifiers(t *testing.T) {
    cases := []struct {
        input, want string
    }{
        {`SETOF public."ViewName"`, `public.ViewName`},
        {`"Schema"."View"`, `Schema.View`},
        {`"MyType"[]`, `MyType`},
        {`public.unquoted`, `public.unquoted`},
    }
    for _, tc := range cases {
        got := extractBaseTypeName(tc.input)
        if got != tc.want {
            t.Errorf("extractBaseTypeName(%q) = %q, want %q", tc.input, got, tc.want)
        }
    }
}

Copy link

@greptile-apps greptile-apps bot Mar 4, 2026

Choose a reason for hiding this comment

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

Quote-stripping happens before case-folding, but after array stripping — works correctly, but edge case with dot-containing quoted schema names

The approach of strings.ReplaceAll(t, "\"", "") correctly handles the primary cases (public."ViewName" → public.ViewName). However, it will silently mangle a (rare but valid) PostgreSQL identifier where the schema name itself is quoted and contains a dot — e.g., "my.schema"."view" would become my.schema.view, which typeMatchesLookup would treat as schema my + name schema.view instead of schema my.schema + name view.

In practice, schema names with dots are extremely uncommon and buildSchemaNameLookup would store the view under my.schema + view, so the lookup key would be my.schema.view — which happens to match the mangled string by coincidence in this case. However if the lookup was keyed differently this could fail.

This is a very low-probability edge case, but it's worth being aware of. The simpler alternative — stripping only quotes that wrap a single token (e.g., via a small regex like strings.Trim per identifier segment) — would be more precise, but for now the current approach covers all realistic PostgreSQL type expressions.

No code change required unless dot-containing schema names are a supported use case.

return t
}

Expand Down