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

Iceberg: Extended documentation for materialized view behavior #11696

Merged
merged 1 commit into from
Apr 8, 2022

Conversation

ulmako
Copy link
Contributor

@ulmako ulmako commented Mar 29, 2022

Description

This PR extends the documentation on iceberg materialized views. I recently dug deeper into understanding the behavior of views and materialized views on the iceberg connector and noticed a behavior I did not expect. After asking about the behavior in slack (see https://trinodb.slack.com/archives/CGB0QHWSW/p1647589373377379) @raunaqmorarka clarified it to me.

To prevent other users from getting hung up on this, I wrote some more phrases about the behavior in the iceberg connector documentation.

Is this change a fix, improvement, new feature, refactoring, or other?
Improvement to documentation

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)
Iceberg Connector

Related issues, pull requests, and links

Discussion in Slack

Documentation

( ) No documentation is needed.
(x) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

(x) No release notes entries required.
( ) Release notes entries required with the following suggested text:

@cla-bot cla-bot bot added the cla-signed label Mar 29, 2022
@github-actions github-actions bot added the docs label Mar 29, 2022
@ulmako ulmako changed the title Extended documentation for iceberg materialized view behavior Iceberg: Extended documentation for materialized view behavior Mar 29, 2022
Copy link
Member

@raunaqmorarka raunaqmorarka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please get rid of the merge commits and update the commit message title to Clarify iceberg materialized views behaviour in docs

the snapshot-ids of all the tables that are part of the materialized
view's query at that point. When the materialized view is queried, the snapshot-ids
are used to check if the data in the storage table is up to date. If the data is
outdated, the storage is considered stale and the materialized view will behave
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the data in the storage table is outdated, the materialized view will behave..

query into the existing table.
query into the existing table. Refreshing the materialized view will also store
the snapshot-ids of all the tables that are part of the materialized
view's query at that point. When the materialized view is queried, the snapshot-ids
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

view's query at that point in materialized view metadata.

Copy link
Member

@anjalinorwood anjalinorwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarified phrasing in one sentence.
Other than that looks good to me. Thanks!

@ulmako
Copy link
Contributor Author

ulmako commented Mar 31, 2022

@raunaqmorarka, @anjalinorwood thanks for your feedback. I made the changes you suggested. Can you take another look?

@raunaqmorarka
Copy link
Member

@hashhar please merge this one if it looks okay to you

Copy link
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM % nit

docs/src/main/sphinx/connector/iceberg.rst Show resolved Hide resolved
@hashhar hashhar merged commit cec437d into trinodb:master Apr 8, 2022
@github-actions github-actions bot added this to the 377 milestone Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants