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

[Bug] _dbt_backup suffixed tables don't drop for dynamic tables #1047

Closed
2 tasks done
j-choe opened this issue May 16, 2024 · 7 comments
Closed
2 tasks done

[Bug] _dbt_backup suffixed tables don't drop for dynamic tables #1047

j-choe opened this issue May 16, 2024 · 7 comments
Labels
bug Something isn't working duplicate

Comments

@j-choe
Copy link

j-choe commented May 16, 2024

Is this a new bug in dbt-core?

  • I believe this is a new bug in dbt-core
  • I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

When creating tables, dbt does the following:

- starts a transaction
- creates the new version of the table in a temporary location (suffixed __dbt_tmp )
- renames the existing version of the table to be the backup version (suffixed __dbt_backup)
- renames the new version of the table (__dbt_tmp → no suffix)
- commits the transaction
- drops the old/backup version (on many databases, drops need to happen outside of transactions)

When a model utilizes dynamic_table as the materialization, it will create the dynamic table as normal, but does not drop the _dbt_backup suffixed table. Looking into dbt debug logs:
drop table if exists "DATABASE"."PUBLIC"."DYNAMIC_TABLE_NAME__dbt_backup" cascade

It seems like dbt is trying to drop a table instead of the renamed dynamic table. Consequently, when utilizing dbt build or dbt run, I'm left with 2 dynamic tables called dynamic_table_name and dynamic_table_name__dbt_backup

Expected Behavior

When running a command, I expect the _dbt_backup suffixed table to be dropped.

Steps To Reproduce

  1. Create a model materialized as "dynamic_table" as test_dynamic_table
  2. Run dbt build --select test_dynamic_table
  3. Run again as dbt build --select test_dynamic_table --full-refresh

Relevant log output

No response

Environment

- OS:
- Python:
- dbt: 1.8
- dbt-snowflake: 1.8.1

Which database adapter are you using with dbt?

snowflake

Additional Context

No response

@j-choe j-choe added bug Something isn't working triage labels May 16, 2024
@jtcohen6 jtcohen6 transferred this issue from dbt-labs/dbt-core May 16, 2024
@jtcohen6 jtcohen6 removed the triage label May 16, 2024
@jtcohen6
Copy link
Contributor

@j-choe Thanks for opening! I have a feeling this could be the same as:

@AartBluestoke
Copy link

AartBluestoke commented May 17, 2024

I have recently been affected by this - yes, it is to do with the rename macro. Generated code looks like

Note the rename doesn't name the database and schema of the target, while naming it when attempting the drop.

-- get the standard backup name
-- drop any pre-existing backup

drop table if exists "DEV_DATAMART"."LMG"."PROMOTIONS__dbt_backup" cascade;

alter table "DEV_DATAMART"."LMG"."PROMOTIONS" rename to PROMOTIONS__dbt_backup;

-- create follows

dbt-adapters\dbt\include\global_project\macros\relations\create_backup.sql passes the full current relation, but only the identifier for the backup relation to the renamer - i don't know if this is significant (ie, does "identifier" include the database and schema? )

{%- macro default__get_create_backup_sql(relation) -%}

    -- get the standard backup name
    {% set backup_relation = make_backup_relation(relation, relation.type) %}

    -- drop any pre-existing backup
    {{ get_drop_sql(backup_relation) }};

    {{ get_rename_sql(relation, backup_relation.identifier) }}

{%- endmacro -%}

@jtcohen6
Copy link
Contributor

@AartBluestoke Good catch. We could either:

  • update the default__get_create_backup_sql macro in dbt-adapters to pass a full Relation instead of just backup_relation.identifier as a string
  • create a new Relation within the get_rename macros that incorporates the new identifier, i.e. cea7f05

@AartBluestoke
Copy link

I would suggest the first.
first, passing a relation to 'get rename sql' makes quite a lot of sense - context matters quite a bit, name alone isn't enough - what if the rename wants to do something specific based on the data type?
second - when passed just a string, the rename says "you know better, lets rename this to exactly what you asked for"

@AartBluestoke
Copy link

@jtcohen6 - #1016 is substantially contributing to this bug

  • without that bug, a re-release of the same code under non-full release doesn't execute anything
  • without that bug, a re-release of the same code under full-release generates the following code:
    create or replace dynamic table dev_DATAMART.lmg.journal_items_wide

note: no preamble to backup the old version.
Question: why do we do the backup then replace on the case where the dynamic tables are changing when a single line create or replace would either succeed or fail, with no need to do potentially destructive move + create statements?
this results in temporary outages where neither the old or the new table are present...

@jtcohen6
Copy link
Contributor

@AartBluestoke We're aware that the Snowflake behavior change described in #1016 is the underlying issue (while this bug was the confounding issue that actually raised the error for users). You can see our next steps for resolving that bug directly:

I think we may need to support a type signature of get_rename_table_sql(relation: Relation, new_name: Union[str, Relation]) to avoid breaking changes — i.e. check the type of new_name to see if it's a Relation or a string, and operate on it accordingly.

@jtcohen6
Copy link
Contributor

jtcohen6 commented May 28, 2024

@jtcohen6 jtcohen6 closed this as not planned Won't fix, can't repro, duplicate, stale May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working duplicate
Projects
None yet
Development

No branches or pull requests

3 participants