From 5ca76dbd66c8b0227d3107a1b1c3f1f9316dbd86 Mon Sep 17 00:00:00 2001 From: Pavel Tiunov Date: Sun, 25 Aug 2024 16:47:41 -0700 Subject: [PATCH] fix: View builds incorrect join graph if join paths partially shared (#8632) Thanks @barakcoh for the fix hint! Fixes #8499 --- .../src/compiler/JoinGraph.js | 1 + .../postgres/views-join-order-2.test.ts | 173 ++++++++++++++++++ .../postgres/views-join-order.test.ts | 10 +- 3 files changed, 179 insertions(+), 5 deletions(-) create mode 100644 packages/cubejs-schema-compiler/test/integration/postgres/views-join-order-2.test.ts diff --git a/packages/cubejs-schema-compiler/src/compiler/JoinGraph.js b/packages/cubejs-schema-compiler/src/compiler/JoinGraph.js index 2f855fd97e987..179ca2ac5ceba 100644 --- a/packages/cubejs-schema-compiler/src/compiler/JoinGraph.js +++ b/packages/cubejs-schema-compiler/src/compiler/JoinGraph.js @@ -145,6 +145,7 @@ export class JoinGraph { let prevNode = root; return joinHints.filter(toJoin => toJoin !== prevNode).map(toJoin => { if (nodesJoined[toJoin]) { + prevNode = toJoin; return { joins: [] }; } const path = this.graph.path(prevNode, toJoin); diff --git a/packages/cubejs-schema-compiler/test/integration/postgres/views-join-order-2.test.ts b/packages/cubejs-schema-compiler/test/integration/postgres/views-join-order-2.test.ts new file mode 100644 index 0000000000000..d69dff1689897 --- /dev/null +++ b/packages/cubejs-schema-compiler/test/integration/postgres/views-join-order-2.test.ts @@ -0,0 +1,173 @@ +import { prepareCompiler } from '../../unit/PrepareCompiler'; +import { dbRunner } from './PostgresDBRunner'; + +describe('Views Join Order 2', () => { + jest.setTimeout(200000); + + const { compiler, joinGraph, cubeEvaluator } = prepareCompiler(` +view(\`View\`, { + description: 'A view', + cubes: [ + { + join_path: A, + includes: ['id', 'name'], + prefix: true, + }, + { + join_path: A.D, + includes: ['id', 'name'], + prefix: true, + }, + { + join_path: A.D.B, + includes: ['id', 'name'], + prefix: true, + }, + { + join_path: A.D.E, + includes: ['id', 'name'], + prefix: true, + }, + ], +}); + +cube('A', { + sql: \` + SELECT 1 id, 'a'::text as "name"\`, + joins: { + B: { + relationship: \`one_to_many\`, + sql: \`\${CUBE.id} = \${B}.fk\`, + }, + D: { + relationship: \`one_to_many\`, + sql: \`\${CUBE.id} = \${D}.fk\`, + }, + }, + dimensions: { + id: { + sql: \`id\`, + type: \`string\`, + primaryKey: true, + }, + name: { + sql: \`\${CUBE}."name"\`, + type: \`string\`, + }, + }, +}); + +cube('B', { + sql: \` + SELECT 2 id, 'b'::text as "name"\`, + joins: { + A: { + relationship: \`many_to_one\`, + sql: \`\${CUBE.fk} = \${A.id}\`, + }, + E: { + sql: \`\${CUBE.name} = \${E.id}\`, + relationship: \`many_to_one\`, + }, + }, + + dimensions: { + id: { + sql: \`id\`, + type: \`string\`, + primaryKey: true, + }, + name: { + sql: \`\${CUBE}."name"\`, + type: \`string\`, + }, + fk: { + sql: \`fk\`, + type: \`string\`, + }, + }, +}); + +cube('E', { + sql: \` + SELECT 4 id, 'e'::text as "name"\`, + dimensions: { + id: { + sql: \`id\`, + type: \`string\`, + primaryKey: true, + }, + name: { + sql: \`\${CUBE}."name"\`, + type: \`string\`, + }, + }, +}); + +cube('D', { + sql: \` + SELECT 3 id, 'd'::text as "name", 1 fk, 2 b_fk, 4 e_fk\`, + joins: { + B: { + relationship: \`one_to_one\`, + sql: \`\${CUBE}.b_fk = \${B}.id\`, + }, + A: { + relationship: \`many_to_one\`, + sql: \`\${CUBE}.fk = \${A}.id\`, + }, + E: { + relationship: \`many_to_one\`, + sql: \`\${CUBE}.e_fk = \${E}.id\`, + }, + }, + dimensions: { + id: { + sql: \`id\`, + type: \`string\`, + primaryKey: true, + }, + name: { + sql: \`\${CUBE}."name"\`, + type: \`string\`, + }, + fk: { + sql: \`fk\`, + type: \`string\`, + }, + bFk: { + sql: \`b_fk\`, + type: \`string\`, + }, + }, +}); + `); + + it('join order', async () => dbRunner.runQueryTest({ + dimensions: [ + 'View.A_id', + 'View.A_name', + 'View.B_id', + 'View.B_name', + 'View.D_id', + 'View.D_name', + 'View.E_id', + 'View.E_name' + ], + timeDimensions: [], + segments: [], + filters: [], + total: true, + renewQuery: false, + limit: 1 + }, [{ + view___a_id: 1, + view___a_name: 'a', + view___b_id: 2, + view___b_name: 'b', + view___d_id: 3, + view___d_name: 'd', + view___e_id: 4, + view___e_name: 'e', + }], { compiler, joinGraph, cubeEvaluator })); +}); diff --git a/packages/cubejs-schema-compiler/test/integration/postgres/views-join-order.test.ts b/packages/cubejs-schema-compiler/test/integration/postgres/views-join-order.test.ts index 10c84e6f5f455..e4e099aaee9a4 100644 --- a/packages/cubejs-schema-compiler/test/integration/postgres/views-join-order.test.ts +++ b/packages/cubejs-schema-compiler/test/integration/postgres/views-join-order.test.ts @@ -34,7 +34,7 @@ cube(\`fact\`, { }); cube(\`product\`, { - sql: \`SELECT 1 as id_product, 1 as id_sub_category, 1 as id_model, 'Product' as description\`, + sql: \`SELECT 1 as id_product, 1 as id_sub_category, 1 as id_model, 'Product'::text as description\`, dimensions: { id_product: { sql: 'id_product', @@ -59,7 +59,7 @@ cube(\`product\`, { }); cube(\`model\`, { - sql: \`SELECT 1 as id_model, 1 as id_brand, 'Model' as description\`, + sql: \`SELECT 1 as id_model, 1 as id_brand, 'Model'::text as description\`, dimensions: { id_model: { sql: 'id_model', @@ -80,7 +80,7 @@ cube(\`model\`, { }); cube(\`brand\`, { - sql: \`SELECT 1 as id_brand, 'Brand' as description\`, + sql: \`SELECT 1 as id_brand, 'Brand'::text as description\`, dimensions: { id_brand: { sql: 'id_brand', @@ -95,7 +95,7 @@ cube(\`brand\`, { }); cube(\`sub_category\`, { - sql: \`SELECT 1 as id_sub_category, 1 as id_category, 'Sub Category' as description\`, + sql: \`SELECT 1 as id_sub_category, 1 as id_category, 'Sub Category'::text as description\`, dimensions: { id_sub_category: { sql: 'id_sub_category', @@ -116,7 +116,7 @@ cube(\`sub_category\`, { }); cube(\`category\`, { - sql: \`SELECT 1 as id_category, 'Category' as description\`, + sql: \`SELECT 1 as id_category, 'Category'::text as description\`, dimensions: { id_category: { sql: 'id_category',