rilldata / rill

Rill is a tool for effortlessly transforming data sets into powerful, opinionated dashboards using SQL. BI-as-code.
https://www.rilldata.com
Apache License 2.0
1.64k stars 111 forks source link

Time Detail Dimension View does not handle DST correctly #3560

Open AndrewRTsao opened 9 months ago

AndrewRTsao commented 9 months ago

Describe the bug There are issues with proper DST handling in the pivot table when opening the time detail dimension view and DST is included as part of the filtered time range. This issue will be further exacerbated if first_day_of_week is configured and set to a different value (e.g. Sunday), resulting in erroneous weekly columns on the wrong days to be displayed along with incorrect calculations. Originally reported by a customer here. We may want to extend the solution covered in #3494 (notion doc) to also apply to time detail dimension views as well.

To Reproduce Steps to reproduce the behavior:

  1. Clone the 311 Operational Metrics locally
  2. Apply the filter to include Nov 5 (daylight savings in the US)
  3. Drill into the time detail dimension view
  4. Check the weekly columns (around Nov 5) with a non-UTC timezone selected (e.g. CST or EST)
  5. Apply the first_day_of_week: 7 property to the dashboard YAML
  6. Check the results and make sure the weekly columns (+ data) are correct

Expected behavior The pivot table should display the correct weekly columns, both data and the date / day shown in the column header, regardless if UTC or a different timezone with DST is being used.

Actual Behavior The pivot table displays incorrect weekly columns when a non-UTC timezone is being used where DST is a consideration.

Screenshots

DST issue with EST and no first_day_of_week set:

1) Pivot table DST issue with EST

No DST issue when using UTC with no first_day_of_week set:

2) Pivot table DST working with UTC

DST issue with CST and first_day_of_week: 7 set:

3) Pivot table DST issue with CST and first_day_of_week configured

No DST issue with UTC and first_day_of_week: 7 set:

4) Pivot table DST working with UTC and first_day_of_week set
egor-ryashin commented 8 months ago

I checked main today, it works correctly. First day - Monday image

First day - Sunday image

Timestamps are 7 days apart.

AndrewRTsao commented 8 months ago

@egor-ryashin So I can confirm that the issue is still reproducible on main (it's likely a corner case). See screenshots - tested on both v0.38.2 and main.

Steps to reproduce:

  1. Use the Rill-311-Ops example project
  2. Use the attached dashboard yaml to change the timezone to America/Chicago and first_day_of_week to Sun
  3. Drill into Median Time to Resolve Ticket (Hours) from main dashboard view
  4. Adjust date range to Oct 29 - Nov 30 and make sure that metric trends is set to week
  5. Note that the column header in the TDD will still show Nov 4 (Sat) as first day of week instead of Nov 5 (Sun)
1) Issue on v38 2 2) Issue on main