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

Exclude "distinct from" from hard coded references flag #411

Merged
merged 3 commits into from
Jan 22, 2024

Conversation

graciegoheen
Copy link
Collaborator

@graciegoheen graciegoheen commented Jan 4, 2024

This is a:

  • bug fix PR with no breaking changes
  • new functionality

Link to Issue

Closes #338

Description & motivation

SQL that uses is distinct from or is not distinct from is being incorrectly flagged as a hard coded reference.

Example:

    select
        old.* exclude (fact_effective_thru, fact_history_is_current) 
        ,current_timestamp() as fact_effective_thru
        ,false as fact_history_is_current
    from
        {{ref(old_state)}} as old
    inner join {{ref(new_state)}} as new
        using(fact_durable_key)
    where
        old.ALL_SOURCE_ATTRIBUTES is distinct from new.ALL_SOURCE_ATTRIBUTES

new.ALL_SOURCE_ATTRIBUTES is being flagged.

I added a negative lookback for distinct to all regex checks.

Integration Test Screenshot

Screenshot 2024-01-04 at 3 55 43 PM

Checklist

  • I have verified that these changes work locally on the following warehouses (Note: it's okay if you do not have access to all warehouses, this helps us understand what has been covered)
    • BigQuery
    • Postgres
    • Redshift
    • Snowflake
    • Databricks
    • DuckDB
    • Trino/Starburst
  • I have updated the README.md (if applicable)
  • I have added tests & descriptions to my models (and macros if applicable)

@graciegoheen graciegoheen changed the title Exclude "is distinct from" from hard coded references flag Exclude "distinct from" from hard coded references flag Jan 4, 2024
@b-per
Copy link
Collaborator

b-per commented Jan 8, 2024

Yeahh, regexes to review!

@dave-connors-3
Copy link
Collaborator

@graciegoheen did you make changes to the seeds at all to catch this new model?

@graciegoheen
Copy link
Collaborator Author

@dave-connors-3 No just added this to int_model_5:

select 1 as id
-- from {{ ref('int_model_4') }}
-- inner join {{ ref('stg_model_1') }}
-- on int_model_4.join_field = stg_model_1.join_field
-- where int_model_4.id is distinct from stg_model_1.id

@dave-connors-3
Copy link
Collaborator

@graciegoheen ah i see i guess we would indeed not expect a new violation there lol

select
*
from {{ ref('fct_model_9') }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this edit needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no lol

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh come on

@graciegoheen graciegoheen merged commit d9b3209 into main Jan 22, 2024
6 of 7 checks passed
@graciegoheen graciegoheen deleted the is_distinct_from branch January 22, 2024 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants