Skip to content

fix(explore): render Jinja before validating legacy chart filters - #41996

Merged
rusackas merged 2 commits into
apache:masterfrom
massucattoj:fix/jinja-filter-time-series-table
Jul 24, 2026
Merged

fix(explore): render Jinja before validating legacy chart filters#41996
rusackas merged 2 commits into
apache:masterfrom
massucattoj:fix/jinja-filter-time-series-table

Conversation

@massucattoj

Copy link
Copy Markdown
Contributor

SUMMARY

Using Jinja templating in a Custom SQL filter (e.g. a HAVING clause) on a legacy chart such as the Time-series Table raises Invalid SQL clause.

Root cause. The legacy viz pipeline validated filter clauses with SQLGlot before rendering Jinja, so the raw template text was parsed as SQL and rejected. This happened in two redundant places:

  1. split_adhoc_filters_into_base_filters (superset/utils/core.py)
  2. BaseViz.query_obj() (superset/viz.py) - re-sanitized the same clauses right after.

The modern chart-data pipeline doesn't have this problem because it renders the template before validating (QueryObject.validate()processor.process_template(clause, force=True)sanitize_clause), which is why only legacy viz types were affected.

Fix. Remove the premature validation instead of duplicating template rendering into the legacy pipeline.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:
Screenshot 2026-07-13 at 08 04 09

After:
Screenshot 2026-07-13 at 07 57 07

TESTING INSTRUCTIONS

Requires ENABLE_TEMPLATE_PROCESSING: True.

  1. Create a Time-series Table chart using the "Vehicle Sales" (cleaned_sales_data) example dataset, with metric SUM(price_each) and one entry in "Time series columns".
  2. Add a filter → Custom SQL → clause HAVING
    sum(price_each) > {% if filter_values('threshold')|length %} {{ filter_values('threshold')[0] }} {% else %} 0 {% endif %}
  3. Update the chart.
    • Before: Invalid SQL clause error.
    • After: chart renders; the issued query contains HAVING ((sum(price_each) > 0)).
  4. Regression check - replace the filter with genuinely invalid SQL (e.g. sum(price_each) > ) broken () and update: the query is still rejected, by the downstream validator (INVALID_SQL_ERROR with parse position).

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags: ENABLE_TEMPLATE_PROCESSING
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • 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

@dosubot dosubot Bot added explore:filter Related to filters in Explore global:jinja Related to Jinja templating labels Jul 13, 2026
@bito-code-review

bito-code-review Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #9c7228

Actionable Suggestions - 0
Review Details
  • Files reviewed - 4 · Commit Range: daa3450..daa3450
    • superset/utils/core.py
    • superset/viz.py
    • tests/unit_tests/core_tests.py
    • tests/unit_tests/test_viz_query_obj.py
  • Files skipped - 0
  • Tools
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@netlify

netlify Bot commented Jul 13, 2026

Copy link
Copy Markdown

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 184b82e
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6a54ceaffa05f9000868d6ac
😎 Deploy Preview https://deploy-preview-41996--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

Comment thread tests/unit_tests/core_tests.py
Comment thread tests/unit_tests/core_tests.py
Comment thread tests/unit_tests/test_viz_query_obj.py
@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.04%. Comparing base (3c69822) to head (184b82e).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
superset/utils/core.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master   #41996   +/-   ##
=======================================
  Coverage   65.04%   65.04%           
=======================================
  Files        2742     2741    -1     
  Lines      153422   153371   -51     
  Branches    35198    35185   -13     
=======================================
- Hits        99789    99761   -28     
+ Misses      51726    51704   -22     
+ Partials     1907     1906    -1     
Flag Coverage Δ
hive 39.07% <0.00%> (+<0.01%) ⬆️
mysql 57.96% <0.00%> (+<0.01%) ⬆️
postgres 58.02% <0.00%> (+<0.01%) ⬆️
presto 41.06% <0.00%> (+<0.01%) ⬆️
python 59.40% <0.00%> (+<0.01%) ⬆️
sqlite 57.63% <0.00%> (+<0.01%) ⬆️
unit 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread superset/utils/core.py

@rusackas rusackas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@rusackas
rusackas merged commit 4ee500e into apache:master Jul 24, 2026
59 checks passed
rusackas pushed a commit that referenced this pull request Jul 24, 2026
Master's #41996 relocated legacy filter sanitization out of viz.py and
utils/core.py into the legacy explore path this branch removes; the
remaining utils signature change merges cleanly, viz.py stays deleted,
and the new test file importing superset.viz is removed with it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
niteshpurohit added a commit to HiMamaInc/superset that referenced this pull request Jul 24, 2026
* refactor(mcp): dedupe list-tool schemas and delete dead middleware (apache#41923)

* fix(ag-grid-table): respect row limit with server pagination (apache#41346)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* feat(table/pivot-table): correct non-additive totals/subtotals via DB rollup [SIP-216] (apache#41184)

Co-authored-by: Superset Dev <dev@superset.apache.org>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com>

* feat(datasets): add RLS filter indicator badge to dataset list and explore view (apache#38807)

Co-authored-by: Evan <evan@preset.io>

* fix(chart): updates counties of kenya map (apache#38019)

Co-authored-by: Zack Adams <zack@Zacks-Laptop.local>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Evan <evan@preset.io>
Co-authored-by: Evan Rusackas <evan@rusackas.com>

* chore(deps): bump actions/setup-go from 6.5.0 to 7.0.0 (apache#42303)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump nanoid from 5.0.9 to 6.0.0 in /superset-frontend (apache#42230)

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: hainenber <dotronghai96@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: hainenber <dotronghai96@gmail.com>
Co-authored-by: Joe Li <joe@preset.io>
Co-authored-by: Evan <evan@preset.io>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

* test(dashboard): migrate dashboard load smoke test to Playwright (apache#41432)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(superset_app_root): when used with oauth (apache#38033)

Signed-off-by: Grégoire Bellon-Gervais <gregoire.bellon-gervais@docaposte.fr>
Co-authored-by: Evan Rusackas <evan@preset.io>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* refactor: engine enforce SQLAlchemy 2.0 (apache#42277)

* fix(ag-grid-table): avoid ambiguous build query import (apache#42313)

* fix: Revert "chore(deps): bump echarts from 5.6.0 to 6.1.0 in /superset-frontend" (apache#42314)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* adding circleci config

* fix(plugin-chart-echarts): import the -obj locale build so time axes render (apache#42317)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* fix(pandas_postprocessing): avoid FutureWarning for max/min in boxplot MINMAX (apache#42272)

* fix(heatmap): correct tooltip axis value lookup and percentage calculations and add tests (apache#41864)

Signed-off-by: yousoph <sophieyou12@gmail.com>
Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

* fix(forecast): resolve time grain robustly for Prophet forecasting (apache#42145)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* fix: Added PostgreSQL 17.X to the supported database versions table in (apache#42280)

* fix(helm): add MCP HTTPRoute configuration (apache#42219)

* fix(dashboard): offer Exit edit mode when there is nothing to discard (apache#42208)

Co-authored-by: Claude Code <noreply@anthropic.com>

* chore(deps): bump actions/labeler from 6.2.0 to 7.0.0 (apache#42332)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump github/codeql-action/analyze from 4.37.0 to 4.37.1 (apache#42331)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump github/codeql-action/init from 4.37.0 to 4.37.1 (apache#42334)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps-dev): bump @formatjs/intl-durationformat from 0.10.17 to 0.10.18 in /superset-frontend (apache#42337)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump caniuse-lite from 1.0.30001805 to 1.0.30001806 in /docs (apache#42333)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump echarts to 6.1.0 with locale and containLabel guards (apache#42315) (apache#42321)

Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com>

* docs: add pattern to the list of organisations using superset (apache#42341)

* chore(deps): bump ag-grid from 36.0.0 to 36.0.1 in /superset-frontend (apache#42338)

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: hainenber <dotronghai96@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: hainenber <dotronghai96@gmail.com>

* fix(mcp): trust dataset is_dttm flag when applying time_grain to VARCHAR temporal columns (apache#42288)

* chore(deps-dev): update taos-ws-py requirement from >=0.6.9 to >=0.7.0 (apache#42344)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps-dev): bump the storybook group in /superset-frontend with 5 updates (apache#42355)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix(native-filters): keep filter value input caret at inline start (apache#42323)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* fix(mcp): truncate query-tool responses instead of hard-failing (apache#42244)

* chore(deps): bump nh3 from 0.3.5 to 0.3.6 (apache#42349)

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Joe Li <joe@preset.io>

* chore(deps): bump pydantic from 2.11.7 to 2.13.4 (apache#42350)

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Joe Li <joe@preset.io>

* chore(deps): bump sqlalchemy-continuum from 1.6.0 to 1.7.0 (apache#42351)

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Joe Li <joe@preset.io>

* fix(embedded): stop rejecting guest chart data built from control-specific params keys (apache#42295)

Co-authored-by: Claude Code <noreply@anthropic.com>

* fix(explore): show the beginning date on time-series x-axis line charts (apache#42046)

* fix(api): add example to get_export_ids_schema so Swagger "Try it out" pre-fills a valid array (apache#42265)

* chore: SQLAlchemy User cascade backref warnings are irrelevant (apache#42360)

* fix(charts): handle async (202) chart-data responses in StatefulChart (apache#42157)

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-authored-by: Evan Rusackas <evan@preset.io>

* feat(KustoKQL): Add support for NULL / IS NOT NULL operator (apache#37890)

Co-authored-by: ag-ramachandran <ramacg@microsoft.com>
Co-authored-by: Joe Li <joe@preset.io>

* chore(deps): bump pillow from 12.2.0 to 12.3.0 (apache#42348)

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Joe Li <joe@preset.io>

* chore(deps): bump flask-compress from 1.17 to 1.24 (apache#42346)

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Joe Li <joe@preset.io>

* chore(deps-dev): bump databricks-sql-connector from 4.2.6 to 4.3.0 (apache#42347)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Joe Li <joe@preset.io>
Co-authored-by: Evan Rusackas <evan@preset.io>

* fix(native-filters): use FILTER_STATE_CACHE_CONFIG timeout for dynamic filter option queries (apache#38910)

* fix(explore): render Jinja before validating legacy chart filters (apache#41996)

* fix(dataset): disable duplicate button when name is empty (apache#42217)

Co-authored-by: AS-MAC-1123 <as-mac-1123@AS-MAC-1123.local>
Co-authored-by: Evan Rusackas <evan@rusackas.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

* docs(gunicorn): correct dead links in values.yaml (apache#42385)

* chore(deps): bump @deck.gl/mapbox from 9.3.6 to 9.3.7 in /superset-frontend in the deckgl group (apache#42377)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump actions/checkout from 7.0.0 to 7.0.1 (apache#42376)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump immer from 11.1.11 to 11.1.15 in /superset-frontend (apache#42378)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump body-parser from 1.20.5 to 1.20.6 in /docs (apache#42370)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps-dev): bump brace-expansion from 1.1.15 to 1.1.16 in /superset-embedded-sdk (apache#42369)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix(security): bump pyasn1 from 0.6.3 to 0.6.4 (apache#42363)

* fix(security): bump pillow from 12.2.0 to 12.3.0 (apache#42362)

* docs(map-tiles): add Yandex Maps Tiles API configuration (apache#42375)

* ci: improve conditional checks for lillio tests and build

---------

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: hainenber <dotronghai96@gmail.com>
Signed-off-by: Grégoire Bellon-Gervais <gregoire.bellon-gervais@docaposte.fr>
Signed-off-by: yousoph <sophieyou12@gmail.com>
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com>
Co-authored-by: Evan Rusackas <evan@preset.io>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Superset Dev <dev@superset.apache.org>
Co-authored-by: SkinnyPigeon <e.blackledge@stuart.com>
Co-authored-by: Zack <adams.z.d@gmail.com>
Co-authored-by: Zack Adams <zack@Zacks-Laptop.local>
Co-authored-by: Evan Rusackas <evan@rusackas.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: hainenber <dotronghai96@gmail.com>
Co-authored-by: Joe Li <joe@preset.io>
Co-authored-by: Grégoire <greggbg@gmail.com>
Co-authored-by: Hans Yu <hans.yu@outlook.de>
Co-authored-by: Elizabeth Thompson <eschutho@gmail.com>
Co-authored-by: yousoph <sophieyou12@gmail.com>
Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com>
Co-authored-by: Amitesh Gupta <143833521+singlaamitesh@users.noreply.github.com>
Co-authored-by: David <39565245+dmunozv04@users.noreply.github.com>
Co-authored-by: Yash Shrivastava <119301033+alephys26@users.noreply.github.com>
Co-authored-by: JUST.in DO IT <justin.park@airbnb.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: jesperct <jmilecelento@gmail.com>
Co-authored-by: Abdul Rehman <76230556+Abdulrehman-PIAIC80387@users.noreply.github.com>
Co-authored-by: jenwitteng <jenwit.amonpongitsara@agoda.com>
Co-authored-by: Ramachandran A G <106139410+ag-ramachandran@users.noreply.github.com>
Co-authored-by: ag-ramachandran <ramacg@microsoft.com>
Co-authored-by: Ujjwal Jain <jainujjwal1609@gmail.com>
Co-authored-by: Jean Massucatto <massucattoj@gmail.com>
Co-authored-by: suvankardas216 <rohanrohan510@gmail.com>
Co-authored-by: AS-MAC-1123 <as-mac-1123@AS-MAC-1123.local>
Co-authored-by: Alejandro Solares <219859296+ASolarers-Rodriguez@users.noreply.github.com>
Co-authored-by: ViktorGo86 <114023094+ViktorGo86@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

explore:filter Related to filters in Explore global:jinja Related to Jinja templating size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants