Skip to content

Commit eff0736

Browse files
KSDaemonKSDaemon
authored
fix(schema-compiler): Fix queries with time dimensions without granularities don't hit pre-aggregations with allow_non_strict_date_range_match=true (#9102)
* code polish (spaces + correct test name) * fix(schema-compiler): Fix queries with time dimensions without granularities don't hit pre-aggregations with allow_non_strict_date_range_match=true * export some functions from backend-shared/time * implement granularity.isAlignedWithDateRange(...) * remove unused/unneeded * fix/implement pre-agg matching * fix test for pre-agg with allowNonStrictDateRangeMatch: true * test for custom granularity match * fix isAlignedWithDateRange for DSTs * fix test * cached granularityHierarchies() for query timezone
1 parent 364d181 commit eff0736

File tree

9 files changed

+268
-89
lines changed

9 files changed

+268
-89
lines changed

‎packages/cubejs-backend-shared/src/time.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ export function subtractInterval(date: moment.Moment, interval: ParsedInterval):
7676
/**
7777
* Returns the closest date prior to date parameter aligned with the origin point
7878
*/
79-
function alignToOrigin(startDate: moment.Moment, interval: ParsedInterval, origin: moment.Moment): moment.Moment {
79+
export const alignToOrigin = (startDate: moment.Moment, interval: ParsedInterval, origin: moment.Moment): moment.Moment => {
8080
let alignedDate = startDate.clone();
8181
let intervalOp;
8282
let isIntervalNegative = false;
@@ -111,17 +111,17 @@ function alignToOrigin(startDate: moment.Moment, interval: ParsedInterval, origi
111111
}
112112

113113
return alignedDate;
114-
}
114+
};
115115

116-
function parsedSqlIntervalToDuration(parsedInterval: ParsedInterval): moment.Duration {
116+
export const parsedSqlIntervalToDuration = (parsedInterval: ParsedInterval): moment.Duration => {
117117
const duration = moment.duration();
118118

119119
Object.entries(parsedInterval).forEach(([key, value]) => {
120120
duration.add(value, key as unitOfTime.DurationConstructor);
121121
});
122122

123123
return duration;
124-
}
124+
};
125125

126126
function checkSeriesForDateRange(intervalStr: string, [startStr, endStr]: QueryDateRange): void {
127127
const intervalParsed = parseSqlInterval(intervalStr);

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

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import { BaseTimeDimension } from './BaseTimeDimension';
2626
import { ParamAllocator } from './ParamAllocator';
2727
import { PreAggregations } from './PreAggregations';
2828
import { SqlParser } from '../parser/SqlParser';
29+
import { Granularity } from './Granularity';
2930

3031
const DEFAULT_PREAGGREGATIONS_SCHEMA = 'stb_pre_aggregations';
3132

@@ -1389,7 +1390,47 @@ export class BaseQuery {
13891390
}
13901391

13911392
granularityHierarchies() {
1392-
return R.fromPairs(Object.keys(standardGranularitiesParents).map(k => [k, this.granularityParentHierarchy(k)]));
1393+
return this.cacheValue(
1394+
// If time dimension custom granularity in data model is defined without
1395+
// timezone information they are treated in query timezone.
1396+
// Because of that it's not possible to correctly precalculate
1397+
// granularities hierarchies on startup as they are specific for each timezone.
1398+
['granularityHierarchies', this.timezone],
1399+
() => R.reduce(
1400+
(hierarchies, cube) => R.reduce(
1401+
(acc, [tdName, td]) => {
1402+
const dimensionKey = `${cube}.${tdName}`;
1403+
1404+
// constructing standard granularities for time dimension
1405+
const standardEntries = R.fromPairs(
1406+
R.keys(standardGranularitiesParents).map(gr => [
1407+
`${dimensionKey}.${gr}`,
1408+
standardGranularitiesParents[gr],
1409+
]),
1410+
);
1411+
1412+
// If we have custom granularities in time dimension
1413+
const customEntries = td.granularities
1414+
? R.fromPairs(
1415+
R.keys(td.granularities).map(granularityName => {
1416+
const grObj = new Granularity(this, { dimension: dimensionKey, granularity: granularityName });
1417+
return [
1418+
`${dimensionKey}.${granularityName}`,
1419+
[granularityName, ...standardGranularitiesParents[grObj.minGranularity()]],
1420+
];
1421+
}),
1422+
)
1423+
: {};
1424+
1425+
return { ...acc, ...standardEntries, ...customEntries };
1426+
},
1427+
hierarchies,
1428+
R.toPairs(this.cubeEvaluator.timeDimensionsForCube(cube)),
1429+
),
1430+
{},
1431+
R.keys(this.cubeEvaluator.evaluatedCubes),
1432+
),
1433+
);
13931434
}
13941435

13951436
granularityParentHierarchy(granularity) {
@@ -1656,7 +1697,8 @@ export class BaseQuery {
16561697

16571698
/**
16581699
*
1659-
* @param {{sql: string, on: {cubeName: string, expression: Function}, joinType: 'LEFT' | 'INNER', alias: string}} customJoin
1700+
* @param {{sql: string, on: {cubeName: string, expression: Function}, joinType: 'LEFT' | 'INNER', alias: string}}
1701+
* customJoin
16601702
* @returns {JoinItem}
16611703
*/
16621704
customSubQueryJoin(customJoin) {

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

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export class BaseTimeDimension extends BaseFilter {
3939
}
4040

4141
// TODO: find and fix all hidden references to granularity to rely on granularityObj instead?
42-
public get granularity(): string | undefined {
42+
public get granularity(): string | null | undefined {
4343
return this.granularityObj?.granularity;
4444
}
4545

@@ -217,7 +217,7 @@ export class BaseTimeDimension extends BaseFilter {
217217
return this.query.dateTimeCast(this.query.paramAllocator.allocateParam(this.dateRange ? this.dateToFormatted() : BUILD_RANGE_END_LOCAL));
218218
}
219219

220-
public dateRangeGranularity() {
220+
public dateRangeGranularity(): string | null {
221221
if (!this.dateRange) {
222222
return null;
223223
}
@@ -229,9 +229,9 @@ export class BaseTimeDimension extends BaseFilter {
229229
);
230230
}
231231

232-
protected rollupGranularityValue: any | null = null;
232+
protected rollupGranularityValue: string | null = null;
233233

234-
public rollupGranularity() {
234+
public rollupGranularity(): string | null {
235235
if (!this.rollupGranularityValue) {
236236
this.rollupGranularityValue =
237237
this.query.cacheValue(
@@ -241,7 +241,18 @@ export class BaseTimeDimension extends BaseFilter {
241241
return this.dateRangeGranularity();
242242
}
243243

244-
return this.query.minGranularity(this.granularityObj.minGranularity(), this.dateRangeGranularity());
244+
if (!this.dateRange) {
245+
return this.granularityObj.minGranularity();
246+
}
247+
248+
// If we have granularity and date range, we need to check
249+
// that the interval and the granularity offset are stacked/fits with date range
250+
if (this.granularityObj.isPredefined() ||
251+
!this.granularityObj.isAlignedWithDateRange([this.dateFromFormatted(), this.dateToFormatted()])) {
252+
return this.query.minGranularity(this.granularityObj.minGranularity(), this.dateRangeGranularity());
253+
}
254+
255+
return this.granularityObj.granularity;
245256
}
246257
);
247258
}
@@ -262,11 +273,7 @@ export class BaseTimeDimension extends BaseFilter {
262273
}
263274

264275
public resolvedGranularity() {
265-
return this.granularityObj?.resolvedGranularity();
266-
}
267-
268-
public isPredefinedGranularity(): boolean {
269-
return this.granularityObj?.isPredefined() || false;
276+
return this.granularityObj ? this.granularityObj.resolvedGranularity() : this.dateRangeGranularity();
270277
}
271278

272279
public wildcardRange() {

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

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
import moment from 'moment-timezone';
22
import {
33
addInterval,
4-
isPredefinedGranularity, parseSqlInterval,
5-
QueryDateRange, timeSeries,
4+
alignToOrigin,
5+
isPredefinedGranularity,
6+
parsedSqlIntervalToDuration,
7+
parseSqlInterval,
8+
QueryDateRange,
9+
timeSeries,
610
timeSeriesFromCustomInterval,
711
TimeSeriesOptions
812
} from '@cubejs-backend/shared';
@@ -159,6 +163,29 @@ export class Granularity {
159163
}
160164
}
161165

166+
public isAlignedWithDateRange([startStr, endStr]: QueryDateRange): boolean {
167+
const intervalParsed = parseSqlInterval(this.granularityInterval);
168+
const grIntervalDuration = parsedSqlIntervalToDuration(intervalParsed);
169+
const msFrom = moment.tz(startStr, this.queryTimezone);
170+
const msTo = moment.tz(endStr, this.queryTimezone).add(1, 'ms');
171+
172+
// We can't simply compare interval milliseconds because of DSTs.
173+
const testDate = msFrom.clone();
174+
while (testDate.isBefore(msTo)) {
175+
testDate.add(grIntervalDuration);
176+
}
177+
if (!testDate.isSame(msTo)) {
178+
return false;
179+
}
180+
181+
const closestDate = alignToOrigin(msFrom, intervalParsed, this.origin);
182+
if (!msFrom.isSame(closestDate)) {
183+
return false;
184+
}
185+
186+
return true;
187+
}
188+
162189
public isNaturalAligned(): boolean {
163190
const intParsed = this.granularityInterval.split(' ');
164191

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

Lines changed: 18 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -439,18 +439,14 @@ export class PreAggregations {
439439
static sortTimeDimensionsWithRollupGranularity(timeDimensions) {
440440
return timeDimensions && R.sortBy(
441441
R.prop(0),
442-
timeDimensions.map(d => (d.isPredefinedGranularity() ?
443-
[d.expressionPath(), d.rollupGranularity(), null] :
444-
// For custom granularities we need to add its name to the list (for exact matches)
445-
[d.expressionPath(), d.rollupGranularity(), d.granularity]
446-
))
442+
timeDimensions.map(d => [d.expressionPath(), d.rollupGranularity()])
447443
) || [];
448444
}
449445

450446
static timeDimensionsAsIs(timeDimensions) {
451447
return timeDimensions && R.sortBy(
452448
R.prop(0),
453-
timeDimensions.map(d => [d.expressionPath(), d.granularity]),
449+
timeDimensions.map(d => [d.expressionPath(), d.resolvedGranularity()]),
454450
) || [];
455451
}
456452

@@ -485,26 +481,6 @@ export class PreAggregations {
485481
);
486482
}
487483

488-
canUsePreAggregationAndCheckIfRefValid(query) {
489-
const transformedQuery = PreAggregations.transformQueryToCanUseForm(query);
490-
return (refs) => PreAggregations.canUsePreAggregationForTransformedQueryFn(
491-
transformedQuery, refs
492-
);
493-
}
494-
495-
checkAutoRollupPreAggregationValid(refs) {
496-
try {
497-
this.autoRollupPreAggregationQuery(null, refs); // TODO null
498-
return true;
499-
} catch (e) {
500-
if (e instanceof UserError || e.toString().indexOf('ReferenceError') !== -1) {
501-
return false;
502-
} else {
503-
throw e;
504-
}
505-
}
506-
}
507-
508484
/**
509485
* Returns function to determine whether pre-aggregation can be used or not
510486
* for specified query, or its value for `refs` if specified.
@@ -560,9 +536,7 @@ export class PreAggregations {
560536
backAlias(references.sortedTimeDimensions || sortTimeDimensions(references.timeDimensions));
561537
const qryTimeDimensions = references.allowNonStrictDateRangeMatch
562538
? transformedQuery.timeDimensions
563-
: transformedQuery.sortedTimeDimensions.map(t => t.slice(0, 2));
564-
// slice above is used to exclude possible custom granularity returned from sortTimeDimensionsWithRollupGranularity()
565-
539+
: transformedQuery.sortedTimeDimensions;
566540
const backAliasMeasures = backAlias(references.measures);
567541
const backAliasSortedDimensions = backAlias(references.sortedDimensions || references.dimensions);
568542
const backAliasDimensions = backAlias(references.dimensions);
@@ -589,12 +563,13 @@ export class PreAggregations {
589563
};
590564

591565
/**
592-
* Wrap granularity string into an array.
593-
* @param {string} granularity
566+
* Expand granularity into array of granularity hierarchy.
567+
* @param {string} dimension Dimension in the form of `cube.timeDimension`
568+
* @param {string} granularity Granularity
594569
* @returns {Array<string>}
595570
*/
596-
const expandGranularity = (granularity) => (
597-
transformedQuery.granularityHierarchies[granularity] ||
571+
const expandGranularity = (dimension, granularity) => (
572+
transformedQuery.granularityHierarchies[`${dimension}.${granularity}`] ||
598573
[granularity]
599574
);
600575

@@ -611,15 +586,15 @@ export class PreAggregations {
611586
references.sortedTimeDimensions ||
612587
sortTimeDimensions(references.timeDimensions);
613588

614-
return expandGranularity(transformedQuery.windowGranularity)
615-
.map(
616-
windowGranularity => R.all(
617-
td => td[1] === windowGranularity,
618-
sortedTimeDimensions,
589+
return sortedTimeDimensions
590+
.map(td => expandGranularity(td[0], transformedQuery.windowGranularity))
591+
.some(
592+
expandedGranularities => expandedGranularities.some(
593+
windowGranularity => sortedTimeDimensions.every(
594+
td => td[1] === windowGranularity
595+
)
619596
)
620-
)
621-
.filter(x => !!x)
622-
.length > 0;
597+
);
623598
};
624599

625600
/**
@@ -628,16 +603,9 @@ export class PreAggregations {
628603
* @returns {Array<Array<string>>}
629604
*/
630605
const expandTimeDimension = (timeDimension) => {
631-
const [dimension, granularity, customGranularity] = timeDimension;
632-
const res = expandGranularity(granularity)
606+
const [dimension, resolvedGranularity] = timeDimension;
607+
return expandGranularity(dimension, resolvedGranularity)
633608
.map((newGranularity) => [dimension, newGranularity]);
634-
635-
if (customGranularity) {
636-
// For custom granularities we add it upfront to the list (for exact matches)
637-
res.unshift([dimension, customGranularity]);
638-
}
639-
640-
return res;
641609
};
642610

643611
/**

‎packages/cubejs-schema-compiler/src/compiler/CubeEvaluator.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -448,8 +448,8 @@ export class CubeEvaluator extends CubeSymbols {
448448

449449
public timeDimensionPathsForCube(cube: any) {
450450
return R.compose(
451-
R.map(nameToDefinition => `${cube}.${nameToDefinition[0]}`),
452-
R.toPairs,
451+
R.map(dimName => `${cube}.${dimName}`),
452+
R.keys,
453453
// @ts-ignore
454454
R.filter((d: any) => d.type === 'time')
455455
// @ts-ignore
@@ -460,12 +460,19 @@ export class CubeEvaluator extends CubeSymbols {
460460
return this.cubeFromPath(cube).measures || {};
461461
}
462462

463+
public timeDimensionsForCube(cube) {
464+
return R.filter(
465+
(d: any) => d.type === 'time',
466+
this.cubeFromPath(cube).dimensions || {}
467+
);
468+
}
469+
463470
public preAggregationsForCube(path: string) {
464471
return this.cubeFromPath(path).preAggregations || {};
465472
}
466473

467474
/**
468-
* Returns pre-aggregations filtered by the spcified selector.
475+
* Returns pre-aggregations filtered by the specified selector.
469476
* @param {{
470477
* scheduled: boolean,
471478
* dataSource: Array<string>,

0 commit comments

Comments
 (0)