Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Model contracts: raise warning for numeric types without specified scale #8721

Merged
merged 6 commits into from
Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 wanring when a contracted model has a numeric field without scale defined
emmyoop marked this conversation as resolved.
Show resolved Hide resolved
emmyoop marked this conversation as resolved.
Show resolved Hide resolved
time: 2023-09-26T13:47:28.645383-05:00
custom:
Author: emmyoop
Issue: "8183"
10 changes: 8 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,30 @@
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 api.Column.create(col['name'], col['data_type'].strip()).is_numeric() -#}
emmyoop marked this conversation as resolved.
Show resolved Hide resolved
emmyoop marked this conversation as resolved.
Show resolved Hide resolved
{%- 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
Loading