Skip to content

Commit 1beba2d

Browse files
KSDaemonKSDaemon
authored
fix(schema-compiler): use query timezone for time granularity origin (#8936)
* fix(schema-compiler): use query timezone for time granularity origin * fix tests * small improvement * fix dateBin implementation across all Queries (timeStampCast → dateTimeCast) * Fix MS SQL timeStampCast * fix ms sql test
1 parent 41b49f4 commit 1beba2d

File tree

13 files changed

+55
-38
lines changed

13 files changed

+55
-38
lines changed

‎packages/cubejs-databricks-jdbc-driver/src/DatabricksQuery.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,9 @@ export class DatabricksQuery extends BaseQuery {
9696
const [intervalFormatted, timeUnit] = this.formatInterval(interval);
9797
const beginOfTime = this.dateTimeCast('\'1970-01-01T00:00:00\'');
9898

99-
return `${this.timeStampCast(`'${origin}'`)} + INTERVAL ${intervalFormatted} *
99+
return `${this.dateTimeCast(`'${origin}'`)} + INTERVAL ${intervalFormatted} *
100100
floor(
101-
date_diff(${timeUnit}, ${this.timeStampCast(`'${origin}'`)}, ${source}) /
101+
date_diff(${timeUnit}, ${this.dateTimeCast(`'${origin}'`)}, ${source}) /
102102
date_diff(${timeUnit}, ${beginOfTime}, ${beginOfTime} + INTERVAL ${intervalFormatted})
103103
)`;
104104
}

‎packages/cubejs-duckdb-driver/src/DuckDBQuery.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,11 @@ export class DuckDBQuery extends BaseQuery {
3535
*/
3636
public dateBin(interval: string, source: string, origin: string): string {
3737
const timeUnit = this.diffTimeUnitForInterval(interval);
38-
const beginOfTime = this.timeStampCast('\'1970-01-01 00:00:00.000\'');
38+
const beginOfTime = this.dateTimeCast('\'1970-01-01 00:00:00.000\'');
3939

40-
return `${this.timeStampCast(`'${origin}'`)}' + INTERVAL '${interval}' *
40+
return `${this.dateTimeCast(`'${origin}'`)}' + INTERVAL '${interval}' *
4141
floor(
42-
date_diff('${timeUnit}', ${this.timeStampCast(`'${origin}'`)}, ${source}) /
42+
date_diff('${timeUnit}', ${this.dateTimeCast(`'${origin}'`)}, ${source}) /
4343
date_diff('${timeUnit}', ${beginOfTime}, ${beginOfTime} + INTERVAL '${interval}')
4444
)::int`;
4545
}

‎packages/cubejs-schema-compiler/src/adapter/BaseQuery.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2781,7 +2781,7 @@ export class BaseQuery {
27812781
* intervals relative to origin timestamp point
27822782
* @param {string} interval (a value expression of type interval)
27832783
* @param {string} source (a value expression of type timestamp/date)
2784-
* @param {string} origin (a value expression of type timestamp/date)
2784+
* @param {string} origin (a value expression of type timestamp/date without timezone)
27852785
* @returns {string}
27862786
*/
27872787
// eslint-disable-next-line @typescript-eslint/no-unused-vars
@@ -2839,7 +2839,7 @@ export class BaseQuery {
28392839
return this.timeGroupedColumn(granularity.granularityFromInterval(), dimension);
28402840
}
28412841

2842-
return this.dateBin(granularity.granularityInterval, dimension, granularity.originFormatted());
2842+
return this.dateBin(granularity.granularityInterval, dimension, granularity.originLocalFormatted());
28432843
}
28442844

28452845
/**

‎packages/cubejs-schema-compiler/src/adapter/ClickHouseQuery.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,10 @@ export class ClickHouseQuery extends BaseQuery {
7373

7474
return `date_add(${timeUnit},
7575
FLOOR(
76-
date_diff(${timeUnit}, ${this.timeStampCast(`'${origin}'`)}, ${source}) /
76+
date_diff(${timeUnit}, ${this.dateTimeCast(`'${origin}'`)}, ${source}) /
7777
date_diff(${timeUnit}, ${beginOfTime}, ${beginOfTime} + ${intervalFormatted})
7878
) * date_diff(${timeUnit}, ${beginOfTime}, ${beginOfTime} + ${intervalFormatted}),
79-
${this.timeStampCast(`'${origin}'`)}
79+
${this.dateTimeCast(`'${origin}'`)}
8080
)`;
8181
}
8282

‎packages/cubejs-schema-compiler/src/adapter/CubeStoreQuery.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ export class CubeStoreQuery extends BaseQuery {
7171
* intervals relative to origin timestamp point.
7272
*/
7373
public dateBin(interval: string, source: string, origin: string): string {
74-
return `DATE_BIN(INTERVAL ${this.formatInterval(interval)}, ${this.timeStampCast(source)}, ${this.timeStampCast(`'${origin}'`)})`;
74+
return `DATE_BIN(INTERVAL ${this.formatInterval(interval)}, ${this.dateTimeCast(source)}, ${this.dateTimeCast(`'${origin}'`)})`;
7575
}
7676

7777
/**

‎packages/cubejs-schema-compiler/src/adapter/Granularity.ts

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export class Granularity {
2525
) {
2626
this.granularity = timeDimension.granularity;
2727
this.predefinedGranularity = isPredefinedGranularity(this.granularity);
28-
this.origin = moment.tz('UTC').startOf('year'); // Defaults to current year start
28+
this.origin = moment.tz(query.timezone).startOf('year'); // Defaults to current year start
2929

3030
if (this.predefinedGranularity) {
3131
this.granularityInterval = `1 ${this.granularity}`;
@@ -43,7 +43,7 @@ export class Granularity {
4343
this.granularityInterval = customGranularity.interval;
4444

4545
if (customGranularity.origin) {
46-
this.origin = moment.tz(customGranularity.origin, 'UTC');
46+
this.origin = moment.tz(customGranularity.origin, query.timezone);
4747
} else if (customGranularity.offset) {
4848
this.granularityOffset = customGranularity.offset;
4949
this.origin = addInterval(this.origin, parseSqlInterval(customGranularity.offset));
@@ -55,10 +55,20 @@ export class Granularity {
5555
return this.predefinedGranularity;
5656
}
5757

58-
public originFormatted(): string {
58+
/**
59+
* @returns origin date string in Query timezone
60+
*/
61+
public originLocalFormatted(): string {
5962
return this.origin.format('YYYY-MM-DDTHH:mm:ss.SSS');
6063
}
6164

65+
/**
66+
* @returns origin date string in UTC timezone
67+
*/
68+
public originUtcFormatted(): string {
69+
return this.origin.clone().utc().format('YYYY-MM-DDTHH:mm:ss.SSSZ');
70+
}
71+
6272
public minGranularity(): string {
6373
if (this.predefinedGranularity) {
6474
return this.granularity;
@@ -86,7 +96,10 @@ export class Granularity {
8696
return timeSeries(this.granularity, dateRange, options);
8797
}
8898

89-
return timeSeriesFromCustomInterval(this.granularityInterval, dateRange, this.origin, options);
99+
// Interval range doesn't take timezone into account and operate in kinda local timezone,
100+
// but origin is treated as a timestamp in query timezone, so we pass it as the naive timestamp
101+
// to be in sync with date range during calculation.
102+
return timeSeriesFromCustomInterval(this.granularityInterval, dateRange, moment(this.originLocalFormatted()), options);
90103
}
91104

92105
public resolvedGranularity(): string {

‎packages/cubejs-schema-compiler/src/adapter/MssqlQuery.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ export class MssqlQuery extends BaseQuery {
7171
}
7272

7373
public timeStampCast(value: string) {
74-
return this.dateTimeCast(value);
74+
return `CAST(${value} AS DATETIMEOFFSET)`;
7575
}
7676

7777
public dateTimeCast(value: string) {
@@ -88,16 +88,16 @@ export class MssqlQuery extends BaseQuery {
8888
* The formula operates with seconds diffs so it won't produce human-expected dates aligned with offset date parts.
8989
*/
9090
public dateBin(interval: string, source: string, origin: string): string {
91-
const beginOfTime = this.timeStampCast('DATEFROMPARTS(1970, 1, 1)');
91+
const beginOfTime = this.dateTimeCast('DATEFROMPARTS(1970, 1, 1)');
9292
const timeUnit = this.diffTimeUnitForInterval(interval);
9393

9494
// Need to explicitly cast one argument of floor to float to trigger correct sign logic
9595
return `DATEADD(${timeUnit},
9696
FLOOR(
97-
CAST(DATEDIFF(${timeUnit}, ${this.timeStampCast(`'${origin}'`)}, ${source}) AS FLOAT) /
97+
CAST(DATEDIFF(${timeUnit}, ${this.dateTimeCast(`'${origin}'`)}, ${source}) AS FLOAT) /
9898
DATEDIFF(${timeUnit}, ${beginOfTime}, ${this.addInterval(beginOfTime, interval)})
9999
) * DATEDIFF(${timeUnit}, ${beginOfTime}, ${this.addInterval(beginOfTime, interval)}),
100-
${this.timeStampCast(`'${origin}'`)}
100+
${this.dateTimeCast(`'${origin}'`)}
101101
)`;
102102
}
103103

‎packages/cubejs-schema-compiler/src/adapter/MysqlQuery.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,10 @@ export class MysqlQuery extends BaseQuery {
7272

7373
return `TIMESTAMPADD(${timeUnit},
7474
FLOOR(
75-
TIMESTAMPDIFF(${timeUnit}, ${this.timeStampCast(`'${origin}'`)}, ${source}) /
75+
TIMESTAMPDIFF(${timeUnit}, ${this.dateTimeCast(`'${origin}'`)}, ${source}) /
7676
TIMESTAMPDIFF(${timeUnit}, '1970-01-01 00:00:00', '1970-01-01 00:00:00' + INTERVAL ${intervalFormatted})
7777
) * TIMESTAMPDIFF(${timeUnit}, '1970-01-01 00:00:00', '1970-01-01 00:00:00' + INTERVAL ${intervalFormatted}),
78-
${this.timeStampCast(`'${origin}'`)}
78+
${this.dateTimeCast(`'${origin}'`)}
7979
)`;
8080
}
8181

‎packages/cubejs-schema-compiler/src/adapter/OracleQuery.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ class OracleFilter extends BaseFilter {
1919
}
2020

2121
/**
22-
* "ILIKE" does't support
22+
* "ILIKE" is not supported
2323
*/
2424
public likeIgnoreCase(column, not, param, type) {
2525
const p = (!type || type === 'contains' || type === 'ends') ? '\'%\' || ' : '';
@@ -30,7 +30,7 @@ class OracleFilter extends BaseFilter {
3030

3131
export class OracleQuery extends BaseQuery {
3232
/**
33-
* "LIMIT" on Oracle it's illegal
33+
* "LIMIT" on Oracle is illegal
3434
* TODO replace with limitOffsetClause override
3535
*/
3636
public groupByDimensionLimit() {

‎packages/cubejs-schema-compiler/src/adapter/SnowflakeQuery.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,10 @@ export class SnowflakeQuery extends BaseQuery {
5353

5454
return `DATEADD(${timeUnit},
5555
FLOOR(
56-
DATEDIFF(${timeUnit}, ${this.timeStampCast(`'${origin}'`)}, ${source}) /
56+
DATEDIFF(${timeUnit}, ${this.dateTimeCast(`'${origin}'`)}, ${source}) /
5757
DATEDIFF(${timeUnit}, ${beginOfTime}, (${beginOfTime} + interval '${intervalFormatted}'))
5858
) * DATEDIFF(${timeUnit}, ${beginOfTime}, (${beginOfTime} + interval '${intervalFormatted}')),
59-
${this.timeStampCast(`'${origin}'`)})`;
59+
${this.dateTimeCast(`'${origin}'`)})`;
6060
}
6161

6262
/**

‎packages/cubejs-schema-compiler/test/integration/mssql/mssql-pre-aggregations.test.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ describe('MSSqlPreAggregations', () => {
1818
sql: \`
1919
select * from ##visitors
2020
\`,
21-
21+
2222
joins: {
2323
visitor_checkins: {
2424
relationship: 'hasMany',
@@ -30,22 +30,22 @@ describe('MSSqlPreAggregations', () => {
3030
count: {
3131
type: 'count'
3232
},
33-
33+
3434
checkinsTotal: {
3535
sql: \`\${checkinsCount}\`,
3636
type: 'sum'
3737
},
38-
38+
3939
uniqueSourceCount: {
4040
sql: 'source',
4141
type: 'countDistinct'
4242
},
43-
43+
4444
countDistinctApprox: {
4545
sql: 'id',
4646
type: 'countDistinctApprox'
4747
},
48-
48+
4949
ratio: {
5050
sql: \`1.0 * \${uniqueSourceCount} / nullif(\${checkinsTotal}, 0)\`,
5151
type: 'number'
@@ -72,13 +72,13 @@ describe('MSSqlPreAggregations', () => {
7272
subQuery: true
7373
}
7474
},
75-
75+
7676
segments: {
7777
google: {
7878
sql: \`source = 'google'\`
7979
}
8080
},
81-
81+
8282
preAggregations: {
8383
default: {
8484
type: 'originalSql'
@@ -125,8 +125,8 @@ describe('MSSqlPreAggregations', () => {
125125
}
126126
}
127127
})
128-
129-
128+
129+
130130
cube('visitor_checkins', {
131131
sql: \`
132132
select * from ##visitor_checkins
@@ -157,7 +157,7 @@ describe('MSSqlPreAggregations', () => {
157157
sql: 'created_at'
158158
}
159159
},
160-
160+
161161
preAggregations: {
162162
main: {
163163
type: 'originalSql'
@@ -168,7 +168,7 @@ describe('MSSqlPreAggregations', () => {
168168
}
169169
}
170170
})
171-
171+
172172
cube('GoogleVisitors', {
173173
extends: visitors,
174174
sql: \`select v.* from \${visitors.sql()} v where v.source = 'google'\`
@@ -276,7 +276,7 @@ describe('MSSqlPreAggregations', () => {
276276

277277
expect(preAggregationsDescription[0].invalidateKeyQueries[0][0].replace(/(\r\n|\n|\r)/gm, '')
278278
.replace(/\s+/g, ' '))
279-
.toMatch('SELECT CASE WHEN CURRENT_TIMESTAMP < DATEADD(day, 7, CAST(@_1 AS DATETIME2)) THEN FLOOR((DATEDIFF(SECOND,\'1970-01-01\', GETUTCDATE())) / 3600) END');
279+
.toMatch('SELECT CASE WHEN CURRENT_TIMESTAMP < DATEADD(day, 7, CAST(@_1 AS DATETIMEOFFSET)) THEN FLOOR((DATEDIFF(SECOND,\'1970-01-01\', GETUTCDATE())) / 3600) END');
280280

281281
return dbRunner
282282
.evaluateQueryWithPreAggregations(query)

‎packages/cubejs-schema-compiler/test/unit/base-query.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1356,7 +1356,7 @@ describe('SQL Generation', () => {
13561356
expect(preAggregations.length).toEqual(1);
13571357
expect(preAggregations[0].invalidateKeyQueries).toEqual([
13581358
[
1359-
'SELECT CASE\n WHEN CURRENT_TIMESTAMP < CAST(@_1 AS DATETIME2) THEN FLOOR((DATEDIFF(SECOND,\'1970-01-01\', GETUTCDATE())) / 3600) END as refresh_key',
1359+
'SELECT CASE\n WHEN CURRENT_TIMESTAMP < CAST(@_1 AS DATETIMEOFFSET) THEN FLOOR((DATEDIFF(SECOND,\'1970-01-01\', GETUTCDATE())) / 3600) END as refresh_key',
13601360
[
13611361
'__TO_PARTITION_RANGE',
13621362
],

‎packages/cubejs-schema-compiler/test/unit/pre-agg-time-dim-match.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,11 @@ describe('Pre Aggregation by filter match tests', () => {
104104
));
105105

106106
it('1 count measure, day, two_weeks_by_1st_feb_00am', () => testPreAggregationMatch(
107-
true, ['count'], 'day', 'two_weeks_by_1st_feb_00am'
107+
true, ['count'], 'day', 'two_weeks_by_1st_feb_00am', 'UTC'
108+
));
109+
110+
it('1 count measure, day, two_weeks_by_1st_feb_00am', () => testPreAggregationMatch(
111+
false, ['count'], 'day', 'two_weeks_by_1st_feb_00am', 'Europe/Berlin'
108112
));
109113

110114
it('1 count measure, day, two_weeks_by_1st_feb_10am', () => testPreAggregationMatch(

0 commit comments

Comments
 (0)