Skip to content

Commit d5678f3

Browse files
mcheshkovmarianore-muttdata
authored andcommitted
fix(schema-compiler): Collect join hints from subquery join conditions (cube-js#9554)
This is necessary to support queries where output references only join subqueries. Before, when query does have subqeury joins, but does not have any measures/dimensions/... join hints would be empty, and join tree would be null, which is almost okay, but to generate query with join subqueries we need to build join query, so join tree is more-or-less required.
1 parent 76527d0 commit d5678f3

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
@@ -324,6 +324,10 @@ export class BaseQuery {
324324
return dimension;
325325
}).filter(R.identity).map(this.newTimeDimension.bind(this));
326326
this.allFilters = this.timeDimensions.concat(this.segments).concat(this.filters);
327+
/**
328+
* @type {Array<{sql: string, on: {expression: Function}, joinType: 'LEFT' | 'INNER', alias: string}>}
329+
*/
330+
this.customSubQueryJoins = this.options.subqueryJoins ?? [];
327331
this.useNativeSqlPlanner = this.options.useNativeSqlPlanner ?? getEnv('nativeSqlPlanner');
328332
this.canUseNativeSqlPlannerPreAggregation = false;
329333
if (this.useNativeSqlPlanner) {
@@ -337,11 +341,6 @@ export class BaseQuery {
337341
this.preAggregationsSchemaOption = this.options.preAggregationsSchema ?? DEFAULT_PREAGGREGATIONS_SCHEMA;
338342
this.externalQueryClass = this.options.externalQueryClass;
339343

340-
/**
341-
* @type {Array<{sql: string, on: {expression: Function}, joinType: 'LEFT' | 'INNER', alias: string}>}
342-
*/
343-
this.customSubQueryJoins = this.options.subqueryJoins ?? [];
344-
345344
// Set the default order only when options.order is not provided at all
346345
// if options.order is set (empty array [] or with data) - use it as is
347346
this.order = this.options.order ?? this.defaultOrder();
@@ -2392,14 +2391,34 @@ export class BaseQuery {
23922391
* @returns {Array<Array<string>>}
23932392
*/
23942393
collectJoinHints(excludeTimeDimensions = false) {
2395-
const membersToCollectFrom = this.allMembersConcat(excludeTimeDimensions)
2396-
.concat(this.join ? this.join.joins.map(j => ({
2397-
getMembers: () => [{
2398-
path: () => null,
2399-
cube: () => this.cubeEvaluator.cubeFromPath(j.originalFrom),
2400-
definition: () => j.join,
2401-
}]
2402-
})) : []);
2394+
const customSubQueryJoinMembers = this.customSubQueryJoins.map(j => {
2395+
const res = {
2396+
path: () => null,
2397+
cube: () => this.cubeEvaluator.cubeFromPath(j.on.cubeName),
2398+
definition: () => ({
2399+
sql: j.on.expression,
2400+
// TODO use actual type even though it isn't used right now
2401+
type: 'number'
2402+
}),
2403+
};
2404+
return {
2405+
getMembers: () => [res],
2406+
};
2407+
});
2408+
2409+
const joinMembers = this.join ? this.join.joins.map(j => ({
2410+
getMembers: () => [{
2411+
path: () => null,
2412+
cube: () => this.cubeEvaluator.cubeFromPath(j.originalFrom),
2413+
definition: () => j.join,
2414+
}]
2415+
})) : [];
2416+
2417+
const membersToCollectFrom = [
2418+
...this.allMembersConcat(excludeTimeDimensions),
2419+
...joinMembers,
2420+
...customSubQueryJoinMembers,
2421+
];
24032422

24042423
return this.collectJoinHintsFromMembers(membersToCollectFrom);
24052424
}

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)