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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20241016-160412.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Always validate an incremental model's `incremental_strategy`
time: 2024-10-16T16:04:12.58581-05:00
custom:
Author: QMalcolm
Issue: "330"
7 changes: 7 additions & 0 deletions dbt-tests-adapter/dbt/tests/adapter/basic/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@
{{ config(materialized="incremental") }}
"""

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

config_materialized_var = """
{{ config(materialized=var("materialized_var", "table"))}}
"""
Expand Down Expand Up @@ -217,3 +221,6 @@
ephemeral_view_sql = config_materialized_view + model_ephemeral
ephemeral_table_sql = config_materialized_table + model_ephemeral
incremental_sql = config_materialized_incremental + model_incremental
incremental_invalid_strategy_sql = (
config_materialized_incremental_invalid_strategy + model_incremental
)
39 changes: 39 additions & 0 deletions dbt-tests-adapter/dbt/tests/adapter/basic/test_incremental.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,45 @@ def test_incremental_not_schema_change(self, project):
assert run_result == RunStatus.Success


class BaseIncrementalBadStrategy:
@pytest.fixture(scope="class")
def project_config_update(self):
return {"name": "incremental"}

@pytest.fixture(scope="class")
def models(self):
return {
"incremental.sql": files.incremental_invalid_strategy_sql,
"schema.yml": files.schema_base_yml,
}

@pytest.fixture(scope="class")
def seeds(self):
return {"base.csv": files.seeds_base_csv, "added.csv": files.seeds_added_csv}

@pytest.fixture(autouse=True)
def clean_up(self, project):
yield
with project.adapter.connection_named("__test"):
relation = project.adapter.Relation.create(
database=project.database, schema=project.test_schema
)
project.adapter.drop_schema(relation)

def test_incremental_invalid_strategy(self, project):
# seed command
results = run_dbt(["seed"])
assert len(results) == 2

# try to run the incremental model, it should fail on the first attempt
results = run_dbt(["run"], expect_pass=False)
QMalcolm marked this conversation as resolved.
Show resolved Hide resolved
assert len(results.results) == 1
assert (
'dbt could not find an incremental strategy macro with the name "get_incremental_bad_strategy_sql"'
in results.results[0].message
)


class Testincremental(BaseIncremental):
pass

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@

{% set to_drop = [] %}

{% set incremental_strategy = config.get('incremental_strategy') or 'default' %}
{% set strategy_sql_macro_func = adapter.get_incremental_strategy_macro(context, incremental_strategy) %}

{% if existing_relation is none %}
{% set build_sql = get_create_table_as_sql(False, target_relation, sql) %}
{% elif full_refresh_mode %}
Expand All @@ -52,9 +55,7 @@
{% endif %}

{#-- Get the incremental_strategy, the macro to use for the strategy, and build the sql --#}
{% set incremental_strategy = config.get('incremental_strategy') or 'default' %}
{% set incremental_predicates = config.get('predicates', none) or config.get('incremental_predicates', none) %}
{% set strategy_sql_macro_func = adapter.get_incremental_strategy_macro(context, incremental_strategy) %}
{% set strategy_arg_dict = ({'target_relation': target_relation, 'temp_relation': temp_relation, 'unique_key': unique_key, 'dest_columns': dest_columns, 'incremental_predicates': incremental_predicates }) %}
{% set build_sql = strategy_sql_macro_func(strategy_arg_dict) %}

Expand Down
Loading