Skip to content

Commit c0341ba

Browse files
authored
fix(schema-compiler): Use join tree for pre-agg matching (#9597)
* prepare test for non-match because of join tree difference * remove unused * fix some types * fix duplicates in join hint collection * Don't even try to match pre-agg if there are customSubQueryJoins * Don't try to match pre-aggs if there are MemberExpressions * add joinTree evaluation for pre-agg * more types and ts fixes * implement join tree comparison * fix tests * fix for 'rollupJoin' pre-aggs * remove comments * fix tests * fix aliasMember set for segments * fix expressionPath() * fix test * add resolveFullMemberPathFn() in BaseQuery * make real full join name path member comparison for pre-agg matching * fix tests * fix tests * add comments * Add more tests * fix tesseract skip tests * mark some tests as skipped for tesseract * sync tests * remove obsolete types * remove comment * fix tests * fix tests * skip tests for tesseract * useful comment
1 parent 909f2a7 commit c0341ba

File tree

12 files changed

+465
-210
lines changed

12 files changed

+465
-210
lines changed

packages/cubejs-schema-compiler/src/adapter/BaseMeasure.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ export class BaseMeasure {
329329

330330
public expressionPath(): string {
331331
if (this.expression) {
332-
return `expr:${this.expression.expressionName}`;
332+
return `expr:${this.expressionName}`;
333333
}
334334

335335
const path = this.path();

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

Lines changed: 108 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,7 @@ export class BaseQuery {
130130
/** @type {import('./BaseTimeDimension').BaseTimeDimension[]} */
131131
timeDimensions;
132132

133-
/**
134-
* @type {import('../compiler/JoinGraph').FinishedJoinTree}
135-
*/
133+
/** @type {import('../compiler/JoinGraph').FinishedJoinTree} */
136134
join;
137135

138136
/**
@@ -332,6 +330,9 @@ export class BaseQuery {
332330
}).filter(R.identity).map(this.newTimeDimension.bind(this));
333331
this.allFilters = this.timeDimensions.concat(this.segments).concat(this.filters);
334332
/**
333+
* For now this might come only from SQL API, it might be some queries that uses measures and filters to
334+
* get the dimensions that are then used as join conditions to get the final results.
335+
* As consequence - if there are such sub query joins - pre-aggregations can't be used.
335336
* @type {Array<{sql: string, on: {expression: Function}, joinType: 'LEFT' | 'INNER', alias: string}>}
336337
*/
337338
this.customSubQueryJoins = this.options.subqueryJoins ?? [];
@@ -365,6 +366,17 @@ export class BaseQuery {
365366
try {
366367
// TODO allJoinHints should contain join hints form pre-agg
367368
this.join = this.joinGraph.buildJoin(this.allJoinHints);
369+
/**
370+
* @type {Record<string, string[]>}
371+
*/
372+
const queryJoinGraph = {};
373+
for (const { originalFrom, originalTo } of (this.join?.joins || [])) {
374+
if (!queryJoinGraph[originalFrom]) {
375+
queryJoinGraph[originalFrom] = [];
376+
}
377+
queryJoinGraph[originalFrom].push(originalTo);
378+
}
379+
this.joinGraphPaths = queryJoinGraph || {};
368380
} catch (e) {
369381
if (this.useNativeSqlPlanner) {
370382
// Tesseract doesn't require join to be prebuilt and there's a case where single join can't be built for multi-fact query
@@ -420,7 +432,7 @@ export class BaseQuery {
420432

421433
/**
422434
*
423-
* @returns {Array<Array<string>>}
435+
* @returns {Array<string | Array<string>>}
424436
*/
425437
get allJoinHints() {
426438
if (!this.collectedJoinHints) {
@@ -708,7 +720,9 @@ export class BaseQuery {
708720
if (this.from) {
709721
return this.simpleQuery();
710722
}
711-
if (!this.options.preAggregationQuery) {
723+
const hasMemberExpressions = this.allMembersConcat(false).some(m => m.isMemberExpression);
724+
725+
if (!this.options.preAggregationQuery && !this.customSubQueryJoins.length && !hasMemberExpressions) {
712726
preAggForQuery =
713727
this.preAggregations.findPreAggregationForQuery();
714728
if (this.options.disableExternalPreAggregations && preAggForQuery?.preAggregation.external) {
@@ -720,8 +734,6 @@ export class BaseQuery {
720734
multipliedMeasures,
721735
regularMeasures,
722736
cumulativeMeasures,
723-
withQueries,
724-
multiStageMembers,
725737
} = this.fullKeyQueryAggregateMeasures();
726738

727739
if (cumulativeMeasures.length === 0) {
@@ -785,7 +797,7 @@ export class BaseQuery {
785797
externalPreAggregationQuery() {
786798
if (!this.options.preAggregationQuery && !this.options.disableExternalPreAggregations && this.externalQueryClass) {
787799
const preAggregationForQuery = this.preAggregations.findPreAggregationForQuery();
788-
if (preAggregationForQuery && preAggregationForQuery.preAggregation.external) {
800+
if (preAggregationForQuery?.preAggregation.external) {
789801
return true;
790802
}
791803
const preAggregationsDescription = this.preAggregations.preAggregationsDescription();
@@ -3810,7 +3822,7 @@ export class BaseQuery {
38103822

38113823
/**
38123824
*
3813-
* @param options
3825+
* @param {unknown} options
38143826
* @returns {this}
38153827
*/
38163828
newSubQuery(options) {
@@ -4964,7 +4976,10 @@ export class BaseQuery {
49644976
*/
49654977
backAliasMembers(members) {
49664978
const query = this;
4967-
return Object.fromEntries(members.flatMap(
4979+
4980+
const buildJoinPath = this.buildJoinPathFn();
4981+
4982+
const aliases = Object.fromEntries(members.flatMap(
49684983
member => {
49694984
const collectedMembers = query.evaluateSymbolSqlWithContext(
49704985
() => query.collectFrom([member], query.collectMemberNamesFor.bind(query), 'collectMemberNamesFor'),
@@ -4982,5 +4997,88 @@ export class BaseQuery {
49824997
.map(d => [query.cubeEvaluator.byPathAnyType(d).aliasMember, memberPath]);
49834998
}
49844999
));
5000+
5001+
/**
5002+
* @type {Record<string, string>}
5003+
*/
5004+
const res = {};
5005+
for (const [original, alias] of Object.entries(aliases)) {
5006+
const [cube, field] = original.split('.');
5007+
const path = buildJoinPath(cube);
5008+
5009+
const [aliasCube, aliasField] = alias.split('.');
5010+
const aliasPath = aliasCube !== cube ? buildJoinPath(aliasCube) : path;
5011+
5012+
if (path) {
5013+
res[`${path}.${field}`] = aliasPath ? `${aliasPath}.${aliasField}` : alias;
5014+
}
5015+
5016+
// Aliases might come from proxied members, in such cases
5017+
// we need to map them to originals too
5018+
if (aliasPath) {
5019+
res[original] = `${aliasPath}.${aliasField}`;
5020+
}
5021+
}
5022+
5023+
return res;
5024+
}
5025+
5026+
buildJoinPathFn() {
5027+
const query = this;
5028+
const { root } = this.join || {};
5029+
5030+
return (target) => {
5031+
const visited = new Set();
5032+
const path = [];
5033+
5034+
/**
5035+
* @param {string} node
5036+
* @returns {boolean}
5037+
*/
5038+
function dfs(node) {
5039+
if (node === target) {
5040+
path.push(node);
5041+
return true;
5042+
}
5043+
5044+
if (visited.has(node)) return false;
5045+
visited.add(node);
5046+
5047+
const neighbors = query.joinGraphPaths[node] || [];
5048+
for (const neighbor of neighbors) {
5049+
if (dfs(neighbor)) {
5050+
path.unshift(node);
5051+
return true;
5052+
}
5053+
}
5054+
5055+
return false;
5056+
}
5057+
5058+
return dfs(root) ? path.join('.') : null;
5059+
};
5060+
}
5061+
5062+
/**
5063+
* Returns a function that constructs the full member path
5064+
* based on the query's join structure.
5065+
* @returns {(function(member: string): (string))}
5066+
*/
5067+
resolveFullMemberPathFn() {
5068+
const { root: queryJoinRoot } = this.join || {};
5069+
5070+
const buildJoinPath = this.buildJoinPathFn();
5071+
5072+
return (member) => {
5073+
const [cube, field] = member.split('.');
5074+
if (!cube || !field) return member;
5075+
5076+
if (cube === queryJoinRoot.root) {
5077+
return member;
5078+
}
5079+
5080+
const path = buildJoinPath(cube);
5081+
return path ? `${path}.${field}` : member;
5082+
};
49855083
}
49865084
}

packages/cubejs-schema-compiler/src/adapter/BaseSegment.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ export class BaseSegment {
9090

9191
public expressionPath(): string {
9292
if (this.expression) {
93-
return `expr:${this.expression.expressionName}`;
93+
return `expr:${this.expressionName}`;
9494
}
9595

9696
const path = this.path();

0 commit comments

Comments
 (0)