-
Notifications
You must be signed in to change notification settings - Fork 633
feat(dashboard): display metadata for index itself besides index table #21680
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
Conversation
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.
Pull Request Overview
This PR enhances the dashboard to display metadata for the index itself (including index items) in addition to the index table information. Key changes include:
- Refactoring the endpoints to separate index table and index item metadata.
- Adding a new API function to join index table data with the corresponding index items.
- Updating the UI to display an additional column for index item counts.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/meta/src/dashboard/mod.rs | Introduces separate endpoints for index tables and index metadata. |
src/meta/src/controller/catalog/list_op.rs | Adds a new controller method to list index items from the catalog. |
dashboard/pages/indexes.tsx | Adds a new column to display index item counts in the dashboard UI. |
dashboard/lib/api/streaming.ts | Implements the merging logic for index table and index item metadata. |
Comments suppressed due to low confidence (2)
src/meta/src/dashboard/mod.rs:153
- [nitpick] Consider renaming the 'list_indexes' function to 'list_index_items' or similar to clearly differentiate it from the 'list_index_tables' endpoint.
pub async fn list_indexes(Extension(srv): Extension<Service>) -> Result<Json<Vec<Index>>> {
dashboard/pages/indexes.tsx:28
- [nitpick] The column header 'Index Len' may be ambiguous; consider using a more descriptive label like 'Index Column Count' to better communicate its purpose.
const indexItemsCountColumn: Column<Index> = {
// Join them by id. | ||
return index_tables.flatMap((x) => { | ||
let index = index_items.find((y) => y.id === x.id) | ||
if (index === undefined) { |
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.
When merging index table and index item data, a missing matching index item leads to the entry being silently dropped. Consider explicitly handling this case (e.g., logging a warning) to avoid potential data discrepancies.
if (index === undefined) { | |
if (index === undefined) { | |
console.warn(`No matching index item found for index table with id: ${x.id}`) |
Copilot uses AI. Check for mistakes.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Previously we only displayed the metadata for the state table of the index, which lacked information about the index itself, espeically the
index_items
, i.e., expression of each index column.Checklist
Documentation
Release note