Skip to content

Commit 525932c

Browse files
KSDaemonwaralexrom
KSDaemon
and
waralexrom
authored
fix(schema-compiler): Fix BigQuery queries datetime/timestamp comparisons (#9683)
* ConvertTZ and date_trunc should not change the data type as time dims should be timestamps * fix intervals math in bq * fix join conditions date time datatypes comparisons for BQ * Add tests * update test snapshots * fix mssql tests * fix mysql tests * fix for redshift * override runningTotals funcs for BQ. * tesseract implementation --------- Co-authored-by: Alexandr Romanenko <[email protected]>
1 parent f9ff494 commit 525932c

33 files changed

+550
-23
lines changed

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

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -958,8 +958,7 @@ export class BaseQuery {
958958
.map(
959959
d => [
960960
d,
961-
// eslint-disable-next-line @typescript-eslint/no-unused-vars
962-
(dateFrom, dateTo, dateField, dimensionDateFrom, dimensionDateTo) => `${dateField} >= ${dimensionDateFrom} AND ${dateField} <= ${dateTo}`
961+
(_dateFrom, dateTo, dateField, dimensionDateFrom, _dimensionDateTo) => `${dateField} >= ${dimensionDateFrom} AND ${dateField} <= ${dateTo}`
963962
]
964963
);
965964
}
@@ -970,7 +969,7 @@ export class BaseQuery {
970969
.map(
971970
d => [
972971
d,
973-
(dateFrom, dateTo, dateField, dimensionDateFrom, dimensionDateTo, isFromStartToEnd) => `${dateField} >= ${this.timeGroupedColumn(granularity, dateFrom)} AND ${dateField} <= ${dateTo}`
972+
(dateFrom, dateTo, dateField, _dimensionDateFrom, _dimensionDateTo, _isFromStartToEnd) => `${dateField} >= ${this.timeGroupedColumn(granularity, dateFrom)} AND ${dateField} <= ${dateTo}`
974973
]
975974
);
976975
}
@@ -980,7 +979,7 @@ export class BaseQuery {
980979
return this.timeDimensions
981980
.filter(td => td.granularity)
982981
.map(
983-
d => [d, (dateFrom, dateTo, dateField, dimensionDateFrom, dimensionDateTo, isFromStartToEnd) => {
982+
d => [d, (dateFrom, dateTo, dateField, _dimensionDateFrom, _dimensionDateTo, isFromStartToEnd) => {
984983
// dateFrom based window
985984
const conditions = [];
986985
if (trailingInterval !== 'unbounded') {
@@ -1803,6 +1802,13 @@ export class BaseQuery {
18031802
const dateJoinConditionSql =
18041803
dateJoinCondition.map(
18051804
([d, f]) => f(
1805+
// Time-series table is generated differently in different dialects,
1806+
// but some dialects (like BigQuery) require strict date types and can not automatically convert
1807+
// between date and timestamp for comparisons, at the same time, time dimensions are expected to be
1808+
// timestamps, so we need to align types for join conditions/comparisons.
1809+
// But we can't do it here, as it would break interval maths used in some types of
1810+
// rolling window join conditions in some dialects (like Redshift), so we need to
1811+
// do casts granularly in rolling window join conditions functions.
18061812
`${d.dateSeriesAliasName()}.${this.escapeColumnName('date_from')}`,
18071813
`${d.dateSeriesAliasName()}.${this.escapeColumnName('date_to')}`,
18081814
`${baseQueryAlias}.${d.aliasName()}`,
@@ -1837,9 +1843,13 @@ export class BaseQuery {
18371843
.join(', ');
18381844
}
18391845

1846+
/**
1847+
* BigQuery has strict date type and can not automatically convert between date
1848+
* and timestamp, so we override dateFromStartToEndConditionSql() in BigQuery Dialect
1849+
* @protected
1850+
*/
18401851
dateFromStartToEndConditionSql(dateJoinCondition, fromRollup, isFromStartToEnd) {
18411852
return dateJoinCondition.map(
1842-
// TODO these weird conversions to be strict typed for big query.
18431853
// TODO Consider adding strict definitions of local and UTC time type
18441854
([d, f]) => ({
18451855
filterToWhere: () => {
@@ -3921,6 +3931,7 @@ export class BaseQuery {
39213931
like_escape: '{{ like_expr }} ESCAPE {{ escape_char }}',
39223932
within_group: '{{ fun_sql }} WITHIN GROUP (ORDER BY {{ within_group_concat }})',
39233933
concat_strings: '{{ strings | join(\' || \' ) }}',
3934+
rolling_window_expr_timestamp_cast: '{{ value }}'
39243935
},
39253936
tesseract: {
39263937
ilike: '{{ expr }} {% if negated %}NOT {% endif %}ILIKE {{ pattern }}', // May require different overloads in Tesseract than the ilike from expressions used in SQLAPI.

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

Lines changed: 101 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export class BigqueryQuery extends BaseQuery {
4242
}
4343

4444
public convertTz(field) {
45-
return `DATETIME(${this.timeStampCast(field)}, '${this.timezone}')`;
45+
return `TIMESTAMP(DATETIME(${field}), '${this.timezone}')`;
4646
}
4747

4848
public timeStampCast(value) {
@@ -58,7 +58,7 @@ export class BigqueryQuery extends BaseQuery {
5858
}
5959

6060
public timeGroupedColumn(granularity, dimension) {
61-
return `DATETIME_TRUNC(${dimension}, ${GRANULARITY_TO_INTERVAL[granularity]})`;
61+
return this.timeStampCast(`DATETIME_TRUNC(${dimension}, ${GRANULARITY_TO_INTERVAL[granularity]})`);
6262
}
6363

6464
/**
@@ -72,7 +72,7 @@ export class BigqueryQuery extends BaseQuery {
7272

7373
return `(${this.dateTimeCast(`'${origin}'`)} + INTERVAL ${intervalFormatted} *
7474
CAST(FLOOR(
75-
DATETIME_DIFF(${source}, ${this.dateTimeCast(`'${origin}'`)}, ${timeUnit}) /
75+
DATETIME_DIFF(${this.dateTimeCast(source)}, ${this.dateTimeCast(`'${origin}'`)}, ${timeUnit}) /
7676
DATETIME_DIFF(${beginOfTime} + INTERVAL ${intervalFormatted}, ${beginOfTime}, ${timeUnit})
7777
) AS INT64))`;
7878
}
@@ -182,31 +182,31 @@ export class BigqueryQuery extends BaseQuery {
182182
}
183183

184184
public subtractInterval(date, interval) {
185-
return `DATETIME_SUB(${date}, INTERVAL ${this.formatInterval(interval)[0]})`;
186-
}
187-
188-
public addInterval(date, interval) {
189-
return `DATETIME_ADD(${date}, INTERVAL ${this.formatInterval(interval)[0]})`;
190-
}
191-
192-
public subtractTimestampInterval(date, interval) {
193185
const [intervalFormatted, timeUnit] = this.formatInterval(interval);
194-
if (['YEAR', 'MONTH', 'QUARTER'].includes(timeUnit)) {
186+
if (['YEAR', 'MONTH', 'QUARTER'].includes(timeUnit) || intervalFormatted.includes('WEEK')) {
195187
return this.timeStampCast(`DATETIME_SUB(DATETIME(${date}), INTERVAL ${intervalFormatted})`);
196188
}
197189

198190
return `TIMESTAMP_SUB(${date}, INTERVAL ${intervalFormatted})`;
199191
}
200192

201-
public addTimestampInterval(date, interval) {
193+
public addInterval(date, interval) {
202194
const [intervalFormatted, timeUnit] = this.formatInterval(interval);
203-
if (['YEAR', 'MONTH', 'QUARTER'].includes(timeUnit)) {
195+
if (['YEAR', 'MONTH', 'QUARTER'].includes(timeUnit) || intervalFormatted.includes('WEEK')) {
204196
return this.timeStampCast(`DATETIME_ADD(DATETIME(${date}), INTERVAL ${intervalFormatted})`);
205197
}
206198

207199
return `TIMESTAMP_ADD(${date}, INTERVAL ${intervalFormatted})`;
208200
}
209201

202+
public subtractTimestampInterval(timestamp, interval) {
203+
return this.subtractInterval(timestamp, interval);
204+
}
205+
206+
public addTimestampInterval(timestamp, interval) {
207+
return this.addInterval(timestamp, interval);
208+
}
209+
210210
public nowTimestampSql() {
211211
return 'CURRENT_TIMESTAMP()';
212212
}
@@ -215,6 +215,90 @@ export class BigqueryQuery extends BaseQuery {
215215
return `UNIX_SECONDS(${this.nowTimestampSql()})`;
216216
}
217217

218+
/**
219+
* Should be protected, but BaseQuery is in js
220+
* Overridden from BaseQuery to support BigQuery strict data types for
221+
* joining conditions (note timeStampCast)
222+
*/
223+
public override runningTotalDateJoinCondition() {
224+
return this.timeDimensions
225+
.map(
226+
d => [
227+
d,
228+
(_dateFrom: string, dateTo: string, dateField: string, dimensionDateFrom: string, _dimensionDateTo: string) => `${dateField} >= ${dimensionDateFrom} AND ${dateField} <= ${this.timeStampCast(dateTo)}`
229+
]
230+
);
231+
}
232+
233+
/**
234+
* Should be protected, but BaseQuery is in js
235+
* Overridden from BaseQuery to support BigQuery strict data types for
236+
* joining conditions (note timeStampCast)
237+
*/
238+
public override rollingWindowToDateJoinCondition(granularity) {
239+
return this.timeDimensions
240+
.filter(td => td.granularity)
241+
.map(
242+
d => [
243+
d,
244+
(dateFrom: string, dateTo: string, dateField: string, _dimensionDateFrom: string, _dimensionDateTo: string, _isFromStartToEnd: boolean) => `${dateField} >= ${this.timeGroupedColumn(granularity, dateFrom)} AND ${dateField} <= ${this.timeStampCast(dateTo)}`
245+
]
246+
);
247+
}
248+
249+
/**
250+
* Should be protected, but BaseQuery is in js
251+
* Overridden from BaseQuery to support BigQuery strict data types for
252+
* joining conditions (note timeStampCast)
253+
*/
254+
public override rollingWindowDateJoinCondition(trailingInterval, leadingInterval, offset) {
255+
offset = offset || 'end';
256+
return this.timeDimensions
257+
.filter(td => td.granularity)
258+
.map(
259+
d => [d, (dateFrom: string, dateTo: string, dateField: string, _dimensionDateFrom: string, _dimensionDateTo: string, isFromStartToEnd: boolean) => {
260+
// dateFrom based window
261+
const conditions: string[] = [];
262+
if (trailingInterval !== 'unbounded') {
263+
const startDate = isFromStartToEnd || offset === 'start' ? dateFrom : dateTo;
264+
const trailingStart = trailingInterval ? this.subtractInterval(startDate, trailingInterval) : startDate;
265+
const sign = offset === 'start' ? '>=' : '>';
266+
conditions.push(`${dateField} ${sign} ${this.timeStampCast(trailingStart)}`);
267+
}
268+
if (leadingInterval !== 'unbounded') {
269+
const endDate = isFromStartToEnd || offset === 'end' ? dateTo : dateFrom;
270+
const leadingEnd = leadingInterval ? this.addInterval(endDate, leadingInterval) : endDate;
271+
const sign = offset === 'end' ? '<=' : '<';
272+
conditions.push(`${dateField} ${sign} ${this.timeStampCast(leadingEnd)}`);
273+
}
274+
return conditions.length ? conditions.join(' AND ') : '1 = 1';
275+
}]
276+
);
277+
}
278+
279+
// Should be protected, but BaseQuery is in js
280+
public override dateFromStartToEndConditionSql(dateJoinCondition, fromRollup, isFromStartToEnd) {
281+
return dateJoinCondition.map(
282+
([d, f]) => ({
283+
filterToWhere: () => {
284+
const timeSeries = d.timeSeries();
285+
return f(
286+
isFromStartToEnd ?
287+
this.timeStampCast(this.paramAllocator.allocateParam(timeSeries[0][0])) :
288+
`${this.timeStampInClientTz(d.dateFromParam())}`,
289+
isFromStartToEnd ?
290+
this.timeStampCast(this.paramAllocator.allocateParam(timeSeries[timeSeries.length - 1][1])) :
291+
`${this.timeStampInClientTz(d.dateToParam())}`,
292+
`${fromRollup ? this.dimensionSql(d) : d.convertedToTz()}`,
293+
`${this.timeStampInClientTz(d.dateFromParam())}`,
294+
`${this.timeStampInClientTz(d.dateToParam())}`,
295+
isFromStartToEnd
296+
);
297+
}
298+
})
299+
);
300+
}
301+
218302
// eslint-disable-next-line no-unused-vars
219303
public preAggregationLoadSql(cube, preAggregation, tableName) {
220304
return this.preAggregationSql(cube, preAggregation);
@@ -250,7 +334,7 @@ export class BigqueryQuery extends BaseQuery {
250334
const templates = super.sqlTemplates();
251335
templates.quotes.identifiers = '`';
252336
templates.quotes.escape = '\\`';
253-
templates.functions.DATETRUNC = 'DATETIME_TRUNC(CAST({{ args[1] }} AS DATETIME), {% if date_part|upper == \'WEEK\' %}{{ \'WEEK(MONDAY)\' }}{% else %}{{ date_part }}{% endif %})';
337+
templates.functions.DATETRUNC = 'TIMESTAMP(DATETIME_TRUNC(CAST({{ args[1] }} AS DATETIME), {% if date_part|upper == \'WEEK\' %}{{ \'WEEK(MONDAY)\' }}{% else %}{{ date_part }}{% endif %}))';
254338
templates.functions.LOG = 'LOG({{ args_concat }}{% if args[1] is undefined %}, 10{% endif %})';
255339
templates.functions.BTRIM = 'TRIM({{ args_concat }})';
256340
templates.functions.STRPOS = 'STRPOS({{ args_concat }})';
@@ -263,7 +347,8 @@ export class BigqueryQuery extends BaseQuery {
263347
templates.expressions.binary = '{% if op == \'%\' %}MOD({{ left }}, {{ right }}){% else %}({{ left }} {{ op }} {{ right }}){% endif %}';
264348
templates.expressions.interval = 'INTERVAL {{ interval }}';
265349
templates.expressions.extract = 'EXTRACT({% if date_part == \'DOW\' %}DAYOFWEEK{% elif date_part == \'DOY\' %}DAYOFYEAR{% else %}{{ date_part }}{% endif %} FROM {{ expr }})';
266-
templates.expressions.timestamp_literal = 'DATETIME(TIMESTAMP(\'{{ value }}\'))';
350+
templates.expressions.timestamp_literal = 'TIMESTAMP(\'{{ value }}\')';
351+
templates.expressions.rolling_window_expr_timestamp_cast = 'TIMESTAMP({{ value }})';
267352
delete templates.expressions.ilike;
268353
delete templates.expressions.like_escape;
269354
templates.filters.like_pattern = 'CONCAT({% if start_wild %}\'%\'{% else %}\'\'{% endif %}, LOWER({{ value }}), {% if end_wild %}\'%\'{% else %}\'\'{% endif %})';

‎packages/cubejs-testing-drivers/fixtures/mssql.json‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,8 @@
161161
"SQL API: Extended nested Rollup over asterisk",
162162
"SQL API: ungrouped pre-agg",
163163
"SQL API: NULLS FIRST/LAST SQL push down",
164-
"SQL API: SQL push down push to cube quoted alias"
164+
"SQL API: SQL push down push to cube quoted alias",
165+
"SQL API: Date/time comparison with SQL push down",
166+
"SQL API: Date/time comparison with date_trunc with SQL push down"
165167
]
166168
}

‎packages/cubejs-testing-drivers/fixtures/mysql.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@
157157
"SQL API: Nested Rollup with aliases",
158158
"SQL API: Nested Rollup over asterisk",
159159
"SQL API: Extended nested Rollup over asterisk",
160-
"SQL API: SQL push down push to cube quoted alias"
160+
"SQL API: SQL push down push to cube quoted alias",
161+
"SQL API: Date/time comparison with date_trunc with SQL push down"
161162
]
162163
}

‎packages/cubejs-testing-drivers/src/tests/testQueries.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2118,5 +2118,25 @@ from
21182118
`);
21192119
expect(res.rows).toMatchSnapshot();
21202120
});
2121+
2122+
executePg('SQL API: Date/time comparison with SQL push down', async (connection) => {
2123+
const res = await connection.query(`
2124+
SELECT MEASURE(BigECommerce.rollingCountBy2Day)
2125+
FROM BigECommerce
2126+
WHERE BigECommerce.orderDate < CAST('2021-01-01' AS TIMESTAMP) AND
2127+
LOWER("city") = 'columbus'
2128+
`);
2129+
expect(res.rows).toMatchSnapshot();
2130+
});
2131+
2132+
executePg('SQL API: Date/time comparison with date_trunc with SQL push down', async (connection) => {
2133+
const res = await connection.query(`
2134+
SELECT MEASURE(BigECommerce.rollingCountBy2Week)
2135+
FROM BigECommerce
2136+
WHERE date_trunc('day', BigECommerce.orderDate) < CAST('2021-01-01' AS TIMESTAMP) AND
2137+
LOWER("city") = 'columbus'
2138+
`);
2139+
expect(res.rows).toMatchSnapshot();
2140+
});
21212141
});
21222142
}

‎packages/cubejs-testing-drivers/test/__snapshots__/athena-export-bucket-s3-full.test.ts.snap‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8007,3 +8007,19 @@ Array [
80078007
},
80088008
]
80098009
`;
8010+
8011+
exports[`Queries with the @cubejs-backend/athena-driver SQL API: Date/time comparison with SQL push down 1`] = `
8012+
Array [
8013+
Object {
8014+
"measure(BigECommerce.rollingCountBy2Day)": "12",
8015+
},
8016+
]
8017+
`;
8018+
8019+
exports[`Queries with the @cubejs-backend/athena-driver SQL API: Date/time comparison with date_trunc with SQL push down 1`] = `
8020+
Array [
8021+
Object {
8022+
"measure(BigECommerce.rollingCountBy2Week)": "12",
8023+
},
8024+
]
8025+
`;

‎packages/cubejs-testing-drivers/test/__snapshots__/bigquery-export-bucket-gcs-full.test.ts.snap‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,21 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3+
exports[`Queries with the @cubejs-backend/bigquery-driver SQL API: Date/time comparison with SQL push down 1`] = `
4+
Array [
5+
Object {
6+
"measure(BigECommerce.rollingCountBy2Day)": "12",
7+
},
8+
]
9+
`;
10+
11+
exports[`Queries with the @cubejs-backend/bigquery-driver SQL API: Date/time comparison with date_trunc with SQL push down 1`] = `
12+
Array [
13+
Object {
14+
"measure(BigECommerce.rollingCountBy2Week)": "12",
15+
},
16+
]
17+
`;
18+
319
exports[`Queries with the @cubejs-backend/bigquery-driver SQL API: NULLS FIRST/LAST SQL push down: nulls_first_last_sql_push_down 1`] = `
420
Array [
521
Object {

‎packages/cubejs-testing-drivers/test/__snapshots__/clickhouse-export-bucket-s3-full.test.ts.snap‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8952,3 +8952,19 @@ Array [
89528952
},
89538953
]
89548954
`;
8955+
8956+
exports[`Queries with the @cubejs-backend/clickhouse-driver export-bucket-s3 SQL API: Date/time comparison with SQL push down 1`] = `
8957+
Array [
8958+
Object {
8959+
"measure(BigECommerce.rollingCountBy2Day)": "12",
8960+
},
8961+
]
8962+
`;
8963+
8964+
exports[`Queries with the @cubejs-backend/clickhouse-driver export-bucket-s3 SQL API: Date/time comparison with date_trunc with SQL push down 1`] = `
8965+
Array [
8966+
Object {
8967+
"measure(BigECommerce.rollingCountBy2Week)": "12",
8968+
},
8969+
]
8970+
`;

‎packages/cubejs-testing-drivers/test/__snapshots__/clickhouse-export-bucket-s3-prefix-full.test.ts.snap‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8952,3 +8952,19 @@ Array [
89528952
},
89538953
]
89548954
`;
8955+
8956+
exports[`Queries with the @cubejs-backend/clickhouse-driver export-bucket-s3-prefix SQL API: Date/time comparison with SQL push down 1`] = `
8957+
Array [
8958+
Object {
8959+
"measure(BigECommerce.rollingCountBy2Day)": "12",
8960+
},
8961+
]
8962+
`;
8963+
8964+
exports[`Queries with the @cubejs-backend/clickhouse-driver export-bucket-s3-prefix SQL API: Date/time comparison with date_trunc with SQL push down 1`] = `
8965+
Array [
8966+
Object {
8967+
"measure(BigECommerce.rollingCountBy2Week)": "12",
8968+
},
8969+
]
8970+
`;

‎packages/cubejs-testing-drivers/test/__snapshots__/clickhouse-full.test.ts.snap‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8952,3 +8952,19 @@ Array [
89528952
},
89538953
]
89548954
`;
8955+
8956+
exports[`Queries with the @cubejs-backend/clickhouse-driver SQL API: Date/time comparison with SQL push down 1`] = `
8957+
Array [
8958+
Object {
8959+
"measure(BigECommerce.rollingCountBy2Day)": "12",
8960+
},
8961+
]
8962+
`;
8963+
8964+
exports[`Queries with the @cubejs-backend/clickhouse-driver SQL API: Date/time comparison with date_trunc with SQL push down 1`] = `
8965+
Array [
8966+
Object {
8967+
"measure(BigECommerce.rollingCountBy2Week)": "12",
8968+
},
8969+
]
8970+
`;

0 commit comments

Comments
 (0)