Fix adding new cols to check_cols in snapshots

[GtheSheep]

resolves #3146

Let me know if I need to open a new issue for this/ this can't be re-opened?
Also, does this require a test? Will add if so

Description

Adds new columns to a snapshot when a new entry is added to the check_cols arg with the check strategy

Checklist

• 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 added information about my change to be included in the CHANGELOG.

[GtheSheep]

The failing test seems to be a 1 second difference between 2 timestamps in tests/functional/simple_snapshot/test_hard_delete_snapshot.py just for one Python/ OS combo 😬

[gshank]

We've seen that test failure before; it's intermittent. I just opened a ticket for it. Will re-run tests to see if they pass

[gshank]

A test case would be great! We're in the process of switching to use pytest in our tests, so the new test should probably go in tests/functional/simple_snapshot.

[gshank]

Are you up for adding a test case?

[GtheSheep]

hey, yes ofc! Just haven't had a chance yet this week, will get to it in the next couple of days if that's cool?

[gshank]

Of course. We're wrapping up things to be included in the next minor release, so just checking in on people :)

[dbeatty10]

@GtheSheep See below for a functional test that reproduces the bug reported in #3146 -- you could utilize it to get started.

They might also ask you to add a check_relations_equal portion to verify that it also produces the intended result.

import pytest
from dbt.tests.util import run_dbt

snapshot_sql = """
{% snapshot snapshot_check_cols_new_column %}
    {{
        config(
            target_database=database,
            target_schema=schema,
            strategy='check',
            unique_key='id',
            check_cols=var("check_cols"),
        )
    }}

    {% if var('version') == 1 %}

        select 1 as id, 'foo' as name

    {% else %}

        select 1 as id, 'foo' as name, 'bar' as other

    {% endif %}

{% endsnapshot %}
"""


@pytest.fixture(scope="class")
def snapshots():
    return {"snapshot_check_cols_new_column.sql": snapshot_sql}


def test_simple_snapshot(project):
    """
    Test that snapshots using the "check" strategy and explicit check_cols support adding columns.

    Approach:
    1. Take a snapshot that checks a single non-id column
    2. Add a new column to the data
    3. Take a snapshot that checks the new non-id column too

    As long as no error is thrown, then the snapshot was successful
    """

    # Snapshot 1
    results = run_dbt(["snapshot", "--vars", "{version: 1, check_cols: ['name']}"])
    assert len(results) == 1

    # Snapshot 2
    results = run_dbt(["snapshot", "--vars", "{version: 2, check_cols: ['name', 'other']}"])
    assert len(results) == 1

[GtheSheep]

@dbeatty10 - Thanks for this, super helpful! I've taken a shot at writing a test but am having some issues with black in the pre-commit hooks, ImportError: cannot import name '_unicodefun' from 'click', bumping it's version doesn't seem to have done the trick either 😦 will give it a shot again tomorrow, thanks for your help!

[dbeatty10]

@GtheSheep The test is failing in CI, so I took a peek.

Is the test passing for you when you run the following command locally?

python -m pytest tests/functional/simple_snapshot/test_changing_check_cols_snapshot.py

In my local environment, I needed to use the following block of code to get it to pass:

expected_csv = """
id,name,other,dbt_scd_id,dbt_updated_at,dbt_valid_from,dbt_valid_to
1,foo,NULL,0d73ad1b216ad884c9f7395d799c912c,2016-07-01 00:00:00.000,2016-07-01 00:00:00.000,2016-07-02 00:00:00.000
1,foo,bar,7df3783934a6a707d51254859260b9ff,2016-07-02 00:00:00.000,2016-07-02 00:00:00.000,
""".lstrip()

The only differences are the two values for the dbt_scd_id column.

[GtheSheep]

@dbeatty10 yep! but I think this was some accidental copy paste pre-commit with the example you linked 😓

[gshank]

Looks good.