Skip to content

feat(dashboard): resolve chain node of mv on mv #162

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
Merged
merged 2 commits into from
Feb 9, 2022
Merged

feat(dashboard): resolve chain node of mv on mv #162

merged 2 commits into from
Feb 9, 2022

Conversation

@BugenZhao
Copy link
Member

@BugenZhao BugenZhao commented Feb 9, 2022 •
edited by cloudcarver
Loading

What's changed and what's your intention?

This PR introduces an optional feature "resolving mv on mv" for dashboard.
Note that the fragments of base MV might be rendered multiple times in different MVs.

Screen Shot 2022-02-09 at 3 52 53 PM

Checklist

  • I have written necessary docs and comments
  • I have added necessary unit tests and integration tests

Refer to a related PR or issue link (optional)

@BugenZhao BugenZhao requested a review from skyzh February 9, 2022 07:55
@github-actions github-actions bot added the type/feature Type: New feature. label Feb 9, 2022
skyzh
skyzh approved these changes Feb 9, 2022
View reviewed changes
Copy link
Contributor

@skyzh skyzh left a comment

Choose a reason for hiding this comment

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

LGTM. But I'd prefer to show the structure of MV fragment only once, and only show "MV Actor xxx" before chain nodes.

@skyzh
Copy link
Contributor

skyzh commented Feb 9, 2022

Would you please test if creating two MVs on the same table, and then joining them would work correctly?

@codecov
Copy link

codecov bot commented Feb 9, 2022

Codecov Report

Merging #162 (aff4899) into main (4e1712f) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               main     #162   +/-   ##
=========================================
  Coverage     74.65%   74.65%           
  Complexity     2654     2654           
=========================================
  Files           834      834           
  Lines         47888    47888           
  Branches       1562     1562           
=========================================
  Hits          35752    35752           
  Misses        11337    11337           
  Partials        799      799
Flag Coverage Δ
java 62.08% <ø> (ø)
rust 80.07% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
rust/meta/src/hummock/compaction.rs 73.85% <0.00%> (-0.66%) ⬇️
rust/storage/src/hummock/version_manager.rs 39.18% <0.00%> (+0.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e1712f...aff4899. Read the comment docs.

@BugenZhao
Copy link
Member Author

BugenZhao commented Feb 9, 2022

Would you please test if creating two MVs on the same table, and then joining them would work correctly?

Seems okay.

image

@BugenZhao
Copy link
Member Author

BugenZhao commented Feb 9, 2022 

dev=> create table t (v int);
CREATE_TABLE
dev=> create materialized view mv1 as select * from t;
CREATE_MATERIALIZED_VIEW
dev=> create materialized view mv2 as select * from mv1;
CREATE_MATERIALIZED_VIEW
dev=> create materialized view mv3 as select * from mv1 inner join mv2 on mv1.v = mv2.v;
CREATE_MATERIALIZED_VIEW

image

@BugenZhao BugenZhao merged commit dab68a0 into main Feb 9, 2022
@BugenZhao BugenZhao deleted the bz/dashboard-mv-on-mv branch February 9, 2022 08:16
@BugenZhao BugenZhao mentioned this pull request Feb 10, 2022
Merged
@cloudcarver cloudcarver mentioned this pull request Feb 14, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@skyzh skyzh skyzh approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

type/feature Type: New feature.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@BugenZhao @skyzh