get_columns() missing @reflection.cache causes a warehouse round-trip on every reflection call
Nobody has claimed this yet.
Assessment
- Difficulty
- 1/5
- Estimated time
- Under an hour
- Newbie friendliness
- 92/100
Research direction
Start in src/databricks/sqlalchemy/base.py at get_columns(), then compare its reflection handling with get_pk_constraint() and the upstream dialects mentioned in the issue. Exercise repeated calls with the same info_cache and verify that the cache is populated and the warehouse round-trip occurs only once.
Written by the indexing model from the issue text.
Description
Summary
DatabricksDialect.get_columns() is missing the @reflection.cache decorator, so it never participates in SQLAlchemy's reflection cache. Every call issues a fresh GetColumns round-trip to the warehouse, however many times the same table is reflected through the same Inspector.
This is the sibling of #72 (get_foreign_keys()), which is addressed in #74. get_columns() is a separate occurrence of the same omission and is not covered by that PR.
Expected behaviour
Like get_pk_constraint(), has_table(), get_table_names(), get_view_names(), get_materialized_view_names(), get_temp_view_names(), get_schema_names() and get_table_comment() — and like get_columns() in SQLAlchemy's own SQLite, PostgreSQL and MySQL dialects, all three of which are decorated — repeated reflection of the same table through one Inspector should cost one round-trip, not one per call.
Inspector.get_columns() explicitly threads the cache through to the dialect (sqlalchemy/engine/reflection.py):
with self._operation_context() as conn:
col_defs = self.dialect.get_columns(
conn, table_name, schema, info_cache=self.info_cache, **kw
)
The info_cache kwarg arrives, lands in **kwargs, and is discarded.
Actual behaviour
Reproduced against main (SQLAlchemy 2.0.52), stubbing out the transport so the call count is directly observable:
info_cache = {}
for _ in range(3):
dialect.get_columns(conn, "t", None, info_cache=info_cache)
| method | calls | server round-trips | info_cache keys |
|---|---|---|---|
get_columns() |
3 | 3 | [] — never populated |
get_pk_constraint() |
3 | 1 | [('get_pk_constraint', ('t',), (('schema', None),))] |
Note the cost is not always a single statement: when cur.columns() returns an empty list, get_columns() follows up with DESCRIBE TABLE EXTENDED to distinguish a genuinely column-less table from a missing one (base.py:156). For such tables an uncached call is two round-trips, repeated every time.
Callers that reuse one Inspector across many lookups feel this directly — Alembic's autogenerate is the common case, as is any long-lived application that reflects per request.
Root cause
src/databricks/sqlalchemy/base.py:139 — get_columns() lacks @reflection.cache. Compare get_pk_constraint() at line 212, which is decorated.
Fix
Add @reflection.cache to get_columns().
Two things worth noting for whoever picks this up, both checked:
- Caching is safe with respect to
Inspector._instantiate_types(), which mutates the returned column dicts in place. It is guarded byif not isinstance(coltype, TypeEngine), so re-running it over an already-instantiated cached list is a no-op. This is the same situation the upstream dialects are in. get_indexes()is also undecorated but returns theEMPTY_INDEXconstant without touching the server, so it needs no cache.get_columns()is the only remaining method where the omission costs a round-trip.
I'm happy to open a PR for this if it's welcome.
- Dominant language
- Python
- Stars
- 24
- Forks
- 18
- PR merge metrics
- No merged PRs in 30d
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from databricks/databricks-sqlalchemy
-
Difficulty 1/5 Under an hour Newbie friendliness 90/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
-
Difficulty 4/5 3-5 days Newbie friendliness 48/100
databricks/databricks-sqlalchemy#73 · 1 comment ·
-
Difficulty 5/5 Over a week Newbie friendliness 35/100
-
Difficulty 5/5 Over a week Newbie friendliness 30/100
All issues in databricks/databricks-sqlalchemy
Similar issues
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
use-agent-os/agent-os#3314 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
BasedHardware/omi#15662 · 1 comment ·
-
documentation help wanted
Difficulty 2/5 1-3 hours Newbie friendliness 90/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
AiursoftWeb/AnduinOS-2#19 ·