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 by if 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 the EMPTY_INDEX constant 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.
Summary
DatabricksDialect.get_columns()is missing the@reflection.cachedecorator, so it never participates in SQLAlchemy's reflection cache. Every call issues a freshGetColumnsround-trip to the warehouse, however many times the same table is reflected through the sameInspector.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()andget_table_comment()— and likeget_columns()in SQLAlchemy's own SQLite, PostgreSQL and MySQL dialects, all three of which are decorated — repeated reflection of the same table through oneInspectorshould cost one round-trip, not one per call.Inspector.get_columns()explicitly threads the cache through to the dialect (sqlalchemy/engine/reflection.py):The
info_cachekwarg 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_cachekeysget_columns()[]— never populatedget_pk_constraint()[('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 withDESCRIBE TABLE EXTENDEDto 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
Inspectoracross 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. Compareget_pk_constraint()at line 212, which is decorated.Fix
Add
@reflection.cachetoget_columns().Two things worth noting for whoever picks this up, both checked:
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.