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 SWAP instead of RENAME for table materialisation on Snowflake #1100

Closed

Conversation

danimcgoo
Copy link

ALTER TABLE REAME is DDL and commits an open transaction. This creates the opportunity for errors to arise if a connection attempts to access the target table, between the renaming of intermediate table and the target table, and before permissions are granted. Snowflake provides an ALTER TABLE SWAP function to allow atomic table swaps.

  • Add snowflake__swap_table macro
  • Update Snowflake table materialisation to use swap table

- Add snowflake__swap_table macro
- Update Snowflake table materialisation to use swap table
@danimcgoo
Copy link
Author

This is a work in progress commit. I haven't worked much with DBT so would like feedback on whether my approach is appropriate.

I considered adding swap_table as a method to the Snowflake Python adapter, but it seems specific to Snowflake so I'm not sure if that makes sense.

cc - @clrcrl

@drewbanin
Copy link
Contributor

Hi @danimcgoo! We had an issue for this (#724) which I recently closed. Our plan is to change the Snowflake table materialization to use the create or replace table ... syntax, which should atomically replace the destination model. We didn't have an issue for this, so I just made one here: #1101

I think your idea is a good one, and it's largely how I imagined solving this problem until we experimented with create or replace view. I think you did a really nice job with this PR :)

Do you buy all of this ^ ?

@danimcgoo
Copy link
Author

Hey @drewbanin, sounds good to me. Same outcome, less code. If the COPY GRANTS option is used, the permissions will carry across, which is perfect.

Do you have a timeline on making the change?

@drewbanin
Copy link
Contributor

That issue isn't currently prioritized, but we are having a deep think about how to make materializations nicer. They're pretty unruly at the moment, and I think a couple of key abstractions will make these materializations more readable/testable/auditable. I think it might be on the order of weeks/months before we turn this around.

If you're in a pinch, you can always copy your updated materialization to a .sql file in your project's macros/ dir, then change the name to something like snowflake_view. Then, you could refer to your custom materialization in your models! More info on this here: https://docs.getdbt.com/docs/creating-new-materializations

@danimcgoo
Copy link
Author

That sounds great. Thanks for the feedback @drewbanin.

@danimcgoo danimcgoo closed this Oct 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants