Skip to content

Commit b5ddd11

Browse files
authored
fix(schema-compiler): Do not drop join hints when view member is not itself owned (#9444)
Without this when following from view to cube member join hints will be collected and ignored, then join hints would be collected for references from cube members to other cube members, and only those would be used for join tree
1 parent 4d1b17a commit b5ddd11

File tree

4 files changed

+297
-47
lines changed

4 files changed

+297
-47
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2501,7 +2501,7 @@ export class BaseQuery {
25012501

25022502
pushMemberNameForCollectionIfNecessary(cubeName, name) {
25032503
const pathFromArray = this.cubeEvaluator.pathFromArray([cubeName, name]);
2504-
if (this.cubeEvaluator.byPathAnyType(pathFromArray).ownedByCube) {
2504+
if (!this.cubeEvaluator.getCubeDefinition(cubeName).isView) {
25052505
const joinHints = this.cubeEvaluator.joinHints();
25062506
if (joinHints && joinHints.length) {
25072507
joinHints.forEach(cube => this.pushCubeNameForCollectionIfNecessary(cube));

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ export class CubeSymbols {
9191
// TODO can we define it when cubeList is defined?
9292
this.cubeList.push(splitViews[viewName]);
9393
this.symbols[viewName] = splitViews[viewName];
94+
this.cubeDefinitions[viewName] = splitViews[viewName];
9495
}
9596
}
9697
}
Lines changed: 271 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,271 @@
1+
import { PostgresQuery } from '../../../src/adapter/PostgresQuery';
2+
import { prepareCompiler } from '../../unit/PrepareCompiler';
3+
import { DataSchemaCompiler } from '../../../src/compiler/DataSchemaCompiler';
4+
import { JoinGraph } from '../../../src/compiler/JoinGraph';
5+
import { CubeEvaluator } from '../../../src/compiler/CubeEvaluator';
6+
7+
describe('Multiple join paths', () => {
8+
jest.setTimeout(200000);
9+
10+
let compiler: DataSchemaCompiler;
11+
let joinGraph: JoinGraph;
12+
let cubeEvaluator: CubeEvaluator;
13+
14+
beforeAll(async () => {
15+
// All joins would look like this
16+
// A-->B-->C-->X
17+
// | ^
18+
// ├-->D-->E---┤
19+
// | |
20+
// └-->F-------┘
21+
// View, pre-aggregations and all interesting parts should use ADEX path
22+
// It should NOT be the shortest one from A to X (that's AFX), nor first in join edges declaration (that's ABCX)
23+
// All join conditions would be essentially `FALSE`, but with different syntax, to be able to test SQL generation
24+
// Also, there should be only one way to cover cubes A and D with joins: A->D join
25+
26+
// TODO in this model queries like [A.a_id, X.x_id] become ambiguous, probably we want to handle this better
27+
28+
// language=JavaScript
29+
const prepared = prepareCompiler(`
30+
cube('A', {
31+
sql: 'SELECT 1 AS a_id, 100 AS a_value',
32+
33+
joins: {
34+
B: {
35+
relationship: 'many_to_one',
36+
sql: "'A' = 'B'",
37+
},
38+
D: {
39+
relationship: 'many_to_one',
40+
sql: "'A' = 'D'",
41+
},
42+
F: {
43+
relationship: 'many_to_one',
44+
sql: "'A' = 'F'",
45+
},
46+
},
47+
48+
dimensions: {
49+
a_id: {
50+
type: 'number',
51+
sql: 'a_id',
52+
primaryKey: true,
53+
},
54+
},
55+
56+
measures: {
57+
a_sum: {
58+
sql: 'a_value',
59+
type: 'sum',
60+
},
61+
},
62+
});
63+
64+
cube('B', {
65+
sql: 'SELECT 1 AS b_id, 100 AS b_value',
66+
67+
joins: {
68+
C: {
69+
relationship: 'many_to_one',
70+
sql: "'B' = 'C'",
71+
},
72+
},
73+
74+
dimensions: {
75+
b_id: {
76+
type: 'number',
77+
sql: 'b_id',
78+
primaryKey: true,
79+
},
80+
},
81+
82+
measures: {
83+
b_sum: {
84+
sql: 'b_value',
85+
type: 'sum',
86+
},
87+
},
88+
});
89+
90+
cube('C', {
91+
sql: 'SELECT 1 AS c_id, 100 AS c_value',
92+
93+
joins: {
94+
X: {
95+
relationship: 'many_to_one',
96+
sql: "'C' = 'X'",
97+
},
98+
},
99+
100+
dimensions: {
101+
c_id: {
102+
type: 'number',
103+
sql: 'c_id',
104+
primaryKey: true,
105+
},
106+
},
107+
108+
measures: {
109+
c_sum: {
110+
sql: 'c_value',
111+
type: 'sum',
112+
},
113+
},
114+
});
115+
116+
cube('D', {
117+
sql: 'SELECT 1 AS d_id, 100 AS d_value',
118+
119+
joins: {
120+
E: {
121+
relationship: 'many_to_one',
122+
sql: "'D' = 'E'",
123+
},
124+
},
125+
126+
dimensions: {
127+
d_id: {
128+
type: 'number',
129+
sql: 'd_id',
130+
primaryKey: true,
131+
},
132+
},
133+
134+
measures: {
135+
d_sum: {
136+
sql: 'd_value',
137+
type: 'sum',
138+
},
139+
},
140+
});
141+
142+
cube('E', {
143+
sql: 'SELECT 1 AS e_id, 100 AS e_value',
144+
145+
joins: {
146+
X: {
147+
relationship: 'many_to_one',
148+
sql: "'E' = 'X'",
149+
},
150+
},
151+
152+
dimensions: {
153+
e_id: {
154+
type: 'number',
155+
sql: 'e_id',
156+
primaryKey: true,
157+
},
158+
},
159+
160+
measures: {
161+
e_sum: {
162+
sql: 'e_value',
163+
type: 'sum',
164+
},
165+
},
166+
});
167+
168+
cube('F', {
169+
sql: 'SELECT 1 AS f_id, 100 AS f_value',
170+
171+
joins: {
172+
X: {
173+
relationship: 'many_to_one',
174+
sql: "'F' = 'X'",
175+
},
176+
},
177+
178+
dimensions: {
179+
f_id: {
180+
type: 'number',
181+
sql: 'f_id',
182+
primaryKey: true,
183+
},
184+
},
185+
186+
measures: {
187+
f_sum: {
188+
sql: 'f_value',
189+
type: 'sum',
190+
},
191+
},
192+
});
193+
194+
cube('X', {
195+
sql: 'SELECT 1 AS x_id, 100 AS x_value',
196+
197+
dimensions: {
198+
x_id: {
199+
type: 'number',
200+
sql: 'x_id',
201+
primaryKey: true,
202+
},
203+
// This member should be:
204+
// * NOT ownedByCube
205+
// * reference only members of same cube
206+
// * included in view
207+
x_id_ref: {
208+
type: 'number',
209+
sql: \`\${x_id} + 1\`,
210+
},
211+
},
212+
213+
measures: {
214+
x_sum: {
215+
sql: 'x_value',
216+
type: 'sum',
217+
},
218+
},
219+
});
220+
221+
view('ADEX_view', {
222+
cubes: [
223+
{
224+
join_path: A,
225+
includes: [
226+
'a_id',
227+
],
228+
prefix: false
229+
},
230+
{
231+
join_path: A.D.E.X,
232+
includes: [
233+
'x_id',
234+
'x_id_ref',
235+
],
236+
prefix: false
237+
},
238+
]
239+
});
240+
`);
241+
242+
({ compiler, joinGraph, cubeEvaluator } = prepared);
243+
});
244+
245+
beforeEach(async () => {
246+
await compiler.compile();
247+
});
248+
249+
describe('View and indirect members', () => {
250+
it('should respect join path from view declaration', async () => {
251+
const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
252+
measures: [],
253+
dimensions: [
254+
'ADEX_view.a_id',
255+
'ADEX_view.x_id_ref',
256+
],
257+
});
258+
259+
const [sql, _params] = query.buildSqlAndParams();
260+
261+
expect(sql).toMatch(/ON 'A' = 'D'/);
262+
expect(sql).toMatch(/ON 'D' = 'E'/);
263+
expect(sql).toMatch(/ON 'E' = 'X'/);
264+
expect(sql).not.toMatch(/ON 'A' = 'B'/);
265+
expect(sql).not.toMatch(/ON 'B' = 'C'/);
266+
expect(sql).not.toMatch(/ON 'C' = 'X'/);
267+
expect(sql).not.toMatch(/ON 'A' = 'F'/);
268+
expect(sql).not.toMatch(/ON 'F' = 'X'/);
269+
});
270+
});
271+
});

0 commit comments

Comments
 (0)