Skip to content

Commit 038f19d

Browse files
committed
fix(schema-compiler): Do not drop join hints when view member is not itself owned
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 e98bb6c commit 038f19d

File tree

2 files changed

+283
-1
lines changed

2 files changed

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

0 commit comments

Comments
 (0)