Skip to content

Commit d3948c9

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 4e56069 commit d3948c9

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
@@ -2545,7 +2545,7 @@ export class BaseQuery {
25452545

25462546
pushMemberNameForCollectionIfNecessary(cubeName, name) {
25472547
const pathFromArray = this.cubeEvaluator.pathFromArray([cubeName, name]);
2548-
if (this.cubeEvaluator.byPathAnyType(pathFromArray).ownedByCube) {
2548+
if (!this.cubeEvaluator.getCubeDefinition(cubeName).isView) {
25492549
const joinHints = this.cubeEvaluator.joinHints();
25502550
if (joinHints && joinHints.length) {
25512551
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)