-
Notifications
You must be signed in to change notification settings - Fork 613
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
fix(meta): display distinct dedup tables in dashboard #7948
Conversation
Signed-off-by: Richard Chien <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #7948 +/- ##
=======================================
Coverage 71.69% 71.69%
=======================================
Files 1116 1116
Lines 179341 179364 +23
=======================================
+ Hits 128571 128589 +18
- Misses 50770 50775 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
LGTM
@@ -93,6 +93,9 @@ where | |||
always!(s.table, "HashAgg"); | |||
} | |||
} | |||
for (distinct_col, dedup_table) in &mut node.distinct_dedup_tables { |
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.
How could it work without assigning a global table ID before this PR? 🥵
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.
It doesn't show up in the dashboard and seems no other place will access the table id (
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.
If the table ID is local for each fragment graph (mview), then tables from multiple views may get conflicted. (note that we use the table id as the storage prefix) 😄 I guess it'll be better to assign a range, for example, from 1000, for global table IDs, then we can check it out earlier. Not a big deal.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
As title.
Checklist
./risedev check
(or alias,./risedev c
)