dhruv-techdev opened a new pull request, #45437:
URL: https://github.com/apache/superset/pull/45437
### SUMMARY
The `get_filters()` and `filter_values()` Jinja macros used a truthiness
check on each filter's value, so any filter whose value is `0` or `false` was
silently skipped. Virtual datasets that use these macros then render SQL that
ignores that filter.
The most common trigger is a dashboard **Range** native filter with a lower
bound of `0`: a 0–100 selection sends `col >= 0` and `col <= 100`, but
`get_filters('col')` returned only the `<= 100` filter. With
`remove_filter=True`, the dropped filter also wasn't marked as removed, so the
template and the outer query disagreed about which filters apply.
The fix replaces the truthiness checks with explicit checks for missing or
empty values:
```diff
# ExtraCache.filter_values
- elif val:
+ elif val is not None:
# ExtraCache.get_filters
- val
+ val not in (None, "", [])
```
`None`, `""` and `[]` are still skipped as before, so filters with no usable
value keep their current behavior. `IS NULL` / `IS NOT NULL` handling is
unchanged. This matches the `val is None` check already used by the
drill-to-detail fallback in the same class (`_get_filters_from_query_context`).
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
`get_filters('col')` for a dashboard Range filter set to 0–100:
| | Result |
|---|---|
| Before | `[{'op': '<=', 'col': 'col', 'val': 100}]` |
| After | `[{'op': '>=', 'col': 'col', 'val': 0}, {'op': '<=', 'col': 'col',
'val': 100}]` |
`filter_values('col')` for an adhoc filter `col == 0`: before `[]`, after
`[0]`.
### TESTING INSTRUCTIONS
1. Create a virtual dataset with:
```sql
SELECT * FROM my_table
WHERE 1=1
{% for f in get_filters('amount', remove_filter=True) %}
AND amount {{ f.op }} {{ f.val }}
{% endfor %}
```
2. Add a chart on it to a dashboard, plus a **Range** native filter on
`amount`.
3. Set the range to 0–100 and apply it, then open the chart's **View query**.
4. Before this change only `AND amount <= 100` is rendered; after it, both
`AND amount >= 0` and `AND amount <= 100` appear.
Automated: `pytest tests/unit_tests/jinja_context_test.py` passes (205
passed, 1 skipped). The 0 / `false` / Range-filter cases were also verified
with ad-hoc unit tests against `ExtraCache`, which failed before the change and
pass after it.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #45436
- [ ] 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]
