Add percentage support for error_if, warn_if

[jaypeedevlin]

resolves #4723, in collaboration with @ehmartens

Description

Adds support for using percentages in the error_if and warn_if configurations.

We'd love some feedback on the following:

• Whether the method is too 'hacky' and/or if there are alternate ways of going about this.
• Where to add tests — we took a look but couldn't find exactly where the configs are currently tested, and additionally noted that the tests are currently moving between two frameworks

Checklist

• I have read the contributing guide and understand what's expected of me
• I have signed the CLA
• I have run this code in development and it appears to resolve the stated issue
• This PR includes tests, or tests are not required/relevant for this PR
• I have opened an issue to add/update docs, or docs changes are not required/relevant for this PR
• I have run changie new to create a changelog entry

[jtcohen6]

Thanks for taking a first crack at this @jaypeedevlin @ehmartens!

I'm going to tag in the Language team to help here, since I think the real question is actually around the metadata that generic tests can make available within their materializations. Ideally, we'd have access to the rendered form of "{{ get_where_subquery(<ref_or_source>) }}", i.e. that which gets passed in as the {{ model }} argument to generic test definitions.

[jtcohen6]

Two thoughts:

• For tests defined on models, this should work. For tests defined on sources, file_key_name is sources.source_name (and missing the name of the specific source table), so I don't think it would work as currently constituted. We could update file_key_name to include sources.source_name.source_table.
• If users have defined a where config, and we're calculating %, I think we would want to filter both the numerator and denominator by that where config. You could reproduce that logic here (config.get("where")), but all it all it seems preferable if there were some where to access the rendered version of {{ model }} (from the generic test query) or {{ test_metadata.kwargs.model }} (from the test node), since that's really the thing we want.

[jaypeedevlin]

Great point on the 'where' @jtcohen6, we totally overlooked that! We've added that in, but not pushed the change yet, while we wait to hear from the language team on how we might approach getting the non-model resources appropriately.

[jtcohen6]

Could you imagine someone wanting to define a % failure threshold for a singular test (as opposed to generic tests)? I can't think of one myself

[jaypeedevlin]

We took a list at that this morning, I think I probably could think of someone wanting to do this (in a rare case), but it wasn't clear to me why the current approach wouldn't work (given that thresholds should work in the same way). Could you help me understand how that might work?

[jtcohen6]

In the case where a singular test depends on multiple nodes, I'm not sure how we can tell which node should represent the denominator?

I also think, in a singular test, you're writing all the SQL yourself, it's pretty trivial to add a / (select count(*) from total * 100 and define warn_if/error_if with percents-as-integers.

[gshank]

I played with some code in #5214. It doesn't handle the rendering bit yet, but thought I'd get feedback on whether this is what we're thinking of.

[jaypeedevlin]

Hey @gshank, are you able to provide any guidance on how to test your changes in #5214 in conjunction with our current branch?

[gshank]

I took another look at this, and we're creating the same strings in the generic_test_builder, and rendering them in 'add_rendered_test_kwargs'. Then at compilation time they are added to the context with the top-level key "_dbt_generic_test_kwargs", and the key 'model'. So I'm wondering if we can use that instead making a new function.

That rendered 'model' string is used by 'build_raw_sql' in the generic_test_builder which builds the test sql.

[jtcohen6]

@gshank Really good point! This "just works" for me if I start storing _dbt_generic_test_kwargs on the test node (which seems easier than trying to pass it through the different Jinja contexts).

f0dc98a

Granted, I'm taking a pretty simple approach here, and just overwriting the raw test_metadata.kwargs with the compiled versions. Is there a reason we shouldn't do that? Doesn't it feel... better? We're not doing anything with the raw versions, not even using it for the same_contents comparison method that powers state:modified.

version: 2
models:
  - name: my_model
    columns:
      - name: id
        tests:
          - unique:
              warn_if: "=5%"
          - not_null:
              config:
                where: 1=1
                error_if: ">5%"

At runtime (using BigQuery):

  select
      count(*) as failures,
      count(*)  != 0 as should_warn,
      count(*) / (select count(*) from (select * from `dbt-test-env`.`dbt_jcohen`.`my_model` where 1=1) dbt_subquery) * 100 >5 as should_error
...
    select
      count(*) as failures,
      count(*) / (select count(*) from `dbt-test-env`.`dbt_jcohen`.`my_model`) * 100 =5 as should_warn,
      count(*)  != 0 as should_error

[jtcohen6]

Instead of count(*) always, should this be {{ fail_calc }} for the denominator as well? That won't work in cases where the generic test query synthesizes a new column for use in the fail_calc, which isn't present in the underlying {{ model }}, but it feels better than assuming it's always a simple count.

[jaypeedevlin]

I'm going to close this PR for now, since we're not actively working on it.

[gracie2339]

Hi All, I found a workaround with this by writing a generic test in dbt and then referencing it in the yml files like any other ordinary relationships test:

This one has a 1% error threshold and works for BigQuery. Feel free to change as you see fit.

{% test relationships_with_1_pct_error_threshold(model, column_name, field, to) %}

with parent as (
    select
        {{ field }} as id
    from {{ to }}
),

child as (
    select 
        {{ column_name }} as id
    from {{ model }}
),

error_threshold as (
    select
        cast(count(distinct id) * .01 as int) as num
    from parent
)

select distinct id
from child 
where id is not null
    and id not in (select id from parent)
    and (
        select count(distinct id) 
        from child
        where id is not null
            and id not in (select id from parent)
        ) > (select num from error_threshold)

{% endtest %}