From a53bf951c07ffa16e79e993ed578d5c67185b8a5 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Tue, 21 Jan 2025 12:40:44 +0200 Subject: [PATCH 01/10] =?UTF-8?q?just=20typos=20in=C2=A0comments=20and?= =?UTF-8?q?=C2=A0moving=20code=20around?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/adapter/BaseQuery.js | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index 6f354fd3e589f..da47995769df9 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -945,8 +945,8 @@ export class BaseQuery { ).concat(multiStageMembers.map(m => `SELECT * FROM ${m.alias}`)); } - // Move regular measures to multiplied ones if there're same - // cubes to calculate. Most of the times it'll be much faster to + // Move regular measures to multiplied ones if there are same + // cubes to calculate. Most of the time it'll be much faster to // calculate as there will be only single scan per cube. if ( regularMeasures.length && @@ -1499,6 +1499,24 @@ export class BaseQuery { ); } + overTimeSeriesSelect(cumulativeMeasures, dateSeriesSql, baseQuery, dateJoinConditionSql, baseQueryAlias) { + const forSelect = this.overTimeSeriesForSelect(cumulativeMeasures); + return `SELECT ${forSelect} FROM ${dateSeriesSql}` + + ` LEFT JOIN (${baseQuery}) ${this.asSyntaxJoin} ${baseQueryAlias} ON ${dateJoinConditionSql}` + + this.groupByClause(); + } + + overTimeSeriesForSelect(cumulativeMeasures) { + return this.dimensions.map(s => s.cumulativeSelectColumns()).concat(this.dateSeriesSelect()).concat( + cumulativeMeasures.map(s => s.cumulativeSelectColumns()), + ).filter(c => !!c) + .join(', '); + } + + dateSeriesSelect() { + return this.timeDimensions.map(d => d.dateSeriesSelectColumn()); + } + dateFromStartToEndConditionSql(dateJoinCondition, fromRollup, isFromStartToEnd) { return dateJoinCondition.map( // TODO these weird conversions to be strict typed for big query. @@ -1523,24 +1541,6 @@ export class BaseQuery { ); } - overTimeSeriesSelect(cumulativeMeasures, dateSeriesSql, baseQuery, dateJoinConditionSql, baseQueryAlias) { - const forSelect = this.overTimeSeriesForSelect(cumulativeMeasures); - return `SELECT ${forSelect} FROM ${dateSeriesSql}` + - ` LEFT JOIN (${baseQuery}) ${this.asSyntaxJoin} ${baseQueryAlias} ON ${dateJoinConditionSql}` + - this.groupByClause(); - } - - overTimeSeriesForSelect(cumulativeMeasures) { - return this.dimensions.map(s => s.cumulativeSelectColumns()).concat(this.dateSeriesSelect()).concat( - cumulativeMeasures.map(s => s.cumulativeSelectColumns()), - ).filter(c => !!c) - .join(', '); - } - - dateSeriesSelect() { - return this.timeDimensions.map(d => d.dateSeriesSelectColumn()); - } - /** * @param {import('./BaseTimeDimension').BaseTimeDimension} timeDimension * @return {string} From d7818bcc2ee89811791765008e891887f098536b Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Wed, 22 Jan 2025 13:48:05 +0200 Subject: [PATCH 02/10] =?UTF-8?q?fill=20missing=20dateRanges=20for=C2=A0TD?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/cubejs-api-gateway/src/gateway.ts | 23 ++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/cubejs-api-gateway/src/gateway.ts b/packages/cubejs-api-gateway/src/gateway.ts index af90986b4117b..ccf57e0ac409a 100644 --- a/packages/cubejs-api-gateway/src/gateway.ts +++ b/packages/cubejs-api-gateway/src/gateway.ts @@ -1171,6 +1171,7 @@ class ApiGateway { let queryType: QueryType = QueryTypeEnum.REGULAR_QUERY; if (!Array.isArray(query)) { query = this.compareDateRangeTransformer(query); + query = this.fillMissingDateRangesForTimeDimensions(query); if (Array.isArray(query)) { queryType = QueryTypeEnum.COMPARE_DATE_RANGE_QUERY; } @@ -2485,6 +2486,28 @@ class ApiGateway { } }; + protected fillMissingDateRangesForTimeDimensions(query) { + if (!query.timeDimensions) { + return query; + } + + const timeDimensionCount = query.timeDimensions.reduce((acc, item) => { + acc[item.dimension] = acc[item.dimension] || {}; + if (!acc[item.dimension].dateRange && item.dateRange) { + acc[item.dimension].dateRange = item.dateRange; + } + return acc; + }, {}); + + query.timeDimensions.forEach(td => { + if (!td.dateRange && timeDimensionCount[td.dimension].dateRange) { + td.dateRange = timeDimensionCount[td.dimension].dateRange; + } + }); + + return query; + } + protected compareDateRangeTransformer(query) { let queryCompareDateRange; let compareDateRangeTDIndex; From 30ab97dccad1ba61a6f1134ab7a675ec05582f27 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Wed, 22 Jan 2025 13:49:02 +0200 Subject: [PATCH 03/10] fix multiple TDs request SQL generation --- .../src/adapter/BaseQuery.js | 70 +++++++++++++++---- .../src/adapter/BaseTimeDimension.ts | 10 ++- 2 files changed, 64 insertions(+), 16 deletions(-) diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index da47995769df9..eeef5e820bca8 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -1464,18 +1464,59 @@ export class BaseQuery { overTimeSeriesQuery(baseQueryFn, cumulativeMeasure, fromRollup) { const dateJoinCondition = cumulativeMeasure.dateJoinCondition(); + const uniqDateJoinCondition = R.uniqBy(djc => djc[0].dimension, dateJoinCondition); const cumulativeMeasures = [cumulativeMeasure]; if (!this.timeDimensions.find(d => d.granularity)) { - const filters = this.segments.concat(this.filters).concat(this.dateFromStartToEndConditionSql(dateJoinCondition, fromRollup, false)); + const filters = this.segments + .concat(this.filters) + .concat(this.dateFromStartToEndConditionSql( + // If the same time dimension is passed more than once, no need to build the same + // filter condition again and again. Different granularities don't play role here, + // as rollingWindow.granularity is used for filtering. + uniqDateJoinCondition, + fromRollup, + false + )); return baseQueryFn(cumulativeMeasures, filters, false); } - const dateSeriesSql = this.timeDimensions.map(d => this.dateSeriesSql(d)).join(', '); - const filters = this.segments.concat(this.filters).concat(this.dateFromStartToEndConditionSql(dateJoinCondition, fromRollup, true)); + + // We can't do meaningful query if few time dimensions with different ranges passed, + // it won't be possible to join them together without loosing some rows. + const rangedTimeDimensions = this.timeDimensions.filter(d => d.dateRange && d.granularity); + const uniqTimeDimensionWithRanges = R.uniqBy(d => d.dateRange, rangedTimeDimensions); + if (uniqTimeDimensionWithRanges.length > 1) { + throw new Error('Can\'t build query for time dimensions with different date ranges'); + } + + // We need to generate time series table for the lowest granularity among all time dimensions + let dateSeriesDimension; + const dateSeriesGranularity = this.timeDimensions.filter(d => d.granularity) + .reduce((acc, d) => { + const mg = this.minGranularity(acc, d.resolvedGranularity()); + if (mg === d.resolvedGranularity()) { + dateSeriesDimension = d; + } + return mg; + }, undefined); + + const dateSeriesSql = this.dateSeriesSql(dateSeriesDimension); + + // If the same time dimension is passed more than once, no need to build the same + // filter condition again and again. Different granularities don't play role here, + // as rollingWindow.granularity is used for filtering. + const filters = this.segments + .concat(this.filters) + .concat(this.dateFromStartToEndConditionSql( + uniqDateJoinCondition, + fromRollup, + true + )); const baseQuery = this.groupedUngroupedSelect( () => baseQueryFn(cumulativeMeasures, filters), cumulativeMeasure.shouldUngroupForCumulative(), !cumulativeMeasure.shouldUngroupForCumulative() && this.minGranularity( - cumulativeMeasure.windowGranularity(), this.timeDimensions.find(d => d.granularity).resolvedGranularity() + cumulativeMeasure.windowGranularity(), + dateSeriesGranularity ) || undefined ); const baseQueryAlias = this.cubeAlias('base'); @@ -1495,28 +1536,27 @@ export class BaseQuery { dateSeriesSql, baseQuery, dateJoinConditionSql, - baseQueryAlias + baseQueryAlias, + dateSeriesDimension.granularity, ); } - overTimeSeriesSelect(cumulativeMeasures, dateSeriesSql, baseQuery, dateJoinConditionSql, baseQueryAlias) { - const forSelect = this.overTimeSeriesForSelect(cumulativeMeasures); + overTimeSeriesSelect(cumulativeMeasures, dateSeriesSql, baseQuery, dateJoinConditionSql, baseQueryAlias, dateSeriesGranularity) { + const forSelect = this.overTimeSeriesForSelect(cumulativeMeasures, dateSeriesGranularity); return `SELECT ${forSelect} FROM ${dateSeriesSql}` + ` LEFT JOIN (${baseQuery}) ${this.asSyntaxJoin} ${baseQueryAlias} ON ${dateJoinConditionSql}` + this.groupByClause(); } - overTimeSeriesForSelect(cumulativeMeasures) { - return this.dimensions.map(s => s.cumulativeSelectColumns()).concat(this.dateSeriesSelect()).concat( - cumulativeMeasures.map(s => s.cumulativeSelectColumns()), - ).filter(c => !!c) + overTimeSeriesForSelect(cumulativeMeasures, dateSeriesGranularity) { + return this.dimensions + .map(s => s.cumulativeSelectColumns()) + .concat(this.timeDimensions.map(d => d.dateSeriesSelectColumn(null, dateSeriesGranularity))) + .concat(cumulativeMeasures.map(s => s.cumulativeSelectColumns())) + .filter(c => !!c) .join(', '); } - dateSeriesSelect() { - return this.timeDimensions.map(d => d.dateSeriesSelectColumn()); - } - dateFromStartToEndConditionSql(dateJoinCondition, fromRollup, isFromStartToEnd) { return dateJoinCondition.map( // TODO these weird conversions to be strict typed for big query. diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseTimeDimension.ts b/packages/cubejs-schema-compiler/src/adapter/BaseTimeDimension.ts index 69c7a4ffa2ace..5018bc47af4a0 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseTimeDimension.ts +++ b/packages/cubejs-schema-compiler/src/adapter/BaseTimeDimension.ts @@ -85,10 +85,18 @@ export class BaseTimeDimension extends BaseFilter { return this.query.escapeColumnName(`${this.dimension}_series`); } - public dateSeriesSelectColumn(dateSeriesAliasName) { + public dateSeriesSelectColumn(dateSeriesAliasName: string, dateSeriesGranularity: string) { if (!this.granularityObj) { return null; } + + // In case of query with more than one granularity, the time series table was generated + // with the minimal granularity among all. If this is our granularity, we can save + // some cpu cycles without 'date_from' truncation. But if this is not our granularity, + // we need to truncate it to desired. + if (dateSeriesGranularity && this.granularityObj?.granularity !== dateSeriesGranularity) { + return `${this.query.dimensionTimeGroupedColumn(`${dateSeriesAliasName || this.dateSeriesAliasName()}.${this.query.escapeColumnName('date_from')}`, this.granularityObj)} ${this.aliasName()}`; + } return `${dateSeriesAliasName || this.dateSeriesAliasName()}.${this.query.escapeColumnName('date_from')} ${this.aliasName()}`; } From 64b8a39e28b34875e1dddf090b3ae2c03ef7ea38 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Wed, 22 Jan 2025 15:11:57 +0200 Subject: [PATCH 04/10] add tests --- .../postgres/sql-generation.test.ts | 179 +++++++++++++++++- 1 file changed, 178 insertions(+), 1 deletion(-) 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 86184d058e311..1fda04d9c0705 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 @@ -79,6 +79,19 @@ describe('SQL Generation', () => { offset: 'start' } }, + countRollingUnbounded: { + type: 'count', + rollingWindow: { + trailing: 'unbounded' + } + }, + countRollingWeekToDate: { + type: 'count', + rollingWindow: { + type: 'to_date', + granularity: 'week' + } + }, revenue_qtd: { type: 'sum', sql: 'amount', @@ -220,7 +233,14 @@ describe('SQL Generation', () => { }, created_at: { type: 'time', - sql: 'created_at' + sql: 'created_at', + granularities: { + three_days: { + interval: '3 days', + title: '3 days', + origin: '2017-01-01' + } + } }, updated_at: { type: 'time', @@ -778,6 +798,163 @@ describe('SQL Generation', () => { } ])); + it('rolling window with two time dimension granularities', async () => runQueryTest({ + measures: [ + 'visitors.countRollingWeekToDate' + ], + timeDimensions: [ + { + dimension: 'visitors.created_at', + granularity: 'three_days', + dateRange: ['2017-01-01', '2017-01-10'] + }, + { + dimension: 'visitors.created_at', + granularity: 'day', + dateRange: ['2017-01-01', '2017-01-10'] + } + ], + order: [{ + id: 'visitors.created_at' + }], + timezone: 'America/Los_Angeles' + }, [ + { + visitors__count_rolling_week_to_date: null, + visitors__created_at_day: '2017-01-01T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-01T00:00:00.000Z', + }, + { + visitors__count_rolling_week_to_date: '1', + visitors__created_at_day: '2017-01-02T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-01T00:00:00.000Z', + }, + { + visitors__count_rolling_week_to_date: '1', + visitors__created_at_day: '2017-01-03T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-01T00:00:00.000Z', + }, + { + visitors__count_rolling_week_to_date: '2', + visitors__created_at_day: '2017-01-04T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-04T00:00:00.000Z', + }, + { + visitors__count_rolling_week_to_date: '3', + visitors__created_at_day: '2017-01-05T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-04T00:00:00.000Z', + }, + { + visitors__count_rolling_week_to_date: '5', + visitors__created_at_day: '2017-01-06T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-04T00:00:00.000Z', + }, + { + visitors__count_rolling_week_to_date: '5', + visitors__created_at_day: '2017-01-07T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-07T00:00:00.000Z', + }, + { + visitors__count_rolling_week_to_date: '5', + visitors__created_at_day: '2017-01-08T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-07T00:00:00.000Z', + }, + { + visitors__count_rolling_week_to_date: null, + visitors__created_at_day: '2017-01-09T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-07T00:00:00.000Z', + }, + { + visitors__count_rolling_week_to_date: null, + visitors__created_at_day: '2017-01-10T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-10T00:00:00.000Z', + } + ])); + + it('two rolling windows with two time dimension granularities', async () => runQueryTest({ + measures: [ + 'visitors.countRollingUnbounded', + 'visitors.countRollingWeekToDate' + ], + timeDimensions: [ + { + dimension: 'visitors.created_at', + granularity: 'three_days', + dateRange: ['2017-01-01', '2017-01-10'] + }, + { + dimension: 'visitors.created_at', + granularity: 'day', + dateRange: ['2017-01-01', '2017-01-10'] + } + ], + order: [{ + id: 'visitors.created_at' + }], + timezone: 'America/Los_Angeles' + }, [ + { + visitors__count_rolling_unbounded: '1', + visitors__count_rolling_week_to_date: null, + visitors__created_at_day: '2017-01-01T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-01T00:00:00.000Z', + }, + { + visitors__count_rolling_unbounded: '2', + visitors__count_rolling_week_to_date: '1', + visitors__created_at_day: '2017-01-03T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-01T00:00:00.000Z', + }, + { + visitors__count_rolling_unbounded: '2', + visitors__count_rolling_week_to_date: '1', + visitors__created_at_day: '2017-01-02T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-01T00:00:00.000Z', + }, + { + visitors__count_rolling_unbounded: '3', + visitors__count_rolling_week_to_date: '2', + visitors__created_at_day: '2017-01-04T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-04T00:00:00.000Z', + }, + { + visitors__count_rolling_unbounded: '4', + visitors__count_rolling_week_to_date: '3', + visitors__created_at_day: '2017-01-05T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-04T00:00:00.000Z', + }, + { + visitors__count_rolling_unbounded: '6', + visitors__count_rolling_week_to_date: '5', + visitors__created_at_day: '2017-01-06T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-04T00:00:00.000Z', + }, + { + visitors__count_rolling_unbounded: '6', + visitors__count_rolling_week_to_date: '5', + visitors__created_at_day: '2017-01-08T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-07T00:00:00.000Z', + }, + { + visitors__count_rolling_unbounded: '6', + visitors__count_rolling_week_to_date: '5', + visitors__created_at_day: '2017-01-07T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-07T00:00:00.000Z', + }, + { + visitors__count_rolling_unbounded: '6', + visitors__count_rolling_week_to_date: null, + visitors__created_at_day: '2017-01-09T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-07T00:00:00.000Z', + }, + { + visitors__count_rolling_unbounded: '6', + visitors__count_rolling_week_to_date: null, + visitors__created_at_day: '2017-01-10T00:00:00.000Z', + visitors__created_at_three_days: '2017-01-10T00:00:00.000Z', + } + ])); + it('rolling month', async () => runQueryTest({ measures: [ 'visitors.revenueRollingThreeDay' From 10c227b3d4ac739ffb5eb81013ab99f416631bf4 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Thu, 23 Jan 2025 14:33:59 +0200 Subject: [PATCH 05/10] =?UTF-8?q?fill=20daterange=20for=C2=A0all=20time=20?= =?UTF-8?q?dimension=20items=20in=C2=A0query?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- rust/cubesql/cubesql/src/compile/rewrite/converter.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rust/cubesql/cubesql/src/compile/rewrite/converter.rs b/rust/cubesql/cubesql/src/compile/rewrite/converter.rs index d717f23f249f4..a65bcca60336a 100644 --- a/rust/cubesql/cubesql/src/compile/rewrite/converter.rs +++ b/rust/cubesql/cubesql/src/compile/rewrite/converter.rs @@ -1823,16 +1823,16 @@ impl LanguageToLogicalPlanConverter { let values = match_data_node!(node_by_id, params[2], FilterMemberValues); if !is_in_or && op == "inDateRange" { - let existing_time_dimension = - query_time_dimensions.iter_mut().find_map(|td| { + let existing_time_dimensions: Vec<_> = + query_time_dimensions.iter_mut().filter_map(|td| { if td.dimension == member && td.date_range.is_none() { td.date_range = Some(json!(values)); Some(td) } else { None } - }); - if existing_time_dimension.is_none() { + }).collect(); + if existing_time_dimensions.len() == 0 { let dimension = V1LoadRequestQueryTimeDimension { dimension: member.to_string(), granularity: None, From eaa6224e5f982a0105468456e3c26d6b34ccc305 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Thu, 23 Jan 2025 14:35:22 +0200 Subject: [PATCH 06/10] remove this.fillMissingDateRangesForTimeDimensions from gateway --- packages/cubejs-api-gateway/src/gateway.ts | 23 ---------------------- 1 file changed, 23 deletions(-) diff --git a/packages/cubejs-api-gateway/src/gateway.ts b/packages/cubejs-api-gateway/src/gateway.ts index ccf57e0ac409a..af90986b4117b 100644 --- a/packages/cubejs-api-gateway/src/gateway.ts +++ b/packages/cubejs-api-gateway/src/gateway.ts @@ -1171,7 +1171,6 @@ class ApiGateway { let queryType: QueryType = QueryTypeEnum.REGULAR_QUERY; if (!Array.isArray(query)) { query = this.compareDateRangeTransformer(query); - query = this.fillMissingDateRangesForTimeDimensions(query); if (Array.isArray(query)) { queryType = QueryTypeEnum.COMPARE_DATE_RANGE_QUERY; } @@ -2486,28 +2485,6 @@ class ApiGateway { } }; - protected fillMissingDateRangesForTimeDimensions(query) { - if (!query.timeDimensions) { - return query; - } - - const timeDimensionCount = query.timeDimensions.reduce((acc, item) => { - acc[item.dimension] = acc[item.dimension] || {}; - if (!acc[item.dimension].dateRange && item.dateRange) { - acc[item.dimension].dateRange = item.dateRange; - } - return acc; - }, {}); - - query.timeDimensions.forEach(td => { - if (!td.dateRange && timeDimensionCount[td.dimension].dateRange) { - td.dateRange = timeDimensionCount[td.dimension].dateRange; - } - }); - - return query; - } - protected compareDateRangeTransformer(query) { let queryCompareDateRange; let compareDateRangeTDIndex; From c0dfdbf4907b632d1655b54f983aceaa01d6c2b8 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Thu, 23 Jan 2025 14:47:53 +0200 Subject: [PATCH 07/10] =?UTF-8?q?Add=20guard=20for=C2=A0time=20series=20wi?= =?UTF-8?q?thout=20daterange?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/cubejs-schema-compiler/src/adapter/BaseQuery.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index eeef5e820bca8..8b5b9b5944add 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -1480,6 +1480,10 @@ export class BaseQuery { return baseQueryFn(cumulativeMeasures, filters, false); } + if (this.timeDimensions.filter(d => !d.dateRange && d.granularity).length > 0) { + throw new UserError('Time series queries without dateRange aren\'t supported'); + } + // We can't do meaningful query if few time dimensions with different ranges passed, // it won't be possible to join them together without loosing some rows. const rangedTimeDimensions = this.timeDimensions.filter(d => d.dateRange && d.granularity); From 0177327c94bf5f1170c929223c2c0254bb4c3017 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Thu, 23 Jan 2025 14:50:20 +0200 Subject: [PATCH 08/10] cargo fmt --- rust/cubesql/cubesql/src/compile/rewrite/converter.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/rust/cubesql/cubesql/src/compile/rewrite/converter.rs b/rust/cubesql/cubesql/src/compile/rewrite/converter.rs index a65bcca60336a..4daf5b658084a 100644 --- a/rust/cubesql/cubesql/src/compile/rewrite/converter.rs +++ b/rust/cubesql/cubesql/src/compile/rewrite/converter.rs @@ -1823,15 +1823,17 @@ impl LanguageToLogicalPlanConverter { let values = match_data_node!(node_by_id, params[2], FilterMemberValues); if !is_in_or && op == "inDateRange" { - let existing_time_dimensions: Vec<_> = - query_time_dimensions.iter_mut().filter_map(|td| { + let existing_time_dimensions: Vec<_> = query_time_dimensions + .iter_mut() + .filter_map(|td| { if td.dimension == member && td.date_range.is_none() { td.date_range = Some(json!(values)); Some(td) } else { None } - }).collect(); + }) + .collect(); if existing_time_dimensions.len() == 0 { let dimension = V1LoadRequestQueryTimeDimension { dimension: member.to_string(), From 84743f705f0359d5c4d33d37e3097beb83df7272 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Thu, 23 Jan 2025 15:37:55 +0200 Subject: [PATCH 09/10] fix test for cubesql fix --- rust/cubesql/cubesql/src/compile/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rust/cubesql/cubesql/src/compile/mod.rs b/rust/cubesql/cubesql/src/compile/mod.rs index 68cb378265d86..e0649f5565d23 100644 --- a/rust/cubesql/cubesql/src/compile/mod.rs +++ b/rust/cubesql/cubesql/src/compile/mod.rs @@ -2219,7 +2219,10 @@ limit V1LoadRequestQueryTimeDimension { dimension: "KibanaSampleDataEcommerce.order_date".to_string(), granularity: Some("month".to_string()), - date_range: None, + date_range: Some(json!(vec![ + "2023-07-08T00:00:00.000Z".to_string(), + "2023-10-07T23:59:59.999Z".to_string() + ])), } ]), order: Some(vec![]), From 063c5fe9308e2fc5dca6d576386e3f474982ace4 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Mon, 24 Feb 2025 16:42:01 +0200 Subject: [PATCH 10/10] fix notes after review --- .../src/adapter/BaseQuery.js | 15 +++++++-------- .../src/adapter/BaseTimeDimension.ts | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index 8b5b9b5944add..bcd7efb6f3907 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -1485,7 +1485,7 @@ export class BaseQuery { } // We can't do meaningful query if few time dimensions with different ranges passed, - // it won't be possible to join them together without loosing some rows. + // it won't be possible to join them together without losing some rows. const rangedTimeDimensions = this.timeDimensions.filter(d => d.dateRange && d.granularity); const uniqTimeDimensionWithRanges = R.uniqBy(d => d.dateRange, rangedTimeDimensions); if (uniqTimeDimensionWithRanges.length > 1) { @@ -1493,15 +1493,14 @@ export class BaseQuery { } // We need to generate time series table for the lowest granularity among all time dimensions - let dateSeriesDimension; - const dateSeriesGranularity = this.timeDimensions.filter(d => d.granularity) - .reduce((acc, d) => { - const mg = this.minGranularity(acc, d.resolvedGranularity()); + const [dateSeriesDimension, dateSeriesGranularity] = this.timeDimensions.filter(d => d.granularity) + .reduce(([prevDim, prevGran], d) => { + const mg = this.minGranularity(prevGran, d.resolvedGranularity()); if (mg === d.resolvedGranularity()) { - dateSeriesDimension = d; + return [d, mg]; } - return mg; - }, undefined); + return [prevDim, mg]; + }, [null, null]); const dateSeriesSql = this.dateSeriesSql(dateSeriesDimension); diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseTimeDimension.ts b/packages/cubejs-schema-compiler/src/adapter/BaseTimeDimension.ts index 5018bc47af4a0..e84a81c47d247 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseTimeDimension.ts +++ b/packages/cubejs-schema-compiler/src/adapter/BaseTimeDimension.ts @@ -85,7 +85,7 @@ export class BaseTimeDimension extends BaseFilter { return this.query.escapeColumnName(`${this.dimension}_series`); } - public dateSeriesSelectColumn(dateSeriesAliasName: string, dateSeriesGranularity: string) { + public dateSeriesSelectColumn(dateSeriesAliasName: string | null, dateSeriesGranularity?: string) { if (!this.granularityObj) { return null; }