betodealmeida opened a new pull request, #35890:
URL: https://github.com/apache/superset/pull/35890
### SUMMARY
Fixes #34894
When using `get_time_filter(remove_filter=True)` in virtual datasets, time
filters were incorrectly applied twice:
1. Once in the inner query (from the virtual dataset's Jinja template)
2. Again in the outer query WHERE clause (added by Superset's query
generator)
**Root Cause**: Timeseries charts use the special `__timestamp` alias for
time filters, but virtual dataset Jinja templates use actual column names
(e.g., "dttm"). The `removed_filters` check didn't recognize these as referring
to the same column, causing the filter to be re-applied in the outer query.
**Solution**: Enhanced the filter skip logic in `superset/models/helpers.py`
to check both the `__timestamp` alias and the actual datetime column name when
determining if a filter should be skipped.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
**BEFORE (Bug)**: Virtual dataset with `get_time_filter('dttm',
remove_filter=True)` would generate SQL like:
```sql
SELECT * FROM (
SELECT * FROM my_table WHERE dttm >= '2024-01-01' AND dttm < '2024-01-31'
) AS virtual_table
WHERE dttm >= '2024-01-01' AND dttm < '2024-01-31' -- Filter applied again!
```
**AFTER (Fixed)**: The filter is only applied once in the inner query:
```sql
SELECT * FROM (
SELECT * FROM my_table WHERE dttm >= '2024-01-01' AND dttm < '2024-01-31'
) AS virtual_table
-- No duplicate filter in outer query
```
### TESTING INSTRUCTIONS
1. Run the new unit tests to verify the fix:
```bash
pytest tests/unit_tests/models/test_time_filter_double_application.py -v
```
2. Run existing time filter tests to ensure no regressions:
```bash
pytest tests/unit_tests/jinja_context_test.py -k test_get_time_filter -v
```
3. Manual test (optional):
- Create a virtual dataset with SQL using
`get_time_filter('your_time_col', remove_filter=True, target_type='DATE')`
- Create a timeseries chart from this virtual dataset
- Add the chart to a dashboard with a time filter applied
- Inspect the generated SQL (via query inspector or logs) - the time
filter should only appear in the subquery, not duplicated in the outer query
### ADDITIONAL INFORMATION
- [x] Has associated issue: #34894
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
**Code Changes**:
- `superset/models/helpers.py`: Added logic to handle `__timestamp` alias
when checking removed_filters (10 lines)
- `tests/unit_tests/models/test_time_filter_double_application.py`: Added
comprehensive unit tests (360 lines)
**Testing Coverage**:
- Test for basic virtual dataset with `remove_filter=True`
- Test for `__timestamp` alias handling in timeseries queries
- Test for simple non-nested cases
All pre-commit checks pass (mypy, ruff, pylint).
--
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]
