Fold schema-qualified table names to bare identifiers in alias position

[phdoerfler]

Note: The current check failure is due to the usual Your tlBaseVersion 0.28 is behind the latest tag 0.29.0.

As with #867, I put Claude to this one. Same caveat applies — I've reviewed what I could but I'm not deep enough into the aliasing internals to independently vouch for correctness. That said, I have prodded and poked the AI whenever I saw something potentially suspicious such as the string manipulation code until I was satisfied. Ultimately, the fix is small, targeted, and every claim below is backed by a reproducible test. Here's the PR text Claude proposed:

---

Fixes #342.

Symptom

A mapping over a schema-qualified table name (e.g. public.country) that participates in a recursive relationship (Country → City → Country) fails with a cryptic Postgres error:

syntax error at or near "."

Root cause

Grackle treats a table's SQL name as a single opaque string used in three different roles: the SQL reference (public.country, correctly dotted), the alias it's given when a query revisits the table, and a component of synthesized subquery names. Alias and synthesized-name positions both require a bare identifier — a dot is illegal there. When a table name is schema-qualified, both positions render the qualifier verbatim, e.g.:

INNER JOIN public.country AS public.country_alias_1

which Postgres rejects. Four sites in SqlMapping.scala seed one of these positions with the raw qualified name: the alias mint, syntheticName (subquery names for shapes that can't merge, e.g. limits), the union path's subquery naming, the associative (_assoc) derived-table alias, and MSSQL's encapsulateUnionBranch sibling.

Fix

A shared TableName.asIdentifier helper folds a qualifier's dots to underscores, used at all five sites. For unqualified names the fold is the identity, so existing mappings are unaffected. Derived names (_assoc, _base, _encaps) inherit sanitized inputs since they're built from already-folded names.

Collision safety

Folding qualifiers raises an obvious question: can qualified.country (folded to qualified_country) now collide with an unrelated real table actually named qualified_country? No — AliasState's alias mint deduplicates against every name it has already seen at render time, regardless of whether that name came from a real table or a folded synthesized one. A dedicated test joins both qualified.country (recursively aliased) and a real qualified_country table into the same statement and asserts it renders correctly.

One residual, pre-existing edge case: a table name containing a literal dot inside a quoted identifier (not a schema separator) was already broken before this change and remains out of scope.

Tests

Five tests over a new SqlQualifiedNamesMapping/SqlQualifiedNamesSuite (testdata/pg/qualified-names.sql), wired into both doobie-pg and skunk:

• The reported reproducer (recursive Country → City → Country), watched fail with the exact PSQLException before the fix.
• A top-level limit with a child join, and a nested limit — both exercise syntheticName, which the first fix alone didn't cover.
• An associative-field query, exercising the _assoc derived-table alias.
• The collision-safety coexistence pin described above.

MSSQL's encapsulateUnionBranch sibling was fixed by inspection (same pattern, no MSSQL-specific test infrastructure in this suite) — flagging for review attention since it's the one change not directly test-covered. Oracle and MSSQL were deliberately not wired into the shared suite itself; their schema models differ enough from Postgres's that the fixture wouldn't transfer directly (explained in a suite comment).

All green: doobiepg 249/249, skunkJVM 250/250, on both Scala 2.13 and 3.3.7.

Design note

The deeper fix would be a structured TableName(schema, name) instead of one opaque string doing three jobs — that would make "which role needs the bare identifier" a type-level question instead of a per-call-site one. I judged that out of scope for a bug report this size; flagging it as a possible follow-up if maintainers agree the pattern is worth generalizing.

[milessabin]

I think the "deeper fix" described in the design note would be the right way to go.

[phdoerfler]

Fair enough. I'll just close this PR then. I figured it might be worth having this superficial fix in the meantime so the user does not get an unexpected 500 but I do also think a more thorough fix is the way to go ultimately.

[phdoerfler]

nvm my last comment. I confused something. Yes, I also think an own TableName type would be great and I think it should be a reasonably sized change.

[milessabin]

Aside from a compilation warning ... LGTM!