Import a warehouse catalog when a data source is built - #282
Draft
jat255 wants to merge 2 commits into
Draft
Conversation
jat255
added
needs-manual-review
jat255
force-pushed
the
jat255/xp65-source-wiring
branch
from
September 5, 2026 04:58
63dfbf7 to
01c9608
Compare
jat255
force-pushed
the
jat255/xp65-source-wiring
branch
2 times, most recently
from
September 6, 2026 06:29
8770a5a to
75a721c
Compare
A Snowflake or Databricks engine now resolves its selection against the warehouse rather than leaving the readers unused: the session identity is read first, explicitly named relations are access-checked before anything is described, the listing is folded into the authored dictionary, and the session is read again at the end. A selection defaults to the namespace the connection already points at, and exclude drops relations from it. The merge re-keys the dictionary to the warehouse's labels, so a table's definitions are now looked up under the name the author gave it rather than the label it ended up with. Definitions over a column the warehouse spells differently are refused for now, because the compiler cannot yet bind an authored name to the discovered one.
jat255
force-pushed
the
jat255/xp65-source-wiring
branch
from
September 6, 2026 19:20
75a721c to
234dae7
Compare
…ons wherever a source is built Review findings on this branch. The order the import runs in is what makes the source safe, and nothing asserted it. Three separate inversions each passed the whole suite: reading the session identity after the registry rather than before it, probing named relations after the merge rather than before, and moving the closing session re-read ahead of the merge, which is the one thing it exists to cover. Four tests now read the query log as a sequence of steps, and each inversion fails exactly one of them. Only Snowflake was ever exercised. The fake could not answer a Databricks query at all, so wiring `databricks` to the Snowflake reader passed everything. There is now a `FakeDatabricks`, and three tests drive a real Databricks import through it. Both fakes take their row shapes from `catalog-rows.json` rather than restating them, so a change to what a warehouse returns reaches them instead of leaving them answering with a shape the readers no longer expect. `from_engine()` and `from_board()` took a `dictionary` and never compiled its definitions: only `data_source()` did, so a source built through the documented constructors silently carried none, and the grain and unexposed-table checks never ran. Lowering now happens at the end of every constructor. The manifest no longer starts with the relations construction probed marked readable. Carrying that answer forward saved a round trip on first touch at the cost of serving a grant revoked in between, and the sibling implementation re-probes for the same reason. Also: `_selector()` delegates its spelling rules to `_table_entry_id()` rather than restating them; a selection emptied by `exclude`, and a table `exclude` dropped out from under its own definitions, each say so instead of reporting that nothing matched; and the four warehouse-only attributes on `DataSource` carry their real types. Docs: `from_engine()` documents `exclude`; `data_source()` no longer claims DuckDB is the only dialect with an emitter, and names the two refusals this stack added; `query()` says it can now fail because the connection identity moved; and `from_board()` says why it takes a dictionary it does not merge. Tests also cover the four `_selector` spellings and their refusals, a named relation the warehouse lacks, and the two `exclude` refusals that had none.
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
1 participant
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.
Summary
Sixth PR of the catalog import stack (kata task
xp65). Stacked on #281. Python only.Everything this catalog stack has been foundational with nothing calling it. This PR wires it in: a Snowflake or Databricks engine now imports its catalog when the source is built. The session identity is read first; relations named in
tablesare access-checked before anything is described, so a name the caller got wrong fails at construction rather than mid-conversation; the listing is folded into the authored dictionary; and the session is read again at the end, because a role that moved during discovery invalidates what was just learned.A selection defaults to the namespace the connection already points at. A namespace has to be named with a
Selector, becauseANALYTICS.PUBLICon its own does not say whetherPUBLICis a schema or a table; a plain string is read as a relation, the way it is everywhere else.excludeis accepted only for a warehouse, since nothing else has a listing to drop from.The merge re-keys the dictionary to the warehouse's labels, so a table's definitions are now looked up under the name the author gave it. Definitions over a column the warehouse spells differently are refused for now: the compiler cannot yet bind an authored name to the discovered one, and lowering them as written would emit SQL against columns that do not exist. The next PR in the stack replaces that refusal with real binding.
dictionaryis taken byfrom_engine()andfrom_board()because the merge needs it during construction, but definitions are lowered once, indata_source(). Both docstrings say so.Verification
725 Python tests, ruff and pyrefly clean. A fake backend stands in for the network; the fixture-pinned row interpretation and the real dispatch from
data_source()throughfrom_engine()are both exercised. The definitions lookup was confirmed to bite by reverting it and watching the end-to-end test fail.