feat(athena): add build_strategy config for incremental and table materializations - #1830
feat(athena): add build_strategy config for incremental and table materializations#1830dtaniwaki wants to merge 10 commits into
Conversation
d1d5a19 to
4be0baa
Compare
Signed-off-by: Daisuke Taniwaki <daisuketaniwaki@gmail.com>
4be0baa to
5bd01f8
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new Athena adapter config (build_with_subquery) to avoid staging incremental model results into a physical __dbt_tmp CTAS table for append and Iceberg merge, reducing S3 write/read overhead and scan cost by using the compiled model SQL as a subquery instead.
Changes:
- Introduces
build_with_subqueryadapter config and materialization logic to useUSING ({{ compiled_code }})/FROM ({{ compiled_code }})while still creating an empty tmp table for schema comparison. - Adds guardrails/errors for unsupported combinations (Python models,
force_batch,insert_overwrite) and forTOO_MANY_OPEN_PARTITIONSwhen subquery mode is enabled. - Adds unit + functional tests covering SQL generation and end-to-end behavior for merge/append and incompatibility with
force_batch.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| dbt-athena/src/dbt/adapters/athena/impl.py | Adds build_with_subquery to AthenaConfig and documents intended behavior. |
| dbt-athena/src/dbt/include/athena/macros/materializations/models/incremental/incremental.sql | Reads build_with_subquery and wires subquery mode into append + Iceberg merge paths (empty CTAS for schema comparison). |
| dbt-athena/src/dbt/include/athena/macros/materializations/models/incremental/helpers.sql | Extends incremental_insert to optionally insert from a subquery and error on open partitions in subquery mode. |
| dbt-athena/src/dbt/include/athena/macros/materializations/models/incremental/merge.sql | Extends iceberg_merge to optionally USING (subquery) and error on open partitions in subquery mode. |
| dbt-athena/tests/unit/test_config.py | Adds unit coverage for AthenaConfig default/override of build_with_subquery. |
| dbt-athena/tests/unit/test_build_with_subquery.py | Adds unit tests validating generated SQL for insert/merge and open-partitions behavior. |
| dbt-athena/tests/functional/adapter/test_build_with_subquery.py | Adds functional coverage for Iceberg merge/append and Hive append plus force_batch incompatibility. |
| dbt-athena/.changes/unreleased/Features-20260406-225458.yaml | Adds changelog entry for the new feature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {% if force_batch %} | ||
| {% do exceptions.raise_compiler_error('build_with_subquery is incompatible with force_batch. Batching requires data in the tmp table.') %} | ||
| {% endif %} | ||
| {% if strategy == 'insert_overwrite' %} | ||
| {% do exceptions.raise_compiler_error('build_with_subquery is not supported with insert_overwrite strategy.') %} | ||
| {% endif %} |
There was a problem hiding this comment.
The build_with_subquery validation block runs unconditionally, which will raise a compiler error even on --full-refresh runs (e.g., when build_with_subquery=True and force_batch=True). The new functional tests expect full-refresh to succeed and only the incremental run to fail, and it’s also reasonable for full-refresh to ignore this incremental-only optimization. Consider gating these checks to only apply on incremental runs (e.g., when existing_relation is not none and not should_full_refresh()), or update the tests/behavior consistently.
| {% if force_batch %} | |
| {% do exceptions.raise_compiler_error('build_with_subquery is incompatible with force_batch. Batching requires data in the tmp table.') %} | |
| {% endif %} | |
| {% if strategy == 'insert_overwrite' %} | |
| {% do exceptions.raise_compiler_error('build_with_subquery is not supported with insert_overwrite strategy.') %} | |
| {% endif %} | |
| {% if existing_relation is not none and not should_full_refresh() %} | |
| {% if force_batch %} | |
| {% do exceptions.raise_compiler_error('build_with_subquery is incompatible with force_batch. Batching requires data in the tmp table.') %} | |
| {% endif %} | |
| {% if strategy == 'insert_overwrite' %} | |
| {% do exceptions.raise_compiler_error('build_with_subquery is not supported with insert_overwrite strategy.') %} | |
| {% endif %} | |
| {% endif %} |
There was a problem hiding this comment.
I think there're pros and cons in ignoring the error in the full-refresh phase. The error will be eventually raised in the incremental phase, so we should catch it earlier.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
iconara
left a comment
There was a problem hiding this comment.
Nice to see this being implemented, I think it will be appreciated.
| {% if old_tmp_relation is not none %} | ||
| {% do drop_relation(old_tmp_relation) %} | ||
| {% endif %} | ||
| {%- set empty_sql = 'SELECT * FROM (' ~ compiled_code ~ ') _dbt_sbq WITH NO DATA' -%} |
There was a problem hiding this comment.
Is there any chance that compiled_code could end with a comment? I assume that this could happen if for example the model's last line has a comment. In that case a newline is needed before the closing paragraph to avoid the rest of the statement ending up inside of the comment.
| {% if old_tmp_relation is not none %} | ||
| {% do drop_relation(old_tmp_relation) %} | ||
| {% endif %} | ||
| {%- set empty_sql = 'SELECT * FROM (' ~ compiled_code ~ ') _dbt_sbq WITH NO DATA' -%} |
There was a problem hiding this comment.
Same here, a newline before the closing parenthesis is needed if there is a possibility that compiled_code could end with a comment.
Signed-off-by: Daisuke Taniwaki <daisuketaniwaki@gmail.com>
…style Signed-off-by: Daisuke Taniwaki <daisuketaniwaki@gmail.com>
0b73fd6 to
c2217e9
Compare
…ormalization Signed-off-by: Daisuke Taniwaki <daisuketaniwaki@gmail.com>
PR dbt-labs#1830/dbt-labs#1832 のテストが fork #3 で追加された disable_batch_fallback 引数を MockAdapter.run_query_with_partitions_limit_catching に含んでいなかった。
|
@colin-rogers-dbt @iconara Could you review this PR? I think direct subquery insert also solves many situations of HIVE_TOO_MANY_PARTITIONS and ICEBERG_TOO_MANY_OPEN_PARTITIONS. |
iconara
left a comment
There was a problem hiding this comment.
I'm not familiar enough with the way the jinja templates for CTAS/INSERT/MERGE worked before to say anything about that part, but based on the tests I think this is sound. I left a philosophical comment about naming, and how looking at this from an outside perspective, this new way of handling incremental loads is probably how most would expect it to work by default, and the config naming should reflect that (the current default behavior needs to stay the current default, but naming can be used to signal what the "basic" or simplest behavior is).
| build_with_subquery: Use a subquery directly instead of staging data into __dbt_tmp. | ||
| Creates an empty tmp table (WITH NO DATA) for schema comparison, then applies via subquery. | ||
| Supported for Iceberg merge and append strategies, and Hive append. Incompatible with force_batch. | ||
| """ |
There was a problem hiding this comment.
If you aren't familiar with dbt-athena since before this change, you may be confused by the naming of this property. The name is a response to the current default, but the current default is actually not the default behavior that you would expect.
It's natural to add a feature with a config that enables it, but I'm thinking that in this case we are adding something that probably should have been the default mode, and it's only for historical reasons it ended up being added later. At least to me, using a temp table is the alternative way of doing this.
One way to convey this is to use a config that is not true or false, but enables one of two (or more) modes, like incremental_strategy with values "subquery" or "tmp_table" (default).
There was a problem hiding this comment.
That sounds reasonable. incremental_strategy is the same as dbt-core's config like incremental_strategy: insert_overwrite. How about build_strategy?
There was a problem hiding this comment.
What do you think about applying this mode to table materialization? This behavior benefits all the materialization types rather than the incremental materialization.
I made another PR over this PR in dtaniwaki#12.
There was a problem hiding this comment.
build_strategy sounds good to me, that works for table materialization too.
| {% do exceptions.raise_compiler_error('build_with_subquery is not supported with Python models.') %} | ||
| {% endif %} | ||
| {% if force_batch %} | ||
| {% do exceptions.raise_compiler_error('build_with_subquery is incompatible with force_batch. Batching requires data in the tmp table.') %} |
There was a problem hiding this comment.
Nitpick: I would say "Batching requires a temp table"
resolves #1829
docs dbt-labs/docs.getdbt.com/#
Problem
Athena incremental models always stage data into a
__dbt_tmptable via CTAS before applying MERGE or INSERT. The tmp table exists to support automatic batching whenTOO_MANY_OPEN_PARTITIONSoccurs, but many models never hit this limit. For these models, the tmp table is pure overhead — unnecessary S3 writes, doubled scan costs, and added latency.Solution
Add
build_strategyconfig option to control how a model's source SQL is materialized. The defaulttmp_tablepreserves today's behavior;subqueryskips the full CTAS and uses the compiled SQL directly as a subquery.CTAS ... WITH NO DATA(zero data scan) preserves theprocess_schema_changesflow, soon_schema_changeis fully supportedMERGE INTO target USING (subquery) AS src ON ...INSERT INTO target SELECT ... FROM (subquery)CREATE TABLE AS subqueryskips the intermediate tmp tableExample configuration:
Constraints:
subqueryis incompatible withforce_batch— batching requires physical tmp table partition metadatasubqueryis not supported with Python modelssubquery,TOO_MANY_OPEN_PARTITIONSraises an error instead of auto-batching (clear message guides user to switch back totmp_table)Test infrastructure (Terraform)
The functional tests in
tests/functional/adapter/test_build_strategy.pyexercise every combination ofbuild_strategy× incremental strategy × table type against a real Athena account. The Terraform below provisions the minimal AWS resources needed.Terraform (click to expand)
Checklist