mikebridge opened a new pull request, #44909: URL: https://github.com/apache/superset/pull/44909
### SUMMARY A semantic view cannot apply a series (group) limit unless the query has series columns, but the semantic mapper accepted a positive `series_limit` with empty `series_columns` and silently returned all rows. Pivot Table shows a Series limit control and has no Series columns control, so it produced exactly that request. - **Frontend:** the Pivot Table Series limit control is hidden for semantic views (using the control `visibility` pattern). `buildQueryContext` now clears a positive `series_limit` for any semantic-view query whose final `series_columns` is empty, so saved charts that already carry a limit keep working (Pivot Table, and Box Plot with an empty group-by). SQL datasets, and semantic queries that do have series columns, are unchanged. - **Backend:** the semantic mapper raises `QueryObjectValidationError` for a positive limit with no series columns, which the chart-data endpoint returns as a client error. Both changes ship together so the contract stays paired. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF Not captured; control visibility is covered by Jest. ### TESTING INSTRUCTIONS - Pivot Table on a semantic view: Series limit is hidden. A saved Pivot Table with a stored positive limit sends `series_limit: 0`. - Pivot Table on a SQL dataset keeps its limit; a semantic query with series columns keeps its limit. - A raw chart-data request for a semantic view with a positive limit and no series columns returns a validation error. - Automated: `pytest tests/unit_tests/semantic_layers/mapper_test.py`; Jest for `superset-ui-core` `buildQueryContext`, the Pivot Table plugin and Box Plot `buildQuery`. ### 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)) - [ ] Introduces new feature or API - [ ] Removes existing feature or API Risks I'd like reviewers to weigh: - **Requests that do not go through the frontend.** The clearing happens in `buildQueryContext`. A request built elsewhere with this shape now gets the validation error instead of an unlimited result: an already-open browser tab running old assets, a direct API caller, and any server-side run of a chart's stored `query_context` (for example alerts and reports) for a semantic-view chart saved before this change. I have not checked whether such stored contexts exist in practice; re-saving the chart fixes one. - The new error message is not wrapped for translation. - The frontend type hook was checked with a source-resolved `tsc` because the generated declarations in my checkout were stale. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
