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

Implement atomic swap for Snowflake #724

Closed
jtcohen6 opened this issue Apr 4, 2018 · 7 comments
Closed

Implement atomic swap for Snowflake #724

jtcohen6 opened this issue Apr 4, 2018 · 7 comments
Assignees
Labels
enhancement New feature or request snowflake

Comments

@jtcohen6
Copy link
Contributor

jtcohen6 commented Apr 4, 2018

Snowflake doesn't have a concept of transactions as Redshift does. If it succeeds in creating a MODEL_NAME__dbt_tmp view/table but encounters an error before renaming it to MODEL_NAME, the tmp view/table sticks around. The user must manually drop the tmp view/table, or the next run will fail (as below), in either case yielding a disruption to the workflow.

Snowflake error: 002002 (42710): SQL compilation error:
Object 'SCHEMA.MODEL_NAME__DBT_TMP' already exists.

This has been caused by failing post-hooks, which cannot currently be after-committed due to this issue.

A simple solution could be to replace

On MODEL: create view "SCHEMA"."MODEL__dbt_tmp" as (

with

On MODEL: create or replace view "SCHEMA"."MODEL__dbt_tmp" as (

but after a cursory look I'm not sure where that code is generated within dbt.

@cmcarthur
Copy link
Member

look into snowflake atomic swap. then instead of doing:

create temp, drop old, rename

do:

create temp, atomic swap, drop old

@jthandy
Copy link
Member

jthandy commented Jul 3, 2018

@cmcarthur It seems possible in the flow that you're suggesting that there could still be a leftover temporary table in certain error conditions. I had kind of assumed that we would need to explicitly check for the existence of such a temporary table in the materialization code and drop it if it existed. thoughts on that?

@drewbanin
Copy link
Contributor

@jthandy yeah, i think you're right. The more direct approach might be to use

create or replace table/view ...

as noted above by @jtcohen6

@cmcarthur
Copy link
Member

This may not be an issue right now, because of quoting changes and improvements to how we drop tmp relations before running models.

But there is a bigger issue around Snowflake transactions. We write materialization code assuming transactional DDL is possible. Snowflake does not have any transactional DDL. So while this code might work fine today, it would be easy for us to make a simple seeming change that actually creates major dbt bugs.

Kicking this out of 0.10.2 but we should review this later on.

@cmcarthur cmcarthur removed this from the 0.10.2 - Betsy Ross (unreleased) milestone Jul 3, 2018
@drewbanin drewbanin changed the title Snowflake: deal with __dbt_tmp if error before model rename Implement atomic swap for Snowflake Jul 19, 2018
@drewbanin drewbanin added the enhancement New feature or request label Jul 19, 2018
@drewbanin
Copy link
Contributor

We don't need to atomic swap ever on Snowflake, as the best option is to use create or replace {table|view}. If we do need to do a drop/swap in the future, we should use the atomic swap functionality. For the time being though, I don't think there's anything to do here. Going to close this.

@jthandy
Copy link
Member

jthandy commented Oct 23, 2018

@drewbanin is that what we're doing right now--create or replace (...)?

that's new-ish, right?

@drewbanin
Copy link
Contributor

@jthandy yeah - this is (relatively) new. I think we added it in 0.11.0, as we had tons of issues with Snowflake relations when switching models from table <--> view materializations. You can find more info about this in the PR #940

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request snowflake
Projects
None yet
Development

No branches or pull requests

4 participants