Skip to content

Commit ac29330

Browse files
KSDaemonKSDaemon
committed
improve topological sort
1 parent adb08d2 commit ac29330

File tree

2 files changed

+115
-49
lines changed

2 files changed

+115
-49
lines changed

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

Lines changed: 65 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { camelize } from 'inflection';
44

55
import { UserError } from './UserError';
66
import { DynamicReference } from './DynamicReference';
7-
import { camelizeCube, topologicalSort } from './utils';
7+
import { camelizeCube, findCyclesInGraph, topologicalSort } from './utils';
88
import { BaseQuery } from '../adapter';
99

1010
import type { ErrorReporter } from './ErrorReporter';
@@ -47,19 +47,11 @@ export const CURRENT_CUBE_CONSTANTS = ['CUBE', 'TABLE'];
4747

4848
export type CubeDef = any;
4949

50-
export type GraphNode = {
51-
cubeDef: CubeDef;
52-
name: string;
53-
};
54-
55-
export type ReferenceNode = {
56-
name: string;
57-
};
58-
5950
/**
60-
* Treat it as GraphNode depends on ReferenceNode
51+
* Tuple of 2 node names
52+
* Treat it as first node depends on the last
6153
*/
62-
export type GraphEdge = [GraphNode, ReferenceNode?];
54+
export type GraphEdge = [string, string];
6355

6456
export class CubeSymbols {
6557
public symbols: Record<string | symbol, any>;
@@ -115,10 +107,20 @@ export class CubeSymbols {
115107
}
116108
}
117109

118-
private prepareDepsGraph(cubes: CubeDefinition[]): GraphEdge[] {
119-
const graph = new Map<string, GraphEdge>();
110+
private prepareDepsGraph(cubes: CubeDefinition[]): [Map<string, CubeDef>, GraphEdge[]] {
111+
const graphNodes = new Map<string, CubeDef>();
112+
const adjacencyList = new Map<string, Set<string>>(); // To search for cycles
113+
114+
const addEdge = (from: string, to: string) => {
115+
if (!adjacencyList.has(from)) {
116+
adjacencyList.set(from, new Set());
117+
}
118+
adjacencyList.get(from)!.add(to);
119+
};
120120

121121
for (const cube of cubes) {
122+
graphNodes.set(cube.name, cube);
123+
122124
if (cube.isView) {
123125
cube.cubes?.forEach(c => {
124126
const jp = c.joinPath || c.join_path; // View is not camelized yet
@@ -134,27 +136,71 @@ export class CubeSymbols {
134136
[cubeJoinPath] = res.split('.');
135137
}
136138
}
137-
graph.set(`${cube.name}-${cubeJoinPath}`, [{ cubeDef: cube, name: cube.name }, { name: cubeJoinPath }]);
139+
addEdge(cube.name, cubeJoinPath);
138140
}
139141
});
140142

141143
// Legacy-style includes
142144
if (typeof cube.includes === 'function') {
143145
const refs = this.funcArguments(cube.includes);
144146
refs.forEach(ref => {
145-
graph.set(`${cube.name}-${ref}`, [{ cubeDef: cube, name: cube.name }, { name: ref }]);
147+
addEdge(cube.name, ref);
146148
});
147149
}
148150
} else if (cube.joins && Object.keys(cube.joins).length > 0) {
149151
Object.keys(cube.joins).forEach(j => {
150-
graph.set(`${cube.name}-${j}`, [{ cubeDef: cube, name: cube.name }, { name: j }]);
152+
addEdge(cube.name, j);
151153
});
152154
} else {
153-
graph.set(`${cube.name}-none`, [{ cubeDef: cube, name: cube.name }]);
155+
adjacencyList.set(cube.name, new Set());
156+
}
157+
}
158+
159+
const cycles = findCyclesInGraph(adjacencyList);
160+
161+
for (const cycle of cycles) {
162+
const cycleSet = new Set(cycle);
163+
164+
// Validate that cycle doesn't have views
165+
if (cycle.some(node => graphNodes.get(node)?.isView)) {
166+
throw new UserError(`A view cannot be part of a dependency loop. Please review your cube definitions ${cycle.join(', ')} and ensure that no views are included in loops.`);
167+
}
168+
169+
// Let's find external dependencies (who refers to the loop)
170+
const externalNodes = new Set<string>();
171+
for (const [from, toSet] of adjacencyList.entries()) {
172+
if (!cycleSet.has(from)) {
173+
for (const to of toSet) {
174+
if (cycleSet.has(to)) {
175+
externalNodes.add(from);
176+
}
177+
}
178+
}
179+
}
180+
181+
// Remove all edges inside the loop
182+
for (const node of cycle) {
183+
adjacencyList.set(node, new Set([...adjacencyList.get(node)!].filter(n => !cycleSet.has(n))));
184+
}
185+
186+
// If there are external dependencies, point them to every node in the loop
187+
if (externalNodes.size > 0) {
188+
for (const external of externalNodes) {
189+
for (const cube of cycle) {
190+
addEdge(external, cube);
191+
}
192+
}
193+
}
194+
}
195+
196+
const graphEdges: GraphEdge[] = [];
197+
for (const [from, toSet] of adjacencyList) {
198+
for (const to of toSet) {
199+
graphEdges.push([from, to]);
154200
}
155201
}
156202

157-
return Array.from(graph.values());
203+
return [graphNodes, graphEdges];
158204
}
159205

160206
public getCubeDefinition(cubeName: string) {

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

Lines changed: 50 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -57,32 +57,19 @@ export function camelizeCube(cube: any): unknown {
5757
/**
5858
* This is a simple cube-views topological sorting based on Kahn's algorythm.
5959
*/
60-
export function topologicalSort(edges: GraphEdge[]): CubeDef[] {
61-
const graph = new Map();
62-
const outDegree = new Map();
60+
export function topologicalSort([nodes, edges]: [Map<string, CubeDef>, GraphEdge[]]): CubeDef[] {
61+
const inDegree = new Map<string, Set<string>>();
62+
const outDegree = new Map<string, number>();
6363

64-
for (const [from, to] of edges) {
65-
if (!graph.has(from.name)) {
66-
graph.set(from.name, { cubeDef: from.cubeDef, from: [] });
67-
outDegree.set(from.name, 0);
68-
} else {
69-
const n = graph.get(from.name);
70-
if (!n.cubeDef) {
71-
n.cubeDef = from.cubeDef;
72-
}
73-
}
74-
75-
if (to) {
76-
if (!graph.has(to.name)) {
77-
graph.set(to.name, { from: [from.name] });
78-
outDegree.set(to.name, 0);
79-
} else {
80-
const n = graph.get(to.name);
81-
n.from.push(from.name);
82-
}
64+
nodes.forEach(node => {
65+
outDegree.set(node.name, 0);
66+
inDegree.set(node.name, new Set());
67+
});
8368

84-
outDegree.set(from.name, (outDegree.get(from.name) || 0) + 1);
85-
}
69+
for (const [from, to] of edges) {
70+
const n = inDegree.get(to) || new Set();
71+
n.add(from);
72+
outDegree.set(from, (outDegree.get(from) ?? 0) + 1);
8673
}
8774

8875
const queue: string[] = [...outDegree.entries()].filter(([_, deg]) => deg === 0).map(([name]) => name);
@@ -95,22 +82,55 @@ export function topologicalSort(edges: GraphEdge[]): CubeDef[] {
9582
break;
9683
}
9784

98-
const node = graph.get(nodeName);
85+
const from = inDegree.get(nodeName) || new Set();
9986

100-
sorted.push(node.cubeDef);
87+
sorted.push(nodes.get(nodeName));
10188

102-
for (const neighbor of node.from) {
103-
outDegree.set(neighbor, outDegree.get(neighbor) - 1);
89+
for (const neighbor of from) {
90+
outDegree.set(neighbor, (outDegree.get(neighbor) || 1) - 1);
10491
if (outDegree.get(neighbor) === 0) {
10592
queue.push(neighbor);
10693
}
10794
}
10895
}
10996

110-
if (sorted.length !== graph.size) {
111-
const remainingNodes = [...graph.keys()].filter(node => !sorted.includes(node));
97+
if (sorted.length !== nodes.size) {
98+
const remainingNodes = [...nodes.keys()].filter(node => !sorted.includes(node));
11299
throw new Error(`Cyclical dependence detected! Potential problems with ${remainingNodes.join(', ')}.`);
113100
}
114101

115102
return sorted;
116103
}
104+
105+
export function findCyclesInGraph(adjacencyList: Map<string, Set<string>>): string[][] {
106+
const visited = new Set<string>();
107+
const stack = new Set<string>();
108+
const cycles: string[][] = [];
109+
110+
const dfs = (node: string, path: string[]) => {
111+
if (stack.has(node)) {
112+
const cycleStart = path.indexOf(node);
113+
cycles.push(path.slice(cycleStart));
114+
return;
115+
}
116+
if (visited.has(node)) return;
117+
118+
visited.add(node);
119+
stack.add(node);
120+
path.push(node);
121+
122+
for (const neighbor of adjacencyList.get(node) ?? []) {
123+
dfs(neighbor, [...path]);
124+
}
125+
126+
stack.delete(node);
127+
};
128+
129+
for (const node of adjacencyList.keys()) {
130+
if (!visited.has(node)) {
131+
dfs(node, []);
132+
}
133+
}
134+
135+
return cycles;
136+
}

0 commit comments

Comments
 (0)