From efc57452af44ee31092b8dfbb33a7ba23e86bba5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C3=ADas=20Aguirre?= Date: Tue, 31 Aug 2021 00:29:46 -0300 Subject: [PATCH] feat: Support multi-value filtering on same column through FILTER_PARAMS (#2854) Thanks to @omab! * Support multi-value filtering on same column * Test multivalue filter on same column * Simplify filter length check --- .../src/adapter/BaseQuery.js | 38 +++++++------ .../postgres/sql-generation.test.ts | 53 +++++++++++++++++++ 2 files changed, 75 insertions(+), 16 deletions(-) diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index 3b187dabbc17..d7f9de72eaf5 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -2405,26 +2405,32 @@ export class BaseQuery { const cubeName = this.cubeEvaluator.cubeNameFromPath(name); return new Proxy({ cube: cubeName }, { get: (cubeNameObj, propertyName) => { - const filter = - allFilters.find(f => f.dimension === this.cubeEvaluator.pathFromArray([cubeNameObj.cube, propertyName])); + const filters = + allFilters.filter(f => f.dimension === this.cubeEvaluator.pathFromArray([cubeNameObj.cube, propertyName])); return { filter: (column) => { - const filterParams = filter && filter.filterParams(); - if ( - filterParams && filterParams.length - ) { - if (typeof column === 'function') { - // eslint-disable-next-line prefer-spread - return column.apply( - null, - filterParams.map(this.paramAllocator.allocateParam.bind(this.paramAllocator)) - ); - } else { - return filter.conditionSql(column); - } - } else { + if (!filters.length) { return '1 = 1'; } + + return filters.map(filter => { + const filterParams = filter && filter.filterParams(); + if ( + filterParams && filterParams.length + ) { + if (typeof column === 'function') { + // eslint-disable-next-line prefer-spread + return column.apply( + null, + filterParams.map(this.paramAllocator.allocateParam.bind(this.paramAllocator)) + ); + } else { + return filter.conditionSql(column); + } + } else { + return '1 = 1'; + } + }).join(' AND '); } }; } diff --git a/packages/cubejs-schema-compiler/test/integration/postgres/sql-generation.test.ts b/packages/cubejs-schema-compiler/test/integration/postgres/sql-generation.test.ts index cbfdf891871d..6432b9acb394 100644 --- a/packages/cubejs-schema-compiler/test/integration/postgres/sql-generation.test.ts +++ b/packages/cubejs-schema-compiler/test/integration/postgres/sql-generation.test.ts @@ -240,6 +240,33 @@ describe('SQL Generation', () => { } }) + cube('visitor_checkins_sources', { + sql: \` + select id, source from visitor_checkins WHERE \${FILTER_PARAMS.visitor_checkins_sources.source.filter('source')} + \`, + + rewriteQueries: true, + + joins: { + cards: { + relationship: 'hasMany', + sql: \`\${CUBE}.id = \${cards}.visitor_checkin_id\` + } + }, + + dimensions: { + id: { + type: 'number', + sql: 'id', + primaryKey: true + }, + source: { + type: 'string', + sql: 'source' + } + } + }) + cube('cards', { sql: \` select * from cards @@ -1340,6 +1367,32 @@ describe('SQL Generation', () => { ]) ); + it( + 'contains multiple value filter', + () => runQueryTest({ + measures: [], + dimensions: [ + 'visitor_checkins_sources.source' + ], + timeDimensions: [], + timezone: 'America/Los_Angeles', + filters: [{ + dimension: 'visitor_checkins_sources.source', + operator: 'contains', + values: ['goo'] + }, { + dimension: 'visitor_checkins_sources.source', + operator: 'contains', + values: ['gle'] + }], + order: [{ + id: 'visitor_checkins_sources.source' + }] + }, [ + { visitor_checkins_sources__source: 'google' } + ]) + ); + it( 'contains null filter', () => runQueryTest({