Skip to content

Commit bef7838

Browse files
axellpadillaaxellpadilla
authored
Merge pull request #699 from dbt-msft/fix/698-unit-test-empty-rows-tsql
Empty unit test fixtures (rows: []) emit invalid LIMIT 0 syntax
2 parents d709249 + 42c60d7 commit bef7838

4 files changed

Lines changed: 183 additions & 11 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
### v1.10.1
4+
5+
#### Bugfixes
6+
7+
- 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)
8+
39
### v1.10.0
410

511
#### Features

‎dbt/include/sqlserver/macros/adapters/columns.sql‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
1-
{% macro sqlserver__get_empty_subquery_sql(select_sql, select_sql_header=none) %}
1+
{% macro sqlserver__select_starts_with_cte(select_sql) %}
2+
{#-- Strip comments first so a leading comment does not hide the CTE --#}
23
{%- set select_sql_stripped = modules.re.sub('(?s)/\\*.*?\\*/|--[^\n]*\n', '', select_sql) -%}
3-
{% if select_sql_stripped.strip().lower().startswith('with') %}
4+
{{ return(select_sql_stripped.strip().lower().startswith('with')) }}
5+
{% endmacro %}
6+
7+
{% macro sqlserver__get_empty_subquery_sql(select_sql, select_sql_header=none) %}
8+
{% if sqlserver__select_starts_with_cte(select_sql) %}
49
{{ select_sql }}
510
{% else -%}
611
select * from (
@@ -13,15 +18,22 @@
1318

1419
{% macro sqlserver__get_columns_in_query(select_sql) %}
1520
{% set query_label = get_query_options() %}
16-
{% call statement('get_columns_in_query', fetch_result=True, auto_begin=False) -%}
17-
select TOP 0 * from (
18-
{{ select_sql }}
19-
) as __dbt_sbq
20-
where 0 = 1
21-
{{ query_label }}
22-
{% endcall %}
23-
24-
{{ return(load_result('get_columns_in_query').table.columns | map(attribute='name') | list) }}
21+
{% if sqlserver__select_starts_with_cte(select_sql) %}
22+
{#-- A query starting with a CTE cannot be wrapped in a subquery; describe its result set instead of executing it (dbt-msft/dbt-sqlserver#698) --#}
23+
{% call statement('get_columns_in_query', fetch_result=True, auto_begin=False) -%}
24+
exec sp_describe_first_result_set @tsql = N'{{ escape_single_quotes(select_sql) }}'
25+
{% endcall %}
26+
{{ return(load_result('get_columns_in_query').table.columns['name'].values() | list) }}
27+
{% else %}
28+
{% call statement('get_columns_in_query', fetch_result=True, auto_begin=False) -%}
29+
select TOP 0 * from (
30+
{{ select_sql }}
31+
) as __dbt_sbq
32+
where 0 = 1
33+
{{ query_label }}
34+
{% endcall %}
35+
{{ return(load_result('get_columns_in_query').table.columns | map(attribute='name') | list) }}
36+
{% endif %}
2537
{% endmacro %}
2638

2739
{% macro sqlserver__alter_column_type(relation, column_name, new_column_type) %}

‎dbt/include/sqlserver/macros/materializations/models/unit_test/get_fixture_sql.sql‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
{#
2+
dbt-core does not dispatch "get_fixture_sql" or "get_expected_sql", so this file
3+
shadows the implementations from the dbt global project
4+
(macros/unit_test_sql/get_fixture_sql.sql) - adapter package macros take
5+
precedence over the global project. Keep in sync with dbt-core when upgrading.
6+
7+
To ease a future transition if dbt-core adds adapter.dispatch for these macros,
8+
the public wrappers below are intentionally thin. They delegate to the
9+
SQL Server-specific implementations (sqlserver__get_fixture_sql and
10+
sqlserver__get_expected_sql), which contain the copied/adapted upstream logic.
11+
12+
When upstream dispatches these macros, the public wrappers can be deleted and
13+
the sqlserver__ implementations kept as the dispatched handlers.
14+
15+
Changes from upstream (see dbt-msft/dbt-sqlserver#698):
16+
- sqlserver__get_fixture_sql: the empty-rows branch emits "select top 0"
17+
instead of "limit 0", which is not valid T-SQL.
18+
- sqlserver__get_expected_sql: the empty-rows branch emits a "select top 0"
19+
of typed nulls instead of "select * from dbt_internal_unit_test_actual limit 0".
20+
Besides the invalid "limit", sqlserver__get_unit_test_sql wraps the expected
21+
SQL in its own view, where that CTE name is out of scope.
22+
#}
23+
24+
{% macro get_fixture_sql(rows, column_name_to_data_types) %}
25+
{{ return(sqlserver__get_fixture_sql(rows, column_name_to_data_types)) }}
26+
{% endmacro %}
27+
28+
29+
{% macro sqlserver__get_fixture_sql(rows, column_name_to_data_types) %}
30+
-- Fixture for {{ model.name }}
31+
{% set default_row = {} %}
32+
33+
{%- if not column_name_to_data_types -%}
34+
{#-- Use defer_relation IFF it is available in the manifest and 'this' is missing from the database --#}
35+
{%- set this_or_defer_relation = defer_relation if (defer_relation and not load_relation(this)) else this -%}
36+
{%- set columns_in_relation = adapter.get_columns_in_relation(this_or_defer_relation) -%}
37+
38+
{%- set column_name_to_data_types = {} -%}
39+
{%- set column_name_to_quoted = {} -%}
40+
{%- for column in columns_in_relation -%}
41+
42+
{#-- This needs to be a case-insensitive comparison --#}
43+
{%- do column_name_to_data_types.update({column.name|lower: column.data_type}) -%}
44+
{%- do column_name_to_quoted.update({column.name|lower: column.quoted}) -%}
45+
{%- endfor -%}
46+
{%- endif -%}
47+
48+
{%- if not column_name_to_data_types -%}
49+
{{ exceptions.raise_compiler_error("Not able to get columns for unit test '" ~ model.name ~ "' from relation " ~ this ~ " because the relation doesn't exist") }}
50+
{%- endif -%}
51+
52+
{%- for column_name, column_type in column_name_to_data_types.items() -%}
53+
{%- do default_row.update({column_name: (safe_cast("null", column_type) | trim )}) -%}
54+
{%- endfor -%}
55+
56+
{{ validate_fixture_rows(rows, row_number) }}
57+
58+
{%- for row in rows -%}
59+
{%- set formatted_row = format_row(row, column_name_to_data_types) -%}
60+
{%- set default_row_copy = default_row.copy() -%}
61+
{%- do default_row_copy.update(formatted_row) -%}
62+
select
63+
{%- for column_name, column_value in default_row_copy.items() %} {{ column_value }} as {{ column_name_to_quoted[column_name] }}{% if not loop.last -%}, {%- endif %}
64+
{%- endfor %}
65+
{%- if not loop.last %}
66+
union all
67+
{% endif %}
68+
{%- endfor -%}
69+
70+
{%- if (rows | length) == 0 -%}
71+
select top 0
72+
{%- for column_name, column_value in default_row.items() %} {{ column_value }} as {{ column_name_to_quoted[column_name] }}{% if not loop.last -%},{%- endif %}
73+
{%- endfor %}
74+
{%- endif -%}
75+
{% endmacro %}
76+
77+
78+
{% macro get_expected_sql(rows, column_name_to_data_types, column_name_to_quoted) %}
79+
{{ return(sqlserver__get_expected_sql(rows, column_name_to_data_types, column_name_to_quoted)) }}
80+
{% endmacro %}
81+
82+
83+
{% macro sqlserver__get_expected_sql(rows, column_name_to_data_types, column_name_to_quoted) %}
84+
85+
{%- if (rows | length) == 0 -%}
86+
select top 0
87+
{%- for column_name, column_type in column_name_to_data_types.items() %} {{ safe_cast("null", column_type) | trim }} as {{ column_name_to_quoted[column_name] }}{% if not loop.last -%},{%- endif %}
88+
{%- endfor %}
89+
{%- else -%}
90+
{%- for row in rows -%}
91+
{%- set formatted_row = format_row(row, column_name_to_data_types) -%}
92+
select
93+
{%- for column_name, column_value in formatted_row.items() %} {{ column_value }} as {{ column_name_to_quoted[column_name] }}{% if not loop.last -%}, {%- endif %}
94+
{%- endfor %}
95+
{%- if not loop.last %}
96+
union all
97+
{% endif %}
98+
{%- endfor -%}
99+
{%- endif -%}
100+
101+
{% endmacro %}

‎tests/functional/adapter/dbt/test_unit_tests.py‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,41 @@
2828
- {{ tested_column: {yaml_value} }}
2929
"""
3030

31+
my_union_model_sql = """
32+
select tested_column from {{ ref('my_upstream_model') }}
33+
union all
34+
select tested_column from {{ ref('my_other_upstream_model') }}
35+
"""
36+
37+
upstream_model_sql = """
38+
select 1 as tested_column
39+
"""
40+
41+
# `rows: []` must not generate `limit 0`, which is invalid T-SQL (issue #698)
42+
test_empty_fixture_yml = """
43+
unit_tests:
44+
- name: test_empty_given
45+
model: my_union_model
46+
given:
47+
- input: ref('my_upstream_model')
48+
rows:
49+
- {tested_column: 1}
50+
- input: ref('my_other_upstream_model')
51+
rows: []
52+
expect:
53+
rows:
54+
- {tested_column: 1}
55+
- name: test_empty_expect
56+
model: my_union_model
57+
given:
58+
- input: ref('my_upstream_model')
59+
rows: []
60+
- input: ref('my_other_upstream_model')
61+
rows: []
62+
expect:
63+
rows: []
64+
"""
65+
3166

3267
class TestUnitTestCaseInsensitivity(BaseUnitTestCaseInsensivity):
3368
pass
@@ -37,6 +72,24 @@ class TestUnitTestInvalidInput(BaseUnitTestInvalidInput):
3772
pass
3873

3974

75+
class TestUnitTestEmptyFixture:
76+
@pytest.fixture(scope="class")
77+
def models(self):
78+
return {
79+
"my_upstream_model.sql": upstream_model_sql,
80+
"my_other_upstream_model.sql": upstream_model_sql,
81+
"my_union_model.sql": my_union_model_sql,
82+
"schema.yml": test_empty_fixture_yml,
83+
}
84+
85+
def test_empty_fixture_rows(self, project):
86+
results = run_dbt(["run"])
87+
assert len(results) == 3
88+
89+
results = run_dbt(["test", "--select", "my_union_model"])
90+
assert len(results) == 2
91+
92+
4093
class TestUnitTestingTypes(BaseUnitTestingTypes):
4194
@pytest.fixture
4295
def data_types(self):

0 commit comments

Comments
 (0)