You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
get_columns() missing @reflection.cache causes a warehouse round-trip on every reflection call #75
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):
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.
Activity
fix: cache get_columns() to stop a round-trip on every reflection call