richardfogaca opened a new pull request, #35937: URL: https://github.com/apache/superset/pull/35937
## SUMMARY Fixes a bug where charts with "Group remaining as Others" enabled ignore all filters (time range, user filters, RLS filters, custom WHERE clauses). ### Problem When `group_others_when_limit_reached=True`, the query is reconstructed with `sa.select(select_exprs)`, creating a new SQLAlchemy Select object that loses previously applied WHERE and HAVING clauses. ### Solution 1. Added `_reapply_query_filters()` method to systematically re-apply all filters after query reconstruction 2. Fixed `time_filters` initialization - moved before `if granularity:` block to prevent undefined variable error when charts have no time dimension 3. Called filter reapplication at both query reconstruction points (lines 2449 and 2523) ### Changes - **`superset/models/helpers.py`**: New method + initialization fix + 2 integration points - **`tests/unit_tests/models/helpers_test.py`**: 5 unit tests covering all code paths ## BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF **BEFORE:** Outer WHERE clause missing when "Group remaining as Others" is enabled - charts show unfiltered data <img width="1114" height="852" alt="before" src="https://github.com/user-attachments/assets/2ca9c12d-32f6-4109-9b74-2e6100687128"; /> **AFTER:** Outer WHERE clause correctly applied - charts respect time range and all filters <img width="1142" height="842" alt="after" src="https://github.com/user-attachments/assets/38e132cc-18a8-4d44-a377-6a34023bda83"; /> ## TESTING INSTRUCTIONS ### Manual Testing 1. **Create a bar chart** with: - Time dimension + categorical series column - Set time range filter (e.g., "Last 30 days") - Enable series "LIMIT" (e.g., 10) and check "Group remaining as 'Others'" 2. **Verify**: - Chart shows only data within selected time range - SQL query has outer WHERE clause with time filters - "Others" category appears for remaining series 3. **Test edge case**: Create chart without time dimension, add filters, enable "Group remaining as Others" - should work without errors ### ADDITIONAL INFORMATION Fixes #35815 - [X] Has associated issue: https://github.com/apache/superset/issues/35815 - [ ] Required feature flags: - [ ] 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]
