Skip to content

Commit 460d5ff

Browse files
committed
fix(schema-compiler): Collect join hints from subquery join conditions
This is necessary to support queries where output references only join subqueries
1 parent 1deddcc commit 460d5ff

File tree

3 files changed

+71
-13
lines changed

3 files changed

+71
-13
lines changed

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

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,10 @@ export class BaseQuery {
288288
return dimension;
289289
}).filter(R.identity).map(this.newTimeDimension.bind(this));
290290
this.allFilters = this.timeDimensions.concat(this.segments).concat(this.filters);
291+
/**
292+
* @type {Array<{sql: string, on: {expression: Function}, joinType: 'LEFT' | 'INNER', alias: string}>}
293+
*/
294+
this.customSubQueryJoins = this.options.subqueryJoins ?? [];
291295
this.useNativeSqlPlanner = this.options.useNativeSqlPlanner ?? getEnv('nativeSqlPlanner');
292296
this.canUseNativeSqlPlannerPreAggregation = false;
293297
if (this.useNativeSqlPlanner) {
@@ -300,11 +304,6 @@ export class BaseQuery {
300304
this.preAggregationsSchemaOption = this.options.preAggregationsSchema ?? DEFAULT_PREAGGREGATIONS_SCHEMA;
301305
this.externalQueryClass = this.options.externalQueryClass;
302306

303-
/**
304-
* @type {Array<{sql: string, on: {expression: Function}, joinType: 'LEFT' | 'INNER', alias: string}>}
305-
*/
306-
this.customSubQueryJoins = this.options.subqueryJoins ?? [];
307-
308307
// Set the default order only when options.order is not provided at all
309308
// if options.order is set (empty array [] or with data) - use it as is
310309
this.order = this.options.order ?? this.defaultOrder();
@@ -2284,14 +2283,34 @@ export class BaseQuery {
22842283
* @returns {Array<Array<string>>}
22852284
*/
22862285
collectJoinHints(excludeTimeDimensions = false) {
2287-
const membersToCollectFrom = this.allMembersConcat(excludeTimeDimensions)
2288-
.concat(this.join ? this.join.joins.map(j => ({
2289-
getMembers: () => [{
2290-
path: () => null,
2291-
cube: () => this.cubeEvaluator.cubeFromPath(j.originalFrom),
2292-
definition: () => j.join,
2293-
}]
2294-
})) : []);
2286+
const customSubQueryJoinMembers = this.customSubQueryJoins.map(j => {
2287+
const res = {
2288+
path: () => null,
2289+
cube: () => this.cubeEvaluator.cubeFromPath(j.on.cubeName),
2290+
definition: () => ({
2291+
sql: j.on.expression,
2292+
// TODO use actual type even though it isn't used right now
2293+
type: 'number'
2294+
}),
2295+
};
2296+
return {
2297+
getMembers: () => [res],
2298+
};
2299+
});
2300+
2301+
const joinMembers = this.join ? this.join.joins.map(j => ({
2302+
getMembers: () => [{
2303+
path: () => null,
2304+
cube: () => this.cubeEvaluator.cubeFromPath(j.originalFrom),
2305+
definition: () => j.join,
2306+
}]
2307+
})) : [];
2308+
2309+
const membersToCollectFrom = [
2310+
...this.allMembersConcat(excludeTimeDimensions),
2311+
...joinMembers,
2312+
...customSubQueryJoinMembers,
2313+
];
22952314

22962315
return this.collectJoinHintsFromMembers(membersToCollectFrom);
22972316
}

packages/cubejs-testing/test/__snapshots__/smoke-cubesql.test.ts.snap

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,17 @@ Array [
520520
]
521521
`;
522522

523+
exports[`SQL API Postgres (Data) join with grouped query and empty members: join grouped empty members 1`] = `
524+
Array [
525+
Object {
526+
"status": "processed",
527+
},
528+
Object {
529+
"status": "shipped",
530+
},
531+
]
532+
`;
533+
523534
exports[`SQL API Postgres (Data) join with grouped query on coalesce: join grouped on coalesce 1`] = `
524535
Array [
525536
Object {

packages/cubejs-testing/test/smoke-cubesql.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,34 @@ filter_subq AS (
666666
expect(res.rows).toMatchSnapshot('join grouped on coalesce');
667667
});
668668

669+
test('join with grouped query and empty members', async () => {
670+
const query = `
671+
SELECT
672+
top_orders.status
673+
FROM
674+
"Orders"
675+
INNER JOIN
676+
(
677+
SELECT
678+
status,
679+
SUM(totalAmount)
680+
FROM
681+
"Orders"
682+
GROUP BY 1
683+
ORDER BY 2 DESC
684+
LIMIT 2
685+
) top_orders
686+
ON
687+
"Orders".status = top_orders.status
688+
GROUP BY 1
689+
ORDER BY 1
690+
`;
691+
692+
const res = await connection.query(query);
693+
// Expect only top statuses 2 by total amount: processed and shipped
694+
expect(res.rows).toMatchSnapshot('join grouped empty members');
695+
});
696+
669697
test('where segment is false', async () => {
670698
const query =
671699
'SELECT value AS val, * FROM "SegmentTest" WHERE segment_eq_1 IS FALSE ORDER BY value;';

0 commit comments

Comments
 (0)