include primary key in the pinot schema ui #17566
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #17566 +/- ##
============================================
+ Coverage 63.14% 63.17% +0.03%
Complexity 1476 1476
============================================
Files 3172 3172
Lines 189783 189783
Branches 29041 29041
============================================
+ Hits 119842 119902 +60
+ Misses 60629 60568 -61
- Partials 9312 9313 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
yashmayya
left a comment
yashmayya
left a comment
There was a problem hiding this comment.
LGTM, thanks for the enhancement!
There was a problem hiding this comment.
Pull request overview
This PR adds "Primary Key" as a new column in the schema UI table to make it easier to identify primary key columns for upsert and dedup tables without needing to inspect the JSON format.
Changes:
- Added "Primary Key" column to the schema table display
- Updated the column headers and data mapping to include primary key information
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pinot-controller/src/main/resources/app/utils/Utils.tsx | Added logic to extract primary key columns and include them in the schema table data |
| pinot-controller/src/main/resources/app/pages/TenantDetails.tsx | Updated empty state schema table to include the new "Primary Key" column header |
| columns: ['Column', 'Type', 'Field Type', 'Multi Value', 'Primary Key'], | ||
| records: columnList.map((field) => { | ||
| return [field.name, field.dataType, field.fieldType, getMultiValueField(field)]; | ||
| return [field.name, field.dataType, field.fieldType, getMultiValueField(field), primaryKeys.has(field.name)]; |
There was a problem hiding this comment.
The primary key value is returned as a boolean (true/false) which is inconsistent with the 'Multi Value' column that uses getMultiValueField(). Consider creating a similar helper function like getPrimaryKeyValue() to maintain consistency in how boolean values are formatted and displayed across columns.
| columns: ['Column', 'Type', 'Field Type', 'Multi Value', 'Primary Key'], | ||
| records: columnList.map((field) => { | ||
| return [field.name, field.dataType, field.fieldType, getMultiValueField(field)]; | ||
| return [field.name, field.dataType, field.fieldType, getMultiValueField(field), primaryKeys.has(field.name)]; |
There was a problem hiding this comment.
The primary key column only appears when showFieldType is true, but there's no corresponding update for the case when showFieldType is false. This creates an inconsistent API where the primary key information is conditionally available. Consider adding the primary key column to both code paths to ensure consistent data structure.
5 participants
this is a small UI improvement to include "Primary Key" in the schema UI. This is useful for upsert and dedup tables so you can see this without having to look at the json format.
I ran this on test cluster where we can see
truefor this case.