fix(time-comparison): shift offset filter when X-axis is adhoc Custom SQL#40586
Conversation
Code Review Agent Run #f1a130Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40586 +/- ##
=======================================
Coverage 63.94% 63.95%
=======================================
Files 2658 2658
Lines 143011 143011
Branches 32866 32866
=======================================
+ Hits 91454 91456 +2
+ Misses 49994 49992 -2
Partials 1563 1563
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
65c15d2 to
afa24ce
Compare
… SQL When the X-axis is an adhoc Custom SQL column (label != underlying time column), the relative time-offset datetime-series branch matched the existing TEMPORAL_RANGE filter against `granularity or x_axis_label`, which never equaled the dataset column. The filter kept the original range and the offset query AND'd both ranges together, returning an empty intersection. Match against the column resolved from the existing filter via `_get_temporal_column_for_filter` (already used for date-range offsets) so the filter is shifted correctly.
afa24ce to
fdb2d62
Compare
Code Review Agent Run #a27f44Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
betodealmeida
left a comment
There was a problem hiding this comment.
Thanks for fixing this! We need to spend some time cleaning up this logic, it's getting very messy.
SUMMARY
When a chart with a Custom SQL X-axis (adhoc column whose label differs from the underlying time column) uses a relative time comparison like "1 year ago", the offset query's temporal filter never gets shifted. The original time range and the granularity-derived shifted range both end up in the WHERE clause, AND'd together. The intersection covers only the overlap of the two ranges, so the comparison series shows values for just that overlap (e.g. last ~4 months of the requested window) instead of the full range.
In
processing_time_offsets(relative-offset, datetime-series branch), the existing TEMPORAL_RANGE filter was matched againstquery_object_clone.granularity or x_axis_label. For an adhoc Custom SQL X-axis the label is the user's column label (e.g.wedding_date_cast), not the dataset column the filter is on (wedding_date), so the match fails and the filter'svalstays at the original range.The fix uses
_get_temporal_column_for_filter, which is already used by the date-range-offset branch. It prefers the column from an existing TEMPORAL_RANGE filter over the X-axis label.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Same chart, same X-axis Custom SQL, time range 2025-01-01 to 2026-06-01, "1 year ago" comparison. Dataset has year-dependent revenue (2024 baseline + 80/year) so the comparison line is visually offset from the main line.
Before —
1 year agoseries (purple) only renders for the last ~4 months of the range (the overlap of original and shifted windows):After —
1 year agoseries plots across the full range, ~80 below the mainSUM(revenue)line (as expected from the dataset formula):Offset query WHERE clause before/after:
Before (offset has impossible intersection of original + shifted ranges):
After (offset uses the shifted range only):
TESTING INSTRUCTIONS
wedding_date).wedding_date, time grain Month.Unit test:
test_processing_time_offsets_updates_temporal_filter_with_adhoc_x_axis.ADDITIONAL INFORMATION