Skip to content

Commit 352d0a9

Browse files
authored
Merge pull request #702 from dbt-msft/fix/missing-type-handling
feat: safe type expansion, NVARCHAR/NCHAR catalog fix, seed empty cell fix
2 parents 3768fea + 6fd1c2e commit 352d0a9

18 files changed

Lines changed: 1360 additions & 50 deletions

File tree

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,19 @@
22

33
### v1.10.1
44

5+
#### Features
6+
7+
- Add `dbt_sqlserver_enable_safe_type_expansion` behaviour flag to allow safe column type widening during schema expansion: `varchar``nvarchar`, integer family promotions (`bit``tinyint``smallint``int``bigint`), and `numeric`/`decimal` precision/scale upgrades. Gated by the per-model `column_type_expansion_max_rows` config (default 1,000,000 rows). See [#699](https://github.com/dbt-msft/dbt-sqlserver/issues/699).
8+
- Add `prefer_single_alter_column` model config to use a single `ALTER COLUMN` statement instead of the add+update+drop+rename pattern when altering column types on tables.
9+
- Add `string_type_instance()` to preserve the NVARCHAR/NCHAR type family during column expansion, fixing incorrect promotion of NVARCHAR/NCHAR to VARCHAR.
10+
- Add `tinyint` and `bit` to the `is_integer()` type list for correct type detection.
11+
512
#### Bugfixes
613

714
- Fix unit tests with empty fixtures (`rows: []`) generating invalid `limit 0` syntax; emit `top 0` instead. Also fix `get_columns_in_query()` for queries starting with a CTE, which broke unit tests with an empty `expect` block; such queries are now described via `sp_describe_first_result_set` instead of being executed. [#698](https://github.com/dbt-msft/dbt-sqlserver/issues/698)
15+
- Fix catalog generation for NVARCHAR/NCHAR columns: use `user_type_id` instead of `system_type_id` in catalog.sql, preventing them from appearing as `SYSNAME` in `dbt docs`. [#637](https://github.com/dbt-msft/dbt-sqlserver/issues/637)
16+
- Fix `varchar(max)` / `nvarchar(max)` columns being incorrectly treated as size `-1` during type expansion, preventing `varchar(max)``varchar(100)` narrowing and properly allowing `varchar(100)``varchar(max)` expansion.
17+
- Fix seed table ingestion of empty numeric cells by inlining `null` literals instead of binding parameters. [#425](https://github.com/dbt-msft/dbt-sqlserver/issues/425)
818

919
#### Features
1020

README.md

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,18 @@ The same setting is also honoured via `vars:` for backwards compatibility; the b
129129

130130
*(default: `pyodbc`)* Set to `mssql-python` in a profile target to use the `mssql-python` backend instead of `pyodbc`. The adapter fails if the required backend package (Python dependency), such as `pyodbc` or `mssql-python`, is not installed.
131131

132+
### `dbt_sqlserver_enable_safe_type_expansion`
133+
134+
*(default: `false`)* When enabled, allows the adapter to widen column types during incremental model schema expansion beyond same-family string resizes. Supported safe expansions include:
135+
136+
- **Cross-family string**: `varchar`/`char` → `nvarchar`/`nchar` (same or larger size)
137+
- **Integer family**: `bit` → `tinyint` → `smallint` → `int` → `bigint`
138+
- **Integer → numeric**: `int` → `numeric` (with sufficient precision to hold the integer range)
139+
- **Numeric precision/scale**: `numeric(p,s)` → `numeric(p2,s2)` where precision and scale both increase
140+
- **Fixed-money**: `smallmoney` → `money`, `money` → `numeric` (with sufficient precision)
141+
142+
Safe expansions are further gated by `column_type_expansion_max_rows` (default 1,000,000 rows) to avoid long-running operations on large tables.
143+
132144
### `dbt_sqlserver_use_dbt_transactions`
133145

134146
_(default: `false`)_ When enabled, makes dbt's transaction hooks real at the SQL Server level by emitting `BEGIN TRANSACTION` / `COMMIT TRANSACTION` through the adapter's `add_begin_query` and `add_commit_query` methods.
@@ -142,7 +154,28 @@ This mode is opt-in and should be tested carefully with project-specific materia
142154
```yaml
143155
# dbt_project.yml
144156
flags:
145-
dbt_sqlserver_use_dbt_transactions: true # <-- opt-in; default is false
157+
dbt_sqlserver_enable_safe_type_expansion: true
158+
dbt_sqlserver_use_dbt_transactions: true # <-- opt-in; default is false
159+
```
160+
161+
### `column_type_expansion_max_rows`
162+
163+
*(default: `1000000`)* Per-model config that limits when safe type expansion runs. When the target table exceeds this row count, safe type expansion is skipped (basic same-family string resizes still proceed). Set to `-1` to disable the check entirely.
164+
165+
```sql
166+
-- In an incremental model
167+
{{ config(materialized='incremental', unique_key='id',
168+
column_type_expansion_max_rows=500000) }}
169+
```
170+
171+
### `prefer_single_alter_column`
172+
173+
*(default: `false`)* Model-level config that controls how `alter_column_type` changes column types on tables. When `false` (default), the adapter uses the safer approach: add a temporary column, copy data, drop the original, and rename. When `true`, the adapter uses a single `ALTER COLUMN` statement, which is faster on small, medium tables and instant on safe type expansions but may fail for types that cannot be implicitly converted.
174+
175+
```sql
176+
-- In an incremental model
177+
{{ config(materialized='incremental', unique_key='id',
178+
prefer_single_alter_column=true) }}
146179
```
147180

148181
**Compatibility notes:** Enabling `dbt_sqlserver_use_dbt_transactions: true` may expose transaction-state assumptions hidden by autocommit-only mode. Explicit transaction macros may interact with dbt-managed transactions, and cleanup after failed DDL/DML may differ. Review pre/post hooks for in-transaction vs out-of-transaction semantics.

dbt/adapters/sqlserver/sqlserver_adapter.py

Lines changed: 109 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@
1515
from dbt.adapters.base.meta import available
1616
from dbt.adapters.base.relation import BaseRelation
1717
from dbt.adapters.capability import Capability, CapabilityDict, CapabilitySupport, Support
18-
from dbt.adapters.events.types import SchemaCreation
18+
from dbt.adapters.events.logging import AdapterLogger
19+
from dbt.adapters.events.types import ColTypeChange, SchemaCreation
1920
from dbt.adapters.reference_keys import _make_ref_key_dict
2021
from dbt.adapters.relation_configs import RelationConfigChangeAction
2122
from dbt.adapters.sql.impl import CREATE_SCHEMA_MACRO_NAME, SQLAdapter
@@ -30,6 +31,8 @@
3031
from dbt.adapters.sqlserver.sqlserver_connections import SQLServerConnectionManager
3132
from dbt.adapters.sqlserver.sqlserver_relation import SQLServerRelation
3233

34+
logger = AdapterLogger("SQLServer")
35+
3336

3437
class SQLServerAdapter(SQLAdapter):
3538
"""
@@ -111,6 +114,16 @@ def _behavior_flags(self) -> List[BehaviorFlag]:
111114
"The new behaviour is intended to become the default in a future release."
112115
),
113116
},
117+
{
118+
"name": "dbt_sqlserver_enable_safe_type_expansion",
119+
"default": False,
120+
"description": (
121+
"Allow the SQL Server adapter to widen column types during schema expansion. "
122+
"This enables promotions like varchar -> nvarchar, "
123+
"bit -> tinyint -> smallint -> int -> bigint, "
124+
"and numeric(p,s) -> numeric(p2,s2) using alter column."
125+
),
126+
},
114127
{
115128
"name": "dbt_sqlserver_use_dbt_transactions",
116129
"default": False,
@@ -312,6 +325,101 @@ def render_model_constraint(cls, constraint: ModelLevelConstraint) -> Optional[s
312325
else:
313326
return None
314327

328+
def _get_row_count(self, relation) -> int:
329+
"""Return the number of rows in the given relation."""
330+
sql = f"SELECT COUNT_BIG(*) FROM {relation}"
331+
_, cursor = self.connections.add_select_query(sql)
332+
row = cursor.fetchone()
333+
return int(row[0]) if row else 0
334+
335+
def expand_column_types(self, goal, current, max_rows: int = 1000000):
336+
"""Override to ensure we preserve nvarchar/nchar type family during
337+
column expansion. Necessary same-family resizes (e.g. varchar size)
338+
always proceed. Safe type expansions (cross-family promotions like
339+
varchar -> nvarchar) are guarded by column_type_expansion_max_rows.
340+
enable_safe_type_expansion is the future approach for widening."""
341+
342+
reference_columns = {c.name: c for c in self.get_columns_in_relation(goal)}
343+
target_columns = {c.name: c for c in self.get_columns_in_relation(current)}
344+
345+
enable_safe = self.behavior.dbt_sqlserver_enable_safe_type_expansion
346+
347+
row_count_exceeds = False
348+
if enable_safe and max_rows != -1:
349+
if max_rows == 0:
350+
row_count_exceeds = True
351+
logger.info(
352+
"Safe type expansion skipped for %s: column_type_expansion_max_rows is 0.",
353+
current,
354+
)
355+
else:
356+
row_count = self._get_row_count(current)
357+
if row_count > max_rows:
358+
row_count_exceeds = True
359+
logger.warning(
360+
"Safe type expansion skipped for %s: "
361+
"%s rows exceeds column_type_expansion_max_rows (%s). "
362+
"Set column_type_expansion_max_rows=-1 to disable "
363+
"this check, or increase the limit.",
364+
current,
365+
row_count,
366+
max_rows,
367+
)
368+
369+
for column_name, reference_column in reference_columns.items():
370+
target_column = target_columns.get(column_name)
371+
if target_column is None:
372+
continue
373+
374+
if target_column.can_expand_to(reference_column):
375+
pass
376+
elif (
377+
enable_safe
378+
and not row_count_exceeds
379+
and target_column.can_expand_safe(reference_column)
380+
):
381+
pass
382+
else:
383+
continue
384+
385+
if reference_column.is_string():
386+
col_string_size = reference_column.string_size()
387+
new_type = reference_column.string_type_instance(col_string_size)
388+
else:
389+
new_type = reference_column.data_type
390+
fire_event(
391+
ColTypeChange(
392+
orig_type=target_column.data_type,
393+
new_type=new_type,
394+
table=_make_ref_key_dict(current),
395+
)
396+
)
397+
self.alter_column_type(current, column_name, new_type)
398+
399+
@available.parse_none
400+
def expand_target_column_types(
401+
self, from_relation: BaseRelation, to_relation: BaseRelation, max_rows: int = 1000000
402+
) -> None:
403+
if not isinstance(from_relation, self.Relation):
404+
from dbt.adapters.base.impl import MacroArgTypeError
405+
406+
raise MacroArgTypeError(
407+
method_name="expand_target_column_types",
408+
arg_name="from_relation",
409+
got_value=from_relation,
410+
expected_type=self.Relation,
411+
)
412+
if not isinstance(to_relation, self.Relation):
413+
from dbt.adapters.base.impl import MacroArgTypeError
414+
415+
raise MacroArgTypeError(
416+
method_name="expand_target_column_types",
417+
arg_name="to_relation",
418+
got_value=to_relation,
419+
expected_type=self.Relation,
420+
)
421+
self.expand_column_types(from_relation, to_relation, max_rows)
422+
315423
@available
316424
def parse_index(self, raw_index: Any) -> Optional[SQLServerIndexConfig]:
317425
return SQLServerIndexConfig.parse(raw_index)

0 commit comments

Comments
 (0)