From f273d54c675bc5ffb7f74b84c5a6d7e16003447c Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Wed, 27 Sep 2023 08:00:12 -0500 Subject: [PATCH] Model contracts: raise warning for numeric types without specified scale (#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 <111200756+colin-rogers-dbt@users.noreply.github.com> * Apply suggestions from code review --------- Co-authored-by: colin-rogers-dbt <111200756+colin-rogers-dbt@users.noreply.github.com> --- .../unreleased/Features-20230926-134728.yaml | 6 ++ .../macros/adapters/columns.sql | 9 ++- .../contracts/test_contract_precision.py | 63 +++++++++++++++++++ 3 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 .changes/unreleased/Features-20230926-134728.yaml create mode 100644 tests/functional/contracts/test_contract_precision.py diff --git a/.changes/unreleased/Features-20230926-134728.yaml b/.changes/unreleased/Features-20230926-134728.yaml new file mode 100644 index 00000000000..9b1e3157208 --- /dev/null +++ b/.changes/unreleased/Features-20230926-134728.yaml @@ -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" diff --git a/core/dbt/include/global_project/macros/adapters/columns.sql b/core/dbt/include/global_project/macros/adapters/columns.sql index b5a03ec53b5..e1099649cf0 100644 --- a/core/dbt/include/global_project/macros/adapters/columns.sql +++ b/core/dbt/include/global_project/macros/adapters/columns.sql @@ -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 %} diff --git a/tests/functional/contracts/test_contract_precision.py b/tests/functional/contracts/test_contract_precision.py new file mode 100644 index 00000000000..ee5ad4bb50c --- /dev/null +++ b/tests/functional/contracts/test_contract_precision.py @@ -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