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
feat: add support for materialized views #408
feat: add support for materialized views #408
Conversation
Here is the summary of changes. You added 6 region tags.
This comment is generated by snippet-bot.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor naming ruminations, and a paste edit for the license header.
Thanks for putting this together!
google/cloud/bigquery/table.py
Outdated
"partitioning_type": "timePartitioning", | ||
"external_data_configuration": "externalDataConfiguration", | ||
"friendly_name": "friendlyName", | ||
"mv_enable_refresh": "materializedView", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we can use a better prefix? For logical views we're already using view_
prefix, maybe mview or matview? My worry that mv is too terse here and/or people will overlook them in favor of the logical properties.
google/cloud/bigquery/table.py
Outdated
|
||
@mv_query.deleter | ||
def mv_query(self): | ||
"""Delete SQL query defining the table as a view.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: s/view/materialized view/ since we're dealing with multiple kinds now.
|
||
|
||
def alter_materialized_view(override_values={}): | ||
# [START bigquery_alter_materialized_view] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This tag makes me itchy, and expecting a DDL style ALTER MATERIALIZED VIEW statement rather than an api update call. Should we correct the tag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, corrected to bigquery_update_materialized_view
.
The corresponding docs section has room for both. https://cloud.google.com/bigquery/docs/materialized-views#alter For example the bq
table shows bq query
with DDL and bq update
options.
@@ -0,0 +1,93 @@ | |||
# Copyright 2016 Google Inc. All Rights Reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2020?
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #407🦕