-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add WHEN STALE option to CREATE MATERIALIZED VIEW
#27356
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
base: master
Are you sure you want to change the base?
Conversation
This is a preparatory change that allows connectors to store information about the WHEN STALE behavior. The option cannot yet be used in practice, as execution will still fail with a NOT_SUPPORTED error.
Previously, analysis was performed using AllowAllAccessControl, so the test setup was ignored.
Behavior: - `FAIL` - If the MV is stale, queries referencing it will fail. Analysis of the underlying query is never performed while querying the MV. - `INLINE` - Preserves the current behavior. Even if the MV is stale, it is expanded like a logical view, and the underlying query is analyzed. Motivation: - Avoid analyzing the underlying query every time, which can be costly when it references many tables. - Allow using fresh MVs even when some data sources are temporarily unavailable.
| TableHandle tableHandle = metadata.getTableHandle(session, storageTableName) | ||
| .orElseThrow(() -> semanticException(INVALID_VIEW, table, "Storage table '%s' does not exist", storageTableName)); | ||
| return createScopeForMaterializedView(table, name, scope, materializedViewDefinition, Optional.of(tableHandle)); | ||
| return createScopeForMaterializedView(table, name, scope, materializedViewDefinition, Optional.of(tableHandle), useLogicalViewSemantics); |
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'm a bit concerned about this line: the new clause affects the path for fresh MVs (WHEN STALE FAIL skips analysis, while WHEN STALE INLINE runs the analysis), even though the syntax suggests it should only define behavior for stale MVs.
Is analysis of the underlying query even necessary for fresh MVs, regardless of the WHEN STALE clause? I understand that it can detect schema changes in the base tables or changes to the MV definer’s access permissions - but is that the intended behavior?
|
What is our plan on changing this behaviour for existing or new MVs ? |
| if (!plannerContext.getMetadata().getConnectorCapabilities(session, catalogHandle).contains(MATERIALIZED_VIEW_WHEN_STALE_BEHAVIOR)) { | ||
| throw semanticException(NOT_SUPPORTED, statement, "Catalog '%s' does not support WHEN STALE", catalogName); | ||
| } |
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.
Should catalogs need to handle anything in order to support it ? If it is some sort of MV property (like security mode in case of views) - It should be supported for all connectors supporting MVs right ? Or we could fail during the creation or alter phase ?
| } | ||
| // This is a stale materialized view and should be expanded like a logical view | ||
| return createScopeForMaterializedView(table, name, scope, materializedViewDefinition, Optional.empty()); | ||
| return createScopeForMaterializedView(table, name, scope, materializedViewDefinition, Optional.empty(), useLogicalViewSemantics); |
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.
Instead of passing a useLogicalViewSemantics can we create a scope for the storage table ?
|
Apart from the benefit on skipping the analysis what are the other benefits would I get if a run a query as |
|
For context, #15326 |
|
Also, #23387 (reply in thread) and #23747 |
This change is meant to be backward-compatible. The new clause is optional. If it is not specified, or if
Skipping the analysis is not the goal in itself. The goal is to achieve better availability when the source tables are not available and to get closer to the semantics mentioned in the issues that Martin and I linked in this PR - specifically, that within the grace period, the MV should be treated as if it were a regular storage table. And today, as far as I know, the only blocker for this is the analysis performed every time the MV is queried.
This is more or less what I wanted to express in this comment: #27356 (comment). I’m happy to discuss what the syntax should look like to achieve the goals described above. Personally, |
Description
This PR introduces a new optional clause for
CREATE MATERIALIZED VIEWthat allows users to control the behavior of materialized views when they are stale.I will updated the documentation once the proposed syntax and behavior are approved.
New syntax
CREATE MATERIALIZED VIEW mv_name WHEN STALE { FAIL | INLINE } AS SELECT ...Behavior
Motivation
Additional context and related issues
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text: