Skip to content

Commit 5e830c7

Browse files
tauhid621claude
andcommitted
refactor(redshift): move temp relation fallback into get_columns_in_relation
Follow-up to #2097. That PR reached the driver-based temp relation fallback through a new @available method called from redshift__get_columns_in_relation. A new adapter-specific method has no record type behind it, so the conformance harness cannot record or replay it. Move the dispatch into an override of get_columns_in_relation instead. @supports_replay re-applies AdapterGetColumnsInRelationRecord to subclass overrides, so the fallback gets record/replay coverage without a new record type, and porting to v2 is an override of an existing interface method rather than a Redshift-only addition. get_columns_in_temp_relation keeps its name but loses @available: it is now reached only from get_columns_in_relation, which is where the call is recorded. Behaviour is unchanged -- the SHOW branch already required a qualified relation, so unqualified relations took the legacy branch either way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 38bd0f6 commit 5e830c7

3 files changed

Lines changed: 68 additions & 20 deletions

File tree

dbt-redshift/src/dbt/adapters/redshift/impl.py

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from collections import namedtuple
1212
from dbt.adapters.base import PythonJobHelper
1313
from dbt.adapters.base.impl import AdapterConfig, ConstraintSupport, FreshnessResponse
14+
from dbt.adapters.base.column import Column as BaseColumn
1415
from dbt.adapters.base.meta import available
1516
from dbt.adapters.base.relation import BaseRelation
1617
from dbt.adapters.capability import (
@@ -239,8 +240,26 @@ def use_show_apis(self) -> bool:
239240
"""
240241
return bool(self.config.credentials.datasharing)
241242

242-
@available
243-
def get_columns_in_temp_relation(self, relation: BaseRelation) -> List[Any]:
243+
def get_columns_in_relation(self, relation: BaseRelation) -> List[BaseColumn]:
244+
"""Describe a relation, falling back to the driver when the catalog cannot see it.
245+
246+
On a datashare consumer database, temp relations stay queryable but are absent from
247+
``information_schema.columns``, ``pg_attribute`` and ``svv_columns``; the empty
248+
result makes ``on_schema_change='sync_all_columns'`` drop every column in the
249+
target. Only unqualified relations can take the fallback, since the driver query
250+
has no database or schema to qualify itself with.
251+
252+
In an override rather than in the macro so that ``@supports_replay`` records the
253+
fallback under the existing ``AdapterGetColumnsInRelationRecord``.
254+
"""
255+
columns = super().get_columns_in_relation(relation)
256+
257+
if not columns and not relation.database and not relation.schema:
258+
return self.get_columns_in_temp_relation(relation)
259+
260+
return columns
261+
262+
def get_columns_in_temp_relation(self, relation: BaseRelation) -> List[BaseColumn]:
244263
"""Describe a temporary relation from the driver instead of the catalog.
245264
246265
Needed when the connection's database is a datashare consumer database: temp

dbt-redshift/src/dbt/include/redshift/macros/adapters.sql

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -130,24 +130,7 @@
130130
{% if redshift__use_show_apis() and relation.database and relation.schema %}
131131
{{ return(redshift__get_columns_in_relation_show(relation)) }}
132132
{% else %}
133-
{%- set columns = redshift__get_columns_in_relation_legacy(relation) -%}
134-
135-
{#-
136-
A temp relation with no columns means the catalog could not see it, not that it has
137-
none: when the connection's database is a datashare consumer database, temp relations
138-
stay queryable but are absent from information_schema.columns, pg_attribute and
139-
svv_columns. Left unhandled, an empty list makes on_schema_change='sync_all_columns'
140-
treat every column in the target as removed and drop it.
141-
142-
Only temp relations are affected, and only they can be described this way: the driver
143-
query is unqualified, so it resolves to the right relation only when there is no
144-
database or schema to qualify it with.
145-
-#}
146-
{%- if columns | length == 0 and not relation.database and not relation.schema -%}
147-
{{ return(adapter.get_columns_in_temp_relation(relation)) }}
148-
{%- endif -%}
149-
150-
{{ return(columns) }}
133+
{{ return(redshift__get_columns_in_relation_legacy(relation)) }}
151134
{% endif %}
152135
{% endmacro %}
153136

dbt-redshift/tests/unit/test_temp_relation_columns.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import pytest
1919
from dbt_common.exceptions import DbtRuntimeError
2020

21+
from dbt.adapters.sql import SQLAdapter
2122
from dbt.adapters.redshift.impl import (
2223
TYPE_OID_TO_DATA_TYPE,
2324
TYPE_OID_TO_INFORMATION_SCHEMA_DATA_TYPE,
@@ -140,6 +141,51 @@ def quote(identifier):
140141
_temp_relation_data_type = RedshiftAdapter._temp_relation_data_type
141142

142143

144+
class TestGetColumnsInRelationFallback:
145+
"""When the catalog result falls back to the driver, and when it must not."""
146+
147+
def _adapter(self):
148+
# Bypass __init__: the override needs only a real RedshiftAdapter for zero-arg super().
149+
adapter = RedshiftAdapter.__new__(RedshiftAdapter)
150+
adapter.get_columns_in_temp_relation = mock.Mock(return_value=["from_driver"])
151+
return adapter
152+
153+
def test_catalog_result_is_returned_without_touching_the_driver(self):
154+
adapter = self._adapter()
155+
relation = mock.Mock(database=None, schema=None, identifier="model__dbt_tmp123")
156+
157+
with mock.patch.object(
158+
SQLAdapter, "get_columns_in_relation", return_value=["from_catalog"]
159+
):
160+
assert RedshiftAdapter.get_columns_in_relation(adapter, relation) == ["from_catalog"]
161+
162+
adapter.get_columns_in_temp_relation.assert_not_called()
163+
164+
def test_invisible_temp_relation_falls_back_to_the_driver(self):
165+
adapter = self._adapter()
166+
relation = mock.Mock(database=None, schema=None, identifier="model__dbt_tmp123")
167+
168+
with mock.patch.object(SQLAdapter, "get_columns_in_relation", return_value=[]):
169+
assert RedshiftAdapter.get_columns_in_relation(adapter, relation) == ["from_driver"]
170+
171+
adapter.get_columns_in_temp_relation.assert_called_once_with(relation)
172+
173+
def test_qualified_relation_with_no_columns_does_not_fall_back(self):
174+
# The driver query is unqualified, so it would resolve to the wrong relation.
175+
adapter = self._adapter()
176+
relation = mock.Mock(database="db", schema="sch", identifier="my_model")
177+
178+
with mock.patch.object(SQLAdapter, "get_columns_in_relation", return_value=[]):
179+
assert RedshiftAdapter.get_columns_in_relation(adapter, relation) == []
180+
181+
adapter.get_columns_in_temp_relation.assert_not_called()
182+
183+
def test_driver_fallback_is_not_exposed_to_jinja(self):
184+
# @available here would put a method with no record type back in the Jinja context.
185+
assert "get_columns_in_temp_relation" not in RedshiftAdapter._available_
186+
assert "get_columns_in_relation" in RedshiftAdapter._available_
187+
188+
143189
class TestGetColumnsInTempRelation:
144190
def _adapter_with_columns(self, columns):
145191
adapter = _StubAdapter(columns)

0 commit comments

Comments
 (0)