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

Ft Add materialization for Snowflake materialized view object type #181

Closed
wants to merge 39 commits into from

Conversation

leahtessem
Copy link

@leahtessem leahtessem commented Jun 28, 2022

Resolves #1162
This is an issue opened for dbt-core repository, but because the code I have written is specific to (and only tested on Snowflake), I feel that it fits in this repository. This PR (or parts of it) can be adapted and moved to live in dbt-core if desired.

Description

  • Adding materialization for Snowflake materialized views
    • including support for cluster_by config entry
    • Adding SnowflakeRelationType with materialized view type (including space)
    • add methods to SnowflakeRelation to determine if relation is MV
    • add get relation type method that includes new MV addition
  • Update snowflake__list_relations_without_caching macro to include materialized views
    • MVs do not return from show terse objects so it now has to be two calls (one for tables & one for views)
  • Also added macro for create or replace-ing materialized view similar to how the base view macro was built out in dbt-core

Handling switching between view types

  • Add handling in view materialization to drop the object if a materialized view exists with the desired view name (similar to how tables are handled in the create_or_replace_view macro in dbt-core)
  • Add onto incremental materialization to drop the MV if one exists with the desired incremental table name

Added MVs in the switching relation type tests as well as their own test file

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt-snowflake next" section.

@cla-bot cla-bot bot added the cla:yes label Jun 28, 2022
@leahtessem leahtessem changed the title Ft materialized views Ft Add materialization for Snowflake materialized view object type Jun 28, 2022
@leahtessem
Copy link
Author

Hi @dbt-labs/oss-maintainers, hoping to get a review here. I would like feedback on whether any of these changes should go to dbt-core instead and making sure that my tests are sufficient.

Getting this change merged in would allow my organization to have all of our snowflake objects managed in one place. Currently, our materialized views are managed in a way that is disjoint and prone to failure. It would also address issue dbt-labs/dbt-core#1162 and add the feature that has been desired since 2018.

Thanks!

@leahtessem leahtessem marked this pull request as draft August 17, 2022 15:48
@leahtessem leahtessem marked this pull request as ready for review August 17, 2022 15:48
@jtcohen6 jtcohen6 self-assigned this Aug 17, 2022
@jtcohen6
Copy link
Contributor

@leahtessem Thanks for the PR, and sorry for the delay!

I owe you a response here. To make a long story short, I think we're unlikely to merge this into dbt-core / adapters for native support — at least for now. For the last few years, we've kept "experimental" implementations of materialized views in this repo: https://github.com/dbt-labs/dbt-labs-experimental-features/tree/main/materialized-views

I'd like to say more about that, though. It's always worth reexamining our long-standing assumptions. I'll plan to swing back in the next few days with a fuller explanation.

@Fleid
Copy link
Contributor

Fleid commented Feb 1, 2023

Closing as my current thinking is that we won't support Snowflake's Materialized Views, but Dynamic Tables instead.

Thanks a lot for your contribution, it will inspire our work going forward on that topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Materialized Views
3 participants