From f5022c4eca57c5487a4f49a05f451665328241a1 Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Wed, 9 Apr 2025 10:46:11 +0200 Subject: [PATCH 1/4] test(schema-compiler): Use actual view in pre-aggregations tests --- .../postgres/pre-aggregations.test.ts | 70 +++++++------------ 1 file changed, 24 insertions(+), 46 deletions(-) diff --git a/packages/cubejs-schema-compiler/test/integration/postgres/pre-aggregations.test.ts b/packages/cubejs-schema-compiler/test/integration/postgres/pre-aggregations.test.ts index b3d191d8f5066..0964d74d6e636 100644 --- a/packages/cubejs-schema-compiler/test/integration/postgres/pre-aggregations.test.ts +++ b/packages/cubejs-schema-compiler/test/integration/postgres/pre-aggregations.test.ts @@ -7,6 +7,7 @@ import { dbRunner } from './PostgresDBRunner'; describe('PreAggregations', () => { jest.setTimeout(200000); + // language=JavaScript const { compiler, joinGraph, cubeEvaluator } = prepareCompiler(` cube(\`visitors\`, { sql: \` @@ -444,29 +445,6 @@ describe('PreAggregations', () => { } }) - cube('VisitorView', { - sql: \`SELECT 1\`, - - measures: { - checkinsTotal: { - sql: \`\${visitors.checkinsTotal}\`, - type: 'number', - } - }, - - dimensions: { - source: { - sql: \`\${visitors.source}\`, - type: 'string', - }, - - createdAt: { - sql: \`\${visitors.createdAt}\`, - type: 'time' - } - }, - }); - cube('LambdaVisitors', { extends: visitors, @@ -2084,20 +2062,20 @@ describe('PreAggregations', () => { it('simple view', () => compiler.compile().then(() => { const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, { measures: [ - 'VisitorView.checkinsTotal' + 'visitors_view.checkinsTotal' ], dimensions: [ - 'VisitorView.source' + 'visitors_view.source' ], timezone: 'America/Los_Angeles', preAggregationsSchema: '', timeDimensions: [{ - dimension: 'VisitorView.createdAt', + dimension: 'visitors_view.createdAt', granularity: 'day', dateRange: ['2016-12-30', '2017-01-05'] }], order: [{ - id: 'VisitorView.createdAt' + id: 'visitors_view.createdAt' }], }); @@ -2117,19 +2095,19 @@ describe('PreAggregations', () => { expect(res).toEqual( [ { - visitor_view__source: 'some', - visitor_view__created_at_day: '2017-01-02T00:00:00.000Z', - visitor_view__checkins_total: '3' + visitors_view__source: 'some', + visitors_view__created_at_day: '2017-01-02T00:00:00.000Z', + visitors_view__checkins_total: '3' }, { - visitor_view__source: 'some', - visitor_view__created_at_day: '2017-01-04T00:00:00.000Z', - visitor_view__checkins_total: '2' + visitors_view__source: 'some', + visitors_view__created_at_day: '2017-01-04T00:00:00.000Z', + visitors_view__checkins_total: '2' }, { - visitor_view__source: 'google', - visitor_view__created_at_day: '2017-01-05T00:00:00.000Z', - visitor_view__checkins_total: '1' + visitors_view__source: 'google', + visitors_view__created_at_day: '2017-01-05T00:00:00.000Z', + visitors_view__checkins_total: '1' } ] ); @@ -2139,20 +2117,20 @@ describe('PreAggregations', () => { it('simple view non matching time-dimension granularity', () => compiler.compile().then(() => { const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, { measures: [ - 'VisitorView.checkinsTotal' + 'visitors_view.checkinsTotal' ], dimensions: [ - 'VisitorView.source' + 'visitors_view.source' ], timezone: 'America/Los_Angeles', preAggregationsSchema: '', timeDimensions: [{ - dimension: 'VisitorView.createdAt', + dimension: 'visitors_view.createdAt', granularity: 'month', dateRange: ['2016-12-30', '2017-01-05'] }], order: [{ - id: 'VisitorView.createdAt' + id: 'visitors_view.createdAt' }], }); @@ -2172,14 +2150,14 @@ describe('PreAggregations', () => { expect(res).toEqual( [ { - visitor_view__source: 'google', - visitor_view__created_at_month: '2017-01-01T00:00:00.000Z', - visitor_view__checkins_total: '1' + visitors_view__source: 'google', + visitors_view__created_at_month: '2017-01-01T00:00:00.000Z', + visitors_view__checkins_total: '1' }, { - visitor_view__source: 'some', - visitor_view__created_at_month: '2017-01-01T00:00:00.000Z', - visitor_view__checkins_total: '5' + visitors_view__source: 'some', + visitors_view__created_at_month: '2017-01-01T00:00:00.000Z', + visitors_view__checkins_total: '5' } ] ); From e98bb6c9f0961832097f122e014c8357e81dca6a Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Wed, 9 Apr 2025 10:48:57 +0200 Subject: [PATCH 2/4] fix(schema-compiler): Register split views in cubeDefinitions --- packages/cubejs-schema-compiler/src/compiler/CubeSymbols.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/cubejs-schema-compiler/src/compiler/CubeSymbols.ts b/packages/cubejs-schema-compiler/src/compiler/CubeSymbols.ts index dffa59b9e017c..b6dc024e772a4 100644 --- a/packages/cubejs-schema-compiler/src/compiler/CubeSymbols.ts +++ b/packages/cubejs-schema-compiler/src/compiler/CubeSymbols.ts @@ -91,6 +91,7 @@ export class CubeSymbols { // TODO can we define it when cubeList is defined? this.cubeList.push(splitViews[viewName]); this.symbols[viewName] = splitViews[viewName]; + this.cubeDefinitions[viewName] = splitViews[viewName]; } } } From 038f19d890c6bf2a2151826232a375a1f7680931 Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Tue, 8 Apr 2025 13:39:37 +0200 Subject: [PATCH 3/4] 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 --- .../src/adapter/BaseQuery.js | 2 +- .../view-and-indirect-members.test.ts | 282 ++++++++++++++++++ 2 files changed, 283 insertions(+), 1 deletion(-) create mode 100644 packages/cubejs-schema-compiler/test/integration/postgres/view-and-indirect-members.test.ts diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index 1333d9d4dd10a..c83c50a50fa38 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -2501,7 +2501,7 @@ export class BaseQuery { pushMemberNameForCollectionIfNecessary(cubeName, name) { const pathFromArray = this.cubeEvaluator.pathFromArray([cubeName, name]); - if (this.cubeEvaluator.byPathAnyType(pathFromArray).ownedByCube) { + if (!this.cubeEvaluator.getCubeDefinition(cubeName).isView) { const joinHints = this.cubeEvaluator.joinHints(); if (joinHints && joinHints.length) { joinHints.forEach(cube => this.pushCubeNameForCollectionIfNecessary(cube)); diff --git a/packages/cubejs-schema-compiler/test/integration/postgres/view-and-indirect-members.test.ts b/packages/cubejs-schema-compiler/test/integration/postgres/view-and-indirect-members.test.ts new file mode 100644 index 0000000000000..361c659fe3f08 --- /dev/null +++ b/packages/cubejs-schema-compiler/test/integration/postgres/view-and-indirect-members.test.ts @@ -0,0 +1,282 @@ +import { PostgresQuery } from '../../../src/adapter/PostgresQuery'; +import { prepareCompiler } from '../../unit/PrepareCompiler'; +import { DataSchemaCompiler } from '../../../src/compiler/DataSchemaCompiler'; +import { JoinGraph } from '../../../src/compiler/JoinGraph'; +import { CubeEvaluator } from '../../../src/compiler/CubeEvaluator'; + +describe('View and indirect members', () => { + jest.setTimeout(200000); + + let compiler: DataSchemaCompiler; + let joinGraph: JoinGraph; + let cubeEvaluator: CubeEvaluator; + + beforeAll(async () => { + // All joins would look like this + // A-->B-->C-->X + // | ^ + // ├-->D-->E---┤ + // | | + // └-->F-------┘ + // View would use ADEX path + // It should NOT be the shortest one, nor first in join edges declaration + // All join conditions would be essentially `FALSE`, but with different syntax, to be able to test SQL generation + + // TODO in this model queries like [A.a_id, X.x_id] become ambiguous, probably we want to handle this better + + // language=JavaScript + const prepared = prepareCompiler(` + cube('A', { + sql: 'SELECT 1 AS a_id, 100 AS a_value', + + joins: { + B: { + relationship: 'many_to_one', + sql: "'A' = 'B'", + }, + D: { + relationship: 'many_to_one', + sql: "'A' = 'D'", + }, + F: { + relationship: 'many_to_one', + sql: "'A' = 'F'", + }, + }, + + dimensions: { + a_id: { + type: 'number', + sql: 'a_id', + primaryKey: true, + }, + }, + + measures: { + a_sum: { + sql: 'a_value', + type: 'sum', + }, + }, + }); + + cube('B', { + sql: 'SELECT 1 AS b_id, 100 AS b_value', + + joins: { + C: { + relationship: 'many_to_one', + sql: "'B' = 'C'", + }, + }, + + dimensions: { + b_id: { + type: 'number', + sql: 'b_id', + primaryKey: true, + }, + }, + + measures: { + b_sum: { + sql: 'b_value', + type: 'sum', + }, + }, + }); + + cube('C', { + sql: 'SELECT 1 AS c_id, 100 AS c_value', + + joins: { + X: { + relationship: 'many_to_one', + sql: "'C' = 'X'", + }, + }, + + dimensions: { + c_id: { + type: 'number', + sql: 'c_id', + primaryKey: true, + }, + }, + + measures: { + c_sum: { + sql: 'c_value', + type: 'sum', + }, + }, + }); + + cube('D', { + sql: 'SELECT 1 AS d_id, 100 AS d_value', + + joins: { + E: { + relationship: 'many_to_one', + sql: "'D' = 'E'", + }, + }, + + dimensions: { + d_id: { + type: 'number', + sql: 'd_id', + primaryKey: true, + }, + }, + + measures: { + d_sum: { + sql: 'd_value', + type: 'sum', + }, + }, + }); + + cube('E', { + sql: 'SELECT 1 AS e_id, 100 AS e_value', + + joins: { + X: { + relationship: 'many_to_one', + sql: "'E' = 'X'", + }, + }, + + dimensions: { + e_id: { + type: 'number', + sql: 'e_id', + primaryKey: true, + }, + }, + + measures: { + e_sum: { + sql: 'e_value', + type: 'sum', + }, + }, + }); + + cube('F', { + sql: 'SELECT 1 AS f_id, 100 AS f_value', + + joins: { + X: { + relationship: 'many_to_one', + sql: "'F' = 'X'", + }, + }, + + dimensions: { + f_id: { + type: 'number', + sql: 'f_id', + primaryKey: true, + }, + }, + + measures: { + f_sum: { + sql: 'f_value', + type: 'sum', + }, + }, + }); + + cube('X', { + sql: 'SELECT 1 AS x_id, 100 AS x_value', + + dimensions: { + x_id: { + type: 'number', + sql: 'x_id', + primaryKey: true, + }, + // This member should be: + // * NOT ownedByCube + // * reference only members of same cube + // * included in view + x_id_ref: { + type: 'number', + sql: \`\${x_id} + 1\`, + }, + }, + + measures: { + x_sum: { + sql: 'x_value', + type: 'sum', + }, + }, + }); + + view('ADEX_view', { + cubes: [ + { + join_path: A, + includes: [ + 'a_id', + ], + prefix: false + }, + { + join_path: A.D, + includes: [ + 'd_id', + ], + prefix: false + }, + { + join_path: A.D.E, + includes: [ + 'e_id', + ], + prefix: false + }, + { + join_path: A.D.E.X, + includes: [ + 'x_id', + 'x_id_ref', + ], + prefix: false + }, + ] + }); + `); + + ({ compiler, joinGraph, cubeEvaluator } = prepared); + }); + + beforeEach(async () => { + await compiler.compile(); + }); + + it('should respect join path from view declaration', async () => { + const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: [], + dimensions: [ + 'ADEX_view.a_id', + 'ADEX_view.x_id_ref', + ], + }); + + const [sql, _params] = query.buildSqlAndParams(); + + expect(sql).toMatch(/ON 'A' = 'D'/); + expect(sql).toMatch(/ON 'D' = 'E'/); + expect(sql).toMatch(/ON 'E' = 'X'/); + expect(sql).not.toMatch(/ON 'A' = 'B'/); + expect(sql).not.toMatch(/ON 'B' = 'C'/); + expect(sql).not.toMatch(/ON 'C' = 'X'/); + expect(sql).not.toMatch(/ON 'A' = 'F'/); + expect(sql).not.toMatch(/ON 'F' = 'X'/); + }); +}); From 16f97defce476ce59bfcd4de580581c20e111c7b Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Thu, 10 Apr 2025 19:46:52 +0200 Subject: [PATCH 4/4] refactor(schema-compiler): Rename tests for multiple join paths --- ...rs.test.ts => multiple-join-paths.test.ts} | 57 ++++++++----------- 1 file changed, 23 insertions(+), 34 deletions(-) rename packages/cubejs-schema-compiler/test/integration/postgres/{view-and-indirect-members.test.ts => multiple-join-paths.test.ts} (82%) diff --git a/packages/cubejs-schema-compiler/test/integration/postgres/view-and-indirect-members.test.ts b/packages/cubejs-schema-compiler/test/integration/postgres/multiple-join-paths.test.ts similarity index 82% rename from packages/cubejs-schema-compiler/test/integration/postgres/view-and-indirect-members.test.ts rename to packages/cubejs-schema-compiler/test/integration/postgres/multiple-join-paths.test.ts index 361c659fe3f08..383184054a124 100644 --- a/packages/cubejs-schema-compiler/test/integration/postgres/view-and-indirect-members.test.ts +++ b/packages/cubejs-schema-compiler/test/integration/postgres/multiple-join-paths.test.ts @@ -4,7 +4,7 @@ import { DataSchemaCompiler } from '../../../src/compiler/DataSchemaCompiler'; import { JoinGraph } from '../../../src/compiler/JoinGraph'; import { CubeEvaluator } from '../../../src/compiler/CubeEvaluator'; -describe('View and indirect members', () => { +describe('Multiple join paths', () => { jest.setTimeout(200000); let compiler: DataSchemaCompiler; @@ -18,9 +18,10 @@ describe('View and indirect members', () => { // ├-->D-->E---┤ // | | // └-->F-------┘ - // View would use ADEX path - // It should NOT be the shortest one, nor first in join edges declaration + // View, pre-aggregations and all interesting parts should use ADEX path + // It should NOT be the shortest one from A to X (that's AFX), nor first in join edges declaration (that's ABCX) // All join conditions would be essentially `FALSE`, but with different syntax, to be able to test SQL generation + // Also, there should be only one way to cover cubes A and D with joins: A->D join // TODO in this model queries like [A.a_id, X.x_id] become ambiguous, probably we want to handle this better @@ -226,20 +227,6 @@ describe('View and indirect members', () => { ], prefix: false }, - { - join_path: A.D, - includes: [ - 'd_id', - ], - prefix: false - }, - { - join_path: A.D.E, - includes: [ - 'e_id', - ], - prefix: false - }, { join_path: A.D.E.X, includes: [ @@ -259,24 +246,26 @@ describe('View and indirect members', () => { await compiler.compile(); }); - it('should respect join path from view declaration', async () => { - const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, { - measures: [], - dimensions: [ - 'ADEX_view.a_id', - 'ADEX_view.x_id_ref', - ], - }); + describe('View and indirect members', () => { + it('should respect join path from view declaration', async () => { + const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: [], + dimensions: [ + 'ADEX_view.a_id', + 'ADEX_view.x_id_ref', + ], + }); - const [sql, _params] = query.buildSqlAndParams(); + const [sql, _params] = query.buildSqlAndParams(); - expect(sql).toMatch(/ON 'A' = 'D'/); - expect(sql).toMatch(/ON 'D' = 'E'/); - expect(sql).toMatch(/ON 'E' = 'X'/); - expect(sql).not.toMatch(/ON 'A' = 'B'/); - expect(sql).not.toMatch(/ON 'B' = 'C'/); - expect(sql).not.toMatch(/ON 'C' = 'X'/); - expect(sql).not.toMatch(/ON 'A' = 'F'/); - expect(sql).not.toMatch(/ON 'F' = 'X'/); + expect(sql).toMatch(/ON 'A' = 'D'/); + expect(sql).toMatch(/ON 'D' = 'E'/); + expect(sql).toMatch(/ON 'E' = 'X'/); + expect(sql).not.toMatch(/ON 'A' = 'B'/); + expect(sql).not.toMatch(/ON 'B' = 'C'/); + expect(sql).not.toMatch(/ON 'C' = 'X'/); + expect(sql).not.toMatch(/ON 'A' = 'F'/); + expect(sql).not.toMatch(/ON 'F' = 'X'/); + }); }); });