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

Use old_relation for renaming in default materializations #2161

Closed
jtcohen6 opened this issue Feb 26, 2020 · 1 comment · Fixed by #3547
Closed

Use old_relation for renaming in default materializations #2161

jtcohen6 opened this issue Feb 26, 2020 · 1 comment · Fixed by #3547
Labels
bug Something isn't working
Milestone

Comments

@jtcohen6
Copy link
Contributor

Describe the bug

In the default view and table materializations, dbt should rename old_relation to backup_relation, instead of renaming target_relation to backup_relation. The issue here is that, if old_relation.type != target_relation.type, dbt will run the wrong alter ... rename statement.

I think this is as simple as switching these to

  {% if old_relation is not none %}
    {{ adapter.rename_relation(old_relation, backup_relation) }}
  {% endif %}

Steps To Reproduce

  • Create + run a view model
  • Change config to table + rerun

This does not break on any of the core adapters:

  • Redshift and Postgres use alter ... rename, but they support alter table for both tables + views
  • Snowflake and BigQuery don't use alter ... rename, they instead use create or replace

It does break on Presto, which distinguishes between alter view and alter table. dbt will attempt to run alter table (because target_relation.type is table) against an existing view and return an error.

Expected behavior

dbt should use the type of the old relation when it attempts to rename it.

The output of dbt --version:

dbt --version
installed version: 0.15.2
   latest version: 0.15.2

Up to date!
@jtcohen6 jtcohen6 added bug Something isn't working triage labels Feb 26, 2020
@drewbanin drewbanin removed the triage label Feb 28, 2020
@drewbanin drewbanin added this to the Octavius Catto milestone Feb 28, 2020
@drewbanin
Copy link
Contributor

Thanks for the report @jtcohen6. We can get this fixed in a future release, but I'd advise addressing this in a materialization override in the presto plugin for the time being!

@drewbanin drewbanin removed this from the Octavius Catto milestone Feb 28, 2020
mpcarter added a commit to vertica/dbt-vertica that referenced this issue Mar 8, 2020
When you change materialization for deployed view to a table,
dbt attempts to rename original view with ALTER TABLE.
dbt-labs/dbt-core#2161
@jtcohen6 jtcohen6 added this to the Oh-Twenty-One milestone Jul 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants