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.

Sign up for GitHub

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

Jump to bottom

Materialized Views Touch Ups #3818

Closed
1 task done
mikealfare opened this issue Jul 26, 2023 · 6 comments · Fixed by #3779
Closed
1 task done

Materialized Views Touch Ups #3818

mikealfare opened this issue Jul 26, 2023 · 6 comments · Fixed by #3779
Assignees
@matthewshaver
Labels
content Improvements or additions to content improvement Use this when an area of the docs needs improvement as it's currently unclear

Comments

@mikealfare
Copy link
Contributor

mikealfare commented Jul 26, 2023 •
edited
Loading

Contributions

  • I have read the contribution docs, and understand what's expected of me.

Link to the page on docs.getdbt.com requiring updates

https://docs.getdbt.com/reference/resource-configs/postgres-configs#materialized-view
https://docs.getdbt.com/reference/resource-configs/redshift-configs#materialized-view
https://docs.getdbt.com/reference/resource-configs/snowflake-configs

What part(s) of the page would you like to see updated?

dbt-postgres

  • on_configuration_change no longer has a value of skip, it was renamed to continue since skip already means something else in a very similar context
    • the skip bullet should be updated to continue and we should avoid the use of the word skipped
    • the key differentiation is that downstream models will still execute despite this model not running
  • indexes is an adapter-specific supported configuration; they work just like indexes on tables

dbt-redshift

  • similar to dbt-postgres, skip should be updated to on_configuration_change
  • auto_refresh is a new configuration specific to materialized views in dbt-redshift
    • it should be called out as a new configuration just like on_configuration_change; it's a boolean with default of False and corresponds to autorefresh on a Redshift materialized view
    • should it be included in the example code snippet?
  • backup, sort, sort_type, and dist are supported configurations; they work just like their counterparts on tables

dbt-snowflake

  • similar to dbt-postgres, skip should be updated to on_configuration_change
  • downstream is not currently supported for target_lag
  • warehouse should be snowflake_warehouse, which is an existing configuration that was reused for dynamic tables

Additional information

No response
@mikealfare mikealfare added content Improvements or additions to content improvement Use this when an area of the docs needs improvement as it's currently unclear labels Jul 26, 2023
@amychen1776
Copy link
Collaborator

I agree with auto_refresh being part of the code snippet for dbt-redshift. It's more user friendly this way.

In terms of Snowflake - we are following up with the patch pretty early on so I think it's okay to include on the docs with an *. Let's make sure to have a code snippet that includes in the on_configuration_change values that work.

@matthewshaver matthewshaver self-assigned this Jul 26, 2023
@mikealfare
Copy link
Contributor Author

mikealfare commented Jul 27, 2023 •
edited
Loading

we are following up with the patch pretty early on

The patch is ready for review. I anticipate getting it into the release without a patch.

Let's make sure to have a code snippet that includes in the on_configuration_change values that work.

The values of on_configuration_change are all the same. The attributes that can make use of this are target_lag and snowflake_warehouse.

@dataders dataders mentioned this issue Jul 31, 2023
Merged
7 tasks
@dataders
Copy link
Contributor

@mikealfare can you edit the above issue to reflect what isn't addressed in #3779?

@mikealfare
Copy link
Contributor Author

@dataders I updated the description to reflect the work from #3779. I also submitted a review of that PR that overlaps a bit with the information above. The biggest change is the update to dbt-snowflake. Now this is basically down to on_configuration_change=='continue' and mentioning supported adapter-specific parameters.

matthewshaver added a commit that referenced this issue Aug 1, 2023
@matthewshaver
update and detail MV and DT support (#3779)
dafc0ff 
## What are you changing in this pull request and why?
<!---
Describe your changes and why you're making them. If linked to an open
issue or a pull request on dbt Core, then link to them here! 

To learn more about the writing conventions used in the dbt Labs docs,
see the [Content style
guide](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/content-style-guide.md).
-->

resolves: #3494
resolves: #3818

- drop (for now) docs about BQ MVs
- added section on Dynamic Table support
- added limitation sections related to MV/DTs to postgres redshift and
snowflake

questions:
- is it our convention to describe configuration parameters when they
are describing already on the data platform docs? I'm thinking of
`target_lag` specifically. 👀 @matthewshaver

## Checklist
<!--
Uncomment if you're publishing docs for a prerelease version of dbt
(delete if not applicable):
- [ ] Add versioning components, as described in [Versioning
Docs](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/single-sourcing-content.md#versioning-entire-pages)
- [ ] Add a note to the prerelease version [Migration
Guide](https://github.com/dbt-labs/docs.getdbt.com/tree/current/website/docs/guides/migration/versions)
-->
- [ ] Review the [Content style
guide](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/content-style-guide.md)
and [About
versioning](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/single-sourcing-content.md#adding-a-new-version)
so my content adheres to these guidelines.
- [ ] Add a checklist item for anything that needs to happen before this
PR is merged, such as "needs technical review" or "change base branch."

Adding new pages (delete if not applicable):
- [ ] Add page to `website/sidebars.js`
- [ ] Provide a unique filename for the new page

Removing or renaming existing pages (delete if not applicable):
- [ ] Remove page from `website/sidebars.js`
- [ ] Add an entry `website/static/_redirects`
- [ ] [Ran link
testing](https://github.com/dbt-labs/docs.getdbt.com#running-the-cypress-tests-locally)
to update the links that point to the deleted page
@mikealfare
Copy link
Contributor Author

mikealfare commented Aug 3, 2023 •
edited
Loading

The docs that appear here don't have any of the changes in the write-up above. We're getting bug reports from users that dbt is not working according to the docs.

@mikealfare mikealfare reopened this Aug 3, 2023
@mikealfare
Copy link
Contributor Author

This is no longer necessary to be re-opened. I've submitted a separate PR to correct the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@matthewshaver matthewshaver

Labels
content Improvements or additions to content improvement Use this when an area of the docs needs improvement as it's currently unclear
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

update and detail MV and DT support dbt-labs/docs.getdbt.com
4 participants
@dataders @mikealfare @amychen1776 @matthewshaver