Skip to content

Commit 38bd0f6

Browse files
will-sargent-dbtlabsclaudetauhid621
authored
fix(redshift): describe temp relations from the driver when the catalog cannot see them (#2097)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Tauhid Anjum <tauhidanjum@gmail.com>
1 parent c9160c9 commit 38bd0f6

10 files changed

Lines changed: 791 additions & 1 deletion

File tree

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
kind: Fixes
2+
body: Describe temporary relations from the driver when the connection targets a datashare
3+
consumer database, where they are invisible to all catalog views. Previously this made
4+
on_schema_change='sync_all_columns' drop every column in the target table.
5+
time: 2026-07-28T12:00:03.029795-07:00
6+
custom:
7+
Author: will-sargent-dbtlabs
8+
Issue: "1947 1991"

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

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,46 @@
4242
GET_RELATIONS_MACRO_NAME = "redshift__get_relations"
4343
SHOW_TABLES_FROM_SCHEMA_MACRO_NAME = "redshift__show_tables_from_schema"
4444

45+
# Redshift type OID -> SQL data type name, as reported by both the legacy path's
46+
# information_schema.columns.data_type and SHOW COLUMNS. Used to describe temp relations
47+
# from the driver's cursor description (see get_columns_in_temp_relation); the names have
48+
# to match whichever of those two paths described the target relation, or schema comparison
49+
# reports a type change on every run. Verified against SHOW COLUMNS on Redshift 1.0.358853
50+
# -- note bpchar reports as "character", and varbyte as "binary varying".
51+
TYPE_OID_TO_DATA_TYPE: Dict[int, str] = {
52+
16: "boolean", # bool
53+
20: "bigint", # int8
54+
21: "smallint", # int2
55+
23: "integer", # int4
56+
25: "character varying", # text
57+
700: "real", # float4
58+
701: "double precision", # float8
59+
1042: "character", # bpchar
60+
1043: "character varying", # varchar
61+
1082: "date", # date
62+
1083: "time without time zone", # time
63+
1114: "timestamp without time zone", # timestamp
64+
1184: "timestamp with time zone", # timestamptz
65+
1188: "interval year to month", # intervaly2m
66+
1190: "interval day to second", # intervald2s
67+
1266: "time with time zone", # timetz
68+
1700: "numeric", # numeric
69+
2935: "hllsketch", # hllsketch
70+
3000: "geometry", # geometry
71+
3001: "geography", # geography
72+
4000: "super", # super
73+
6551: "binary varying", # varbyte
74+
}
75+
76+
# information_schema reports its internal type name where SHOW COLUMNS reports the SQL name,
77+
# so a fallback column only compares equal if it follows whichever path described the target.
78+
# Everything else in the map above agrees across both; these are the exceptions, verified in
79+
# tests/functional/test_type_oid_mapping.py.
80+
TYPE_OID_TO_INFORMATION_SCHEMA_DATA_TYPE: Dict[int, str] = {
81+
1188: "intervaly2m", # SHOW COLUMNS: interval year to month
82+
1190: "intervald2s", # SHOW COLUMNS: interval day to second
83+
}
84+
4585
REDSHIFT_SKIP_AUTOCOMMIT_TRANSACTION_STATEMENTS = BehaviorFlag(
4686
name="redshift_skip_autocommit_transaction_statements",
4787
default=False,
@@ -199,6 +239,111 @@ def use_show_apis(self) -> bool:
199239
"""
200240
return bool(self.config.credentials.datasharing)
201241

242+
@available
243+
def get_columns_in_temp_relation(self, relation: BaseRelation) -> List[Any]:
244+
"""Describe a temporary relation from the driver instead of the catalog.
245+
246+
Needed when the connection's database is a datashare consumer database: temp
247+
relations stay queryable but are invisible to ``information_schema.columns``,
248+
``pg_attribute`` and ``svv_columns`` alike, and ``SHOW COLUMNS`` cannot address
249+
them because they carry no database or schema.
250+
251+
Sizes come from ``cursor.ps["row_desc"]``'s ``type_modifier``, not from
252+
``cursor.description``: ``redshift_connector`` hardcodes the PEP 249
253+
size/precision/scale fields to ``None`` (see its ``Cursor._getDescription``).
254+
Without real sizes, every sized column looks changed against the target on the
255+
next run and gets needlessly rewritten.
256+
"""
257+
sql = f"select * from {self.quote(relation.identifier)} limit 0"
258+
_, cursor = self.connections.add_select_query(sql)
259+
260+
# `cursor.ps["row_desc"]` is an internal, undocumented structure of
261+
# `redshift_connector`, not a public API -- there is no supported alternative for
262+
# reaching `type_modifier`. `_getDescription` builds `cursor.description` from this
263+
# same list, in the same order, so index-aligned lookup here is safe.
264+
try:
265+
row_descriptions = cursor.ps["row_desc"]
266+
except (AttributeError, KeyError, TypeError):
267+
row_descriptions = []
268+
269+
columns = []
270+
for i, description in enumerate(cursor.description or []):
271+
# PEP 249: (name, type_code, display_size, internal_size, precision, scale, null_ok)
272+
column_name = description[0]
273+
type_code = description[1]
274+
275+
type_modifier = None
276+
if i < len(row_descriptions):
277+
type_modifier = row_descriptions[i].get("type_modifier")
278+
279+
data_type = self._temp_relation_data_type(type_code)
280+
281+
# Only string and exact-numeric types feed size into Column.data_type, which is
282+
# what schema comparison comes down to. For every other type the size fields are
283+
# ignored, so leave them unset.
284+
char_size = None
285+
numeric_precision = None
286+
numeric_scale = None
287+
if type_modifier is not None and type_modifier >= 0:
288+
# Postgres wire-protocol convention: VARHDRSZ (4 bytes) is added to both a
289+
# string's declared length and a numeric's packed (precision, scale) pair.
290+
# -1 means "no modifier" (unconstrained/default size) and is left unset.
291+
raw_modifier = type_modifier - 4
292+
if data_type in ("character varying", "character"):
293+
char_size = raw_modifier if raw_modifier > 0 else None
294+
elif data_type == "numeric":
295+
numeric_precision = raw_modifier >> 16
296+
numeric_scale = raw_modifier & 0xFFFF
297+
298+
columns.append(
299+
self.Column(
300+
column=column_name,
301+
dtype=data_type,
302+
char_size=char_size,
303+
numeric_precision=numeric_precision,
304+
numeric_scale=numeric_scale,
305+
)
306+
)
307+
308+
return columns
309+
310+
def _temp_relation_data_type(self, type_code: Any) -> str:
311+
"""Map a driver type code onto the name the target relation's describer reports.
312+
313+
The target goes through SHOW COLUMNS when ``datasharing`` is on and through
314+
information_schema when it is off, and the two disagree on a few type names.
315+
"""
316+
if not self.use_show_apis():
317+
data_type = TYPE_OID_TO_INFORMATION_SCHEMA_DATA_TYPE.get(type_code)
318+
if data_type is not None:
319+
return data_type
320+
321+
data_type = TYPE_OID_TO_DATA_TYPE.get(type_code)
322+
if data_type is not None:
323+
return data_type
324+
325+
# Unmapped OID. The driver's own label is closer than nothing, but its lookup is an
326+
# IntEnum call that raises for codes it doesn't recognise either -- and a column
327+
# whose type cannot be named at all is better reported than described with an
328+
# invented name, which would only fail later as invalid DDL.
329+
try:
330+
fallback = str(self.connections.data_type_code_to_name(type_code)).lower()
331+
except Exception as exc:
332+
raise dbt_common.exceptions.DbtRuntimeError(
333+
f"Cannot describe temporary relation column: Redshift returned type code "
334+
f"{type_code}, which neither dbt-redshift nor redshift_connector "
335+
f"recognises. Please report this at "
336+
f"https://github.com/dbt-labs/dbt-adapters/issues"
337+
) from exc
338+
339+
logger.debug(
340+
f"No known Redshift data type for type code {type_code}; "
341+
f"falling back to {fallback!r}. Schema comparison for this column may be "
342+
f"inaccurate -- please report this at "
343+
f"https://github.com/dbt-labs/dbt-adapters/issues"
344+
)
345+
return fallback
346+
202347
@available
203348
def drop_without_cascade(self) -> bool:
204349
"""Whether to omit CASCADE from DROP statements.

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,24 @@
130130
{% if redshift__use_show_apis() and relation.database and relation.schema %}
131131
{{ return(redshift__get_columns_in_relation_show(relation)) }}
132132
{% else %}
133-
{{ return(redshift__get_columns_in_relation_legacy(relation)) }}
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) }}
134151
{% endif %}
135152
{% endmacro %}
136153

dbt-redshift/tests/functional/adapter/datashare_consumer/__init__.py

Whitespace-only changes.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import pytest
2+
3+
from tests.functional.adapter.datashare_consumer.fixtures import (
4+
REDSHIFT_TEST_DATASHARE_DBNAME,
5+
)
6+
7+
8+
@pytest.fixture(autouse=True, scope="session")
9+
def _skip_without_datashare_db():
10+
"""Skip every test in this directory when the env var is not set."""
11+
if not REDSHIFT_TEST_DATASHARE_DBNAME:
12+
pytest.skip("REDSHIFT_TEST_DATASHARE_DBNAME not set")
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import os
2+
3+
import pytest
4+
5+
# Name of a database created FROM A DATASHARE on the cluster under test, i.e. one that
6+
# svv_redshift_databases reports with database_type = 'shared'. The connection's dbname is
7+
# pointed directly at it, which is the configuration these tests exist to cover.
8+
#
9+
# Setting one up (Redshift Serverless, two namespaces):
10+
#
11+
# for role in producer consumer; do
12+
# aws redshift-serverless create-namespace --namespace-name rs-$role \
13+
# --admin-username admin --admin-user-password '<password>' --db-name dev
14+
# aws redshift-serverless create-workgroup --workgroup-name rs-$role-wg \
15+
# --namespace-name rs-$role --base-capacity 8 --publicly-accessible
16+
# done
17+
#
18+
# Both workgroups need inbound 5439 from the test runner. The namespace GUIDs below come from
19+
# `get-namespace --query namespace.namespaceId`. Then:
20+
#
21+
# -- producer namespace, database `dev`
22+
# create schema shared_sch;
23+
# create table shared_sch.orders (id int);
24+
# create datashare repro_share;
25+
# alter datashare repro_share add schema shared_sch;
26+
# alter datashare repro_share add table shared_sch.orders;
27+
# grant usage on datashare repro_share to namespace '<consumer-namespace-guid>';
28+
#
29+
# -- consumer namespace
30+
# create database ds_consumer from datashare repro_share of namespace '<producer-guid>';
31+
#
32+
# then set REDSHIFT_TEST_DATASHARE_DBNAME=ds_consumer and point the test profile's host at
33+
# the consumer workgroup.
34+
REDSHIFT_TEST_DATASHARE_DBNAME = os.getenv("REDSHIFT_TEST_DATASHARE_DBNAME", "")
35+
36+
37+
class DatashareConsumerMixin:
38+
"""Connect directly to a datashare consumer database, with datasharing enabled.
39+
40+
This is deliberately different from CrossDatabaseMixin, which leaves the connection on
41+
the default database and only retargets models via `+database`. That arrangement works;
42+
connecting straight at the consumer database is the one that breaks, because temporary
43+
relations are then invisible to every catalog view.
44+
"""
45+
46+
@pytest.fixture(scope="class")
47+
def profiles_config_update(self, dbt_profile_target, unique_schema):
48+
return {
49+
"test": {
50+
"outputs": {
51+
"default": {
52+
**dbt_profile_target,
53+
"schema": unique_schema,
54+
"dbname": REDSHIFT_TEST_DATASHARE_DBNAME,
55+
"datasharing": True,
56+
}
57+
},
58+
"target": "default",
59+
}
60+
}
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
"""
2+
Incremental on_schema_change against a connection pointed directly at a datashare consumer
3+
database -- the configuration that reproduces dbt-labs/dbt-adapters#1947 and #1991.
4+
5+
The existing coverage passes because it never exercises this arrangement:
6+
7+
| test | connection dbname | model database |
8+
|--------------------------------------------|-------------------|-----------------------|
9+
| TestIncrementalOnSchemaChangeWithDatasharing | default | same as connection |
10+
| TestIncrementalCrossDatabase* | default | cross-db via +database|
11+
| these tests | consumer database | same as connection |
12+
13+
In the last row, temporary relations are created and stay queryable but are absent from
14+
information_schema.columns, pg_attribute and svv_columns, so column introspection returns
15+
nothing. Before the driver-based fallback, sync_all_columns read that as "the source has no
16+
columns" and dropped every column in the target.
17+
"""
18+
19+
from dbt.tests.adapter.incremental.test_incremental_on_schema_change import (
20+
BaseIncrementalOnSchemaChange,
21+
)
22+
from dbt.tests.util import get_connection, run_dbt
23+
import pytest
24+
25+
from tests.functional.adapter.datashare_consumer.fixtures import DatashareConsumerMixin
26+
27+
28+
class TestIncrementalOnSchemaChangeDatashareConsumer(
29+
DatashareConsumerMixin, BaseIncrementalOnSchemaChange
30+
):
31+
"""The full on_schema_change suite, run against a datashare consumer database."""
32+
33+
34+
_MODEL_SYNC_ALL_COLUMNS = """
35+
{{
36+
config(
37+
materialized='incremental',
38+
unique_key='id',
39+
on_schema_change='sync_all_columns'
40+
)
41+
}}
42+
43+
with source_data as (
44+
select 1 as id, 'aaa' as field_1, 2.5::numeric(18,2) as field_2, 'x'::char(5) as field_3
45+
union all select 2 as id, 'bbb' as field_1, 3.5::numeric(18,2) as field_2, 'y'::char(5) as field_3
46+
)
47+
48+
select * from source_data
49+
50+
{% if is_incremental() %}
51+
where id not in (select id from {{ this }})
52+
{% endif %}
53+
"""
54+
55+
56+
class TestIncrementalTempRelationColumnsPreserved(DatashareConsumerMixin):
57+
"""Targeted regression test: an incremental run must not drop the target's columns.
58+
59+
Also guards the type mapping: because the source relation is described from the driver
60+
and the target from SHOW COLUMNS, any disagreement in reported data types shows up as a
61+
perpetual schema change. Running twice and asserting a stable column set catches that --
62+
char and numeric columns are included specifically because they are the ones whose
63+
reported type carries a size.
64+
"""
65+
66+
@pytest.fixture(scope="class")
67+
def models(self):
68+
return {"incremental_sync_all_columns.sql": _MODEL_SYNC_ALL_COLUMNS}
69+
70+
def _columns(self, project):
71+
relation = project.adapter.Relation.create(
72+
database=project.database,
73+
schema=project.test_schema,
74+
identifier="incremental_sync_all_columns",
75+
)
76+
with get_connection(project.adapter):
77+
return project.adapter.get_columns_in_relation(relation)
78+
79+
def test_columns_survive_incremental_run(self, project):
80+
run_dbt(["run", "--select", "incremental_sync_all_columns"])
81+
before = self._columns(project)
82+
assert [c.name for c in before] == ["id", "field_1", "field_2", "field_3"]
83+
84+
run_dbt(["run", "--select", "incremental_sync_all_columns"])
85+
after = self._columns(project)
86+
87+
assert [c.name for c in after] == ["id", "field_1", "field_2", "field_3"]
88+
# Data types must be stable too, otherwise the driver-described source and the
89+
# SHOW COLUMNS-described target disagree and dbt alters the column type every run.
90+
assert [c.dtype for c in after] == [c.dtype for c in before]
91+
92+
def test_third_run_is_still_stable(self, project):
93+
"""A type-mapping mismatch would keep re-triggering; a third run pins that down."""
94+
run_dbt(["run", "--select", "incremental_sync_all_columns"])
95+
run_dbt(["run", "--select", "incremental_sync_all_columns"])
96+
first = self._columns(project)
97+
run_dbt(["run", "--select", "incremental_sync_all_columns"])
98+
assert [(c.name, c.dtype) for c in self._columns(project)] == [
99+
(c.name, c.dtype) for c in first
100+
]

0 commit comments

Comments
 (0)