Skip to content

fix(drill-by): keep the clicked time bucket when drilling from a grained temporal axis - #45612

Open
aminghadersohi wants to merge 28 commits into
apache:masterfrom
aminghadersohi:fix-drill-by-grained-temporal-range
Open

aminghadersohi wants to merge 28 commits into
apache:masterfrom
aminghadersohi:fix-drill-by-grained-temporal-range

Conversation

@aminghadersohi

@aminghadersohi aminghadersohi commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

SUMMARY

Grained Drill by carries the clicked temporal bucket as a grained equality, like Drill to detail, while preserving the chart's existing time filters and effective dashboard grain. This supports independent date-column filters, nested buckets, custom SQL axes, markerless line/area clicks and either Mixed Timeseries query. Downloads and Edit chart use the same predicates.

In Explore, changing the bucket's column or operator removes its inherited grain. Picking a date range therefore compares raw dates; comparator-only and clause-only edits preserve bucket semantics. The Custom SQL tab is unavailable while a filter carries a grain because converting that predicate to portable raw SQL would lose its meaning. Late responses cannot replace results from a subsequent drill.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

The existing screenshots below are historical captures, not fresh acceptance evidence for this revision. The local health endpoint was unavailable; I did not re-capture them or run fresh live UI/SQL acceptance.

Screenshots

Before After
Month chart under a dashboard Time grain of Year, Drill by deal_size from the 2003 bar (1,000): before, only January 2003 (Medium 23 + Small 16 = 39); after, all of 2003 (Medium 502 + Small 444 + Large 54 = 1,000)
Horizontal Day chart under a dashboard Time grain of Year, Drill by from the 2004 bar (1,345): before, only 2004-01-01 (0 rows); after, all of 2004 (1,345)
Month chart under a dashboard Time grain of Year and Time range 2003-06-01 : 2004-01-01, Drill by from the 2003 bar (754): Medium 379 + Small 332 + Large 43 = 754, only the part of 2003 inside the range (new)
Monthly bar chart, no dashboard filters, Drill by deal_size from the Oct 2003 bar: before (merge-base), every month (2,823 rows); after, only Oct 2003 (Medium 86 + Small 65 + Large 7 = 158)

Captured at ac639621. Local docker-compose-light env at the PR head, examples dataset cleaned_sales_data. Before = the previous PR head for the Time grain rows, the merge-base for the last row.

TESTING INSTRUCTIONS

Validation for this revision:

  • Broader Jest scope (FilterControl, utils, Drill by, Drill to detail and filter conversion): 1,058 passed across 87 suites.
  • Touched Timeseries, Mixed Timeseries, Sunburst and bucket-helper suites: 130 passed across five suites.
  • Final FilterControl scope after the SQL-tab guard: 128 passed across ten suites.
  • Docker Python sibling suites: 321 passed.
  • Docker semantic-layer CI coverage gate: 1,827 passed, 100.00% statement/branch coverage.
  • Pre-commit against changed branch files: passed, including mypy, frontend type checking and repository-pinned Ruff.
# From superset-frontend; scoped Jest runs
CI=true npm run test -- --runInBand src/explore/components/controls/FilterControl
CI=true npm run test -- --runInBand --runTestsByPath \
  plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.test.tsx \
  plugins/plugin-chart-echarts/test/Timeseries/EchartsTimeseries.test.tsx \
  plugins/plugin-chart-echarts/test/MixedTimeseries/EchartsMixedTimeseries.test.tsx \
  plugins/plugin-chart-echarts/test/Sunburst/EchartsSunburst.test.tsx \
  plugins/plugin-chart-echarts/test/utils/xAxisDrillByFilter.test.ts

# In the Python Docker test environment, using this checkout's coverage/pytest configuration
python -m pytest --durations-min=0.5 --cov=superset/semantic_layers/ \
  tests/unit_tests/semantic_layers/ --cache-clear --cov-fail-under=100 \
  -m 'not manual and not llm_compliance'
python -m pytest -n 2 --dist loadfile --cov-report= --cov=superset \
  tests/unit_tests/common/test_drill_by_time_column.py \
  tests/unit_tests/common/test_query_context_factory.py \
  tests/unit_tests/core_tests.py \
  tests/unit_tests/mcp_service/chart/test_chart_utils.py \
  -m 'not manual and not llm_compliance'

The date-picker and other operator-edit regressions fail on the reviewed head (five failures) and pass after the change. The additional Custom SQL guard regression fails before its guard and passes after it. Invalid bucket metadata and scalar fallback cases cover the mapper branches missing from the previous CI coverage gate.

Manual checks when a development environment is available:

  • Drill a monthly order_date bucket, choose Edit chart, and pick 2004-01-15 : 2004-02-15. The resulting predicate must have op: TEMPORAL_RANGE with no grain, selecting raw dates rather than month-truncated dates.
  • Verify a bucket filter has no Custom SQL tab; ordinary SQL filters keep their tab.
  • Compare chart counts, Drill by, downloads and Edit chart with saved/native/legacy time ranges, independent date-column filters, nested buckets and either Mixed Timeseries query.
  • Repeat with markerless line/area charts and forced-categorical temporal axes.

Remaining: fresh live UI/SQL acceptance was not run because the local health endpoint is unavailable. Historical screenshots below are not acceptance evidence for this revision. Fresh GitHub CI is running; its final result is unknown.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration — follow [SIP-59] Proposal for Database migration standards #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

…ned temporal axis

Drill by from a grained temporal bucket sends the bucket as a TEMPORAL_RANGE
filter, but the backend replaces the value of every TEMPORAL_RANGE filter with
the query's time_range. The bucket was widened to the chart's time range
("No filter" returned every row) or to the dashboard's Time range.

The Drill by modal now carries the innermost TEMPORAL_RANGE drill filter as the
drilled query's time_range, and replaces a dashboard time_range override in
extra_form_data with it. The filter stays a TEMPORAL_RANGE, so the dataset
timezone and Hours offset are still applied to its bounds.
Carry the clicked bucket through reserved legacy dashboard time filters as well as native filter overrides. Preserve categorical filters and the source form data, and exercise the outgoing query rather than only its form data.\n\nWait for the modal transition before asserting breadcrumb visibility.
@netlify

netlify Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for superset-docs-preview ready!

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

@github-actions github-actions Bot added the doc Namespace | Anything related to documentation label Oct 22, 2026
@codecov

codecov Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.68345% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.04%. Comparing base (230d9b0) to head (d5b6c32).

Files with missing lines Patch % Lines
...ugin-chart-echarts/src/utils/xAxisDrillByFilter.ts 96.00% 2 Missing ⚠️
...rts/src/MixedTimeseries/EchartsMixedTimeseries.tsx 95.83% 1 Missing ⚠️
...chart-echarts/src/Timeseries/EchartsTimeseries.tsx 96.42% 1 Missing ⚠️
...tend/src/components/Chart/DrillBy/DrillByModal.tsx 92.85% 1 Missing ⚠️
superset/models/helpers.py 94.11% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #45612      +/-   ##
==========================================
+ Coverage   85.03%   85.04%   +0.01%     
==========================================
  Files        3080     3080              
  Lines      209437   209549     +112     
  Branches    49385    49455      +70     
==========================================
+ Hits       178095   178218     +123     
+ Misses      27960    27948      -12     
- Partials     3382     3383       +1     
Flag Coverage Δ
hive 34.19% <0.00%> (-0.01%) ⬇️
javascript 81.40% <95.90%> (+0.03%) ⬆️
mysql 52.01% <17.64%> (-0.01%) ⬇️
postgres 52.03% <17.64%> (-0.01%) ⬇️
presto 35.85% <17.64%> (-0.01%) ⬇️
python 87.96% <94.11%> (+<0.01%) ⬆️
sqlite 51.78% <17.64%> (-0.01%) ⬇️
unit 82.90% <94.11%> (+<0.01%) ⬆️

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.

…ange

A grained Drill by replaced the chart's or dashboard's time range with the
clicked bucket. When that range covered only part of the bucket (a Year bar
under a Jun-Dec range, a Quarter bar under a mid-quarter range, a Year bar
after a monthly cross-filter), the drill returned rows the chart never
displayed, e.g. 1,000 rows for a bar that showed 754.

Resolve the chart's effective time range (dashboard override, legacy
dashboard time filter, chart time range, or the chart's own TEMPORAL_RANGE
filter on the drilled column) through /api/v1/time_range/ and query only the
overlap of that range and the bucket. The chart data request waits for the
range, so it is sent once. If the range cannot be resolved, the bucket is
used as before.
@aminghadersohi

Copy link
Copy Markdown
Contributor Author

Acceptance at 4f27f24 (local docker-compose-light, examples dataset cleaned_sales_data, dashboard with 29 charts and 11 native filters; admin, a Gamma viewer and an embedded guest).

Works. Timeseries Drill by from a grained bucket returns the clicked bucket's rows. Chart time range "No filter": Day 34, Week 98, Month (Oct 2003) 158, Quarter (2003 Q4) 524, Year (2003) 1,000, each equal to a SQL count (merge-base: 2,823). Under a dashboard Time range 2003-01-01 : 2005-01-01: Oct 2003 158 (merge-base: 2,345). Hours offset ±5h, nested Drill by (754 → 379 → 158), Drill by after a cross-filter, CSV export and the Edit chart link agree with the clicked bar. Ungrained and categorical Drill by keep the chart and dashboard ranges.

Remaining (not fixed, outside this diff): under a dashboard Time grain filter set to Year, a Month chart's 2003 bar (1,000) drills to 39 rows (Jan 2003 only) and a Day chart's to 0, because EchartsTimeseries.tsx builds the drill bucket from formData.timeGrainSqla (the saved grain) at both getTemporalXAxisDrillByFilter calls, while Drill to detail in the same handler uses the resolved grain. Sunburst still sends order_date == '2003' and returns all 2,823 rows for a 1,000-row ring; it needs #45571. Minor and pre-existing: on a dataset with a timezone west of UTC, Month buckets are one day too long (bar 296, drill 306) because getTimeBucketRange adds a calendar month to a shifted label, and a forced-categorical temporal axis shows the raw epoch in the breadcrumb.

Fixed in 4f27f24. When the chart's own range, a dashboard Time range or a time cross-filter covers only part of the bucket, d0ea5a4 queried the whole bucket: a Year bar showing 754 under a Jun–Dec 2003 range drilled to 1,000 rows (the merge-base gave 754), and a 2003 Q4 bar of 173 under 2003-11-15 : 2004-02-15 drilled to 524. The modal now resolves the chart's range through /api/v1/time_range/ and queries the overlap: 754 and 173. Regression tests in DrillByModal.test.tsx, "keeps only the part inside a narrower dashboard time range" and "keeps only the part inside the chart's own time range", fail at d0ea5a4 and pass at 4f27f24.

Siblings checked:

  • superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx:395 — fixed in 4f27f24
  • superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx:428 — not affected: CSV export uses the same drilled form data; rows 43 / 379 / 332 = 754
  • superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx:581 — not affected: the Edit chart link uses the same drilled form data; stored time_range 2003-06-01T00:00:00 : 2004-01-01T00:00:00
  • superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx:494 — not affected: a nested drill reads the outer range from the original form data; 754 → 379 → 158
  • superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx:742 — not affected: a time cross-filter's range is read as the outer range; a Year bar after an Oct 2003 cross-filter drills to 158

@aminghadersohi

Copy link
Copy Markdown
Contributor Author

Acceptance of the dashboard Time grain change at ac63962: ship.

I ran 40 Drill by actions in a local docker-compose-light env on the examples dataset cleaned_sales_data, and every drilled total matched the clicked bar and a direct SQL count:

  • Dashboard Time grain Year over Month, Day, Week and line charts, horizontal Month and Day charts, and a forced-categorical temporal axis: 2003 = 1,000 and 2004 = 1,345. Quarter (2003 Q4 = 524), Month (Oct 2003 = 158, Nov 2003 = 296) and Day (2003-11-12 = 34) grains also match.
  • Year grain combined with a dashboard Time range of 2003-06-01 : 2004-01-01 returns 754. After an Oct 2003 cross-filter with the grain switched to Year in the filter bar, the drill returns 158, and a nested drill (Small, then product_line) returns 65.
  • Without filters, Day/Week/Month/Quarter/Year drills return 34/98/158/524/1,000. The ungrained axis keeps == and categorical Drill by keeps the chart range (1,384).
  • A Gamma viewer and an embedded guest (guest token) get the same totals, with no console errors.

With EchartsTimeseries.tsx reverted to 4f27f24, the Year-grain drills returned 39 and 0 rows for bars of 1,000 and 1,345. The four new EchartsTimeseries.test.tsx cases fail at that file and pass at ac63962. Screenshots in the description were re-captured at this head.

This PR does not change Sunburst; that goes in #45571.

@aminghadersohi
aminghadersohi marked this pull request as ready for review October 22, 2026 14:08
@codeant-ai-for-open-source

codeant-ai-for-open-source Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

CodeAnt  CodeAnt PR Risk

🟢  Low Risk

  • The PR appears safe to merge; temporal bucket drills preserve chart time filters, and superseded requests are prevented from updating the modal.

Assessed commit: d5b6c32e92c7

@github-actions github-actions Bot added the requires:rebase Requires rebasing on top of current master label Oct 23, 2026
@github-actions github-actions Bot removed the requires:rebase Requires rebasing on top of current master label Oct 23, 2026

@rebenitez1802 rebenitez1802 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Request changes: Drill by now keeps the clicked time bucket, and that part works. But on charts without a saved time_range it also overwrites the chart's own filter on a different date column. Details and an applyable fix are inline.

@aminghadersohi

Copy link
Copy Markdown
Contributor Author

@rebenitez1802 I reproduced the review body's independent-date-column regression. Fixed in 6c75c08: a drill without a query-wide range leaves it unset; with independent temporal filters, saved/native/legacy range overrides are represented by the resolved clicked filter rather than overwriting other columns. Other date filters and outer drill buckets retain their values.

Regression test: Sunburst Drill by preserves an independent date filter without a saved time range in superset-frontend/src/components/Chart/DrillBy/DrillByTemporalFilters.test.tsx. The three temporal drill preserves an independent date filter in query and CSV with saved range %s cases cover absent, No filter and bounded saved ranges, plus chart-data, CSV and Edit chart payloads. Two helper cases cover dashboard/legacy overrides and both mixed-query filter fields.

The same commit also carries clicked grained buckets from markerless line/area charts and both Mixed Timeseries queries, and formats forced-categorical temporal bucket labels. Regression tests: forced categorical temporal Drill by formats the bucket instead of an epoch label and Mixed Timeseries forced categorical temporal Drill by keeps a readable bucket; the parameterized markerless and mixed-query regressions cover the other entry points. The focused comparison is 17 failures with the starting source, 17 passes with this commit. Three ungrained scope/label compatibility cases pass both before and after. An additional exact-match label guard prevents the readable-label change from leaking into ungrained forced-categorical drills.

Siblings checked:

  • superset-frontend/src/components/Chart/DrillBy/drillByTimeRange.ts:191 — fixed in 6c75c08
  • superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx:419 — fixed in 6c75c08
  • superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx:683 — fixed in 6c75c08
  • superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/EchartsMixedTimeseries.tsx:222 — fixed in 6c75c08
  • superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:1149 — fixed in 6c75c08
  • superset-frontend/plugins/plugin-chart-echarts/src/utils/xAxisDrillByFilter.ts:170 — fixed in 6c75c08
  • superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.tsx:156 — not affected: forwards the bucket and formattedVal to the shared modal; scope-selection tests pass.
  • superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/EchartsSunburst.tsx — not affected: creates its own segment filter and uses the shared modal, covered by the added independent-date regression.
  • superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx — not affected: its equality-based Month-cell path is outside this temporal-bucket change.
  • superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx — not affected: its equality-based Month-cell path is outside this temporal-bucket change.
  • superset-frontend/src/components/Chart/DrillBy/useResultsTableView.tsx:34 — not affected: reload and CSV/XLSX use the modal's shared normalized form data.

Validation: Jest: 829 passed, 1 skipped (45 suites); America/New_York: 98 passed (4 suites); Docker backend temporal-query tests: 97 passed. Changed-file pre-commit (including type checking) passed. The four stale-response cases from #45612 (comment) still pass. Fresh live UI/SQL acceptance was not run: the local health endpoint was unavailable.

Remaining: cross-filter-chip ISO display and differences in breadcrumb column names are unchanged; only the epoch bucket-label correction is included in this update. Right-click discoverability and Table/AG Grid Month-cell behavior are intentionally untouched.

@github-actions github-actions Bot added i18n Namespace | Anything related to localization i18n:spanish Translation related to Spanish language i18n:italian Translation related to Italian language i18n:french Translation related to French language i18n:chinese Translation related to Chinese language i18n:japanese Translation related to Japanese language i18n:russian Translation related to Russian language i18n:korean Translation related to Korean language i18n:dutch i18n:slovak i18n:ukrainian i18n:portuguese i18n:brazilian i18n:traditional-chinese i18n:persian i18n:czech i18n:latvian labels Oct 26, 2026
aminghadersohi and others added 2 commits October 27, 2026 23:18
…d-temporal-range

# Conflicts:
#	superset/translations/ar/LC_MESSAGES/messages.po
#	superset/translations/ca/LC_MESSAGES/messages.po
#	superset/translations/cs/LC_MESSAGES/messages.po
#	superset/translations/de/LC_MESSAGES/messages.po
#	superset/translations/en/LC_MESSAGES/messages.po
#	superset/translations/es/LC_MESSAGES/messages.po
#	superset/translations/fa/LC_MESSAGES/messages.po
#	superset/translations/fi/LC_MESSAGES/messages.po
#	superset/translations/fr/LC_MESSAGES/messages.po
#	superset/translations/it/LC_MESSAGES/messages.po
#	superset/translations/ja/LC_MESSAGES/messages.po
#	superset/translations/ko/LC_MESSAGES/messages.po
#	superset/translations/lv/LC_MESSAGES/messages.po
#	superset/translations/messages.pot
#	superset/translations/mi/LC_MESSAGES/messages.po
#	superset/translations/nl/LC_MESSAGES/messages.po
#	superset/translations/pl/LC_MESSAGES/messages.po
#	superset/translations/pt/LC_MESSAGES/messages.po
#	superset/translations/pt_BR/LC_MESSAGES/messages.po
#	superset/translations/ro/LC_MESSAGES/messages.po
#	superset/translations/ru/LC_MESSAGES/messages.po
#	superset/translations/sk/LC_MESSAGES/messages.po
#	superset/translations/sl/LC_MESSAGES/messages.po
#	superset/translations/sr/LC_MESSAGES/messages.po
#	superset/translations/sr_Latn/LC_MESSAGES/messages.po
#	superset/translations/ta/LC_MESSAGES/messages.po
#	superset/translations/th/LC_MESSAGES/messages.po
#	superset/translations/tr/LC_MESSAGES/messages.po
#	superset/translations/uk/LC_MESSAGES/messages.po
#	superset/translations/zh/LC_MESSAGES/messages.po
#	superset/translations/zh_TW/LC_MESSAGES/messages.po
@github-actions github-actions Bot added the requires:rebase Requires rebasing on top of current master label Nov 4, 2026
@github-actions github-actions Bot added requires:rebase Requires rebasing on top of current master and removed requires:rebase Requires rebasing on top of current master labels Nov 4, 2026
…d-temporal-range

# Conflicts:
#	superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/EchartsTimeseries.test.tsx
@github-actions github-actions Bot removed the requires:rebase Requires rebasing on top of current master label Nov 5, 2026

This branch has not been deployed

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

Labels

doc Namespace | Anything related to documentation i18n:brazilian i18n:chinese Translation related to Chinese language i18n:czech i18n:dutch i18n:french Translation related to French language i18n:italian Translation related to Italian language i18n:japanese Translation related to Japanese language i18n:korean Translation related to Korean language i18n:latvian i18n:persian i18n:portuguese i18n:russian Translation related to Russian language i18n:slovak i18n:spanish Translation related to Spanish language i18n:traditional-chinese i18n:ukrainian i18n Namespace | Anything related to localization packages plugins risk:chart-metadata size/XXL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants