Give TableId a catalog component - #274
Merged
Merged
Conversation
jat255
marked this pull request as draft
September 4, 2026 05:49
jat255
added
needs-manual-review
jat255
force-pushed
the
jat255/xp65-catalog-core
branch
from
September 5, 2026 04:58
606312e to
c335b87
Compare
jat255
force-pushed
the
jat255/xp65-catalog-core
branch
from
September 6, 2026 02:07
c335b87 to
82a8ff9
Compare
jat255
marked this pull request as ready for review
September 6, 2026 05:01
jat255
removed
the
needs-manual-review
Collaborator
Author
A warehouse names a table catalog.schema.table, and TableId had two levels, so everything before the last dot became the schema. `ANALYTICS.PUBLIC.ORDERS` quoted to "ANALYTICS.PUBLIC"."ORDERS": one identifier containing a dot, naming a schema no warehouse has. That is a live bug for engine sources today, not only a gap ahead of catalog import. Three levels now, with the components kept apart rather than folded into one string. `parts` is what quoting and probing walk, so neither has to know how many levels a backend has. A catalog without a schema is refused, since there is no level between them to leave out, and a name with more than three parts is refused rather than guessed at. The engine inspector joins the levels above the table into one dotted `schema`, which is SQLAlchemy's convention, instead of passing only the schema and silently probing the wrong namespace. No dialect available to the suite has three levels, so that argument is asserted by recording it. First of the catalog import stack (kata xp65).
The quoting test covered DuckDBBackend, which has its own implementation, so the changed SQLAlchemy preparer path was untested. Confirmed to bite by folding the outer components back into one quote_schema() call. Found by roborev job 328.
Closes #302. R's string parser folded everything before the last dot into the schema, so "ANALYTICS.PUBLIC.ORDERS" quoted as "ANALYTICS.PUBLIC"."ORDERS": one identifier with a dot inside it, naming a schema no warehouse has. The fold predates catalog support (97db857); the Databricks/Snowflake work added catalogs through DBI::Id and never revisited string parsing. Three parts now split into catalog, schema, and table, and more than three are refused, matching the Python side. strsplit() drops a trailing empty piece, so a trailing dot is checked separately; the shared fixture caught R accepting "orders." as a bare name. The parsing rules are user-observable in both languages, so they move into tests/shared/table-names.json with a runner in each suite, and the per-language copies added earlier in this stack come out.
jat255
force-pushed
the
jat255/xp65-catalog-core
branch
from
September 6, 2026 05:36
4e3f039 to
c1b0251
Compare
|
Preview deployed to Connect ( Deployed from commit c1b0251. |
|
Preview deployed to Connect ( Deployed from commit c1b0251. |
simonpcouch
approved these changes
Sep 6, 2026
|
Cleaned up 1 preview bundle(s) on https://dogfood.team.pct.posit.it: 368171 |
|
Cleaned up 1 preview bundle(s) on https://connect.staging.pct.posit.it: 2641 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First PR of the catalog import stack (kata xp65). Stacked on #273.
Summary
A warehouse names a table
catalog.schema.table, andTableIdhad two levels, so everything before the last dot became the schema.ANALYTICS.PUBLIC.ORDERSquoted to"ANALYTICS.PUBLIC"."ORDERS": one identifier with a dot inside it, naming a schema no warehouse has. That is a live bug for engine sources today, not only a gap ahead of catalog import, which is why it is a fix rather than a feature.Three levels now, kept apart rather than folded into a string.
partsis what quoting and probing walk, so neither has to know how many levels a backend has.Two inputs are now refused rather than accepted. A
TableIdwith a catalog but no schema, because there is no level between them to leave out. And a name with more than three parts, which previously became a schema containing dots.The engine inspector joins the levels above the table into one dotted
schema, which is SQLAlchemy's convention, rather than passing the schema alone and probing the wrong namespace. No dialect available to the suite has three levels, so that argument is asserted by recording it rather than by a real probe, which is the one piece of test-double here and the comment says why.R Changes
@simonpcouch there are some R behavior changes in this PR, to address an issue with how catalogs are handled on the R side, filed as #302 and fixed here. R's string parser now reads three parts as
catalog.schema.tableand refuses more than three, matching the Python behavior above. The shared fixture caught R'sstrsplit()silently dropping a trailing empty piece, so"orders."parsed as a bare name; a trailing dot is now checked separately.The parsing rules are user-observable in both languages, so they live in
tests/shared/table-names.jsonwith a runner in each suite, replacing the per-language copies added earlier in this stack.