From f9cc8d5e818c1e83cf0391d5b8b03e515055d1f6 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Tue, 17 Sep 2024 23:16:30 +0300 Subject: [PATCH 1/4] =?UTF-8?q?fix(schema-compiler):=20fix=20FILTER=5FPARA?= =?UTF-8?q?MS=20propagation=20from=20view=20to=C2=A0cube's=20SQL=20query?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/adapter/BaseQuery.js | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index 3bb6734aaddef..ede49e5986b7e 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -3716,6 +3716,9 @@ export class BaseQuery { return null; } const filterMembers = BaseQuery.extractFilterMembers(filter); + console.log('filterMembers:', filterMembers, '\ngroupMembers:', groupMembers, '\naliases:', aliases); + console.log('filterMembers.every(m => (groupMembers.indexOf(m) !== -1)):', Object.keys(filterMembers).every(m => (groupMembers.indexOf(m) !== -1))); + console.log('filterMembers.every(m => (groupMembers.indexOf(m) !== -1 || aliases.indexOf(m) !== -1)):', Object.keys(filterMembers).every(m => (groupMembers.indexOf(m) !== -1 || aliases.indexOf(m) !== -1))); if (filterMembers && Object.keys(filterMembers).every(m => (groupMembers.indexOf(m) !== -1 || aliases.indexOf(m) !== -1))) { return filter; } @@ -3758,7 +3761,7 @@ export class BaseQuery { return newGroupFilter({ operator: filter.operator, values }).filterToWhere(); } - const filterParams = filter.filterParams(); + const filterParams = filter && filter.filterParams(); const filterParamArg = filterParamArgs.filter(p => { const member = p.__member(); return member === filter.measure || @@ -3806,11 +3809,15 @@ export class BaseQuery { .map(v => (v.query ? v.query.allBackAliasMembersExceptSegments() : {})) .reduce((a, b) => ({ ...a, ...b }), {}) : {}; + // Filtering aliases that somehow relate to this group members + const aliasesForGroupMembers = Object.entries(aliases) + .filter(([key, value]) => groupMembers.includes(key)) + .map(([_key, value]) => value); const filter = BaseQuery.findAndSubTreeForFilterGroup( newGroupFilter({ operator: 'and', values: allFilters }), groupMembers, newGroupFilter, - Object.values(aliases) + aliasesForGroupMembers ); return `(${BaseQuery.renderFilterParams(filter, filterParamArgs, allocateParam, newGroupFilter, aliases)})`; @@ -3846,15 +3853,16 @@ export class BaseQuery { .map(v => (v.query ? v.query.allBackAliasMembersExceptSegments() : {})) .reduce((a, b) => ({ ...a, ...b }), {}) : {}; - // Filtering aliases that somehow relate to this cube - const filteredAliases = Object.entries(aliases) - .filter(([key, value]) => key.startsWith(cubeNameObj.cube) || value.startsWith(cubeNameObj.cube)) - .reduce((acc, [key, value]) => ({ ...acc, [key]: value }), {}); + // Filtering aliases that somehow relate to this group member + const groupMember = cubeEvaluator.pathFromArray([cubeNameObj.cube, propertyName]); + const aliasesForGroupMembers = Object.entries(aliases) + .filter(([key, _value]) => key === groupMember) + .map(([_key, value]) => value); const filter = BaseQuery.findAndSubTreeForFilterGroup( newGroupFilter({ operator: 'and', values: allFilters }), - [cubeEvaluator.pathFromArray([cubeNameObj.cube, propertyName])], + [groupMember], newGroupFilter, - Object.values(filteredAliases) + aliasesForGroupMembers ); return `(${BaseQuery.renderFilterParams(filter, [this], allocateParam, newGroupFilter, aliases)})`; From 07c01524e1bf77eaf4712a9ddbc224fbc68bcd45 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Tue, 17 Sep 2024 23:17:50 +0300 Subject: [PATCH 2/4] =?UTF-8?q?Add=20tests=20for=C2=A0fix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../test/unit/base-query.test.ts | 226 +++++++++++++++++- .../cubejs-schema-compiler/test/unit/utils.ts | 39 +++ 2 files changed, 262 insertions(+), 3 deletions(-) diff --git a/packages/cubejs-schema-compiler/test/unit/base-query.test.ts b/packages/cubejs-schema-compiler/test/unit/base-query.test.ts index 00f4efa2664c8..270065882a5df 100644 --- a/packages/cubejs-schema-compiler/test/unit/base-query.test.ts +++ b/packages/cubejs-schema-compiler/test/unit/base-query.test.ts @@ -6,7 +6,8 @@ import { createCubeSchemaWithCustomGranularities, createCubeSchemaYaml, createJoinedCubesSchema, - createSchemaYaml + createSchemaYaml, + createSchemaYamlForGroupFilterParamsTests } from './utils'; import { BigqueryQuery } from '../../src/adapter/BigqueryQuery'; @@ -1398,8 +1399,60 @@ describe('SQL Generation', () => { }, ], }); - const cubeSQL = query.cubeSql('Order'); - expect(cubeSQL).toContain('select * from order where ((type = $0$))'); + const queryAndParams = query.buildSqlAndParams(); + const queryString = queryAndParams[0]; + expect(queryString).toContain('select * from order where ((type = ?))'); + }); + + it('propagate filter params within cte from view into cube\'s query', async () => { + /** @type {Compilers} */ + const compiler = prepareYamlCompiler( + createSchemaYaml({ + cubes: [{ + name: 'Order', + sql: `WITH cte as (select * + from order + where {FILTER_PARAMS.Order.type.filter('type')} + ) + select * from cte`, + measures: [{ + name: 'count', + type: 'count', + }], + dimensions: [{ + name: 'type', + sql: 'type', + type: 'string' + }] + }], + views: [{ + name: 'orders_view', + cubes: [{ + join_path: 'Order', + prefix: true, + includes: [ + 'type', + 'count', + ] + }] + }] + }) + ); + + await compiler.compiler.compile(); + const query = new BaseQuery(compiler, { + measures: ['orders_view.Order_count'], + filters: [ + { + member: 'orders_view.Order_type', + operator: 'equals', + values: ['online'], + }, + ], + }); + const queryAndParams = query.buildSqlAndParams(); + const queryString = queryAndParams[0]; + expect(/select\s+\*\s+from\s+order\s+where\s+\(\(type\s=\s\?\)\)/.test(queryString)).toBeTruthy(); }); }); @@ -1527,6 +1580,173 @@ describe('SQL Generation', () => { const cubeSQL = query.cubeSql('Order'); expect(cubeSQL).toContain('where ((((dim0 = $0$) AND (dim1 = $1$)) OR ((dim0 = $2$) AND (dim1 = $3$))))'); }); + + it('propagate 1 filter param from view into cube\'s query', async () => { + /** @type {Compilers} */ + const compiler = prepareYamlCompiler( + createSchemaYamlForGroupFilterParamsTests( + `select * + from order + where {FILTER_GROUP( + FILTER_PARAMS.Order.dim0.filter('dim0') + , FILTER_PARAMS.Order.dim1.filter('dim1') + )}` + ) + ); + + await compiler.compiler.compile(); + const query = new PostgresQuery(compiler, { + measures: ['orders_view.Order_count'], + filters: [ + { + member: 'orders_view.Order_dim0', + operator: 'equals', + values: ['online'], + }, + ], + }); + const queryAndParams = query.buildSqlAndParams(); + const queryString = queryAndParams[0]; + expect(/select\s+\*\s+from\s+order\s+where\s+\(\(dim0\s=\s\$1\)\)/.test(queryString)).toBeTruthy(); + }); + + it('propagate 2 filter params from view into cube\'s query', async () => { + /** @type {Compilers} */ + const compiler = prepareYamlCompiler( + createSchemaYamlForGroupFilterParamsTests( + `select * + from order + where {FILTER_GROUP( + FILTER_PARAMS.Order.dim0.filter('dim0'), + FILTER_PARAMS.Order.dim1.filter('dim1') + )}` + ) + ); + + await compiler.compiler.compile(); + const query = new PostgresQuery(compiler, { + measures: ['orders_view.Order_count'], + filters: [ + { + member: 'orders_view.Order_dim0', + operator: 'equals', + values: ['online'], + }, + { + member: 'orders_view.Order_dim1', + operator: 'equals', + values: ['online'], + }, + ], + }); + const queryAndParams = query.buildSqlAndParams(); + const queryString = queryAndParams[0]; + expect(/select\s+\*\s+from\s+order\s+where\s+\(\(dim0\s=\s\$1\)\s+AND\s+\(dim1\s+=\s+\$2\)\)/.test(queryString)).toBeTruthy(); + }); + + it('propagate 1 filter param within cte from view into cube\'s query', async () => { + /** @type {Compilers} */ + const compiler = prepareYamlCompiler( + createSchemaYamlForGroupFilterParamsTests( + `with cte as ( + select * + from order + where + {FILTER_GROUP( + FILTER_PARAMS.Order.dim0.filter('dim0'), + FILTER_PARAMS.Order.dim1.filter('dim1') + )} + ) + select * from cte` + ) + ); + + await compiler.compiler.compile(); + const query = new PostgresQuery(compiler, { + measures: ['orders_view.Order_count'], + filters: [ + { + member: 'orders_view.Order_dim0', + operator: 'equals', + values: ['online'], + }, + ], + }); + const queryAndParams = query.buildSqlAndParams(); + const queryString = queryAndParams[0]; + expect(/select\s+\*\s+from\s+order\s+where\s+\(\(dim0\s=\s\$1\)\)/.test(queryString)).toBeTruthy(); + }); + + it('propagate 2 filter params within cte from view into cube\'s query', async () => { + /** @type {Compilers} */ + const compiler = prepareYamlCompiler( + createSchemaYamlForGroupFilterParamsTests( + `with cte as ( + select * + from order + where + {FILTER_GROUP( + FILTER_PARAMS.Order.dim0.filter('dim0'), + FILTER_PARAMS.Order.dim1.filter('dim1') + )} + ) + select * from cte` + ) + ); + + await compiler.compiler.compile(); + const query = new PostgresQuery(compiler, { + measures: ['orders_view.Order_count'], + filters: [ + { + member: 'orders_view.Order_dim0', + operator: 'equals', + values: ['online'], + }, + { + member: 'orders_view.Order_dim1', + operator: 'equals', + values: ['online'], + }, + ], + }); + const queryAndParams = query.buildSqlAndParams(); + const queryString = queryAndParams[0]; + expect(/select\s+\*\s+from\s+order\s+where\s+\(\(dim0\s=\s\$1\)\s+AND\s+\(dim1\s+=\s+\$2\)\)/.test(queryString)).toBeTruthy(); + }); + + it('propagate 1 filter param within cte (ref as cube and view dimensions)', async () => { + /** @type {Compilers} */ + const compiler = prepareYamlCompiler( + createSchemaYamlForGroupFilterParamsTests( + `with cte as ( + select * + from order + where + {FILTER_GROUP( + FILTER_PARAMS.Order.dim0.filter('dim0'), + FILTER_PARAMS.orders_view.dim0.filter('dim0') + )} + ) + select * from cte` + ) + ); + + await compiler.compiler.compile(); + const query = new PostgresQuery(compiler, { + measures: ['orders_view.Order_count'], + filters: [ + { + member: 'orders_view.Order_dim0', + operator: 'equals', + values: ['online'], + }, + ], + }); + const queryAndParams = query.buildSqlAndParams(); + const queryString = queryAndParams[0]; + expect(/select\s+\*\s+from\s+order\s+where\s+\(\(dim0\s=\s\$1\)\)/.test(queryString)).toBeTruthy(); + }); }); }); diff --git a/packages/cubejs-schema-compiler/test/unit/utils.ts b/packages/cubejs-schema-compiler/test/unit/utils.ts index 2556e74b34d88..9d5b1189df4d4 100644 --- a/packages/cubejs-schema-compiler/test/unit/utils.ts +++ b/packages/cubejs-schema-compiler/test/unit/utils.ts @@ -224,6 +224,45 @@ export function createSchemaYaml(schema: CreateSchemaOptions): string { return YAML.dump(schema); } +export function createSchemaYamlForGroupFilterParamsTests(cubeDefSql: string): string { + return createSchemaYaml({ + cubes: [ + { + name: 'Order', + sql: cubeDefSql, + measures: [{ + name: 'count', + type: 'count', + }], + dimensions: [ + { + name: 'dim0', + sql: 'dim0', + type: 'string' + }, + { + name: 'dim1', + sql: 'dim1', + type: 'string' + } + ] + }, + ], + views: [{ + name: 'orders_view', + cubes: [{ + join_path: 'Order', + prefix: true, + includes: [ + 'count', + 'dim0', + 'dim1', + ] + }] + }] + }); +} + export function createCubeSchemaYaml({ name, sqlTable }: CreateCubeSchemaOptions): string { return ` cubes: From 68133150abd4c11426d83b04f3618c5ea8e75a4a Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Tue, 17 Sep 2024 23:32:03 +0300 Subject: [PATCH 3/4] Remove console.log --- packages/cubejs-schema-compiler/src/adapter/BaseQuery.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index ede49e5986b7e..1fed5f2264c0e 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -3716,9 +3716,6 @@ export class BaseQuery { return null; } const filterMembers = BaseQuery.extractFilterMembers(filter); - console.log('filterMembers:', filterMembers, '\ngroupMembers:', groupMembers, '\naliases:', aliases); - console.log('filterMembers.every(m => (groupMembers.indexOf(m) !== -1)):', Object.keys(filterMembers).every(m => (groupMembers.indexOf(m) !== -1))); - console.log('filterMembers.every(m => (groupMembers.indexOf(m) !== -1 || aliases.indexOf(m) !== -1)):', Object.keys(filterMembers).every(m => (groupMembers.indexOf(m) !== -1 || aliases.indexOf(m) !== -1))); if (filterMembers && Object.keys(filterMembers).every(m => (groupMembers.indexOf(m) !== -1 || aliases.indexOf(m) !== -1))) { return filter; } From f573cba5ac220d0cf379d3d01751b1610c0c4d7c Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Fri, 20 Sep 2024 12:01:48 +0300 Subject: [PATCH 4/4] small code polish --- packages/cubejs-schema-compiler/src/adapter/BaseQuery.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index 1fed5f2264c0e..5e68f25be6871 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -3758,7 +3758,7 @@ export class BaseQuery { return newGroupFilter({ operator: filter.operator, values }).filterToWhere(); } - const filterParams = filter && filter.filterParams(); + const filterParams = filter.filterParams(); const filterParamArg = filterParamArgs.filter(p => { const member = p.__member(); return member === filter.measure ||