Format all timestamps in query timezone
- Dominant language
- Rust
- Stars
- 20.8k
- Forks
- 2.1k
- Avg merge
- 1d 2h
- Merged PRs (30d)
- 181
Description
**Is your feature request related to a problem? Please describe.**
Results with timestamps are inconsistently converted to the query timezone depending
on if it is used as a dimension, a time dimension, or a time dimension with granularity.
This makes presenting time data error-prone because it's unclear what timezone the data is formatted in since none of the results contain offsets.
**Describe the solution you'd like**
Always format timestamps in the query timezone to reduce timezone pitfalls.
**Describe alternatives you've considered**
1. Another option would be to include the offset (or `Z`) when the time differs from the query timezone,
making it unambiguous.
2. Using [SQL_UTILS.convertTz](https://cube.dev/docs/cube#context-variables-sql-utils) is a workaround, but requires defining additional dimensions
and introduces pitfalls such as accidentally applying the offset twice in time dimensions:
> Dimensions that use SQL_UTILS.convertTz() should not be used as timeDimensions in queries. Doing so will apply the conversion multiple times and yield wrong results.
**Additional context**
Example Schema for BigQuery:
```
cube('Test', {
sql: `
SELECT TIMESTAMP('2021-06-15T00:00:00', 'America/Los_Angeles') AS Created,
TIMESTAMP('2021-06-15T00:00:00', 'America/Los_Angeles') AS Updated
`,
dimensions: {
created: {
sql: 'Created',
type: 'time',
},
updated: {
sql: 'Updated',
type: 'time',
},
},
});
```
Query:
```
{
"timeDimensions": [
{
"dimension": "Test.created",
"dateRange": [
"2021-06-15",
"2021-06-15"
],
"granularity": "second"
}
],
"dimensions": [
"Test.updated"
],
"timezone": "America/Los_Angeles"
}
```
Results:
```
[
{
// These are in the query timezone
"Test.created.second": "2021-06-15T00:00:00.000",
"Test.created": "2021-06-15T00:00:00.000"
// This one is not in the query timezone
// and it's ambiguous which timezone it is in.
// Expected: "2021-06-15T00:00:00.000" or "2021-06-15T07:00:00.000Z"
"Test.updated": "2021-06-15T07:00:00.000",
}
]
```
Implementation:
A potential path could be to convert in the API Gateway as part of `transformValue`, but a better implementation could be in the drivers.
```diff
diff --git a/packages/cubejs-api-gateway/src/gateway.ts b/packages/cubejs-api-gateway/src/gateway.ts
index c4a00ab6..17901314 100644
--- a/packages/cubejs-api-gateway/src/gateway.ts
+++ b/packages/cubejs-api-gateway/src/gateway.ts
@@ -96,9 +96,9 @@ const prepareAnnotation = (metaConfig: MetaConfig[], query: any) => {
};
};
-const transformValue = (value, type) => {
+const transformValue = (value, type, timezone) => {
if (value && (type === 'time' || value instanceof Date)) { // TODO support for max time
- return (value instanceof Date ? moment(value) : moment.utc(value)).format(moment.HTML5_FMT.DATETIME_LOCAL_MS);
+ return (value instanceof Date ? moment(value) : moment.tz(value, timezone)).format(moment.HTML5_FMT.DATETIME_LOCAL_MS);
}
return value && value.value ? value.value : value; // TODO move to sql adapter
};
@@ -117,7 +117,7 @@ const transformData = (aliasToMemberNameMap, annotation, data, query, queryType)
const transformResult = [
memberName,
- transformValue(p[1], annotationForMember.type)
+ transformValue(p[1], annotationForMember.type, query.timezone)
];
const path = memberName.split('.');
```
Related:
- https://github.com/cube-js/cube.js/issues/443
- https://github.com/cube-js/cube.js/issues/689
- https://cube-js.slack.com/archives/CC0403RRR/p1612912462478500?thread_ts=1612839814.416800&cid=CC0403RRR
Contributor guide
Assessment
This issue has not been assessed yet.