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

Always ensure valid incremental strategy #331

Merged
merged 7 commits into from
Oct 18, 2024

Conversation

QMalcolm
Copy link
Contributor

resolves #330

Problem

Previously if you set an invalid incremental_strategy for an incremental model, it'll succeed on the first run, and then fail on the second run
I.e. if you have

{{config(materialized="incremental", incremental_strategy="bad_strategy")}}

select * from {{ ref("input_model") }}

{% if is_incremental() %}
where event_time >= (select coalesce(max(event_time),'1900-01-01') from {{ this }} )
{% endif %}

That was always succeeding when the model was either built for the first time or rebuilt via --full-refresh. You only got an error about the strategy the next time the model was run.

Solution

Change the order of operations in incremental.sql to ensure that a incremental model's incremental_strategy is always validated

Checklist

  • I have read the contributing guide and understand what's expected of me
  • 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
  • This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc.) or this PR has already received feedback and approval from Product or DX

Right now, this test will fail. That is because the fix will be in the
next commit. Presently if an incremental model is being built for the
first time (because the relation in the data warehouse doesn't exist)
or if the incremental model is being run as a full refresh, then validation
that the incremental strategy _doesn't happen_. Technically for _most_
incremental strategies, this is "okay" because the incremental strategy
isn't used when creating the relation for the first time or during full
refresh. The exception is the microbatch strategy, which is part of the
problem. The other part of the problem is that this leads to an experience
where if you write up an incremental model (with an invalid strategy) and
then run it to ensure it works, you won't know anything is wrong until it
is run for the second time.
QMalcolm added a commit to dbt-labs/dbt-postgres that referenced this pull request Oct 17, 2024
…rs dependencies

This is necessary because the changes we are making in dbt-adapters via dbt-labs/dbt-adapters#331
only get tested when run with an adapter. So this branch on dbt-postgres is not just
for ensuring the test is run, but also proof for the open PR in dbt-adapters that
the tests work. To show that though, we have to update the dependencies to temporarily
point at the dbt-adapters branch that has the work (because the work hasn't been merged
to main yet)
@QMalcolm QMalcolm requested a review from mikealfare October 17, 2024 17:03
@mikealfare mikealfare enabled auto-merge (squash) October 18, 2024 03:28
@mikealfare mikealfare merged commit 937c8c7 into main Oct 18, 2024
10 checks passed
@mikealfare mikealfare deleted the qmalcolm--330-always-ensure-valid-materialization branch October 18, 2024 03:45
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.

[Bug] On first-run/full-refresh the incremental strategy of a model isn't validated
2 participants