Add time zone support

[stanhu]

Right now re:dash assumes that all timestamps are in the local time zone. However, Redshift (and many other databases, depending on the configuration) default to UTC, and thus re:dash parses the data incorrectly. What's confusing is that in Highcharts also formats the data in local time, leading to this awkward display:

Notice how the data is highlighted at the beginning of April 23 (which hasn't happened yet in GMT), but the date is showing April 22 4:00 pm (which hasn't happened yet in Pacific Standard Time).

I temporarily worked around this by patching ng_highchart.js:

         if (moment.isMoment(this.x)) {
-          var s = '<b>' + this.x.toDate().toLocaleString() + '</b>',
+          var s = '<b>' + this.x.toDate().toISOString() + '</b>',
               pointsCount = this.points.length;

resources.js:

             if (angular.isNumber(v)) {
               columnTypes[k] = 'float';
             } else if (_.isString(v) && v.match(/^\d{4}-\d{2}-\d{2}T/)) {
-              row[k] = moment(v);
+              row[k] = moment.utc(v);
               columnTypes[k] = 'datetime';
             } else if (_.isString(v) && v.match(/^\d{4}-\d{2}-\d{2}/)) {
-              row[k] = moment(v);
+              row[k] = moment.utc(v);
               columnTypes[k] = 'date';
             } else if (typeof(v) == 'object' && v !== null) {
               row[k] = JSON.stringify(v);

I'd imagine there will be cases where some users have their database configured to output a specific standard time and others will use UTC. What's the best way to support this feature? Options:

1. Store the UTC offset in the query_result (so moment will parse the right value)
2. Allow configuration option in re:dash for time zone (e.g. UTC, America/Los_Angeles)

What do you think, @arikfr?

[stanhu]

Oh, I see 267c32b at least fixed the time to be stored UTC internally, but it didn't fix my use case.

We still need to consider how the time is displayed in the chart.

[arikfr]

Actually, there is no assumption about the timezone of the data, and the plan was to show the dates/times as they are returned from the database to avoid such issues and let the querying user control the timezone. About a month ago, I started using the toLocaleString method to print dates and times, to make sure that dates are presented with correct formatting (MM/DD for US, DD/MM for Israel for example).

But looks like this resulted in two bugs:

1. Inconsistency between how the time presented in the the chart axis (which handled by HighCharts) and the rest of the system (tooltips, table).
2. In some configurations, it will show the wrong time. From what I can see, as long as the data is UTC and your machine has correct time zone it should actually work.

I think that the best thing to do, is to revert to previous method of handling this (just print the time as is) and find a way to correctly print the dates without using toLocaleString.

But just to verify my theory in #2 above, can you check what time zone you have configured on your machine?

[stanhu]

@arikfr, my browser is configured for Pacific Standard Time (UTC-0700), and the database usually uses UTC. moment() assumes parsing of the local time of the browser if no time zone information is available, which can happen quite frequently when you do a regular SQL query (e.g. extracting a day via SELECT round(extract('epoch' FROM timestamp) / 3600). Requiring that the SQL output the time zone information in all cases seems unrealistic; either re:dash needs to determine this automatically or allow the user to specify it.

[arikfr]

Requiring that the SQL output the time zone information in all cases seems unrealistic

I agree.

I think that we should make as a little assumptions about the data as possible and just show the data as is, regardless of the time zone. Does this sound right to you?

[stanhu]

@arikfr I like the idea, but I'd imagine this may still lead to confusion if a UTC offset or time zone isn't shown somewhere. Also, Moment.js will enforce either UTC or local time zone. Is it really practical to create a timestamp that has no time zone associated with it?

[arikfr]

Maybe there should be a configuration option of default timezone? And we will apply this to timestamps without timezone? (using UTC by default)

[stanhu]

Yes, definitely.

[stanhu]

I was looking into finally fixing this because this is an annoying bug. I think it would be easier to fix this issue if this comment were addressed:

// TODO: we should stop manipulating incoming data, and switch to relaying on the column type set by the backend.
// This logic is prone to errors, and better be removed. Kept for now, for backward compatability.

That way we can tell if a value is a TIMESTAMP type and parse accordingly. Should we fix this by storing the column type mappings as a field in the query_result table?

[stanhu]

Oh, it looks like data_type is available in the query result.

[arikfr]

@stanhu see what I did in #617. I think it should address all (most?) of the issues mentioned on this thread. It will be perfect once we add per-user configuration of date/time format & default timezone (I created #618 to track this).

If you think it doesn't fully address your issue, please reopen.

Thanks!