From 2b6ef377f64ffa7bb0a92b0c7aae52ca895e1788 Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Tue, 11 Mar 2025 19:43:13 +0200 Subject: [PATCH 1/7] test(schema-compiler): Add member expressions on views test suite --- .../member-expressions-on-views.test.ts | 646 ++++++++++++++++++ 1 file changed, 646 insertions(+) create mode 100644 packages/cubejs-schema-compiler/test/integration/postgres/member-expressions-on-views.test.ts diff --git a/packages/cubejs-schema-compiler/test/integration/postgres/member-expressions-on-views.test.ts b/packages/cubejs-schema-compiler/test/integration/postgres/member-expressions-on-views.test.ts new file mode 100644 index 0000000000000..4f87bb189b074 --- /dev/null +++ b/packages/cubejs-schema-compiler/test/integration/postgres/member-expressions-on-views.test.ts @@ -0,0 +1,646 @@ +import { PostgresQuery } from '../../../src'; +import { DataSchemaCompiler } from '../../../src/compiler/DataSchemaCompiler'; +import { JoinGraph } from '../../../src/compiler/JoinGraph'; +import { CubeEvaluator } from '../../../src/compiler/CubeEvaluator'; + +import { prepareCompiler } from '../../unit/PrepareCompiler'; +import { dbRunner } from './PostgresDBRunner'; + +describe('Member expressions on views', () => { + jest.setTimeout(200000); + + // language=JavaScript + const model = ` + cube('single_cube', { + sql: \` + SELECT 1 AS id, 'foo' AS dim, 'one' AS test_dim, 100 AS val UNION ALL + SELECT 2 AS id, 'foo' AS dim, 'two' AS test_dim, 300 AS val UNION ALL + SELECT 3 AS id, 'bar' AS dim, 'three' AS test_dim, 500 AS val + \`, + measures: { + val_sum: { + type: 'sum', + sql: 'val' + }, + val_avg: { + type: 'avg', + sql: 'val' + }, + }, + dimensions: { + id: { + type: 'number', + sql: 'id', + primaryKey: true, + }, + dim: { + type: 'string', + sql: 'dim', + }, + test_dim: { + type: 'string', + sql: 'test_dim', + } + } + }); + + view('single_view', { + cubes: [ + { + join_path: 'single_cube', + includes: [ + 'dim', + 'test_dim', + 'val_sum', + 'val_avg', + ] + }, + ] + }); + + cube('many_to_one_root', { + sql: \` + SELECT 1 AS id, 1 AS child_id, 'foo' AS dim, 'one' AS test_dim, 100 AS val UNION ALL + SELECT 2 AS id, 1 AS child_id, 'foo' AS dim, 'two' AS test_dim, 300 AS val UNION ALL + SELECT 3 AS id, 2 AS child_id, 'foo' AS dim, 'two' AS test_dim, 800 AS val UNION ALL + SELECT 4 AS id, 3 AS child_id, 'bar' AS dim, 'three' AS test_dim, 500 AS val + \`, + joins: { + many_to_one_child: { + relationship: 'many_to_one', + sql: \`\${CUBE.child_id} = \${many_to_one_child.id}\` + }, + }, + measures: { + val_sum: { + type: 'sum', + sql: 'val' + }, + val_avg: { + type: 'avg', + sql: 'val' + }, + }, + dimensions: { + id: { + type: 'number', + sql: 'id', + primaryKey: true, + }, + child_id: { + type: 'number', + sql: 'child_id', + }, + dim: { + type: 'string', + sql: 'dim', + }, + test_dim: { + type: 'string', + sql: 'test_dim', + } + } + }); + + cube('many_to_one_child', { + sql: \` + SELECT 1 AS id, 'foo' AS dim, 'one' AS test_dim, 100 AS val UNION ALL + SELECT 2 AS id, 'foo' AS dim, 'two' AS test_dim, 300 AS val UNION ALL + SELECT 3 AS id, 'bar' AS dim, 'three' AS test_dim, 500 AS val + \`, + measures: { + val_sum: { + type: 'sum', + sql: 'val' + }, + val_avg: { + type: 'avg', + sql: 'val' + }, + }, + dimensions: { + id: { + type: 'number', + sql: 'id', + primaryKey: true, + }, + dim: { + type: 'string', + sql: 'dim', + }, + test_dim: { + type: 'string', + sql: 'test_dim', + } + } + }); + + view('many_to_one_view', { + cubes: [ + { + join_path: 'many_to_one_root', + includes: [ + 'dim', + 'test_dim', + 'val_sum', + 'val_avg', + ], + prefix: true, + }, + { + join_path: 'many_to_one_root.many_to_one_child', + includes: [ + 'dim', + 'test_dim', + 'val_sum', + 'val_avg', + ], + prefix: true, + }, + ] + }); + + cube('one_to_many_root', { + sql: \` + SELECT 1 AS id, 'foo' AS dim, 'one' AS test_dim, 100 AS val UNION ALL + SELECT 2 AS id, 'foo' AS dim, 'two' AS test_dim, 300 AS val UNION ALL + SELECT 3 AS id, 'bar' AS dim, 'three' AS test_dim, 500 AS val UNION ALL + SELECT 4 AS id, 'bar' AS dim, 'four' AS test_dim, 500 AS val UNION ALL + SELECT 5 AS id, 'bar' AS dim, 'five' AS test_dim, 500 AS val + \`, + joins: { + one_to_many_child: { + relationship: 'one_to_many', + sql: \`\${CUBE.id} = \${one_to_many_child.parent_id}\` + }, + }, + measures: { + val_sum: { + type: 'sum', + sql: 'val' + }, + val_avg: { + type: 'avg', + sql: 'val' + }, + }, + dimensions: { + id: { + type: 'number', + sql: 'id', + primaryKey: true, + }, + dim: { + type: 'string', + sql: 'dim', + }, + test_dim: { + type: 'string', + sql: 'test_dim', + } + } + }); + + cube('one_to_many_child', { + sql: \` + SELECT 1 AS id, 1 AS parent_id, 'foo' AS dim, 'one' AS test_dim, 100 AS val UNION ALL + SELECT 2 AS id, 1 AS parent_id, 'bar' AS dim, 'two' AS test_dim, 300 AS val UNION ALL + SELECT 3 AS id, 2 AS parent_id, 'foo' AS dim, 'three' AS test_dim, 500 AS val + \`, + measures: { + val_sum: { + type: 'sum', + sql: 'val' + }, + val_avg: { + type: 'avg', + sql: 'val' + }, + }, + dimensions: { + id: { + type: 'number', + sql: 'id', + primaryKey: true, + }, + parent_id: { + type: 'number', + sql: 'parent_id', + }, + dim: { + type: 'string', + sql: 'dim', + }, + test_dim: { + type: 'string', + sql: 'test_dim', + } + } + }); + + view('one_to_many_view', { + cubes: [ + { + join_path: 'one_to_many_root', + includes: [ + 'dim', + 'test_dim', + 'val_sum', + 'val_avg', + ], + prefix: true, + }, + { + join_path: 'one_to_many_root.one_to_many_child', + includes: [ + 'dim', + 'test_dim', + 'val_sum', + 'val_avg', + ], + prefix: true, + }, + ] + }); + + `; + + let compiler: DataSchemaCompiler; + let joinGraph: JoinGraph; + let cubeEvaluator: CubeEvaluator; + + beforeAll(async () => { + ({ compiler, joinGraph, cubeEvaluator } = prepareCompiler(model)); + await compiler.compile(); + }); + + async function runQueryTest(q: unknown, expectedResult: unknown): Promise { + const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, q); + + const res = await dbRunner.testQuery(query.buildSqlAndParams()); + + expect(res).toEqual(expectedResult); + } + + // Every test have this in common: + // Request single dimension and avg measure from every cube, just to trigger full key query where possible + // Then, on top of that, each test would request one additional measure, with member expression inside + + // TODO add test with calculation in measure based on two different dimensions from same cube + // TODO add test with calculation in measure based on two different dimensions from different cubes + + type Config = { + cubeName: string, + baseQuery: { + measures: Array, + dimensions: Array, + order: Array<{id: string, desc: boolean}>, + }, + baseExpectedResults: Array>, + testMeasures: Array<{ + name: string, + expression: string, + expectedResults: Array> + }>, + }; + + const configs: Array = [ + { + cubeName: 'single_cube', + baseQuery: { + measures: [ + 'single_cube.val_avg', + ], + dimensions: [ + 'single_cube.dim', + ], + order: [{ + id: 'single_cube.dim', + desc: false, + }] + }, + baseExpectedResults: [ + { + single_cube__dim: 'bar', + single_cube__val_avg: '500.0000000000000000', + }, + { + single_cube__dim: 'foo', + single_cube__val_avg: '200.0000000000000000', + }, + ], + testMeasures: [ + { + name: 'val_sum', + // eslint-disable-next-line no-template-curly-in-string + expression: '${single_cube.val_sum}', + expectedResults: [ + { + single_cube_val_sum: '500', + }, + { + single_cube_val_sum: '400', + }, + ], + }, + { + name: 'distinct_dim', + // eslint-disable-next-line no-template-curly-in-string + expression: 'COUNT(DISTINCT ${single_cube.test_dim})', + expectedResults: [ + { + single_cube_distinct_dim: '1', + }, + { + single_cube_distinct_dim: '2', + }, + ], + }, + ], + }, + { + cubeName: 'single_view', + baseQuery: { + measures: [ + 'single_view.val_avg', + ], + dimensions: [ + 'single_view.dim', + ], + order: [{ + id: 'single_view.dim', + desc: false, + }] + }, + baseExpectedResults: [ + { + single_view__dim: 'bar', + single_view__val_avg: '500.0000000000000000', + }, + { + single_view__dim: 'foo', + single_view__val_avg: '200.0000000000000000', + }, + ], + testMeasures: [ + { + name: 'val_sum', + // eslint-disable-next-line no-template-curly-in-string + expression: '${single_view.val_sum}', + expectedResults: [ + { + single_view_val_sum: '500', + }, + { + single_view_val_sum: '400', + }, + ], + }, + { + name: 'distinct_dim', + // eslint-disable-next-line no-template-curly-in-string + expression: 'COUNT(DISTINCT ${single_view.test_dim})', + expectedResults: [ + { + single_view_distinct_dim: '1', + }, + { + single_view_distinct_dim: '2', + }, + ], + }, + ], + }, + { + cubeName: 'many_to_one_view', + baseQuery: { + measures: [ + 'many_to_one_view.many_to_one_root_val_avg', + 'many_to_one_view.many_to_one_child_val_avg', + ], + dimensions: [ + 'many_to_one_view.many_to_one_root_dim', + 'many_to_one_view.many_to_one_child_dim', + ], + order: [ + { + id: 'many_to_one_view.many_to_one_root_dim', + desc: false, + }, + { + id: 'many_to_one_view.many_to_one_child_dim', + desc: false, + } + ] + }, + baseExpectedResults: [ + { + many_to_one_view__many_to_one_root_dim: 'bar', + many_to_one_view__many_to_one_child_dim: 'bar', + many_to_one_view__many_to_one_root_val_avg: '500.0000000000000000', + many_to_one_view__many_to_one_child_val_avg: '500.0000000000000000', + }, + { + many_to_one_view__many_to_one_root_dim: 'foo', + many_to_one_view__many_to_one_child_dim: 'foo', + many_to_one_view__many_to_one_root_val_avg: '400.0000000000000000', + many_to_one_view__many_to_one_child_val_avg: '200.0000000000000000', + }, + ], + testMeasures: [ + { + name: 'root_val_sum', + // eslint-disable-next-line no-template-curly-in-string + expression: '${many_to_one_view.many_to_one_root_val_sum}', + expectedResults: [ + { + many_to_one_view_root_val_sum: '500', + }, + { + many_to_one_view_root_val_sum: '1200', + }, + ], + }, + { + name: 'root_distinct_dim', + // eslint-disable-next-line no-template-curly-in-string + expression: 'COUNT(DISTINCT ${many_to_one_view.many_to_one_root_test_dim})', + expectedResults: [ + { + many_to_one_view_root_distinct_dim: '1', + }, + { + many_to_one_view_root_distinct_dim: '2', + }, + ], + }, + { + name: 'child_val_sum', + // eslint-disable-next-line no-template-curly-in-string + expression: '${many_to_one_view.many_to_one_child_val_sum}', + expectedResults: [ + { + many_to_one_view_child_val_sum: '500', + }, + { + many_to_one_view_child_val_sum: '400', + }, + ], + }, + { + name: 'child_distinct_dim', + // eslint-disable-next-line no-template-curly-in-string + expression: 'COUNT(DISTINCT ${many_to_one_view.many_to_one_child_test_dim})', + expectedResults: [ + { + many_to_one_view_child_distinct_dim: '1', + }, + { + many_to_one_view_child_distinct_dim: '2', + }, + ], + }, + ], + }, + { + cubeName: 'one_to_many_view', + baseQuery: { + measures: [ + 'one_to_many_view.one_to_many_root_val_avg', + 'one_to_many_view.one_to_many_child_val_avg', + ], + dimensions: [ + 'one_to_many_view.one_to_many_root_dim', + 'one_to_many_view.one_to_many_child_dim', + ], + order: [ + { + id: 'one_to_many_view.one_to_many_root_dim', + desc: false, + }, + { + id: 'one_to_many_view.one_to_many_child_dim', + desc: false, + } + ] + }, + baseExpectedResults: [ + { + one_to_many_view__one_to_many_root_dim: 'bar', + one_to_many_view__one_to_many_child_dim: null, + one_to_many_view__one_to_many_root_val_avg: '500.0000000000000000', + one_to_many_view__one_to_many_child_val_avg: null, + }, + { + one_to_many_view__one_to_many_root_dim: 'foo', + one_to_many_view__one_to_many_child_dim: 'bar', + one_to_many_view__one_to_many_root_val_avg: '100.0000000000000000', + one_to_many_view__one_to_many_child_val_avg: '300.0000000000000000', + }, + { + one_to_many_view__one_to_many_root_dim: 'foo', + one_to_many_view__one_to_many_child_dim: 'foo', + one_to_many_view__one_to_many_root_val_avg: '200.0000000000000000', + one_to_many_view__one_to_many_child_val_avg: '300.0000000000000000', + }, + ], + testMeasures: [ + { + name: 'root_val_sum', + // eslint-disable-next-line no-template-curly-in-string + expression: '${one_to_many_view.one_to_many_root_val_sum}', + expectedResults: [ + { + one_to_many_view_root_val_sum: '1500', + }, + { + one_to_many_view_root_val_sum: '100', + }, + { + one_to_many_view_root_val_sum: '400', + }, + ], + }, + { + name: 'root_distinct_dim', + // eslint-disable-next-line no-template-curly-in-string + expression: 'COUNT(DISTINCT ${one_to_many_view.one_to_many_root_test_dim})', + expectedResults: [ + { + one_to_many_view_root_distinct_dim: '3', + }, + { + one_to_many_view_root_distinct_dim: '1', + }, + { + one_to_many_view_root_distinct_dim: '2', + }, + ], + }, + { + name: 'child_val_sum', + // eslint-disable-next-line no-template-curly-in-string + expression: '${one_to_many_view.one_to_many_child_val_sum}', + expectedResults: [ + { + one_to_many_view_child_val_sum: null, + }, + { + one_to_many_view_child_val_sum: '300', + }, + { + one_to_many_view_child_val_sum: '600', + }, + ], + }, + { + name: 'child_distinct_dim', + // eslint-disable-next-line no-template-curly-in-string + expression: 'COUNT(DISTINCT ${one_to_many_view.one_to_many_child_test_dim})', + expectedResults: [ + { + one_to_many_view_child_distinct_dim: '0', + }, + { + one_to_many_view_child_distinct_dim: '1', + }, + { + one_to_many_view_child_distinct_dim: '2', + }, + ], + }, + ], + }, + ]; + + for (const { cubeName, baseQuery, baseExpectedResults, testMeasures } of configs) { + describe(cubeName, () => { + for (const { name, expression, expectedResults } of testMeasures) { + it(name, async () => runQueryTest( + { + ...baseQuery, + measures: [ + ...baseQuery.measures, + { + // eslint-disable-next-line no-new-func + expression: new Function( + cubeName, + `return \`${expression}\`;` + ), + name: `${cubeName}_${name}`, + expressionName: `${cubeName}_${name}`, + // eslint-disable-next-line no-template-curly-in-string + definition: expression, + cubeName, + }, + ], + }, + expectedResults.map((r, i) => ({ + ...baseExpectedResults[i], + ...r, + })) + )); + } + }); + } +}); From d86c7f21be7f5a4fea86b358df36ad91fe40eeba Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Wed, 12 Mar 2025 15:28:49 +0200 Subject: [PATCH 2/7] fix(schema-compiler): Support member expression measures in error message in checkShouldBuildJoinForMeasureSelect --- packages/cubejs-schema-compiler/src/adapter/BaseQuery.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index a90ecc14df3ea..d4de9cc96c47a 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -1983,8 +1983,9 @@ export class BaseQuery { if (R.any(cubeName => keyCubeName !== cubeName, cubes)) { const measuresJoin = this.joinGraph.buildJoin(joinHints); if (measuresJoin.multiplicationFactor[keyCubeName]) { + const measureName = measure.isMemberExpression ? measure.expressionName : measure.measure; throw new UserError( - `'${measure.measure}' references cubes that lead to row multiplication. Please rewrite it using sub query.` + `'${measureName}' references cubes that lead to row multiplication. Please rewrite it using sub query.` ); } return true; From 2cbd1845a5f6020512af907491ce2ce5b65f00e3 Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Wed, 5 Mar 2025 16:59:50 +0200 Subject: [PATCH 3/7] fix(schema-compiler): Handle member expressions in keys in renderedReference --- packages/cubejs-schema-compiler/src/adapter/BaseQuery.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index d4de9cc96c47a..1f359abac3fd3 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -1055,9 +1055,14 @@ export class BaseQuery { outerMeasuresJoinFullKeyQueryAggregate(innerMembers, outerMembers, toJoin) { const renderedReferenceContext = { renderedReference: R.pipe( - R.map(m => [m.measure || m.dimension, m.aliasName()]), + R.map(m => { + const member = m.measure ? m.measure : m.dimension; + const memberPath = typeof member === 'string' + ? member + : this.cubeEvaluator.pathFromArray([m.expressionCubeName, m.expressionName]); + return [memberPath, m.aliasName()]; + }), R.fromPairs, - // eslint-disable-next-line @typescript-eslint/no-unused-vars )(innerMembers), }; From 94e2c3b46eed56e153b3a20b992df1730eb10b62 Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Wed, 5 Mar 2025 17:09:32 +0200 Subject: [PATCH 4/7] fix(schema-compiler): Handle measures with dimension-only member expressions in fullKeyQueryAggregateMeasures `fullKeyQueryAggregate` is using measure.cube() a lot, like for grouping measures to key cube This can be wrong for measure targeting view: we want to attach measure to subquery for its definition cube, and there should be no subquery for view itself It's expected that `fullKeyQueryAggregateMeasures` would resolve and prepare all measures from its original form in query to actual leaf measures in cubes, then build subqueries with those, and then `joinFullKeyQueryAggregate` would use `renderedReference` to point to these leaf measures Hard case is a measure that: * targeting view * is a member expression * references only dimensions from that view Measure like that are "leaf" - there's nowhere to push it, member expression has to be directly in leaf subquery If measure references dimension from a single cube it can be multiplied (think `SUM(${view.deep_dimension})`) So, three points: * it must be accounted for in `multipliedMeasures` * it must be attached to a proper cube subquery * it must use renderedReference correctly `collectRootMeasureToHieararchy` will not drop such measure completely. Now it will check is there's 0 measures collected, try to collect all referenced members to gather used cubes and detect multiplication, and add new measure in hierarchy. Because new returned measure is patched, it will be attached to correct cube subquery in `fullKeyQueryAggregate` Then `outerMeasuresJoinFullKeyQueryAggregate` needs to generate proper alias for it, so outer unpatched measure would pick up alias from inner patched one --- .../src/adapter/BaseQuery.js | 82 ++++++++++++++++++- 1 file changed, 78 insertions(+), 4 deletions(-) diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index 1f359abac3fd3..c57cd6c4d8eca 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -1059,7 +1059,7 @@ export class BaseQuery { const member = m.measure ? m.measure : m.dimension; const memberPath = typeof member === 'string' ? member - : this.cubeEvaluator.pathFromArray([m.expressionCubeName, m.expressionName]); + : this.cubeEvaluator.pathFromArray([m.measure?.originalCubeName ?? m.expressionCubeName, m.expressionName]); return [memberPath, m.aliasName()]; }), R.fromPairs, @@ -1702,6 +1702,8 @@ export class BaseQuery { .filter(f => R.none(m => m.measure === f.measure, this.measures)); return R.fromPairs(this.measures.concat(notAddedMeasureFilters).map(m => { + const measureName = typeof m.measure === 'string' ? m.measure : `${m.measure.cubeName}.${m.measure.name}`; + const collectedMeasures = this.collectFrom( [m], this.collectMultipliedMeasures(context), @@ -1715,7 +1717,57 @@ export class BaseQuery { const cubeName = m.expressionCubeName ? `\`${m.expressionCubeName}\` ` : ''; throw new UserError(`The query contains \`COUNT(*)\` expression but cube/view ${cubeName}is missing \`count\` measure`); } - return [typeof m.measure === 'string' ? m.measure : `${m.measure.cubeName}.${m.measure.name}`, collectedMeasures]; + if (collectedMeasures.length === 0 && m.isMemberExpression) { + // `m` is member expression measure, but does not reference any other measure + // Consider this dimensions-only measure. This can happen at least in 2 cases: + // 1. Ad-hoc aggregation over dimension: SELECT MAX(dim) FROM cube + // 2. Ungrouped query with SQL pushdown will render every column as measure: SELECT dim1 FROM cube WHERE LOWER(dim2) = 'foo'; + // Measures like this needs a special treatment to attach them to cube and decide if they are multiplied or not + // This would return measure object in `measure`, not path + // TODO return measure object for every measure + + const memberNamesForMeasure = this.collectFrom( + [m], + this.collectMemberNamesFor.bind(this), + context ? ['collectMemberNamesFor', JSON.stringify(context)] : 'collectMemberNamesFor', + this.queryCache + ); + const cubeNamesForMeasure = R.pipe( + R.map(mem => this.cubeEvaluator.parsePathAnyType(mem)[0]), + // Filtering views, because collectMemberNamesFor can return both view.dim and cube.dim + R.filter(cubeName => { + const cubeDef = this.cubeEvaluator.getCubeDefinition(cubeName); + return !cubeDef.isView; + }), + // Single member expression can reference multiple dimensions from same cube + R.uniq, + )( + memberNamesForMeasure + ); + + let cubeNameToAttach; + switch (cubeNamesForMeasure.length) { + case 1: + [cubeNameToAttach] = cubeNamesForMeasure; + break; + default: + throw new Error(`Expected single cube for dimension-only measure ${measureName}, got ${cubeNamesForMeasure}`); + } + + const multiplied = this.multipliedJoinRowResult(cubeNameToAttach) || false; + + const attachedMeasure = { + ...m.measure, + originalCubeName: m.measure.cubeName, + cubeName: cubeNameToAttach + }; + + return [measureName, [{ + multiplied, + measure: attachedMeasure, + }]]; + } + return [measureName, collectedMeasures]; })); } @@ -1982,9 +2034,31 @@ export class BaseQuery { } checkShouldBuildJoinForMeasureSelect(measures, keyCubeName) { + // When member expression references view, it would have to collect join hints from view + // Consider join A->B, as many-to-one, so B is multiplied and A is not, and member expression like SUM(AB_view.dimB) + // Both `collectCubeNamesFor` and `collectJoinHintsFor` would return too many cubes here + // They both walk join hints, and gather every cube present there + // For view we would get both A and B, because join hints would go from join tree root + // Even though expression references only B, and should be OK to use it with B as keyCube + // So this check would build new join tree from both A and B, B will be multiplied, and that would break check + return measures.map(measure => { - const cubes = this.collectFrom([measure], this.collectCubeNamesFor.bind(this), 'collectCubeNamesFor'); - const joinHints = this.collectFrom([measure], this.collectJoinHintsFor.bind(this), 'collectJoinHintsFor'); + const memberNamesForMeasure = this.collectFrom( + [measure], + this.collectMemberNamesFor.bind(this), + 'collectMemberNamesFor', + ); + + const nonViewMembers = memberNamesForMeasure + .filter(mem => { + const cubeName = this.cubeEvaluator.parsePathAnyType(mem)[0]; + const cubeDef = this.cubeEvaluator.getCubeDefinition(cubeName); + return !cubeDef.isView; + }) + .map(m => this.memberInstanceByPath(m)); + + const cubes = this.collectFrom(nonViewMembers, this.collectCubeNamesFor.bind(this), 'collectCubeNamesFor'); + const joinHints = this.collectFrom(nonViewMembers, this.collectJoinHintsFor.bind(this), 'collectJoinHintsFor'); if (R.any(cubeName => keyCubeName !== cubeName, cubes)) { const measuresJoin = this.joinGraph.buildJoin(joinHints); if (measuresJoin.multiplicationFactor[keyCubeName]) { From 732c40b33df466f367d9a835124e8170d17f8466 Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Wed, 12 Mar 2025 21:46:39 +0200 Subject: [PATCH 5/7] fix(schema-compiler): Support zero reference measures other than COUNT(*) --- .../src/adapter/BaseQuery.js | 7 +++ .../member-expressions-on-views.test.ts | 55 +++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index c57cd6c4d8eca..1fb95c35d2e44 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -1747,6 +1747,13 @@ export class BaseQuery { let cubeNameToAttach; switch (cubeNamesForMeasure.length) { + case 0: + // For zero reference measure there's nothing to derive info about measure from + // So it assume that it's a regular measure, and it will be evaluated on top of join tree + return [measureName, [{ + multiplied: false, + measure: m.measure, + }]]; case 1: [cubeNameToAttach] = cubeNamesForMeasure; break; diff --git a/packages/cubejs-schema-compiler/test/integration/postgres/member-expressions-on-views.test.ts b/packages/cubejs-schema-compiler/test/integration/postgres/member-expressions-on-views.test.ts index 4f87bb189b074..57da96aab18c2 100644 --- a/packages/cubejs-schema-compiler/test/integration/postgres/member-expressions-on-views.test.ts +++ b/packages/cubejs-schema-compiler/test/integration/postgres/member-expressions-on-views.test.ts @@ -330,6 +330,19 @@ describe('Member expressions on views', () => { }, ], testMeasures: [ + { + name: 'one_sum', + // eslint-disable-next-line no-template-curly-in-string + expression: 'SUM(1)', + expectedResults: [ + { + single_cube_one_sum: '1', + }, + { + single_cube_one_sum: '2', + }, + ], + }, { name: 'val_sum', // eslint-disable-next-line no-template-curly-in-string @@ -383,6 +396,19 @@ describe('Member expressions on views', () => { }, ], testMeasures: [ + { + name: 'one_sum', + // eslint-disable-next-line no-template-curly-in-string + expression: 'SUM(1)', + expectedResults: [ + { + single_view_one_sum: '1', + }, + { + single_view_one_sum: '2', + }, + ], + }, { name: 'val_sum', // eslint-disable-next-line no-template-curly-in-string @@ -448,6 +474,19 @@ describe('Member expressions on views', () => { }, ], testMeasures: [ + { + name: 'one_sum', + // eslint-disable-next-line no-template-curly-in-string + expression: 'SUM(1)', + expectedResults: [ + { + many_to_one_view_one_sum: '1', + }, + { + many_to_one_view_one_sum: '3', + }, + ], + }, { name: 'root_val_sum', // eslint-disable-next-line no-template-curly-in-string @@ -545,6 +584,22 @@ describe('Member expressions on views', () => { }, ], testMeasures: [ + { + name: 'one_sum', + // eslint-disable-next-line no-template-curly-in-string + expression: 'SUM(1)', + expectedResults: [ + { + one_to_many_view_one_sum: '3', + }, + { + one_to_many_view_one_sum: '1', + }, + { + one_to_many_view_one_sum: '2', + }, + ], + }, { name: 'root_val_sum', // eslint-disable-next-line no-template-curly-in-string From 881d823621b2aa4559a7fa2a6bd3af9a2be4c155 Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Mon, 17 Mar 2025 08:53:24 +0200 Subject: [PATCH 6/7] refactor(schema-compiler): Use ownedByCube instead of isView for filtering --- .../src/adapter/BaseQuery.js | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index 1fb95c35d2e44..883bad795dc94 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -1733,12 +1733,10 @@ export class BaseQuery { this.queryCache ); const cubeNamesForMeasure = R.pipe( - R.map(mem => this.cubeEvaluator.parsePathAnyType(mem)[0]), - // Filtering views, because collectMemberNamesFor can return both view.dim and cube.dim - R.filter(cubeName => { - const cubeDef = this.cubeEvaluator.getCubeDefinition(cubeName); - return !cubeDef.isView; - }), + R.map(member => this.memberInstanceByPath(member)), + // collectMemberNamesFor can return both view.dim and cube.dim + R.filter(member => member.definition().ownedByCube), + R.map(member => member.cube().name), // Single member expression can reference multiple dimensions from same cube R.uniq, )( @@ -2057,12 +2055,8 @@ export class BaseQuery { ); const nonViewMembers = memberNamesForMeasure - .filter(mem => { - const cubeName = this.cubeEvaluator.parsePathAnyType(mem)[0]; - const cubeDef = this.cubeEvaluator.getCubeDefinition(cubeName); - return !cubeDef.isView; - }) - .map(m => this.memberInstanceByPath(m)); + .map(member => this.memberInstanceByPath(member)) + .filter(member => member.definition().ownedByCube); const cubes = this.collectFrom(nonViewMembers, this.collectCubeNamesFor.bind(this), 'collectCubeNamesFor'); const joinHints = this.collectFrom(nonViewMembers, this.collectJoinHintsFor.bind(this), 'collectJoinHintsFor'); From 7893f39cdf08bd11f09687f5f3f1bcd45977aad1 Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Mon, 17 Mar 2025 09:07:42 +0200 Subject: [PATCH 7/7] refactor(schema-compiler): Extract dimensionOnlyMeasureToHierarchy method --- .../src/adapter/BaseQuery.js | 99 ++++++++++--------- 1 file changed, 51 insertions(+), 48 deletions(-) diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index 883bad795dc94..7d5e075e08cac 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -1697,13 +1697,60 @@ export class BaseQuery { ${this.query()}`; } + dimensionOnlyMeasureToHierarchy(context, m) { + const measureName = typeof m.measure === 'string' ? m.measure : `${m.measure.cubeName}.${m.measure.name}`; + const memberNamesForMeasure = this.collectFrom( + [m], + this.collectMemberNamesFor.bind(this), + context ? ['collectMemberNamesFor', JSON.stringify(context)] : 'collectMemberNamesFor', + this.queryCache + ); + const cubeNamesForMeasure = R.pipe( + R.map(member => this.memberInstanceByPath(member)), + // collectMemberNamesFor can return both view.dim and cube.dim + R.filter(member => member.definition().ownedByCube), + R.map(member => member.cube().name), + // Single member expression can reference multiple dimensions from same cube + R.uniq, + )( + memberNamesForMeasure + ); + + let cubeNameToAttach; + switch (cubeNamesForMeasure.length) { + case 0: + // For zero reference measure there's nothing to derive info about measure from + // So it assume that it's a regular measure, and it will be evaluated on top of join tree + return [measureName, [{ + multiplied: false, + measure: m.measure, + }]]; + case 1: + [cubeNameToAttach] = cubeNamesForMeasure; + break; + default: + throw new Error(`Expected single cube for dimension-only measure ${measureName}, got ${cubeNamesForMeasure}`); + } + + const multiplied = this.multipliedJoinRowResult(cubeNameToAttach) || false; + + const attachedMeasure = { + ...m.measure, + originalCubeName: m.measure.cubeName, + cubeName: cubeNameToAttach + }; + + return [measureName, [{ + multiplied, + measure: attachedMeasure, + }]]; + } + collectRootMeasureToHieararchy(context) { const notAddedMeasureFilters = R.flatten(this.measureFilters.map(f => f.getMembers())) .filter(f => R.none(m => m.measure === f.measure, this.measures)); return R.fromPairs(this.measures.concat(notAddedMeasureFilters).map(m => { - const measureName = typeof m.measure === 'string' ? m.measure : `${m.measure.cubeName}.${m.measure.name}`; - const collectedMeasures = this.collectFrom( [m], this.collectMultipliedMeasures(context), @@ -1725,53 +1772,9 @@ export class BaseQuery { // Measures like this needs a special treatment to attach them to cube and decide if they are multiplied or not // This would return measure object in `measure`, not path // TODO return measure object for every measure - - const memberNamesForMeasure = this.collectFrom( - [m], - this.collectMemberNamesFor.bind(this), - context ? ['collectMemberNamesFor', JSON.stringify(context)] : 'collectMemberNamesFor', - this.queryCache - ); - const cubeNamesForMeasure = R.pipe( - R.map(member => this.memberInstanceByPath(member)), - // collectMemberNamesFor can return both view.dim and cube.dim - R.filter(member => member.definition().ownedByCube), - R.map(member => member.cube().name), - // Single member expression can reference multiple dimensions from same cube - R.uniq, - )( - memberNamesForMeasure - ); - - let cubeNameToAttach; - switch (cubeNamesForMeasure.length) { - case 0: - // For zero reference measure there's nothing to derive info about measure from - // So it assume that it's a regular measure, and it will be evaluated on top of join tree - return [measureName, [{ - multiplied: false, - measure: m.measure, - }]]; - case 1: - [cubeNameToAttach] = cubeNamesForMeasure; - break; - default: - throw new Error(`Expected single cube for dimension-only measure ${measureName}, got ${cubeNamesForMeasure}`); - } - - const multiplied = this.multipliedJoinRowResult(cubeNameToAttach) || false; - - const attachedMeasure = { - ...m.measure, - originalCubeName: m.measure.cubeName, - cubeName: cubeNameToAttach - }; - - return [measureName, [{ - multiplied, - measure: attachedMeasure, - }]]; + return this.dimensionOnlyMeasureToHierarchy(context, m); } + const measureName = typeof m.measure === 'string' ? m.measure : `${m.measure.cubeName}.${m.measure.name}`; return [measureName, collectedMeasures]; })); }