Skip to content

Commit

Permalink
Model contracts: raise warning for numeric types without specified sc…
Browse files Browse the repository at this point in the history
…ale (#8721)

* add warning when contracting fields don't have precision

* rename files

* changelog

* move tests out of adapter zone

* Update core/dbt/include/global_project/macros/adapters/columns.sql

Co-authored-by: colin-rogers-dbt <[email protected]>

* Apply suggestions from code review

---------

Co-authored-by: colin-rogers-dbt <[email protected]>
  • Loading branch information
2 people authored and QMalcolm committed Oct 6, 2023
1 parent e2fa61c commit f273d54
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 2 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Features-20230926-134728.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Features
body: Raise a warning when a contracted model has a numeric field without scale defined
time: 2023-09-26T13:47:28.645383-05:00
custom:
Author: emmyoop
Issue: "8183"
9 changes: 7 additions & 2 deletions core/dbt/include/global_project/macros/adapters/columns.sql
Original file line number Diff line number Diff line change
Expand Up @@ -36,24 +36,29 @@
limit 0
{% endmacro %}


{% macro get_empty_schema_sql(columns) -%}
{{ return(adapter.dispatch('get_empty_schema_sql', 'dbt')(columns)) }}
{% endmacro %}

{% macro default__get_empty_schema_sql(columns) %}
{%- set col_err = [] -%}
{%- set col_naked_numeric = [] -%}
select
{% for i in columns %}
{%- set col = columns[i] -%}
{%- if col['data_type'] is not defined -%}
{{ col_err.append(col['name']) }}
{%- do col_err.append(col['name']) -%}
{#-- If this column's type is just 'numeric' then it is missing precision/scale, raise a warning --#}
{%- elif col['data_type'].strip().lower() in ('numeric', 'decimal', 'number') -%}
{%- do col_naked_numeric.append(col['name']) -%}
{%- endif -%}
{% set col_name = adapter.quote(col['name']) if col.get('quote') else col['name'] %}
cast(null as {{ col['data_type'] }}) as {{ col_name }}{{ ", " if not loop.last }}
{%- endfor -%}
{%- if (col_err | length) > 0 -%}
{{ exceptions.column_type_missing(column_names=col_err) }}
{%- elif (col_naked_numeric | length) > 0 -%}
{{ exceptions.warn("Detected columns with numeric type and unspecified precision/scale, this can lead to unintended rounding: " ~ col_naked_numeric ~ "`") }}
{%- endif -%}
{% endmacro %}

Expand Down
63 changes: 63 additions & 0 deletions tests/functional/contracts/test_contract_precision.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import pytest
from dbt.tests.util import run_dbt_and_capture


my_numeric_model_sql = """
select
1.234 as non_integer
"""

model_schema_numerics_yml = """
version: 2
models:
- name: my_numeric_model
config:
contract:
enforced: true
columns:
- name: non_integer
data_type: numeric
"""

model_schema_numerics_precision_yml = """
version: 2
models:
- name: my_numeric_model
config:
contract:
enforced: true
columns:
- name: non_integer
data_type: numeric(38,3)
"""


class TestModelContractNumericNoPrecision:
@pytest.fixture(scope="class")
def models(self):
return {
"my_numeric_model.sql": my_numeric_model_sql,
"schema.yml": model_schema_numerics_yml,
}

def test_contracted_numeric_without_precision(self, project):
expected_msg = "Detected columns with numeric type and unspecified precision/scale, this can lead to unintended rounding: ['non_integer']"
_, logs = run_dbt_and_capture(["run"], expect_pass=True)
assert expected_msg in logs
_, logs = run_dbt_and_capture(["--warn-error", "run"], expect_pass=False)
assert "Compilation Error in model my_numeric_model" in logs
assert expected_msg in logs


class TestModelContractNumericPrecision:
@pytest.fixture(scope="class")
def models(self):
return {
"my_numeric_model.sql": my_numeric_model_sql,
"schema.yml": model_schema_numerics_precision_yml,
}

def test_contracted_numeric_with_precision(self, project):
expected_msg = "Detected columns with numeric type and unspecified precision/scale, this can lead to unintended rounding: ['non_integer']"
_, logs = run_dbt_and_capture(["run"], expect_pass=True)
assert expected_msg not in logs

0 comments on commit f273d54

Please sign in to comment.