Skip to content

Commit 7324166

Browse files
committed
fix recursion for time shifts plain values
1 parent 97007aa commit 7324166

File tree

2 files changed

+19
-38
lines changed

2 files changed

+19
-38
lines changed

rust/cubesqlplanner/cubesqlplanner/src/physical_plan_builder/builder.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,10 @@ impl PhysicalPlanBuilderContext {
4747
dimension.calendar_time_shift_for_interval(&shift.interval)
4848
{
4949
return Either::Right((dim_key.clone(), cts.clone()));
50-
} else if let Some(calendar_pk) = dimension.time_shift_pk() {
50+
} else if let Some(calendar_pk) = dimension.time_shift_pk_full_name() {
5151
let mut shift = shift.clone();
5252
shift.interval = shift.interval.inverse();
53-
return Either::Left((calendar_pk.full_name(), shift.clone()));
53+
return Either::Left((calendar_pk, shift.clone()));
5454
}
5555
}
5656
Either::Left((key.clone(), shift.clone()))

rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/symbols/dimension_symbol.rs

Lines changed: 17 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ pub struct DimensionSymbol {
4242
is_reference: bool, // Symbol is a direct reference to another symbol without any calculations
4343
is_view: bool,
4444
time_shift: Vec<CalendarDimensionTimeShift>,
45-
time_shift_pk: Option<Rc<MemberSymbol>>,
45+
time_shift_pk_full_name: Option<String>,
4646
is_self_time_shift_pk: bool, // If the dimension itself is a primary key and has time shifts,
4747
// we can not reevaluate itself again while processing time shifts
4848
// to avoid infinite recursion. So we raise this flag instead.
@@ -60,7 +60,7 @@ impl DimensionSymbol {
6060
case: Option<DimensionCaseDefinition>,
6161
definition: Rc<dyn DimensionDefinition>,
6262
time_shift: Vec<CalendarDimensionTimeShift>,
63-
time_shift_pk: Option<Rc<MemberSymbol>>,
63+
time_shift_pk_full_name: Option<String>,
6464
is_self_time_shift_pk: bool,
6565
) -> Rc<Self> {
6666
Rc::new(Self {
@@ -74,7 +74,7 @@ impl DimensionSymbol {
7474
case,
7575
is_view,
7676
time_shift,
77-
time_shift_pk,
77+
time_shift_pk_full_name,
7878
is_self_time_shift_pk,
7979
})
8080
}
@@ -117,8 +117,8 @@ impl DimensionSymbol {
117117
&self.time_shift
118118
}
119119

120-
pub fn time_shift_pk(&self) -> Option<Rc<MemberSymbol>> {
121-
self.time_shift_pk.clone()
120+
pub fn time_shift_pk_full_name(&self) -> Option<String> {
121+
self.time_shift_pk_full_name.clone()
122122
}
123123

124124
pub fn full_name(&self) -> String {
@@ -239,8 +239,8 @@ impl DimensionSymbol {
239239
.iter()
240240
.find(|shift| shift.interval == *interval)
241241
{
242-
if let Some(pk) = &self.time_shift_pk {
243-
return Some((pk.full_name(), ts.clone()));
242+
if let Some(pk) = &self.time_shift_pk_full_name {
243+
return Some((pk.clone(), ts.clone()));
244244
} else if self.is_self_time_shift_pk {
245245
return Some((self.full_name(), ts.clone()));
246246
}
@@ -394,7 +394,7 @@ impl SymbolFactory for DimensionSymbolFactory {
394394

395395
// If the cube is a calendar, we need to find the primary key member
396396
// so that we can use it for time shifts processing.
397-
let time_shift_pk = if is_calendar && !time_shift.is_empty() {
397+
let time_shift_pk = if is_calendar {
398398
let pk_members = cube_evaluator
399399
.static_data()
400400
.primary_keys
@@ -404,35 +404,16 @@ impl SymbolFactory for DimensionSymbolFactory {
404404

405405
if pk_members.iter().any(|pk| **pk == name) {
406406
is_self_time_shift_pk = true;
407-
// If the dimension itself is a primary key and has time shifts,
408-
// we can not reevaluate itself again as this will lead
409-
// to infinite recursion. So we raise this flag instead.
410-
None
411-
} else {
412-
let pk_member = pk_members
413-
.into_iter()
414-
.map(|primary_key| -> Result<_, CubeError> {
415-
let key_dimension_name = format!("{}.{}", cube_name, primary_key);
416-
let pk_member = compiler.add_dimension_evaluator(key_dimension_name)?;
417-
418-
Ok(pk_member)
419-
})
420-
.collect::<Result<Vec<_>, _>>()?
421-
.into_iter()
422-
.filter(|pk_member| {
423-
if let Ok(pk_dimension) = pk_member.as_dimension() {
424-
// TODO: What if calendar cube is joined via non-time dimension?
425-
if pk_dimension.dimension_type() == "time" {
426-
return true;
427-
}
428-
}
429-
false
430-
})
431-
.collect::<Vec<_>>()
432-
.first()
433-
.cloned();
434-
pk_member
435407
}
408+
409+
if pk_members.len() > 1 {
410+
return Err(CubeError::user(format!(
411+
"Cube '{}' has multiple primary keys, but only one is allowed for calendar cubes",
412+
cube_name
413+
)));
414+
}
415+
416+
pk_members.first().map(|pk| format!("{}.{}", cube_name, pk))
436417
} else {
437418
None
438419
};

0 commit comments

Comments
 (0)