Skip to content

fix(cache): warm native dashboard filter defaults - #43073

Open
endimonan wants to merge 6 commits into
apache:masterfrom
endimonan:fix/dashboard-cache-warm-up-filters
Open

fix(cache): warm native dashboard filter defaults#43073
endimonan wants to merge 6 commits into
apache:masterfrom
endimonan:fix/dashboard-cache-warm-up-filters

Conversation

@endimonan

@endimonan endimonan commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

SUMMARY

Fixes #43024.

Dashboard cache warm-up now includes persisted native filter defaults represented by extra_form_data.filters. It reuses the existing dashboard filter-context helper for scope and default-value extraction, then preserves browser query ordering by placing dashboard filters before the chart's saved filters:

  1. legacy dashboard filters;
  2. native dashboard filters;
  3. saved chart filters.

Explicit extra_filters supplied to the warm-up endpoint keep their existing precedence. The change is intentionally limited to filter clauses; other native-filter context fields and plugin-specific query normalization are outside this PR.

Related regression coverage: #43025.
Related discussion: #42382.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Not applicable. This change affects backend query construction and cache-key compatibility.

TESTING INSTRUCTIONS

Run the focused unit tests:

pytest \
  tests/unit_tests/charts/test_dashboard_filter_context.py \
  tests/unit_tests/commands/chart/warm_up_cache_test.py \
  tests/unit_tests/views/test_utils.py

Run the causal integration regression:

pytest tests/integration_tests/charts/api_tests.py \
  -k test_warm_up_cache_native_defaults_hit_browser_query_cache

The integration test persists a legacy default, a native select default, and a saved chart filter, warms the chart cache, sends the browser-equivalent request with force=false, verifies the legacy → native → chart order, and asserts is_cached=true.

ADDITIONAL INFORMATION

@netlify

netlify Bot commented Aug 11, 2026

Copy link
Copy Markdown

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 38eb2de
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6a7c39c02e93550008192820
😎 Deploy Preview https://deploy-preview-43073--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.

@codecov

codecov Bot commented Aug 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.67%. Comparing base (a501fed) to head (0649aae).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #43073      +/-   ##
==========================================
+ Coverage   66.64%   66.67%   +0.03%     
==========================================
  Files        2863     2863              
  Lines      162154   162116      -38     
  Branches    37404    37384      -20     
==========================================
+ Hits       108065   108090      +25     
+ Misses      52030    51943      -87     
- Partials     2059     2083      +24     
Flag Coverage Δ
hive 38.32% <20.00%> (+<0.01%) ⬆️
mysql 57.96% <100.00%> (+0.08%) ⬆️
postgres 57.99% <100.00%> (+0.08%) ⬆️
presto 40.29% <20.00%> (+<0.01%) ⬆️
python 59.39% <100.00%> (+0.08%) ⬆️
sqlite 57.62% <100.00%> (+0.08%) ⬆️
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.

@endimonan
endimonan force-pushed the fix/dashboard-cache-warm-up-filters branch from d07470c to cccd3a5 Compare August 11, 2026 21:15
@endimonan endimonan changed the title fix(cache): align dashboard cache warm-up filters fix(cache): warm native dashboard filter defaults Aug 11, 2026
@endimonan
endimonan marked this pull request as ready for review August 12, 2026 12:57
@dosubot dosubot Bot added dashboard:native-filters Related to the native filters of the Dashboard infra:caching Infra setup and configuration related to caching labels Aug 12, 2026
Comment thread superset/commands/chart/warm_up_cache.py
@bito-code-review

Copy link
Copy Markdown
Contributor

The flagged issue is correct. In superset/commands/chart/warm_up_cache.py, the change from extend to list concatenation (+) prepends dashboard filters to the existing query.filter list. This causes dashboard filters to be applied alongside explicit filters rather than replacing them, which can lead to conflicts if both sets of filters target the same column.

To resolve this, you should ensure that explicit filters take precedence. One approach is to filter out any dashboard-derived filters that conflict with the explicit filters before merging, or to maintain the previous behavior where explicit filters are treated as authoritative.

Would you like me to implement a fix for this issue? I can also check the rest of the PR for other comments if you would like.

superset/commands/chart/warm_up_cache.py

# Example of preserving precedence
                existing_cols = {f.get("col") for f in query.filter}
                filtered_dashboard_filters = [
                    f for f in dashboard_filters if f.get("col") not in existing_cols
                ]
                query.filter.extend(filtered_dashboard_filters)

@bito-code-review

bito-code-review Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #8d7e21

Actionable Suggestions - 0
Filtered by Review Rules

Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.

  • superset/views/utils.py - 1
Review Details
  • Files reviewed - 5 · Commit Range: cccd3a5..0649aae
    • superset/commands/chart/warm_up_cache.py
    • superset/views/utils.py
    • tests/integration_tests/charts/api_tests.py
    • tests/unit_tests/commands/chart/warm_up_cache_test.py
    • tests/unit_tests/views/test_utils.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

@endimonan

endimonan commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

The flagged issue is correct. In superset/commands/chart/warm_up_cache.py, the change from extend to list concatenation (+) prepends dashboard filters to the existing query.filter list. This causes dashboard filters to be applied alongside explicit filters rather than replacing them, which can lead to conflicts if both sets of filters target the same column.

To resolve this, you should ensure that explicit filters take precedence. One approach is to filter out any dashboard-derived filters that conflict with the explicit filters before merging, or to maintain the previous behavior where explicit filters are treated as authoritative.

Would you like me to implement a fix for this issue? I can also check the rest of the PR for other comments if you would like.

superset/commands/chart/warm_up_cache.py

# Example of preserving precedence
                existing_cols = {f.get("col") for f in query.filter}
                filtered_dashboard_filters = [
                    f for f in dashboard_filters if f.get("col") not in existing_cols
                ]
                query.filter.extend(filtered_dashboard_filters)

Thanks for taking a look at this. The flagged issue isn't correct. Although this query-building flow is not so straightforward to me, so I went back and checked both the previous behavior and the frontend implementation.

Before this change, query.filter.extend(dashboard_filters) already combined both sets of filters. It did not replace the saved chart filters. This change only corrects their order to match buildQueryObject: dashboard or explicit filters first, followed by the saved chart filters.

Explicit extra_filters still take precedence over filters read from the dashboard metadata. Filtering them by column would change the query and could bring back the cache-key mismatch.

@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.

Lookin good. Backed by an integration test that actually asserts is_cached=true on the browser-equivalent request; CI's are green and mergeable is clean. Solid, well-scoped. I'll merge soon...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dashboard:native-filters Related to the native filters of the Dashboard infra:caching Infra setup and configuration related to caching size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

warm_up_cache produces wrong cache key for dashboards with native filters

2 participants