feat: Allow custom schema def for tmp tables generated by incremental #659

pierrebzl · 2024-05-24T20:06:40Z

Description

As mentioned in #613, I would like for all the temporary intermediate tables to be created in a different schema that we could specify as a config argument of the model.

This would enable us to control and limit access or visibility of those tmp tables.
In some cases of parallel processing using unique_tmp_table_suffix flag, we end up creating a lot of those transient tables and we would like to avoid them to appear under the production/staging schemas of our glue catalog.

Models used to test - Optional

Checklist

You followed contributing section
You kept your Pull Request small and focused on a single feature or bug fix.
You added unit testing when necessary
You added functional testing when necessary

nicor88 · 2024-05-25T09:01:02Z

Few notes/consideration:

the integration tests failed because of an issue with snapshots, I retriggered the ci, but if the issue persist must be addressed. Edit: the issue persist, please have a look
I'm wondering if such property could apply to materialized tables. There are few cases where tmp tables are created:
- in case of hive ha tables
- in case we have more than 100 partitions
- in case of iceberg the initial table is a tmp table, then renamed - that is the only case where there is an extra layer of complexity, because we need to guarantee that the final S3 layout is properly specified

tests/functional/adapter/test_incremental_tmp_schema.py

dbt/adapters/athena/impl.py

…le-in-tmp-chemas

pierrebzl · 2024-05-27T20:23:15Z

Thank you for your feedback.

I'm wondering if such property could apply to materialized tables
Yes, I think it definitely could be by adjusting the table materialization macro.
I was thinking to add this

  {% set tmp_schema = config.get('tmp_schema') %}

  {%- if tmp_schema is not none -%}
    {%- set tmp_schema = tmp_schema -%}
  {% else %}
    {%- set tmp_schema = schema -%}
  {%- endif -%}

then pass schema=tmp_schema to api.Relation.create( https://github.com/dbt-athena/dbt-athena/blob/v1.8.1/dbt/include/athena/macros/materializations/models/table/table.sql#L30
Maybe it can be part of a separate PR, what do you think?

I would need to explore more, I'm not very familiar with all the different use case of materialized table you mentioned.
Also this would require to write new tests.

nicor88 · 2024-05-27T20:45:23Z

Let's keep table materialization out, and even consider to use another issue to track that idea :)

dbt/include/athena/macros/materializations/models/incremental/incremental.sql

dbt/adapters/athena/impl.py

nicor88 · 2024-05-27T20:51:59Z

@pierrebzl looks good, few nits, also let's consider to use temp_schema as model parameter for consistency with the code implementation, thanks

nicor88

Great job 💯

This PR contains the following updates: | Package | Update | Change | |---|---|---| | [dbt-athena-community](https://togithub.com/dbt-athena/dbt-athena) | minor | `==1.7.2` -> `==1.8.2` | --- ### Release Notes <details> <summary>dbt-athena/dbt-athena (dbt-athena-community)</summary> ### [`v1.8.2`](https://togithub.com/dbt-athena/dbt-athena/releases/tag/v1.8.2) [Compare Source](https://togithub.com/dbt-athena/dbt-athena/compare/v1.8.1...v1.8.2) ### What's Changed #### Fixes - fix: Add wait_random_exponential for query retries by [@svdimchenko](https://togithub.com/svdimchenko) in [https://github.com/dbt-athena/dbt-athena/pull/655](https://togithub.com/dbt-athena/dbt-athena/pull/655) - fix: Resolve error when cloning Python models ([#645](https://togithub.com/dbt-athena/dbt-athena/issues/645)) by [@jeancochrane](https://togithub.com/jeancochrane) in [https://github.com/dbt-athena/dbt-athena/pull/651](https://togithub.com/dbt-athena/dbt-athena/pull/651) - fix: Fixed table_type for GOVERNED tables by [@svdimchenko](https://togithub.com/svdimchenko) in [https://github.com/dbt-athena/dbt-athena/pull/661](https://togithub.com/dbt-athena/dbt-athena/pull/661) #### Features - feat: Set unique table suffix to allow parallel incremental executions by [@pierrebzl](https://togithub.com/pierrebzl) in [https://github.com/dbt-athena/dbt-athena/pull/650](https://togithub.com/dbt-athena/dbt-athena/pull/650) - feat: Allow custom schema def for tmp tables generated by incremental by [@pierrebzl](https://togithub.com/pierrebzl) in [https://github.com/dbt-athena/dbt-athena/pull/659](https://togithub.com/dbt-athena/dbt-athena/pull/659) - feat: Implement iceberg retry logic by [@svdimchenko](https://togithub.com/svdimchenko) in [https://github.com/dbt-athena/dbt-athena/pull/657](https://togithub.com/dbt-athena/dbt-athena/pull/657) #### Dependencies - chore: Update moto requirement from ~=5.0.7 to ~=5.0.8 by [@dependabot](https://togithub.com/dependabot) in [https://github.com/dbt-athena/dbt-athena/pull/660](https://togithub.com/dbt-athena/dbt-athena/pull/660) - chore: Bumped version to 1.8.2 for release by [@svdimchenko](https://togithub.com/svdimchenko) in [https://github.com/dbt-athena/dbt-athena/pull/663](https://togithub.com/dbt-athena/dbt-athena/pull/663) #### Docs - docs: Cleanup README grammar, punctuation, and capitalisation by [@dfsnow](https://togithub.com/dfsnow) in [https://github.com/dbt-athena/dbt-athena/pull/654](https://togithub.com/dbt-athena/dbt-athena/pull/654) #### New Contributors - [@jeancochrane](https://togithub.com/jeancochrane) made their first contribution in [https://github.com/dbt-athena/dbt-athena/pull/651](https://togithub.com/dbt-athena/dbt-athena/pull/651) - [@pierrebzl](https://togithub.com/pierrebzl) made their first contribution in [https://github.com/dbt-athena/dbt-athena/pull/650](https://togithub.com/dbt-athena/dbt-athena/pull/650) **Full Changelog**: dbt-labs/dbt-athena@v1.8.1...v1.8.2 ### [`v1.8.1`](https://togithub.com/dbt-athena/dbt-athena/releases/tag/v1.8.1) [Compare Source](https://togithub.com/dbt-athena/dbt-athena/compare/v1.7.2...v1.8.1) #### What's Changed ##### Relevant notes ⚠️ 1.8.1 version is equivalent to 1.8.0 in term of features and fixes. You can install the changes from this release via pip install dbt-athena-community==1.8.1 ##### Features - feat: Add column meta to glue column parameters by [@SoumayaMauthoorMOJ](https://togithub.com/SoumayaMauthoorMOJ) in [https://github.com/dbt-athena/dbt-athena/pull/644](https://togithub.com/dbt-athena/dbt-athena/pull/644) ##### Dependencies - chore: Update moto requirement from ~=5.0.6 to ~=5.0.7 by [@dependabot](https://togithub.com/dependabot) in [https://github.com/dbt-athena/dbt-athena/pull/648](https://togithub.com/dbt-athena/dbt-athena/pull/648) ##### Docs - docs: Cleanup Python models section of README by [@dfsnow](https://togithub.com/dfsnow) in [https://github.com/dbt-athena/dbt-athena/pull/643](https://togithub.com/dbt-athena/dbt-athena/pull/643) #### New Contributors - [@dfsnow](https://togithub.com/dfsnow) made their first contribution in [https://github.com/dbt-athena/dbt-athena/pull/643](https://togithub.com/dbt-athena/dbt-athena/pull/643) </details> --- ### Configuration 📅 **Schedule**: Branch creation - "before 4am on the first day of the month" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://togithub.com/renovatebot/renovate).

pierrebzl added 2 commits May 24, 2024 15:31

refactor: alllow to reuse functions to parse dbt test run output

9b44e57

feat: allow custom schema def for tmp tables generated by incremental

2eb788e

pierrebzl marked this pull request as ready for review May 24, 2024 20:24

pierrebzl requested review from jessedobbelaere, Jrmyy, mattiamatrix, nicor88 and svdimchenko as code owners May 24, 2024 20:24

nicor88 added the enable-functional-tests Label to trigger functional testing label May 25, 2024

nicor88 reviewed May 25, 2024

View reviewed changes

tests/functional/adapter/test_incremental_tmp_schema.py Outdated Show resolved Hide resolved

nicor88 reviewed May 25, 2024

View reviewed changes

dbt/adapters/athena/impl.py Outdated Show resolved Hide resolved

pierrebzl added 3 commits May 27, 2024 15:55

fix: adjust macro name, create schema if not exist

d90b20a

Merge remote-tracking branch 'upstream/main' into tmp-incremental-tab…

788abf5

…le-in-tmp-chemas

fix: readme linting

8165167

nicor88 reviewed May 27, 2024

View reviewed changes

dbt/include/athena/macros/materializations/models/incremental/incremental.sql Outdated Show resolved Hide resolved

nicor88 reviewed May 27, 2024

View reviewed changes

dbt/adapters/athena/impl.py Outdated Show resolved Hide resolved

fix: rename arg to temp_schema and fix macro name

18493a2

pierrebzl mentioned this pull request May 28, 2024

[Feature] Allow to define a different schema for tmp tables created during table materialization #662

Open

1 task

Merge branch 'main' into tmp-incremental-tables-in-tmp-schema

165a3bc

nicor88 approved these changes May 28, 2024

View reviewed changes

nicor88 added enable-functional-tests Label to trigger functional testing and removed enable-functional-tests Label to trigger functional testing labels May 28, 2024

nicor88 merged commit 97430f9 into dbt-labs:main May 28, 2024
11 of 13 checks passed

nicor88 mentioned this pull request May 28, 2024

[Feature] Control glue database/schema for tmp tables generated by incremental models #613

Closed

1 task

pierrebzl mentioned this pull request Jun 16, 2024

feat: Allow different schema for tmp tables created during table materialization #664

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Allow custom schema def for tmp tables generated by incremental #659

feat: Allow custom schema def for tmp tables generated by incremental #659

pierrebzl commented May 24, 2024 •

edited

Loading

nicor88 commented May 25, 2024 •

edited

Loading

pierrebzl commented May 27, 2024 •

edited

Loading

nicor88 commented May 27, 2024

nicor88 commented May 27, 2024

nicor88 left a comment

feat: Allow custom schema def for tmp tables generated by incremental #659

feat: Allow custom schema def for tmp tables generated by incremental #659

Conversation

pierrebzl commented May 24, 2024 • edited Loading

Description

Models used to test - Optional

Checklist

nicor88 commented May 25, 2024 • edited Loading

pierrebzl commented May 27, 2024 • edited Loading

nicor88 commented May 27, 2024

nicor88 commented May 27, 2024

nicor88 left a comment

Choose a reason for hiding this comment

pierrebzl commented May 24, 2024 •

edited

Loading

nicor88 commented May 25, 2024 •

edited

Loading

pierrebzl commented May 27, 2024 •

edited

Loading