Skip to content

Commit f8d8bff

Browse files
committed
return back loop for join resolution
1 parent 3d50b65 commit f8d8bff

File tree

1 file changed

+63
-18
lines changed

1 file changed

+63
-18
lines changed

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

Lines changed: 63 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -438,32 +438,77 @@ 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+
let joinMembersJoinHints = [];
443+
const newCollectedHints = [];
444+
445+
// One cube may join the other cube via transitive joined cubes,
446+
// members from which are referenced in the join `on` clauses.
447+
// We need to collect such join hints and push them upfront of the joining one
448+
// but only if they don't exist yet. Cause in other case we might affect what
449+
// join path will be constructed in join graph.
450+
// It is important to use queryLevelJoinHints during the calculation if it is set.
451+
452+
const constructJH = () => R.uniq(this.enrichHintsWithJoinMap([
442453
...this.queryLevelJoinHints,
454+
...(rootOfJoin ? [rootOfJoin] : []),
455+
...newCollectedHints,
443456
...allMembersJoinHints,
444457
...customSubQueryJoinHints,
445458
],
446-
queryJoinMaps);
459+
queryJoinMaps));
447460

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

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

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

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

0 commit comments

Comments
 (0)