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

Feature/incremental predicates #78

Closed
wants to merge 2 commits into from

Conversation

dave-connors-3
Copy link
Contributor

resolves dbt-core issue #3923 for snowflake users

Description

This PR enables users to supply arbitrary filters on their incremental models to reduce the table scan resources when using Snowflake.

-- sample config

{{
    config(
        incremental_strategy='merge',
        materialized='incremental',
        unique_key = 'cc_call_center_sk',
        incremental_predicates = [{
            "source_col":"cc_call_center_id",
            "expression":"like 'AAA%'"
        },
        {
            "source_col":"cc_call_center_id",
            "expression":"not like 'BBB%'"
        }]
   

Log output:

23:05:58.503176 [debug] [Thread-1  ]: On model.staging_party.sample_callcenter: merge into analytics.dbt_dconnors.sample_callcenter as DBT_INTERNAL_DEST
        using analytics.dbt_dconnors.sample_callcenter__dbt_tmp as DBT_INTERNAL_SOURCE
        on 
    DBT_INTERNAL_SOURCE.cc_call_center_sk = DBT_INTERNAL_DEST.cc_call_center_sk
    and DBT_INTERNAL_DEST.cc_call_center_id like 'AAA%' and DBT_INTERNAL_DEST.cc_call_center_id not like 'BBB%' 

TODO before merge:
[ ] Merge dbt-core PR#4546

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 updated the CHANGELOG.md and added information about my change to the "dbt-snowflake next" section.

@cla-bot cla-bot bot added the cla:yes label Jan 3, 2022
@VersusFacit
Copy link
Contributor

VersusFacit commented Jan 20, 2022

Oh, hi there, the™️ @dave-connors-3 👋

Love the contribution, and I kind of want to resuscitate this at a leisurely pace, since it seems there's some sizable work already in the hopper here.

  • Mind updating the Readme to link to the relevant issue? I had trouble locating it myself. I wonder if it was closed 🤔 found it hiding in Core!
  • a changelog entry (one sentence) would be nice too!
  • any additional context outside the original issue that's needed before trying to merge these through?

Get those things there, and I can see about reviewing this.

Edit: Dave and I chatted it up. They'll be leisurely keeping up on this. Just wanted to log that's its still in the works at least round this time

@dave-connors-3
Copy link
Contributor Author

hey @VersusFacit ! i think we can close this PR, and take it up after The Great Materialization Refactor work currently going on

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

Successfully merging this pull request may close these issues.

2 participants