From 9a4d13fe077bf1b724a746e80961ad15f2c55ba4 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Fri, 11 Jul 2025 16:11:11 +0300 Subject: [PATCH 1/8] extend cube validator to support named timeshifts --- .../src/compiler/CubeValidator.ts | 25 +++++++++++++------ .../unit/__snapshots__/schema.test.ts.snap | 6 ++--- .../test/unit/fixtures/calendar_orders.yml | 7 ++++++ .../test/unit/fixtures/custom_calendar.js | 3 +-- .../test/unit/fixtures/custom_calendar.yml | 3 +-- 5 files changed, 28 insertions(+), 16 deletions(-) diff --git a/packages/cubejs-schema-compiler/src/compiler/CubeValidator.ts b/packages/cubejs-schema-compiler/src/compiler/CubeValidator.ts index 9ca271d51b4bb..86ad93f29da20 100644 --- a/packages/cubejs-schema-compiler/src/compiler/CubeValidator.ts +++ b/packages/cubejs-schema-compiler/src/compiler/CubeValidator.ts @@ -576,9 +576,12 @@ const timeShiftItemRequired = Joi.object({ const timeShiftItemOptional = Joi.object({ timeDimension: Joi.func(), // not required - interval: regexTimeInterval.required(), - type: Joi.string().valid('next', 'prior').required(), -}); + interval: regexTimeInterval, + name: identifier, + type: Joi.string().valid('next', 'prior'), +}) + .xor('name', 'interval') + .and('interval', 'type'); const MeasuresSchema = Joi.object().pattern(identifierRegex, Joi.alternatives().conditional(Joi.ref('.multiStage'), [ { @@ -623,6 +626,16 @@ const MeasuresSchema = Joi.object().pattern(identifierRegex, Joi.alternatives(). ] )); +const CalendarTimeShiftItem = Joi.object({ + name: identifier, + interval: regexTimeInterval, + type: Joi.string().valid('next', 'prior'), + sql: Joi.func().required(), +}) + .or('name', 'interval') + .with('interval', 'type') + .with('type', 'interval'); + const DimensionsSchema = Joi.object().pattern(identifierRegex, Joi.alternatives().try( inherit(BaseDimensionWithoutSubQuery, { case: Joi.object().keys({ @@ -667,11 +680,7 @@ const DimensionsSchema = Joi.object().pattern(identifierRegex, Joi.alternatives( inherit(BaseDimensionWithoutSubQuery, { type: Joi.any().valid('time').required(), sql: Joi.func().required(), - timeShift: Joi.array().items(Joi.object({ - interval: regexTimeInterval.required(), - type: Joi.string().valid('next', 'prior').required(), - sql: Joi.func().required(), - })), + timeShift: Joi.array().items(CalendarTimeShiftItem), }) )); diff --git a/packages/cubejs-schema-compiler/test/unit/__snapshots__/schema.test.ts.snap b/packages/cubejs-schema-compiler/test/unit/__snapshots__/schema.test.ts.snap index 0ffe9c8cc20c4..c33f7d388ad7b 100644 --- a/packages/cubejs-schema-compiler/test/unit/__snapshots__/schema.test.ts.snap +++ b/packages/cubejs-schema-compiler/test/unit/__snapshots__/schema.test.ts.snap @@ -87,9 +87,8 @@ Object { "type": "prior", }, Object { - "interval": "1 year", + "name": "retail_date_prev_year", "sql": [Function], - "type": "prior", }, Object { "interval": "2 year", @@ -273,9 +272,8 @@ Object { "type": "prior", }, Object { - "interval": "1 year", + "name": "retail_date_prev_year", "sql": [Function], - "type": "prior", }, Object { "interval": "2 year", diff --git a/packages/cubejs-schema-compiler/test/unit/fixtures/calendar_orders.yml b/packages/cubejs-schema-compiler/test/unit/fixtures/calendar_orders.yml index db039a67552cc..c0ffd1e03c45f 100644 --- a/packages/cubejs-schema-compiler/test/unit/fixtures/calendar_orders.yml +++ b/packages/cubejs-schema-compiler/test/unit/fixtures/calendar_orders.yml @@ -46,6 +46,13 @@ cubes: interval: 1 year type: prior + - name: count_shifted_named + type: count + multi_stage: true + sql: "{count}" + time_shift: + - name: retail_date_prev_year + - name: completed_count type: count filters: diff --git a/packages/cubejs-schema-compiler/test/unit/fixtures/custom_calendar.js b/packages/cubejs-schema-compiler/test/unit/fixtures/custom_calendar.js index 78f434a20fdfe..693557871c234 100644 --- a/packages/cubejs-schema-compiler/test/unit/fixtures/custom_calendar.js +++ b/packages/cubejs-schema-compiler/test/unit/fixtures/custom_calendar.js @@ -41,8 +41,7 @@ cube(`custom_calendar_js`, { sql: `{CUBE.retail_date_prev_month}`, }, { - interval: '1 year', - type: 'prior', + name: 'retail_date_prev_year', sql: `{CUBE.retail_date_prev_year}`, }, { diff --git a/packages/cubejs-schema-compiler/test/unit/fixtures/custom_calendar.yml b/packages/cubejs-schema-compiler/test/unit/fixtures/custom_calendar.yml index aa0a2271a8629..14bc0ebdcc03e 100644 --- a/packages/cubejs-schema-compiler/test/unit/fixtures/custom_calendar.yml +++ b/packages/cubejs-schema-compiler/test/unit/fixtures/custom_calendar.yml @@ -45,8 +45,7 @@ cubes: type: prior sql: "{CUBE.retail_date_prev_month}" - - interval: 1 year - type: prior + - name: retail_date_prev_year sql: "{CUBE.retail_date_prev_year}" # All needed intervals should be defined explicitly From 2c838e099a586642f5e341c347668dcaec0fef00 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Fri, 11 Jul 2025 19:47:23 +0300 Subject: [PATCH 2/8] Initial support for named time shifts --- .../src/cube_bridge/measure_definition.rs | 3 +- .../src/cube_bridge/timeshift_definition.rs | 5 +- .../src/logical_plan/multistage/common.rs | 8 ++- .../logical_plan/multistage/leaf_measure.rs | 8 ++- .../src/physical_plan_builder/builder.rs | 26 ++++++--- .../planners/multi_stage/applied_state.rs | 45 ++++++++++++++- .../multi_stage/multi_stage_query_planner.rs | 2 +- .../sql_evaluator/sql_nodes/time_shift.rs | 2 +- .../sql_evaluator/symbols/dimension_symbol.rs | 55 +++++++++++++++---- .../sql_evaluator/symbols/measure_symbol.rs | 39 +++++++++---- 10 files changed, 151 insertions(+), 42 deletions(-) diff --git a/rust/cubesqlplanner/cubesqlplanner/src/cube_bridge/measure_definition.rs b/rust/cubesqlplanner/cubesqlplanner/src/cube_bridge/measure_definition.rs index ee1c8a0f19acb..bc0e581ee01d8 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/cube_bridge/measure_definition.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/cube_bridge/measure_definition.rs @@ -15,7 +15,8 @@ use std::rc::Rc; #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] pub struct TimeShiftReference { - pub interval: String, + pub interval: Option, + pub name: Option, #[serde(rename = "type")] pub shift_type: Option, #[serde(rename = "timeDimension")] diff --git a/rust/cubesqlplanner/cubesqlplanner/src/cube_bridge/timeshift_definition.rs b/rust/cubesqlplanner/cubesqlplanner/src/cube_bridge/timeshift_definition.rs index 1f507831188c9..f51081ac7ee15 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/cube_bridge/timeshift_definition.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/cube_bridge/timeshift_definition.rs @@ -11,9 +11,10 @@ use std::rc::Rc; #[derive(Deserialize, Serialize, Clone, Debug, PartialEq, Eq, Hash)] pub struct TimeShiftDefinitionStatic { - pub interval: String, + pub interval: Option, #[serde(rename = "type")] - pub timeshift_type: String, + pub timeshift_type: Option, + pub name: Option, } #[nativebridge::native_bridge(TimeShiftDefinitionStatic)] diff --git a/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/multistage/common.rs b/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/multistage/common.rs index 5ca1ddd776f6a..b3eaa47b31531 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/multistage/common.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/multistage/common.rs @@ -58,7 +58,13 @@ impl PrettyPrint for MultiStageAppliedState { &format!( "- {}: {}", time_shift.dimension.full_name(), - time_shift.interval.to_sql() + if let Some(interval) = &time_shift.interval { + interval.to_sql() + } else if let Some(name) = &time_shift.name { + format!("{} (named)", name.to_string()) + } else { + "None".to_string() + } ), &details_state, ); diff --git a/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/multistage/leaf_measure.rs b/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/multistage/leaf_measure.rs index 214ab412fb8d8..fe454c2c03f37 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/multistage/leaf_measure.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/multistage/leaf_measure.rs @@ -30,7 +30,13 @@ impl PrettyPrint for MultiStageLeafMeasure { &format!( "- {}: {}", time_shift.dimension.full_name(), - time_shift.interval.to_sql() + if let Some(interval) = &time_shift.interval { + interval.to_sql() + } else if let Some(name) = &time_shift.name { + format!("{} (named)", name.to_string()) + } else { + "None".to_string() + } ), &details_state, ); diff --git a/rust/cubesqlplanner/cubesqlplanner/src/physical_plan_builder/builder.rs b/rust/cubesqlplanner/cubesqlplanner/src/physical_plan_builder/builder.rs index d585a7f98b4ab..29c7c3aa2b178 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/physical_plan_builder/builder.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/physical_plan_builder/builder.rs @@ -43,14 +43,24 @@ impl PhysicalPlanBuilderContext { .iter() .partition_map(|(key, shift)| { if let Ok(dimension) = shift.dimension.as_dimension() { - if let Some((dim_key, cts)) = - dimension.calendar_time_shift_for_interval(&shift.interval) - { - return Either::Right((dim_key.clone(), cts.clone())); - } else if let Some(calendar_pk) = dimension.time_shift_pk_full_name() { - let mut shift = shift.clone(); - shift.interval = shift.interval.inverse(); - return Either::Left((calendar_pk, shift.clone())); + if let Some(dim_shift_name) = &shift.name { + if let Some((dim_key, cts)) = + dimension.calendar_time_shift_for_named_interval(dim_shift_name) + { + return Either::Right((dim_key.clone(), cts.clone())); + } else if let Some(_calendar_pk) = dimension.time_shift_pk_full_name() { + // TODO: Handle case when named shift is not found + } + } else if let Some(dim_shift_interval) = &shift.interval { + if let Some((dim_key, cts)) = + dimension.calendar_time_shift_for_interval(dim_shift_interval) + { + return Either::Right((dim_key.clone(), cts.clone())); + } else if let Some(calendar_pk) = dimension.time_shift_pk_full_name() { + let mut shift = shift.clone(); + shift.interval = Some(dim_shift_interval.inverse()); + return Either::Left((calendar_pk, shift.clone())); + } } } Either::Left((key.clone(), shift.clone())) diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/applied_state.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/applied_state.rs index 8ccbec0c29b68..f13f01576a17f 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/applied_state.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/applied_state.rs @@ -2,6 +2,7 @@ use crate::plan::{FilterGroup, FilterItem}; use crate::planner::filter::FilterOperator; use crate::planner::sql_evaluator::{DimensionTimeShift, MeasureTimeShifts, MemberSymbol}; use crate::planner::{BaseDimension, BaseMember, BaseTimeDimension}; +use cubenativeutils::CubeError; use itertools::Itertools; use std::cmp::PartialEq; use std::collections::HashMap; @@ -72,15 +73,25 @@ impl MultiStageAppliedState { .collect_vec(); } - pub fn add_time_shifts(&mut self, time_shifts: MeasureTimeShifts) { + pub fn add_time_shifts(&mut self, time_shifts: MeasureTimeShifts) -> Result<(), CubeError> { let resolved_shifts = match time_shifts { MeasureTimeShifts::Dimensions(dimensions) => dimensions, MeasureTimeShifts::Common(interval) => self .all_time_members() .into_iter() .map(|m| DimensionTimeShift { - interval: interval.clone(), + interval: Some(interval.clone()), dimension: m, + name: None, + }) + .collect_vec(), + MeasureTimeShifts::Named(named_shift) => self + .all_time_members() + .into_iter() + .map(|m| DimensionTimeShift { + interval: None, + dimension: m, + name: Some(named_shift.clone()), }) .collect_vec(), }; @@ -90,13 +101,41 @@ impl MultiStageAppliedState { .dimensions_shifts .get_mut(&ts.dimension.full_name()) { - exists.interval += ts.interval; + if let Some(interval) = exists.interval.clone() { + if let Some(new_interval) = ts.interval { + exists.interval = Some(interval + new_interval); + } else { + return Err(CubeError::internal(format!( + "Cannot use both named ({}) and interval ({}) shifts for the same dimension: {}.", + ts.name.clone().unwrap_or("-".to_string()), + interval.to_sql(), + ts.dimension.full_name(), + ))); + } + } else if let Some(named_shift) = exists.name.clone() { + return if let Some(new_interval) = ts.interval { + Err(CubeError::internal(format!( + "Cannot use both named ({}) and interval ({}) shifts for the same dimension: {}.", + named_shift, + new_interval.to_sql(), + ts.dimension.full_name(), + ))) + } else { + Err(CubeError::internal(format!( + "Cannot use more than one named shifts ({}, {}) for the same dimension: {}.", + ts.name.clone().unwrap_or("-".to_string()), + named_shift, + ts.dimension.full_name(), + ))) + }; + } } else { self.time_shifts .dimensions_shifts .insert(ts.dimension.full_name(), ts); } } + Ok(()) } pub fn time_shifts(&self) -> &TimeShiftState { diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/multi_stage_query_planner.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/multi_stage_query_planner.rs index aac6c384b39b5..df2bb46baa155 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/multi_stage_query_planner.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/multi_stage_query_planner.rs @@ -225,7 +225,7 @@ impl MultiStageQueryPlanner { new_state.add_dimensions(dimensions_to_add); } if let Some(time_shift) = multi_stage_member.time_shift() { - new_state.add_time_shifts(time_shift.clone()); + new_state.add_time_shifts(time_shift.clone())?; } if state.has_filters_for_member(&member_name) { new_state.remove_filter_for_member(&member_name); diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_nodes/time_shift.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_nodes/time_shift.rs index 2f30043a0df38..c3b806276199e 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_nodes/time_shift.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_nodes/time_shift.rs @@ -43,7 +43,7 @@ impl SqlNode for TimeShiftSqlNode { MemberSymbol::Dimension(ev) => { if !ev.is_reference() && ev.dimension_type() == "time" { if let Some(shift) = self.shifts.dimensions_shifts.get(&ev.full_name()) { - let shift = shift.interval.to_sql(); + let shift = shift.interval.clone().unwrap().to_sql(); // Common time shifts should always have an interval let res = templates.add_timestamp_interval(input, shift)?; format!("({})", res) } else { diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/dimension_symbol.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/dimension_symbol.rs index 6d15a6a0f4db0..85da6ed53b65f 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/dimension_symbol.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/dimension_symbol.rs @@ -27,7 +27,8 @@ pub struct DimensionCaseDefinition { #[derive(Clone)] pub struct CalendarDimensionTimeShift { - pub interval: SqlInterval, + pub interval: Option, + pub name: Option, pub sql: Option>, } @@ -234,11 +235,31 @@ impl DimensionSymbol { &self, interval: &SqlInterval, ) -> Option<(String, CalendarDimensionTimeShift)> { - if let Some(ts) = self - .time_shift - .iter() - .find(|shift| shift.interval == *interval) - { + if let Some(ts) = self.time_shift.iter().find(|shift| { + if let Some(s_i) = &shift.interval { + s_i == interval + } else { + false + } + }) { + if let Some(pk) = &self.time_shift_pk { + return Some((pk.full_name(), ts.clone())); + } + } + None + } + + pub fn calendar_time_shift_for_named_interval( + &self, + interval_name: &String, + ) -> Option<(String, CalendarDimensionTimeShift)> { + if let Some(ts) = self.time_shift.iter().find(|shift| { + if let Some(s_n) = &shift.name { + s_n == interval_name + } else { + false + } + }) { if let Some(pk) = &self.time_shift_pk_full_name { return Some((pk.clone(), ts.clone())); } else if self.is_self_time_shift_pk { @@ -369,18 +390,28 @@ impl SymbolFactory for DimensionSymbolFactory { time_shift .iter() .map(|item| -> Result<_, CubeError> { - let interval = item.static_data().interval.parse::()?; - let interval = if item.static_data().timeshift_type == "next" { - -interval - } else { - interval + let interval = match &item.static_data().interval { + Some(raw) => { + let mut iv = raw.parse::()?; + if item.static_data().timeshift_type.as_deref() == Some("next") { + iv = -iv; + } + + Some(iv) + } + None => None, }; + let name = item.static_data().name.clone(); let sql = if let Some(sql) = item.sql()? { Some(compiler.compile_sql_call(&cube_name, sql)?) } else { None }; - Ok(CalendarDimensionTimeShift { interval, sql }) + Ok(CalendarDimensionTimeShift { + interval, + name, + sql, + }) }) .collect::, _>>()? } else { diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/measure_symbol.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/measure_symbol.rs index 0f49e175cb153..bbfa64e8df66f 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/measure_symbol.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/measure_symbol.rs @@ -38,13 +38,16 @@ impl MeasureOrderBy { #[derive(Clone, Debug)] pub struct DimensionTimeShift { - pub interval: SqlInterval, + pub interval: Option, + pub name: Option, pub dimension: Rc, } impl PartialEq for DimensionTimeShift { fn eq(&self, other: &Self) -> bool { - self.interval == other.interval && self.dimension.full_name() == other.dimension.full_name() + self.interval == other.interval + && self.dimension.full_name() == other.dimension.full_name() + && self.name == other.name } } @@ -54,6 +57,7 @@ impl Eq for DimensionTimeShift {} pub enum MeasureTimeShifts { Dimensions(Vec), Common(SqlInterval), + Named(String), } #[derive(Clone)] @@ -567,19 +571,29 @@ impl SymbolFactory for MeasureSymbolFactory { let mut shifts: HashMap = HashMap::new(); let mut common_shift = None; for shift_ref in time_shift_references.iter() { - let interval = shift_ref.interval.parse::()?; - let interval = - if shift_ref.shift_type.as_ref().unwrap_or(&format!("prior")) == "next" { - -interval - } else { - interval - }; + let interval = match &shift_ref.interval { + Some(raw) => { + let mut iv = raw.parse::()?; + if shift_ref + .shift_type + .as_deref() + .unwrap_or("prior") + == "next" + { + iv = -iv; + } + + Some(iv) + } + None => None, + }; + let name = shift_ref.name.clone(); if let Some(time_dimension) = &shift_ref.time_dimension { let dimension = compiler.add_dimension_evaluator(time_dimension.clone())?; let dimension = find_owned_by_cube_child(&dimension)?; let dimension_name = dimension.full_name(); if let Some(exists) = shifts.get(&dimension_name) { - if exists.interval != interval { + if exists.interval != interval || exists.name != name { return Err(CubeError::user(format!( "Different time shifts for one dimension {} not allowed", dimension_name @@ -590,15 +604,16 @@ impl SymbolFactory for MeasureSymbolFactory { dimension_name, DimensionTimeShift { interval: interval.clone(), + name: name.clone(), dimension: dimension.clone(), }, ); }; } else { if common_shift.is_none() { - common_shift = Some(interval); + common_shift = interval; } else { - if common_shift != Some(interval) { + if common_shift != interval { return Err(CubeError::user(format!( "Measure can contain only one common time_shift (without time_dimension).", ))); From 1e9a63e7830acc7d625f2a14367aced5c4b40705 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Fri, 11 Jul 2025 19:52:51 +0300 Subject: [PATCH 3/8] Resolve conficts --- .../src/planner/sql_evaluator/symbols/dimension_symbol.rs | 4 ++-- .../src/planner/sql_evaluator/symbols/measure_symbol.rs | 7 +------ 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/dimension_symbol.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/dimension_symbol.rs index 85da6ed53b65f..dacbe81fd7a0f 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/dimension_symbol.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/dimension_symbol.rs @@ -242,8 +242,8 @@ impl DimensionSymbol { false } }) { - if let Some(pk) = &self.time_shift_pk { - return Some((pk.full_name(), ts.clone())); + if let Some(pk) = &self.time_shift_pk_full_name() { + return Some((pk.clone(), ts.clone())); } } None diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/measure_symbol.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/measure_symbol.rs index bbfa64e8df66f..759a33bbc59ea 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/measure_symbol.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/measure_symbol.rs @@ -574,12 +574,7 @@ impl SymbolFactory for MeasureSymbolFactory { let interval = match &shift_ref.interval { Some(raw) => { let mut iv = raw.parse::()?; - if shift_ref - .shift_type - .as_deref() - .unwrap_or("prior") - == "next" - { + if shift_ref.shift_type.as_deref().unwrap_or("prior") == "next" { iv = -iv; } From aa5599efc56b5ed40f7d8e0ea1e2dd85741e0db5 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Fri, 11 Jul 2025 20:35:26 +0300 Subject: [PATCH 4/8] fix Evaluator to support named time shifts --- .../src/compiler/CubeEvaluator.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/cubejs-schema-compiler/src/compiler/CubeEvaluator.ts b/packages/cubejs-schema-compiler/src/compiler/CubeEvaluator.ts index b1321762061ff..e18bd9eb1e5d3 100644 --- a/packages/cubejs-schema-compiler/src/compiler/CubeEvaluator.ts +++ b/packages/cubejs-schema-compiler/src/compiler/CubeEvaluator.ts @@ -28,15 +28,17 @@ export type DimensionDefinition = { }; export type TimeShiftDefinition = { - timeDimension?: (...args: Array) => ToString, - interval: string, - type: 'next' | 'prior', + timeDimension?: (...args: Array) => ToString; + name?: string; + interval?: string; + type?: 'next' | 'prior'; }; export type TimeShiftDefinitionReference = { - timeDimension?: string, - interval: string, - type: 'next' | 'prior', + timeDimension?: string; + name?: string; + interval?: string; + type?: 'next' | 'prior'; }; export type MeasureDefinition = { @@ -393,6 +395,7 @@ export class CubeEvaluator extends CubeSymbols { } if (member.timeShift) { member.timeShiftReferences = member.timeShift.map((s): TimeShiftDefinitionReference => ({ + name: s.name, interval: s.interval, type: s.type, ...(typeof s.timeDimension === 'function' From b1774b5067c37c1254d3299621940f00009fa8de Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Fri, 11 Jul 2025 20:35:45 +0300 Subject: [PATCH 5/8] update measure symbol to support named time shifts --- .../sql_evaluator/symbols/measure_symbol.rs | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/measure_symbol.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/measure_symbol.rs index 759a33bbc59ea..25d71d7a15df6 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/measure_symbol.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/measure_symbol.rs @@ -570,6 +570,7 @@ impl SymbolFactory for MeasureSymbolFactory { { let mut shifts: HashMap = HashMap::new(); let mut common_shift = None; + let mut named_shift = None; for shift_ref in time_shift_references.iter() { let interval = match &shift_ref.interval { Some(raw) => { @@ -604,6 +605,16 @@ impl SymbolFactory for MeasureSymbolFactory { }, ); }; + } else if let Some(name) = &shift_ref.name { + if named_shift.is_none() { + named_shift = Some(name.clone()); + } else { + if named_shift != Some(name.clone()) { + return Err(CubeError::user(format!( + "Measure can contain only one named time_shift (without time_dimension).", + ))); + } + } } else { if common_shift.is_none() { common_shift = interval; @@ -616,12 +627,19 @@ impl SymbolFactory for MeasureSymbolFactory { } } } - if common_shift.is_some() && !shifts.is_empty() { + + if (common_shift.is_some() || named_shift.is_some()) && !shifts.is_empty() { return Err(CubeError::user(format!( "Measure cannot mix common time_shifts (without time_dimension) with dimension-specific ones.", ))); + } else if common_shift.is_some() && named_shift.is_some() { + return Err(CubeError::user(format!( + "Measure cannot mix common unnamed and named time_shifts.", + ))); } else if common_shift.is_some() { Some(MeasureTimeShifts::Common(common_shift.unwrap())) + } else if named_shift.is_some() { + Some(MeasureTimeShifts::Named(named_shift.unwrap())) } else { Some(MeasureTimeShifts::Dimensions( shifts.into_values().collect_vec(), From 8eeacac6e0fecb6745d7fe86dacc299fb4f2e2fe Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Fri, 11 Jul 2025 20:50:33 +0300 Subject: [PATCH 6/8] cargo clippy --- .../src/planner/sql_evaluator/symbols/measure_symbol.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/measure_symbol.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/measure_symbol.rs index 25d71d7a15df6..be71028fc369a 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/measure_symbol.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/measure_symbol.rs @@ -636,10 +636,10 @@ impl SymbolFactory for MeasureSymbolFactory { return Err(CubeError::user(format!( "Measure cannot mix common unnamed and named time_shifts.", ))); - } else if common_shift.is_some() { - Some(MeasureTimeShifts::Common(common_shift.unwrap())) - } else if named_shift.is_some() { - Some(MeasureTimeShifts::Named(named_shift.unwrap())) + } else if let Some(cs) = common_shift { + Some(MeasureTimeShifts::Common(cs)) + } else if let Some(ns) = named_shift { + Some(MeasureTimeShifts::Named(ns)) } else { Some(MeasureTimeShifts::Dimensions( shifts.into_values().collect_vec(), From cc4d0e51d71fa4cc0dce3fa12514769b95d34bfd Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Fri, 11 Jul 2025 22:36:33 +0300 Subject: [PATCH 7/8] update snapshots --- .../test/unit/__snapshots__/schema.test.ts.snap | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/cubejs-schema-compiler/test/unit/__snapshots__/schema.test.ts.snap b/packages/cubejs-schema-compiler/test/unit/__snapshots__/schema.test.ts.snap index c33f7d388ad7b..8debafbcf967c 100644 --- a/packages/cubejs-schema-compiler/test/unit/__snapshots__/schema.test.ts.snap +++ b/packages/cubejs-schema-compiler/test/unit/__snapshots__/schema.test.ts.snap @@ -1892,6 +1892,7 @@ Object { "timeShiftReferences": Array [ Object { "interval": "1 year", + "name": undefined, "timeDimension": "createdAt", "type": "prior", }, From 9617c3a0703ce698c082b9efeeee674819f19232 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Mon, 14 Jul 2025 14:34:52 +0300 Subject: [PATCH 8/8] add tests for named time shifts --- .../integration/postgres/calendars.test.ts | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/packages/cubejs-schema-compiler/test/integration/postgres/calendars.test.ts b/packages/cubejs-schema-compiler/test/integration/postgres/calendars.test.ts index ba2f99d4aa878..8e7ca506a4166 100644 --- a/packages/cubejs-schema-compiler/test/integration/postgres/calendars.test.ts +++ b/packages/cubejs-schema-compiler/test/integration/postgres/calendars.test.ts @@ -73,6 +73,20 @@ cubes: - interval: 1 year type: prior + - name: count_shifted_y_named + type: number + multi_stage: true + sql: "{count}" + time_shift: + - name: one_year + + - name: count_shifted_y1d_named + type: number + multi_stage: true + sql: "{count}" + time_shift: + - name: one_year_and_one_day + - name: count_shifted_calendar_m type: number multi_stage: true @@ -244,6 +258,12 @@ cubes: type: prior sql: "{CUBE}.retail_date_prev_year" + - name: one_year + sql: "{CUBE}.retail_date_prev_year" + + - name: one_year_and_one_day + sql: "({CUBE}.retail_date_prev_year + interval '1 day')" + ##### Retail Dates #### - name: retail_date sql: retail_date @@ -282,6 +302,12 @@ cubes: type: prior sql: "{CUBE}.retail_date_prev_year" + - name: one_year + sql: "{CUBE}.retail_date_prev_year" + + - name: one_year_and_one_day + sql: "({CUBE}.retail_date_prev_year + interval '1 day')" + - name: retail_year sql: "{CUBE}.retail_year_name" type: string @@ -576,6 +602,22 @@ cubes: }, ])); + it('Count shifted by retail year (custom named shift + custom granularity)', async () => runQueryTest({ + measures: ['calendar_orders.count', 'calendar_orders.count_shifted_y_named'], + timeDimensions: [{ + dimension: 'custom_calendar.retail_date', + granularity: 'year', + dateRange: ['2025-02-02', '2026-02-01'] + }], + order: [{ id: 'custom_calendar.retail_date' }] + }, [ + { + calendar_orders__count: '37', + calendar_orders__count_shifted_y_named: '39', + custom_calendar__retail_date_year: '2025-02-02T00:00:00.000Z', + }, + ])); + it('Count shifted by retail month (custom shift + common granularity)', async () => runQueryTest({ measures: ['calendar_orders.count', 'calendar_orders.count_shifted_calendar_m'], timeDimensions: [{ @@ -677,6 +719,23 @@ cubes: custom_calendar__retail_date_week: '2025-04-06T00:00:00.000Z', }, ])); + + it('Count shifted by retail year and another custom calendar year (2 custom named shifts + custom granularity)', async () => runQueryTest({ + measures: ['calendar_orders.count', 'calendar_orders.count_shifted_y_named', 'calendar_orders.count_shifted_y1d_named'], + timeDimensions: [{ + dimension: 'custom_calendar.retail_date', + granularity: 'year', + dateRange: ['2025-02-02', '2026-02-01'] + }], + order: [{ id: 'custom_calendar.retail_date' }] + }, [ + { + calendar_orders__count: '37', + calendar_orders__count_shifted_y_named: '39', + calendar_orders__count_shifted_y1d_named: '39', + custom_calendar__retail_date_year: '2025-02-02T00:00:00.000Z', + }, + ])); }); describe('PK dimension time-shifts', () => { @@ -696,6 +755,22 @@ cubes: }, ])); + it.skip('Count shifted by retail year (custom named shift + custom granularity)1', async () => runQueryTest({ + measures: ['calendar_orders.count', 'calendar_orders.count_shifted_y_named'], + timeDimensions: [{ + dimension: 'custom_calendar.date_val', + granularity: 'year', + dateRange: ['2025-02-02', '2026-02-01'] + }], + order: [{ id: 'custom_calendar.date_val' }] + }, [ + { + calendar_orders__count: '37', + calendar_orders__count_shifted_y_named: '39', + custom_calendar__date_val_year: '2025-02-02T00:00:00.000Z', + }, + ])); + it.skip('Count shifted by retail month (custom shift + common granularity)', async () => runQueryTest({ measures: ['calendar_orders.count', 'calendar_orders.count_shifted_calendar_m'], timeDimensions: [{ @@ -797,6 +872,23 @@ cubes: custom_calendar__date_val_week: '2025-04-06T00:00:00.000Z', }, ])); + + it.skip('Count shifted by retail year and another custom calendar year (2 custom named shifts + custom granularity)', async () => runQueryTest({ + measures: ['calendar_orders.count', 'calendar_orders.count_shifted_y_named', 'calendar_orders.count_shifted_y1d_named'], + timeDimensions: [{ + dimension: 'custom_calendar.date_val', + granularity: 'year', + dateRange: ['2025-02-02', '2026-02-01'] + }], + order: [{ id: 'custom_calendar.date_val' }] + }, [ + { + calendar_orders__count: '37', + calendar_orders__count_shifted_y_named: '39', + calendar_orders__count_shifted_y1d_named: '39', + custom_calendar__date_val_year: '2025-02-02T00:00:00.000Z', + }, + ])); }); }); });