From d2324f04385975b012b3bfa9a4bc0e8ad0c0f842 Mon Sep 17 00:00:00 2001 From: Tobie Tusing Date: Fri, 4 Aug 2023 15:54:23 -0700 Subject: [PATCH 1/8] add primary key coverage for more resource types --- dbt_project.yml | 4 ++++ models/marts/tests/fct_missing_primary_key_tests.sql | 4 ++++ models/marts/tests/fct_test_coverage.sql | 1 + models/marts/tests/intermediate/int_model_test_summary.sql | 2 +- 4 files changed, 10 insertions(+), 1 deletion(-) diff --git a/dbt_project.yml b/dbt_project.yml index c4471679..5f4001f7 100644 --- a/dbt_project.yml +++ b/dbt_project.yml @@ -57,6 +57,10 @@ vars: primary_key_test_macros: [["dbt.test_unique", "dbt.test_not_null"], ["dbt_utils.test_unique_combination_of_columns"]] + # -- Graph variables -- + # node types to test for primary key coverage. acceptable node types: model, source, snapshot, seed + enforced_primary_key_node_types: ["model"] + # -- DAG variables -- models_fanout_threshold: 3 diff --git a/models/marts/tests/fct_missing_primary_key_tests.sql b/models/marts/tests/fct_missing_primary_key_tests.sql index 37bd0c29..2f023ccf 100644 --- a/models/marts/tests/fct_missing_primary_key_tests.sql +++ b/models/marts/tests/fct_missing_primary_key_tests.sql @@ -8,6 +8,10 @@ with tests as ( select * from {{ ref('int_model_test_summary') }} + where resource_type in + ( + {% for resource_type in var('enforced_primary_key_node_types') %}'{{ resource_type }}'{% if not loop.last %},{% endif %} + {% endfor %} ), final as ( diff --git a/models/marts/tests/fct_test_coverage.sql b/models/marts/tests/fct_test_coverage.sql index 872706bb..504aa22a 100644 --- a/models/marts/tests/fct_test_coverage.sql +++ b/models/marts/tests/fct_test_coverage.sql @@ -2,6 +2,7 @@ with test_counts as ( select * from {{ ref('int_model_test_summary') }} + where resource_type = 'model' ), conversion as ( diff --git a/models/marts/tests/intermediate/int_model_test_summary.sql b/models/marts/tests/intermediate/int_model_test_summary.sql index 54116d25..31572b93 100644 --- a/models/marts/tests/intermediate/int_model_test_summary.sql +++ b/models/marts/tests/intermediate/int_model_test_summary.sql @@ -62,7 +62,7 @@ final as ( from all_graph_resources left join agg_test_relationships on all_graph_resources.resource_id = agg_test_relationships.direct_parent_id - where all_graph_resources.resource_type = 'model' + where all_graph_resources.resource_type in ('model', 'seed', 'source', 'snapshot') ) select * from final From c32cf5a07efae2251599b47abb9d8d7f334adcf5 Mon Sep 17 00:00:00 2001 From: Tobie Tusing Date: Fri, 4 Aug 2023 16:01:24 -0700 Subject: [PATCH 2/8] add field --- models/marts/tests/intermediate/int_model_test_summary.sql | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/models/marts/tests/intermediate/int_model_test_summary.sql b/models/marts/tests/intermediate/int_model_test_summary.sql index 31572b93..4b430230 100644 --- a/models/marts/tests/intermediate/int_model_test_summary.sql +++ b/models/marts/tests/intermediate/int_model_test_summary.sql @@ -56,13 +56,15 @@ agg_test_relationships as ( final as ( select all_graph_resources.resource_name, + all_graph_resources.resource_type, all_graph_resources.model_type, coalesce(agg_test_relationships.is_primary_key_tested, FALSE) as is_primary_key_tested, coalesce(agg_test_relationships.number_of_tests_on_model, 0) as number_of_tests_on_model from all_graph_resources left join agg_test_relationships on all_graph_resources.resource_id = agg_test_relationships.direct_parent_id - where all_graph_resources.resource_type in ('model', 'seed', 'source', 'snapshot') + where + all_graph_resources.resource_type in ('model', 'seed', 'source', 'snapshot') ) select * from final From 6aa627b937d98dedc82637f3dfce39027d80c7b9 Mon Sep 17 00:00:00 2001 From: Tobie Tusing Date: Fri, 4 Aug 2023 16:05:34 -0700 Subject: [PATCH 3/8] add missing paren --- models/marts/tests/fct_missing_primary_key_tests.sql | 1 + 1 file changed, 1 insertion(+) diff --git a/models/marts/tests/fct_missing_primary_key_tests.sql b/models/marts/tests/fct_missing_primary_key_tests.sql index 2f023ccf..39eeb1b4 100644 --- a/models/marts/tests/fct_missing_primary_key_tests.sql +++ b/models/marts/tests/fct_missing_primary_key_tests.sql @@ -12,6 +12,7 @@ tests as ( ( {% for resource_type in var('enforced_primary_key_node_types') %}'{{ resource_type }}'{% if not loop.last %},{% endif %} {% endfor %} + ) ), final as ( From 8bc7a7d16f85909b1657ab7063efaeb456f01ef7 Mon Sep 17 00:00:00 2001 From: Tobie Tusing Date: Fri, 4 Aug 2023 16:12:23 -0700 Subject: [PATCH 4/8] remove alias --- models/marts/tests/fct_missing_primary_key_tests.sql | 6 ------ 1 file changed, 6 deletions(-) diff --git a/models/marts/tests/fct_missing_primary_key_tests.sql b/models/marts/tests/fct_missing_primary_key_tests.sql index 39eeb1b4..8d1227e3 100644 --- a/models/marts/tests/fct_missing_primary_key_tests.sql +++ b/models/marts/tests/fct_missing_primary_key_tests.sql @@ -1,9 +1,3 @@ -{{ - config( - alias = 'my_alias', - ) -}} - with tests as ( From 77aa491cce3e0f3f95fba1aa15c73964b5cda717 Mon Sep 17 00:00:00 2001 From: Tobie Tusing Date: Mon, 7 Aug 2023 23:58:05 -0700 Subject: [PATCH 5/8] update docs --- docs/customization/overriding-variables.md | 1 + docs/rules/testing.md | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/docs/customization/overriding-variables.md b/docs/customization/overriding-variables.md index 9e41269e..2d102279 100644 --- a/docs/customization/overriding-variables.md +++ b/docs/customization/overriding-variables.md @@ -9,6 +9,7 @@ Currently, this package uses different variables to adapt the models to your obj | `test_coverage_target` | the minimum acceptable test coverage percentage | 100% | | `documentation_coverage_target` | the minimum acceptable documentation coverage percentage | 100% | | `primary_key_test_macros` | the set(s) of dbt tests used to check validity of a primary key | `[["dbt.test_unique", "dbt.test_not_null"], ["dbt_utils.test_unique_combination_of_columns"]]` | +| `enforced_primary_key_node_types` | the set of node types for you you would like to enforce primary key test coverage. Valid options are `model`, `source`, `snapshot`, `seed` | `model` **Usage notes for `primary_key_test_macros:`** diff --git a/docs/rules/testing.md b/docs/rules/testing.md index 7eae1792..630b3e24 100644 --- a/docs/rules/testing.md +++ b/docs/rules/testing.md @@ -21,6 +21,18 @@ Apply a [uniqueness test](https://docs.getdbt.com/reference/resource-properties/ Additional tests can be configured by applying a [generic test](https://docs.getdbt.com/docs/building-a-dbt-project/tests#generic-tests) in the model's `.yml` entry or by creating a [singular test](https://docs.getdbt.com/docs/building-a-dbt-project/tests#singular-tests) in the `tests` directory of you project. +**Enforcing on more node types and primary key test options (Advanced)** + +You can optionally extend this test to apply to more node types (`source`,`snapshot`, `seed`). By configuring the variable `enforced_primary_key_node_types` to be a set of node types for which you wish to enforce primary key test coverage in addition to (or instead of) just models. Check out the [overriding variables section](../customization/overriding-variables.md) for instructions + +Snapshots should always have a multi-field primary key in order to function, while sources and seeds may not. Depending on your expectations for duplicates and null values, different kinds of primary key tests may be appropriate. Consider your use case carefully. + +Here are some options for primary key tests you can configure: +1. a `dbt.test_unique` applied to a single column, perhaps with `dbt.test_not_null`. Both are [included with DBT core](https://docs.getdbt.com/docs/build/tests#generic-tests). +2. `dbt_utils.test_unique_combination_of_columns` in the [DBT Utils](https://github.com/dbt-labs/dbt-utils) package tests for multi-field uniqueness, but does not enforce that the fields are not null +3. `dbt_constraints.test_primary_key` in the [DBT Constraints](https://github.com/Snowflake-Labs/dbt_constraints) package tests for multi-field uniqueness and that each field is not null + + --- ## Test Coverage From 6b5cb2ed708b82a33e9bbcc248007423a4d8ecb4 Mon Sep 17 00:00:00 2001 From: Tobie Tusing Date: Tue, 8 Aug 2023 00:01:35 -0700 Subject: [PATCH 6/8] fix doc quotes --- docs/customization/overriding-variables.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/customization/overriding-variables.md b/docs/customization/overriding-variables.md index 2d102279..220da045 100644 --- a/docs/customization/overriding-variables.md +++ b/docs/customization/overriding-variables.md @@ -9,7 +9,7 @@ Currently, this package uses different variables to adapt the models to your obj | `test_coverage_target` | the minimum acceptable test coverage percentage | 100% | | `documentation_coverage_target` | the minimum acceptable documentation coverage percentage | 100% | | `primary_key_test_macros` | the set(s) of dbt tests used to check validity of a primary key | `[["dbt.test_unique", "dbt.test_not_null"], ["dbt_utils.test_unique_combination_of_columns"]]` | -| `enforced_primary_key_node_types` | the set of node types for you you would like to enforce primary key test coverage. Valid options are `model`, `source`, `snapshot`, `seed` | `model` +| `enforced_primary_key_node_types` | the set of node types for you you would like to enforce primary key test coverage. Valid options to include are `model`, `source`, `snapshot`, `seed` | `["model"]` **Usage notes for `primary_key_test_macros:`** From 402420bc68b0c7304996f55b6237500dc78a9337 Mon Sep 17 00:00:00 2001 From: Tobie Tusing Date: Tue, 8 Aug 2023 10:45:00 -0700 Subject: [PATCH 7/8] encorporate docs suggestions --- docs/customization/overriding-variables.md | 2 +- docs/rules/testing.md | 8 +------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/docs/customization/overriding-variables.md b/docs/customization/overriding-variables.md index 220da045..73c7e97f 100644 --- a/docs/customization/overriding-variables.md +++ b/docs/customization/overriding-variables.md @@ -22,7 +22,7 @@ For each entry in the parent list, the logic in `int_model_test_summary` will ev Each set of test(s) that define a primary key requirement must be grouped together in a sub-list to ensure they are evaluated together (e.g. [`dbt.test_unique`, `dbt.test_not_null`] ). -*While it's not explicitly tested in this package, we strongly encourage adding a `not_null` test on each of the columns listed in the `dbt_utils.unique_combination_of_columns` tests.* +*While it's not explicitly tested in this package, we strongly encourage adding a `not_null` test on each of the columns listed in the `dbt_utils.unique_combination_of_columns` tests. Alternatively, consider `dbt_constraints.test_primary_key` in the [DBT Constraints](https://github.com/Snowflake-Labs/dbt_constraints) package, which enforces each field in the primary key is non null.* ```yaml title="dbt_project.yml" # set your test and doc coverage to 75% instead diff --git a/docs/rules/testing.md b/docs/rules/testing.md index 630b3e24..e1c1b654 100644 --- a/docs/rules/testing.md +++ b/docs/rules/testing.md @@ -21,18 +21,12 @@ Apply a [uniqueness test](https://docs.getdbt.com/reference/resource-properties/ Additional tests can be configured by applying a [generic test](https://docs.getdbt.com/docs/building-a-dbt-project/tests#generic-tests) in the model's `.yml` entry or by creating a [singular test](https://docs.getdbt.com/docs/building-a-dbt-project/tests#singular-tests) in the `tests` directory of you project. -**Enforcing on more node types and primary key test options (Advanced)** +**Enforcing on more node types(Advanced)** You can optionally extend this test to apply to more node types (`source`,`snapshot`, `seed`). By configuring the variable `enforced_primary_key_node_types` to be a set of node types for which you wish to enforce primary key test coverage in addition to (or instead of) just models. Check out the [overriding variables section](../customization/overriding-variables.md) for instructions Snapshots should always have a multi-field primary key in order to function, while sources and seeds may not. Depending on your expectations for duplicates and null values, different kinds of primary key tests may be appropriate. Consider your use case carefully. -Here are some options for primary key tests you can configure: -1. a `dbt.test_unique` applied to a single column, perhaps with `dbt.test_not_null`. Both are [included with DBT core](https://docs.getdbt.com/docs/build/tests#generic-tests). -2. `dbt_utils.test_unique_combination_of_columns` in the [DBT Utils](https://github.com/dbt-labs/dbt-utils) package tests for multi-field uniqueness, but does not enforce that the fields are not null -3. `dbt_constraints.test_primary_key` in the [DBT Constraints](https://github.com/Snowflake-Labs/dbt_constraints) package tests for multi-field uniqueness and that each field is not null - - --- ## Test Coverage From 37690f8cdf2ba29a28b9ff5518eb3071822f5503 Mon Sep 17 00:00:00 2001 From: Benoit Perigaud <8754100+b-per@users.noreply.github.com> Date: Wed, 9 Aug 2023 15:52:53 +0200 Subject: [PATCH 8/8] Minor correction to docs --- docs/customization/overriding-variables.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/customization/overriding-variables.md b/docs/customization/overriding-variables.md index 73c7e97f..666f683d 100644 --- a/docs/customization/overriding-variables.md +++ b/docs/customization/overriding-variables.md @@ -22,7 +22,7 @@ For each entry in the parent list, the logic in `int_model_test_summary` will ev Each set of test(s) that define a primary key requirement must be grouped together in a sub-list to ensure they are evaluated together (e.g. [`dbt.test_unique`, `dbt.test_not_null`] ). -*While it's not explicitly tested in this package, we strongly encourage adding a `not_null` test on each of the columns listed in the `dbt_utils.unique_combination_of_columns` tests. Alternatively, consider `dbt_constraints.test_primary_key` in the [DBT Constraints](https://github.com/Snowflake-Labs/dbt_constraints) package, which enforces each field in the primary key is non null.* +*While it's not explicitly tested in this package, we strongly encourage adding a `not_null` test on each of the columns listed in the `dbt_utils.unique_combination_of_columns` tests. Alternatively, on Snowflake, consider `dbt_constraints.test_primary_key` in the [dbt Constraints](https://github.com/Snowflake-Labs/dbt_constraints) package, which enforces each field in the primary key is non null.* ```yaml title="dbt_project.yml" # set your test and doc coverage to 75% instead