-
Notifications
You must be signed in to change notification settings - Fork 16.6k
feat(semantic-layer): add frontend support for semantic views #37649
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
base: sl-3-api-integration
Are you sure you want to change the base?
Conversation
Update frontend to support semantic layer datasources: - Add SemanticView to DatasourceType enum - Extend DatasourceKey to handle semantic_view type - Update datasource ID type to support string UUIDs - Update DrillBy modal for semantic view compatibility - Update explore utilities for semantic view form data Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Bito Automatic Review Skipped - Branch Excluded |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| const [datasource_id, datasource_type] = formData.datasource.split('__'); | ||
| const [datasourceIdStr, datasource_type] = formData.datasource.split('__'); | ||
| // Try to parse as integer, fall back to string (UUID) if NaN | ||
| const parsedDatasourceId = parseInt(datasourceIdStr, 10); |
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.
Isn’t there going to be the same parsing problem as you mentioned in 1 of the files above (if the string starts with a number, parseInt will only extract that number)?
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.
And actually, since the id can be a string now, do we need to parse stringified numbers into integers? Cant we just leave them as strings?
|
Yes, the same parsing issue exists in DrillByModal.tsx. Using parseInt directly on datasourceIdStr could incorrectly parse strings starting with numbers (e.g., '85abc' becomes 85), unlike the fix in DatasourceKey.ts which checks if the entire string is numeric. superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx superset-frontend/packages/superset-ui-core/src/query/DatasourceKey.ts |
| const [id, typeString] = formData.datasource.split('__'); | ||
| datasourceId = parseInt(id, 10); | ||
| // Try to parse as integer, fall back to string (UUID) if NaN | ||
| const parsedId = parseInt(id, 10); |
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.
Same as above
SUMMARY
This is 4 of 4 stacked PRs to implement the backend functionality of semantic layers:
This one updates frontend to support semantic layer datasources:
SemanticViewtoDatasourceTypeenumDatasourceKeyto handlesemantic_viewtypeDrillBymodal for semantic view compatibilityBEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
Tested end-to-end (all 4 PRs) with Snowflake.
ADDITIONAL INFORMATION