Skip to content

Commit 91004e1

Browse files
committed
return back loop for join resolution
1 parent ac28b8c commit 91004e1

File tree

1 file changed

+62
-18
lines changed

1 file changed

+62
-18
lines changed

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

Lines changed: 62 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -438,32 +438,76 @@ export class BaseQuery {
438438
const allMembersJoinHints = this.collectJoinHintsFromMembers(this.allMembersConcat(false));
439439
const queryJoinMaps = this.queryJoinMap();
440440
const customSubQueryJoinHints = this.collectJoinHintsFromMembers(this.joinMembersFromCustomSubQuery());
441-
const allJoinHints = this.enrichHintsWithJoinMap([
441+
let rootOfJoin = allMembersJoinHints[0];
442+
const newCollectedHints = [];
443+
444+
// One cube may join the other cube via transitive joined cubes,
445+
// members from which are referenced in the join `on` clauses.
446+
// We need to collect such join hints and push them upfront of the joining one
447+
// but only if they don't exist yet. Cause in other case we might affect what
448+
// join path will be constructed in join graph.
449+
// It is important to use queryLevelJoinHints during the calculation if it is set.
450+
451+
const constructJH = () => R.uniq(this.enrichHintsWithJoinMap([
442452
...this.queryLevelJoinHints,
453+
...(rootOfJoin ? [rootOfJoin] : []),
454+
...newCollectedHints,
443455
...allMembersJoinHints,
444456
...customSubQueryJoinHints,
445457
],
446-
queryJoinMaps);
458+
queryJoinMaps));
447459

448-
const tempJoin = this.joinGraph.buildJoin(allJoinHints);
460+
let prevJoin = null;
461+
let newJoin = null;
449462

450-
if (!tempJoin) {
451-
this.collectedJoinHints = allJoinHints;
452-
return allJoinHints;
453-
}
463+
const isJoinTreesEqual = (a, b) => {
464+
if (!a || !b || a.root !== b.root || a.joins.length !== b.joins.length) {
465+
return false;
466+
}
454467

455-
const joinMembersJoinHints = this.collectJoinHintsFromMembers(this.joinMembersFromJoin(tempJoin));
456-
const allJoinHintsFlatten = new Set(allJoinHints.flat());
457-
const newCollectedHints = joinMembersJoinHints.filter(j => !allJoinHintsFlatten.has(j));
468+
// We don't care about the order of joins on the same level, so
469+
// we can compare them as sets.
470+
const aJoinsSet = new Set(a.joins.map(j => `${j.originalFrom}->${j.originalTo}`));
471+
const bJoinsSet = new Set(b.joins.map(j => `${j.originalFrom}->${j.originalTo}`));
458472

459-
this.collectedJoinHints = this.enrichHintsWithJoinMap([
460-
...this.queryLevelJoinHints,
461-
tempJoin.root,
462-
...newCollectedHints,
463-
...allMembersJoinHints,
464-
...customSubQueryJoinHints,
465-
],
466-
queryJoinMaps);
473+
if (aJoinsSet.size !== bJoinsSet.size) {
474+
return false;
475+
}
476+
477+
for (const val of aJoinsSet) {
478+
if (!bJoinsSet.has(val)) {
479+
return false;
480+
}
481+
}
482+
483+
return true;
484+
};
485+
486+
// Safeguard against infinite loop in case of cyclic joins somehow managed to slip through
487+
let cnt = 0;
488+
let newJoinHintsCollectedCnt;
489+
490+
do {
491+
const allJoinHints = constructJH();
492+
prevJoin = newJoin;
493+
newJoin = this.joinGraph.buildJoin(allJoinHints);
494+
const allJoinHintsFlatten = new Set(allJoinHints.flat());
495+
const joinMembersJoinHints = this.collectJoinHintsFromMembers(this.joinMembersFromJoin(newJoin));
496+
497+
const iterationCollectedHints = joinMembersJoinHints.filter(j => !allJoinHintsFlatten.has(j));
498+
newCollectedHints.push(...iterationCollectedHints);
499+
newJoinHintsCollectedCnt = iterationCollectedHints.length;
500+
cnt++;
501+
if (newJoin) {
502+
rootOfJoin = newJoin.root;
503+
}
504+
} while (newJoin?.joins.length > 0 && !isJoinTreesEqual(prevJoin, newJoin) && cnt < 10000 && newJoinHintsCollectedCnt > 0);
505+
506+
if (cnt >= 10000) {
507+
throw new UserError('Can not construct joins for the query, potential loop detected');
508+
}
509+
510+
this.collectedJoinHints = constructJH();
467511
}
468512
return this.collectedJoinHints;
469513
}

0 commit comments

Comments
 (0)