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

[CT-2861] [Bug] Changing the latest_version of a versioned model should mark all unpinned refs as state:modified #8189

Closed
2 tasks done
Tracked by #7979
joellabes opened this issue Jul 23, 2023 · 3 comments · Fixed by #8264
Closed
2 tasks done
Tracked by #7979
Assignees
Labels

Comments

@joellabes
Copy link
Contributor

joellabes commented Jul 23, 2023

Is this a new bug in dbt-core?

  • I believe this is a new bug in dbt-core
  • I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

I added a versioned model to our internal analytics project in https://github.com/dbt-labs/internal-analytics/pull/1766, which removed the org column from the below model. The new version was prerelease, so I marked v1 as the canonical version:

models:
  - name: rpt_demographics_by_department
    columns:
      ...
    versions:
      - v: 1
        columns:
          ...
        tests:
          - dbt_utils.unique_combination_of_columns:
              combination_of_columns:
                - date_month
                - org
                - technical_classification  
      - v: 2
        defined_in: rpt_demographics_by_department
        tests:
          - dbt_utils.unique_combination_of_columns:
              combination_of_columns:
                - date_month
                - cohort_name
                - technical_classification  

    latest_version: 1

In https://github.com/dbt-labs/internal-analytics/pull/1811, I deleted the final line of that YAML, which meant that v2 took precedence over v1.

All our Slim CI checks passed with flying colours. After merging, the next production run failed:

21:56:36  Database Error in test validate_dei_demographic_numbers (tests/validate_dei_demographic_numbers.sql)
21:56:36    000904 (42000): SQL compilation error: error line 20 at position 12
21:56:36    invalid identifier 'ORG'

because of this code block:

    {%- for col in summary_columns %}
        select 
            org,
            technical_classification,
            '{{ col }}' as demographic,
            {{ col }} as demographic_total_responses
        from {{ ref('rpt_demographics_by_department') }}
        where date_month = date_trunc(month, getdate())
       {% if not loop.last %} union all {% endif %}
    {% endfor %}

The fact that the org column isn't in v2 should have been called out in CI, and would have been if I'd done a dbt build without any modified selection.

Expected Behavior

Changing which version of a model is used by unpinned refs should be considered a modification in all calling files.

Steps To Reproduce

  1. Create and deploy a model with a breaking change, but don't mark it as the default immediately. (Also ensure you have some other model or test that queries the unpinned version of that model.)
  2. After deployment, update the yaml to make the new breaking version the latest version.

Relevant log output

No response

Environment

- OS:
- Python:
- dbt: 1.5.3

Which database adapter are you using with dbt?

No response

Additional Context

No response

@joellabes joellabes added bug Something isn't working triage labels Jul 23, 2023
@github-actions github-actions bot changed the title [Bug] Changing the latest_version of a versioned model should mark all unpinned refs as state:modified [CT-2861] [Bug] Changing the latest_version of a versioned model should mark all unpinned refs as state:modified Jul 23, 2023
@graciegoheen graciegoheen self-assigned this Jul 26, 2023
@graciegoheen
Copy link
Contributor

Confirmed with @MichelleArk that this looks like a bug! Thanks @joellabes for opening.

@MichelleArk
Copy link
Contributor

Couple notes from implementation:

Noticed that none of the other model node attributes participate in state:modified selection, which may be surprising behaviour. Confirmed with @jtcohen6 that expected behaviour of state:modified would be to pick up changes in access, latest_version, and deprecation_date.

This detection would be at the top-level state:modified method as opposed to within state:modified.config, since these attributes are not technically 'configs' and . and it does preserve, for end users, a way of using state:modified subselectors to ignore those attribute changes (since they don't technically affect whether a model build is going to succeed or fail in the DWH)

@MichelleArk
Copy link
Contributor

Fixed in #8264

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants