You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "dhruv-techdev (via GitHub)" <gi...@apache.org> on 2026/10/18 10:37:49 UTC
[PR] fix(jinja): keep filters with 0 or false values in get_filters and fi… [superset]
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(jinja): keep filters with 0 or false values in get_filters and fi… [superset]
Posted by "dhruv-techdev (via GitHub)" <gi...@apache.org>.
dhruv-techdev commented on PR #45437:
URL: https://github.com/apache/superset/pull/45437#issuecomment-6371438890
Thanks for reviewing and approving! Appreciate the help.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(jinja): keep filters with 0 or false values in get_filters and fi… [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #45437:
URL: https://github.com/apache/superset/pull/45437#discussion_r4306719098
##########
superset/jinja_context.py:
##########
@@ -504,7 +504,7 @@ def filter_values(
val = flt.get("val")
if isinstance(val, list):
return_val.extend(val)
- elif val:
+ elif val is not None:
return_val.append(val)
Review Comment:
**Suggestion:** For a query-context filter with an empty-string value, `filter_values()` now returns `[""]` instead of treating it as absent, changing rendered SQL and suppressing the default.
**Assessment:** 🟠 `Major` · 🔁 `Occurrence: Rarely` · 🏷️ `Incorrect condition logic`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=62d11ff375804ff5b4c0df3cfe2a17a9&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=62d11ff375804ff5b4c0df3cfe2a17a9&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/jinja_context.py
**Line:** 507:508
**Comment:**
*Incorrect Condition Logic: For a query-context filter with an empty-string value, `filter_values()` now returns `[""]` instead of treating it as absent, changing rendered SQL and suppressing the default.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45437&comment_hash=354e5ee16ec754896f8eee2b5fed460be2774aee3d6082fd0b4fb2f098559ed5&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45437&comment_hash=354e5ee16ec754896f8eee2b5fed460be2774aee3d6082fd0b4fb2f098559ed5&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(jinja): keep filters with 0 or false values in get_filters and fi… [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #45437:
URL: https://github.com/apache/superset/pull/45437#issuecomment-6248912483
## CodeAnt PR Risk: Low Risk
- The PR appears safe to merge: get_filters still excludes empty strings, while filter_values preserves other non-None values such as 0 and False.
**Assessed commit:** `0646be256e2f`
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(jinja): keep filters with 0 or false values in get_filters and fi… [superset]
Posted by "dhruv-techdev (via GitHub)" <gi...@apache.org>.
dhruv-techdev commented on PR #45437:
URL: https://github.com/apache/superset/pull/45437#issuecomment-6278319496
Please let me know if you'd like the tests structured differently, or if there's anything else you'd like me to change. I'm happy to make any adjustments. Thanks again!
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(jinja): keep filters with 0 or false values in get_filters and fi… [superset]
Posted by "dhruv-techdev (via GitHub)" <gi...@apache.org>.
dhruv-techdev commented on PR #45437:
URL: https://github.com/apache/superset/pull/45437#issuecomment-6278302443
Good call, thanks @sadpandajoe ! I've added regression tests in tests/unit_tests/jinja_context_test.py:
get_filters returns filters with value 0 / False, and marks them as removed with remove_filter=True
a native Range filter of 0–100 keeps both the >= 0 and <= 100 bounds
filter_values returns 0 / False
filter_values still treats "" as absent and falls back to the default (the case CodeAnt flagged)
Five of the new tests fail against master and all pass with this change. The full jinja_context_test.py passes locally (211 passed, 1 skipped).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(jinja): keep filters with 0 or false values in get_filters and fi… [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #45437:
URL: https://github.com/apache/superset/pull/45437#issuecomment-6278345045
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | dcfece29ea215b2884c471b0f2067cf051024d20 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6ad692719023cf2be34788d0 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-45437--superset-docs-preview.netlify.app](https://deploy-preview-45437--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcrRDkAgFADQf7nPmpQp_sBn3FRTCy0xzfy73s7DeSGmY4sZJoAGVMJ9Waute1rvdo_EYjgLsS5kk8iN4TJnjdrEcJRZ1zqgHkYmupEyvlimDO-FlJrC9wM) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-45437--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(jinja): keep filters with 0 or false values in get_filters and fi… [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #45437:
URL: https://github.com/apache/superset/pull/45437#issuecomment-6303230056
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 82e46a97afb1e2c2f5f2d47db31567657d688863 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6ad7ead660e060b055cedf60 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-45437--superset-docs-preview.netlify.app](https://deploy-preview-45437--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcrLDYAgDADQXXqW2As1cQPHKLREDArBTzTG3eX2Du-FUvNaDhgBOnCVNz83h3j3S9wWNoHT_pgQ06HVXJxO3VsULSk_k7RKLIOyEKEioUNrvUoghO8H) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-45437--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(jinja): keep filters with 0 or false values in get_filters and fi… [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe merged PR #45437:
URL: https://github.com/apache/superset/pull/45437
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(jinja): keep filters with 0 or false values in get_filters and fi… [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #45437:
URL: https://github.com/apache/superset/pull/45437#issuecomment-6303477010
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/45437?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `0%` with `1 line` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 65.24%. Comparing base ([`7ff5709`](https://app.codecov.io/gh/apache/superset/commit/7ff5709882fc9005da05a21654e3e2a167b29f70?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`82e46a9`](https://app.codecov.io/gh/apache/superset/commit/82e46a97afb1e2c2f5f2d47db31567657d688863?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/45437?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/jinja\_context.py](https://app.codecov.io/gh/apache/superset/pull/45437?src=pr&el=tree&filepath=superset%2Fjinja_context.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvamluamFfY29udGV4dC5weQ==) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/45437?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (7ff5709) and HEAD (82e46a9). Click for more details.
>
> <details><summary>HEAD has 4 uploads less than BASE</summary>
>
>| Flag | BASE (7ff5709) | HEAD (82e46a9) |
>|------|------|------|
>|python|7|3|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #45437 +/- ##
===========================================
- Coverage 83.98% 65.24% -18.75%
===========================================
Files 3055 3055
Lines 201048 201048
Branches 46967 46967
===========================================
- Hits 168855 131165 -37690
- Misses 28948 66933 +37985
+ Partials 3245 2950 -295
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/45437/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/45437/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.89% <0.00%> (ø)` | |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/45437/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/45437/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/45437/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `36.65% <0.00%> (ø)` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/45437/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `52.73% <0.00%> (-34.47%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/45437/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `52.62% <0.00%> (ø)` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/45437/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/45437?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(jinja): keep filters with 0 or false values in get_filters and fi… [superset]
Posted by "dhruv-techdev (via GitHub)" <gi...@apache.org>.
dhruv-techdev commented on PR #45437:
URL: https://github.com/apache/superset/pull/45437#issuecomment-6248866411
Hi @betodealmeida @rusackas, could you please review this PR and, if everything looks good, approve and help get it merged? Happy to address any feedback. Thanks!
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org