aminghadersohi opened a new pull request, #45571: URL: https://github.com/apache/superset/pull/45571
### SUMMARY A Sunburst saved with a time grain on a temporal hierarchy level (for example by the MCP chart tools) keeps the grain only in `granularity_sqla` / `time_grain_sqla`. The Sunburst control panel has neither control, and Explore rebuilds form data from the panel's controls only, so opening the chart in Explore drops both keys and queries the temporal level at its raw values: a monthly chart with 3 rows shows 90 daily rows in Explore. Design. The Sunburst panel gets a real Time grain control (`time_grain_sqla`, shown only when a hierarchy level is temporal, with no default grain for new charts) plus the hidden `temporal_columns_lookup` control, the pattern the Pivot Table uses. `buildQuery` sends each temporal level as the BASE_AXIS column reference that `normalizeTimeColumn` emits for a physical x-axis (`isColumnReference: true`, `timeGrain`), and drops a dormant grain when no level is temporal and there is no legacy `granularity` subject. The server-side form-data query builder mirrors this, so MCP/compile/fallback queries match Explore. The saved `columns` stay plain names, nothing new is written to `params`, and the MCP temporal-state check also accepts a grain whose subject is a hierarchy level the lookup marks temporal (it still checks the dataset column is temporal). How this meets the constraints: - Embedded guest anti-tamper check: the grained level collapses back to the saved column name (`_denormalize_base_axis_column`), and `params` is unchanged, so an Explore re-save that drops `granularity_sqla` does not trip `_columns_metrics_modified`. A grained reference to a column outside the hierarchy is still rejected. - Non-temporal levels: only levels the lookup (or the legacy `granularity_sqla`) marks temporal get a grain; with none, the grain is not sent, so a semantic view does not raise "A time column must be specified". - Dashboard Time Grain filter: the grain is read from the query's `extras.time_grain_sqla`, which `extra_form_data.time_grain_sqla` overrides, so the filter changes the grained level (also for charts saved without a grain). - Cross-filters: `EchartsSunburst` builds the filter `col` from `formData.columns`, which stays a plain name, so `_validate_filters` on semantic views accepts it. - Visible and editable: the grain is a normal Explore control; renaming or editing levels re-derives the grained columns at query time. - Existing charts without a grain: no grain is added on load or save, so their queries are unchanged. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF Before: an MCP-created Sunburst with a monthly temporal level returns 3 rows through MCP but 90 daily rows when opened in Explore. After: Explore shows the Time grain control set to Month and queries the 3 monthly rows. ### TESTING INSTRUCTIONS 1. Create a Sunburst whose hierarchy has a temporal level with a monthly grain (via the MCP `generate_chart` tool, or in Explore by choosing Time grain = Month). 2. Open it in Explore: the Time grain control shows Month and the temporal ring has one segment per month. 3. Save it, add it to a dashboard with a Time Grain native filter, change the filter to Year: the ring regroups by year. 4. With a hierarchy of only categorical columns, the Time grain control is hidden and no grain is sent. Automated: - `npx jest src/explore/controlUtils/sunburstTimeGrain.test.ts plugins/plugin-chart-echarts/test/Sunburst` - `pytest tests/unit_tests/mcp_service/chart/test_sunburst_chart.py tests/unit_tests/security/manager_test.py tests/unit_tests/semantic_layers/mapper_test.py tests/unit_tests/common/test_form_data_query_context.py` The new tests (12 Jest, 7 pytest) all fail on master and pass with this change. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [x] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
