Skip to content

Commit 58bfe5e

Browse files
waralexromwaralexrom
authored
fix(cube): Issue with calculated measures in pre-aggragations (cube-js#10429)
1 parent bf7146d commit 58bfe5e

4 files changed

Lines changed: 87 additions & 71 deletions

File tree

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

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export type PreAggregationForQuery = {
3939
preAggregationName: string;
4040
cube: string;
4141
canUsePreAggregation: boolean;
42+
leafMeasureMatch?: boolean;
4243
preAggregationId: string;
4344
preAggregation: PreAggregationDefinitionExtended;
4445
references: PreAggregationReferences;
@@ -78,7 +79,8 @@ export type RollupJoinItem = JoinEdgeWithMembers & {
7879

7980
export type RollupJoin = RollupJoinItem[];
8081

81-
export type CanUsePreAggregationFn = (references: PreAggregationReferences) => boolean;
82+
export type CanUsePreAggregationResult = { canUse: boolean; leafMeasureMatch: boolean };
83+
export type CanUsePreAggregationFn = (references: PreAggregationReferences) => CanUsePreAggregationResult;
8284

8385
/**
8486
* TODO: Write a real type.
@@ -642,15 +644,15 @@ export class PreAggregations {
642644
/**
643645
* Determine whether pre-aggregation can be used or not.
644646
*/
645-
const canUsePreAggregationNotAdditive: CanUsePreAggregationFn = (references: PreAggregationReferences): boolean => {
647+
const canUsePreAggregationNotAdditive: CanUsePreAggregationFn = (references: PreAggregationReferences): CanUsePreAggregationResult => {
646648
const qryTimeDimensions = references.allowNonStrictDateRangeMatch
647649
? transformedQuery.timeDimensions
648650
: transformedQuery.sortedTimeDimensions;
649651

650652
const refTimeDimensions = backAlias(sortTimeDimensions(references.timeDimensions));
651653
const backAliasMeasures = backAlias(references.measures);
652654
const backAliasDimensions = backAlias(references.dimensions);
653-
return ((
655+
const canUse = ((
654656
transformedQuery.hasNoTimeDimensionsWithoutGranularity
655657
) && (
656658
!transformedQuery.hasCumulativeMeasures
@@ -670,6 +672,7 @@ export class PreAggregations {
670672
// TODO do we need backAlias here?
671673
R.all(m => backAliasMeasures.includes(m), transformedQuery.leafMeasures)
672674
));
675+
return { canUse: !!canUse, leafMeasureMatch: false };
673676
};
674677

675678
/**
@@ -714,7 +717,7 @@ export class PreAggregations {
714717
.map((newGranularity) => [dimension, newGranularity]);
715718
};
716719

717-
const canUsePreAggregationLeafMeasureAdditive: CanUsePreAggregationFn = (references): boolean => {
720+
const canUsePreAggregationLeafMeasureAdditive: CanUsePreAggregationFn = (references): CanUsePreAggregationResult => {
718721
/**
719722
* Array of 2-element arrays with dimension and granularity.
720723
* @type {Array<Array<string>>}
@@ -736,7 +739,7 @@ export class PreAggregations {
736739
if (transformedQuery.leafMeasures.some(m => references.multipliedMeasures?.includes(m)) ||
737740
transformedQuery.measures.some(m => backAliasMultipliedMeasures.includes(m))
738741
) {
739-
return false;
742+
return { canUse: false, leafMeasureMatch: true };
740743
}
741744
}
742745

@@ -773,32 +776,36 @@ export class PreAggregations {
773776
dimensionsMatch(transformedQuery.sortedUsedCubePrimaryKeys, true) || dimensionsMatch(transformedQuery.sortedUsedCubePrimaryKeys, false)
774777
)
775778
) {
776-
return false;
779+
return { canUse: false, leafMeasureMatch: true };
777780
}
778781
}
779782

780783
const backAliasMeasures = backAlias(references.measures);
781-
return ((
784+
const canUse = ((
782785
windowGranularityMatches(references)
783786
) && (
784787
R.all(
785788
(m: string) => references.measures.indexOf(m) !== -1,
786789
references.rollups.length > 0 ? transformedQuery.leafMeasures : transformedQuery.leafMeasuresFullPaths,
787-
) || R.all(
790+
) || (transformedQuery.isAdditive && R.all(
788791
m => backAliasMeasures.indexOf(m) !== -1,
789792
transformedQuery.measures,
790-
)
793+
))
791794
) && (
792795
dimensionsMatch(transformedQuery.sortedDimensions, true) && timeDimensionsMatch(queryTimeDimensionsList, true) ||
793796
dimensionsMatch(transformedQuery.ownedDimensions, false) && timeDimensionsMatch(ownedQueryTimeDimensionsList, false)
794797
));
798+
return { canUse: !!canUse, leafMeasureMatch: true };
795799
};
796800

797801
const canUseFn: CanUsePreAggregationFn =
798802
(
799803
transformedQuery.leafMeasureAdditive && !transformedQuery.hasMultipliedMeasures && !transformedQuery.hasMultiStage || transformedQuery.ungrouped
800-
) ? ((r: PreAggregationReferences): boolean => canUsePreAggregationLeafMeasureAdditive(r) ||
801-
canUsePreAggregationNotAdditive(r))
804+
) ? ((r: PreAggregationReferences): CanUsePreAggregationResult => {
805+
const leaf = canUsePreAggregationLeafMeasureAdditive(r);
806+
if (leaf.canUse) return leaf;
807+
return canUsePreAggregationNotAdditive(r);
808+
})
802809
: canUsePreAggregationNotAdditive;
803810

804811
if (refs) {
@@ -935,7 +942,7 @@ export class PreAggregations {
935942
}
936943

937944
public getRollupPreAggregationByName(cube, preAggregationName): PreAggregationForQueryWithTableName | {} {
938-
const canUsePreAggregation = () => true;
945+
const canUsePreAggregation: CanUsePreAggregationFn = () => ({ canUse: true, leafMeasureMatch: false });
939946
const preAggregation = R.pipe(
940947
R.toPairs,
941948
R.filter(([_, a]) => a.type === 'rollup' || a.type === 'rollupJoin' || a.type === 'rollupLambda'),
@@ -1079,13 +1086,17 @@ export class PreAggregations {
10791086
canUsePreAggregation: CanUsePreAggregationFn
10801087
): PreAggregationForQuery {
10811088
const references = this.evaluateAllReferences(cube, preAggregation, preAggregationName);
1089+
const rollupResult = preAggregation.type === 'rollup'
1090+
? canUsePreAggregation(references)
1091+
: { canUse: false, leafMeasureMatch: false };
10821092
const preAggObj: PreAggregationForQuery = {
10831093
preAggregationName,
10841094
preAggregation,
10851095
cube,
10861096
// For rollupJoin and rollupLambda we need to enrich references with data
10871097
// from the underlying rollups which are collected later;
1088-
canUsePreAggregation: preAggregation.type === 'rollup' ? canUsePreAggregation(references) : false,
1098+
canUsePreAggregation: rollupResult.canUse,
1099+
leafMeasureMatch: rollupResult.leafMeasureMatch,
10891100
references,
10901101
preAggregationId: `${cube}.${preAggregationName}`
10911102
};
@@ -1107,10 +1118,12 @@ export class PreAggregations {
11071118
references.rollupsReferences.push(preAgg.references);
11081119
});
11091120
const rollupJoin = this.buildRollupJoin(preAggObj, preAggregationsToJoin);
1121+
const joinResult = canUsePreAggregation(references);
11101122

11111123
return {
11121124
...preAggObj,
1113-
canUsePreAggregation: canUsePreAggregation(references),
1125+
canUsePreAggregation: joinResult.canUse,
1126+
leafMeasureMatch: joinResult.leafMeasureMatch,
11141127
preAggregationsToJoin,
11151128
rollupJoin,
11161129
};
@@ -1160,9 +1173,11 @@ export class PreAggregations {
11601173
referencedPreAggregations.forEach(preAgg => {
11611174
references.rollupsReferences.push(preAgg.references);
11621175
});
1176+
const lambdaResult = canUsePreAggregation(references);
11631177
return {
11641178
...preAggObj,
1165-
canUsePreAggregation: canUsePreAggregation(references),
1179+
canUsePreAggregation: lambdaResult.canUse,
1180+
leafMeasureMatch: lambdaResult.leafMeasureMatch,
11661181
referencedPreAggregations,
11671182
};
11681183
} else {
@@ -1566,6 +1581,13 @@ export class PreAggregations {
15661581
return Object.fromEntries(measures
15671582
.flatMap(path => {
15681583
const measure = this.query.newMeasure(path);
1584+
1585+
// Skip non-additive measures in leaf-measure match mode —
1586+
// they will be recomputed from their leaf measures
1587+
if (preAggregationForQuery.leafMeasureMatch && !measure.isAdditive()) {
1588+
return [];
1589+
}
1590+
15691591
const measurePath = measure.path();
15701592
const column = this.query.ungrouped ? measure.aliasName() : (this.query.aggregateOnGroupedColumn(
15711593
measure.measureDefinition(),

‎packages/cubejs-schema-compiler/test/integration/postgres/pre-aggregations-calculated-measures.test.ts‎

Lines changed: 46 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -139,57 +139,51 @@ describe('PreAggregationsMultiStage', () => {
139139
140140
`);
141141

142-
if (getEnv('nativeSqlPlanner') && getEnv('nativeSqlPlannerPreAggregations')) {
143-
it('calculated measure pre-aggregation', () => compiler.compile().then(() => {
144-
const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
145-
measures: [
146-
'visitor_checkins.average',
147-
'visitor_checkins.revenue',
148-
'visitor_checkins.count'
149-
],
150-
dimensions: [
151-
'visitors.source'
152-
],
153-
timezone: 'America/Los_Angeles',
154-
preAggregationsSchema: '',
155-
cubestoreSupportMultistage: true
156-
});
157-
158-
const preAggregationsDescription: any = query.preAggregations?.preAggregationsDescription();
159-
const sqlAndParams = query.buildSqlAndParams();
160-
expect(preAggregationsDescription[0].tableName).toEqual('vis_average_pre_agg');
161-
expect(sqlAndParams[0]).toContain('vis_average_pre_agg');
162-
163-
return dbRunner.evaluateQueryWithPreAggregations(query).then(res => {
164-
expect(res).toEqual(
165-
166-
[
167-
{
168-
vis__source: null,
169-
vc__average: null,
170-
vc__revenue: null,
171-
vc__count: '0'
172-
},
173-
{
174-
vis__source: 'google',
175-
vc__average: '6.0000000000000000',
176-
vc__revenue: '6',
177-
vc__count: '1'
178-
},
179-
{
180-
vis__source: 'some',
181-
vc__average: '3.0000000000000000',
182-
vc__revenue: '15',
183-
vc__count: '5'
184-
}
185-
]
186-
187-
);
188-
});
189-
}));
190-
} else {
191-
it.skip('multi stage pre-aggregations', () => {
192-
// Skipping because it works only in Tesseract
142+
it('calculated measure pre-aggregation', () => compiler.compile().then(() => {
143+
const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
144+
measures: [
145+
'visitor_checkins.average',
146+
'visitor_checkins.revenue',
147+
'visitor_checkins.count'
148+
],
149+
dimensions: [
150+
'visitors.source'
151+
],
152+
timezone: 'America/Los_Angeles',
153+
preAggregationsSchema: '',
154+
cubestoreSupportMultistage: true
193155
});
194-
}
156+
157+
const preAggregationsDescription: any = query.preAggregations?.preAggregationsDescription();
158+
const sqlAndParams = query.buildSqlAndParams();
159+
expect(preAggregationsDescription[0].tableName).toEqual('vis_average_pre_agg');
160+
expect(sqlAndParams[0]).toContain('vis_average_pre_agg');
161+
162+
return dbRunner.evaluateQueryWithPreAggregations(query).then(res => {
163+
expect(res).toEqual(
164+
165+
[
166+
{
167+
vis__source: null,
168+
vc__average: null,
169+
vc__revenue: null,
170+
vc__count: '0'
171+
},
172+
{
173+
vis__source: 'google',
174+
vc__average: '6.0000000000000000',
175+
vc__revenue: '6',
176+
vc__count: '1'
177+
},
178+
{
179+
vis__source: 'some',
180+
vc__average: '3.0000000000000000',
181+
vc__revenue: '15',
182+
vc__count: '5'
183+
}
184+
]
185+
186+
);
187+
});
188+
}));
195189
});

‎packages/cubejs-schema-compiler/test/unit/pre-agg-by-filter-match.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,12 @@ describe('Pre Aggregation by filter match tests', () => {
7777
segments: querySegments?.map(s => `cube.${s}`),
7878
});
7979

80-
const usePreAggregation = PreAggregations.canUsePreAggregationForTransformedQueryFn(
80+
const result = PreAggregations.canUsePreAggregationForTransformedQueryFn(
8181
PreAggregations.transformQueryToCanUseForm(query),
8282
refs
8383
);
8484

85-
expect(usePreAggregation).toEqual(expecting);
85+
expect((result as any).canUse).toEqual(expecting);
8686
}
8787

8888
it('1 Dimension, 1 Filter', () => testPreAggregationMatch(

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,12 @@ describe('Pre Aggregation by filter match tests', () => {
8383
timezone: queryTimeZone,
8484
});
8585

86-
const usePreAggregation = PreAggregations.canUsePreAggregationForTransformedQueryFn(
86+
const result = PreAggregations.canUsePreAggregationForTransformedQueryFn(
8787
PreAggregations.transformQueryToCanUseForm(query),
8888
refs
8989
);
9090

91-
expect(usePreAggregation).toEqual(expecting);
91+
expect((result as any).canUse).toEqual(expecting);
9292
}
9393

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

0 commit comments

Comments
 (0)