fix(trends): redo dst fix with toDate
#32111
Conversation
This reverts commit 4df8220.
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
34438c5 to
1d60f05
Compare
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
toDate
There was a problem hiding this comment.
PR Summary
This PR fixes DST (Daylight Saving Time) transition issues in trends queries by using toDate for timestamp conversions instead of toStartOfDay, particularly affecting daily intervals.
- Modified
trends_query_builder.pyto usetoDatefor daily intervals and adjusted date_from_start_of_interval_for_addition handling in date subqueries - Added comprehensive DST transition tests covering Egypt's UTC+02 to UTC+03 shift and US Pacific timezone changes
- Removed redundant parentheses around ast.Constant arguments in query_date_range.py date conversion methods
- Fixed issue where queries with day windows crossing DST boundaries were incorrectly returning 0 after the transition
4 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
| response[0]["data"], | ||
| [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23], | ||
| ) | ||
|
|
There was a problem hiding this comment.
style: The test data setup creates events with increasing counts per hour (0 to 23 events), but the assertion only checks for the raw hour numbers. Consider asserting the actual event counts to verify the aggregation logic.
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
ab25a83 to
1d60f05
Compare
06851eb to
461a9e7
Compare
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
|
Size Change: 0 B Total Size: 3.65 MB ℹ️ View Unchanged
|
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
Reverts the revert #32075 of #31984
Now uses
toDateto convert timestamps for daily intervals in a DST-compatible manner.