From 22b6d8108195573f854155df1e7ec60cb60d7386 Mon Sep 17 00:00:00 2001 From: Dmitriy Rusov Date: Tue, 29 Oct 2024 14:08:39 +0100 Subject: [PATCH 1/5] dev --- .../cubejs-schema-compiler/src/adapter/PreAggregations.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js b/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js index 4495281416a00..ca930743eafe4 100644 --- a/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js +++ b/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js @@ -1192,7 +1192,7 @@ export class PreAggregations { const targetMeasuresReferences = this.measureAliasesRenderedReference(preAggregationForQuery); const columnsFor = (targetReferences, references, preAggregation) => Object.keys(targetReferences).map( - member => `${references[this.query.cubeEvaluator.pathFromArray([preAggregation.cube, member.split('.')[1]])]} ${targetReferences[member]}` + member => `${references[this.query.cubeEvaluator.pathFromArray([preAggregation.cube, member.replace(/^[^.]*\./, '')])]} ${targetReferences[member]}` ); const tables = preAggregationForQuery.referencedPreAggregations.map(preAggregation => { @@ -1346,7 +1346,7 @@ export class PreAggregations { R.map((td) => { const timeDimension = this.query.newTimeDimension(td); return [ - td.dimension, + `${td.dimension}.${td.granularity}`, this.query.escapeColumnName(timeDimension.unescapedAliasName(rollupGranularity)), ]; }), From 17885c3fbf3007a19d522b77ef0ac9ef55f6fb43 Mon Sep 17 00:00:00 2001 From: Dmitriy Rusov Date: Wed, 30 Oct 2024 09:15:24 +0100 Subject: [PATCH 2/5] test --- .../src/adapter/PreAggregations.js | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js b/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js index ca930743eafe4..2a6d6d71c7596 100644 --- a/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js +++ b/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js @@ -1190,6 +1190,8 @@ export class PreAggregations { const targetDimensionsReferences = this.dimensionsRenderedReference(preAggregationForQuery); const targetTimeDimensionsReferences = this.timeDimensionsRenderedReference(rollupGranularity, preAggregationForQuery); const targetMeasuresReferences = this.measureAliasesRenderedReference(preAggregationForQuery); + // Removing duplicated dimension from targetTimeDimensionsReferences + Object.keys(targetDimensionsReferences).forEach(key => { delete targetTimeDimensionsReferences[key]; }); const columnsFor = (targetReferences, references, preAggregation) => Object.keys(targetReferences).map( member => `${references[this.query.cubeEvaluator.pathFromArray([preAggregation.cube, member.replace(/^[^.]*\./, '')])]} ${targetReferences[member]}` @@ -1275,8 +1277,8 @@ export class PreAggregations { const renderedReference = { ...(this.measuresRenderedReference(preAggregationForQuery)), - ...(this.dimensionsRenderedReference(preAggregationForQuery)), ...(this.timeDimensionsRenderedReference(rollupGranularity, preAggregationForQuery)), + ...(this.dimensionsRenderedReference(preAggregationForQuery)), }; return this.query.evaluateSymbolSqlWithContext( @@ -1342,16 +1344,28 @@ export class PreAggregations { } timeDimensionsRenderedReference(rollupGranularity, preAggregationForQuery) { + const rollupTimeDimensions = this.rollupTimeDimensions(preAggregationForQuery).flatMap(td => { + const timeDimension = this.query.newTimeDimension(td); + return [ + { td, timeDimension }, + { td, timeDimension, withPostfix: true } + ]; + }); + return R.pipe( - R.map((td) => { - const timeDimension = this.query.newTimeDimension(td); + R.map(({ td, timeDimension, withPostfix }) => { + const dimension = [ + td.dimension, + ...(withPostfix ? [td.granularity] : []) + ].join('.'); + return [ - `${td.dimension}.${td.granularity}`, + dimension, this.query.escapeColumnName(timeDimension.unescapedAliasName(rollupGranularity)), ]; }), R.fromPairs, - )(this.rollupTimeDimensions(preAggregationForQuery)); + )(rollupTimeDimensions); } rollupMembers(preAggregationForQuery, type) { From 313376299c808030c437514369b3318bbb36f8f3 Mon Sep 17 00:00:00 2001 From: Dmitriy Rusov Date: Wed, 30 Oct 2024 09:36:04 +0100 Subject: [PATCH 3/5] test --- .../cubejs-schema-compiler/src/adapter/PreAggregations.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js b/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js index 2a6d6d71c7596..3cc3c918d69f2 100644 --- a/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js +++ b/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js @@ -1190,8 +1190,6 @@ export class PreAggregations { const targetDimensionsReferences = this.dimensionsRenderedReference(preAggregationForQuery); const targetTimeDimensionsReferences = this.timeDimensionsRenderedReference(rollupGranularity, preAggregationForQuery); const targetMeasuresReferences = this.measureAliasesRenderedReference(preAggregationForQuery); - // Removing duplicated dimension from targetTimeDimensionsReferences - Object.keys(targetDimensionsReferences).forEach(key => { delete targetTimeDimensionsReferences[key]; }); const columnsFor = (targetReferences, references, preAggregation) => Object.keys(targetReferences).map( member => `${references[this.query.cubeEvaluator.pathFromArray([preAggregation.cube, member.replace(/^[^.]*\./, '')])]} ${targetReferences[member]}` @@ -1211,6 +1209,12 @@ export class PreAggregations { columns: columnsFor(targetDimensionsReferences, dimensionsReferences, preAggregation) .concat(columnsFor(targetTimeDimensionsReferences, timeDimensionsReferences, preAggregation)) .concat(columnsFor(targetMeasuresReferences, measuresReferences, preAggregation)) + .reduce((acc, v) => { + if (!acc.includes(v)) { + acc.push(v); + } + return acc; + }, []), }; }); if (tables.length === 1) { From a234e1f7f28f21ac281483540df19cb3edf0d5ee Mon Sep 17 00:00:00 2001 From: Dmitriy Rusov Date: Wed, 30 Oct 2024 10:18:15 +0100 Subject: [PATCH 4/5] add comments --- .../cubejs-schema-compiler/src/adapter/PreAggregations.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js b/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js index 3cc3c918d69f2..1e63fb7bccf0d 100644 --- a/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js +++ b/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js @@ -1209,6 +1209,8 @@ export class PreAggregations { columns: columnsFor(targetDimensionsReferences, dimensionsReferences, preAggregation) .concat(columnsFor(targetTimeDimensionsReferences, timeDimensionsReferences, preAggregation)) .concat(columnsFor(targetMeasuresReferences, measuresReferences, preAggregation)) + // Deduplicate columns, for case when targetTimeDimensionsReferences has the same column as targetDimensionsReferences + // Concat order is important, targetDimensionsReferences should override targetTimeDimensionsReferences .reduce((acc, v) => { if (!acc.includes(v)) { acc.push(v); @@ -1281,6 +1283,7 @@ export class PreAggregations { const renderedReference = { ...(this.measuresRenderedReference(preAggregationForQuery)), + // Merge order is important, dimensionsRenderedReference must override timeDimensionsRenderedReference ...(this.timeDimensionsRenderedReference(rollupGranularity, preAggregationForQuery)), ...(this.dimensionsRenderedReference(preAggregationForQuery)), }; @@ -1348,6 +1351,9 @@ export class PreAggregations { } timeDimensionsRenderedReference(rollupGranularity, preAggregationForQuery) { + // We have to support 2 time dimensions references + // In the case where the original time column was added as a dimension in the rollup - the engine will use the original value overridden using dimension references + // Otherwise, the cube will use a truncated time representation, but with the original column name const rollupTimeDimensions = this.rollupTimeDimensions(preAggregationForQuery).flatMap(td => { const timeDimension = this.query.newTimeDimension(td); return [ From 7811ed2c077b4093de6f0ac094910fdf4e24aedc Mon Sep 17 00:00:00 2001 From: Dmitriy Rusov Date: Wed, 30 Oct 2024 10:18:47 +0100 Subject: [PATCH 5/5] clear --- packages/cubejs-schema-compiler/src/adapter/PreAggregations.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js b/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js index 1e63fb7bccf0d..d14a2ed9ff218 100644 --- a/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js +++ b/packages/cubejs-schema-compiler/src/adapter/PreAggregations.js @@ -1210,7 +1210,7 @@ export class PreAggregations { .concat(columnsFor(targetTimeDimensionsReferences, timeDimensionsReferences, preAggregation)) .concat(columnsFor(targetMeasuresReferences, measuresReferences, preAggregation)) // Deduplicate columns, for case when targetTimeDimensionsReferences has the same column as targetDimensionsReferences - // Concat order is important, targetDimensionsReferences should override targetTimeDimensionsReferences + // Concat order is important, targetDimensionsReferences must override targetTimeDimensionsReferences .reduce((acc, v) => { if (!acc.includes(v)) { acc.push(v);