Skip to content

Commit f9398ab

Browse files
authored
fix(tesseract): apply time_shift to FILTER_PARAMS column in shifted CTEs (cube-js#11191)
1 parent b99d546 commit f9398ab

7 files changed

Lines changed: 124 additions & 1 deletion

File tree

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import { getEnv } from '@cubejs-backend/shared';
2+
import { prepareYamlCompiler } from '../../unit/PrepareCompiler';
3+
import { dbRunner } from './PostgresDBRunner';
4+
5+
// Regression for CORE-543 (GitHub #11030):
6+
// A multi_stage time_shift measure over a cube whose `sql` uses FILTER_PARAMS
7+
// must render the prior-period CTE with the FILTER_PARAMS column shifted by the
8+
// same interval as the time-shift predicate. Otherwise the current-period
9+
// bounds on the un-shifted column contradict the shifted predicate, the
10+
// prior-period CTE is always empty, and the YoY measure collapses to null.
11+
describe('Multi-Stage time_shift + FILTER_PARAMS', () => {
12+
jest.setTimeout(200000);
13+
14+
const { compiler, joinGraph, cubeEvaluator } = prepareYamlCompiler(`
15+
cubes:
16+
- name: orders
17+
sql: >
18+
SELECT * FROM (
19+
SELECT 1 as id, '2024-03-14T00:00:00.000Z'::timestamptz as created_at, 100 as amount
20+
union all
21+
SELECT 2 as id, '2025-03-14T00:00:00.000Z'::timestamptz as created_at, 300 as amount
22+
) AS t
23+
WHERE {FILTER_PARAMS.orders.date.filter('created_at')}
24+
25+
dimensions:
26+
- name: id
27+
sql: id
28+
type: number
29+
primary_key: true
30+
31+
- name: date
32+
sql: created_at
33+
type: time
34+
35+
measures:
36+
- name: revenue
37+
sql: amount
38+
type: sum
39+
40+
- name: revenue_1_y_ago
41+
sql: "{revenue}"
42+
multi_stage: true
43+
type: number
44+
time_shift:
45+
- time_dimension: date
46+
interval: 1 year
47+
type: prior
48+
`);
49+
50+
if (getEnv('nativeSqlPlanner')) {
51+
it('prior-year time_shift measure is not emptied by FILTER_PARAMS', async () => dbRunner.runQueryTest({
52+
measures: ['orders.revenue', 'orders.revenue_1_y_ago'],
53+
timeDimensions: [
54+
{
55+
dimension: 'orders.date',
56+
granularity: 'year',
57+
dateRange: ['2025-01-01', '2025-12-31'],
58+
}
59+
],
60+
timezone: 'UTC',
61+
}, [
62+
{
63+
orders__date_year: '2025-01-01T00:00:00.000Z',
64+
orders__revenue: '300',
65+
orders__revenue_1_y_ago: '100',
66+
},
67+
], { compiler, joinGraph, cubeEvaluator }));
68+
} else {
69+
// This test is working only in tesseract
70+
test.skip('prior-year time_shift measure is not emptied by FILTER_PARAMS', () => { expect(1).toBe(1); });
71+
}
72+
});

rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/base_filter.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,14 @@ impl ToSql for BaseFilter {
2525
.filter_params_columns
2626
.get(&symbol_to_match.full_name())
2727
{
28+
let time_shift = visitor
29+
.time_shifts()
30+
.dimensions_shifts
31+
.get(&symbol_to_match.full_name())
32+
.and_then(|shift| shift.interval.as_ref());
2833
return self.typed_filter().to_sql_for_filter_params(
2934
filter_params_column,
35+
time_shift,
3036
&query_tools,
3137
templates,
3238
filters_ctx,

rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/typed_filter.rs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use crate::planner::filter::typed_filter::{resolve_base_symbol, FilterOp, TypedF
77
use crate::planner::query_tools::QueryTools;
88
use crate::planner::sql_templates::PlanSqlTemplates;
99
use crate::planner::FiltersContext;
10+
use crate::planner::SqlInterval;
1011
use cubenativeutils::CubeError;
1112
use std::rc::Rc;
1213

@@ -48,6 +49,7 @@ impl TypedFilter {
4849
pub fn to_sql_for_filter_params(
4950
&self,
5051
column: &FilterParamsColumn,
52+
time_shift: Option<&SqlInterval>,
5153
query_tools: &Rc<QueryTools>,
5254
plan_templates: &PlanSqlTemplates,
5355
filters_context: &FiltersContext,
@@ -56,8 +58,23 @@ impl TypedFilter {
5658

5759
match column {
5860
FilterParamsColumn::String(column_sql) => {
61+
// Inside a time-shifted CTE the FILTER_PARAMS column must carry the
62+
// same shift as the regular time-dimension filter, otherwise its
63+
// current-period bounds contradict the shifted predicate and empty
64+
// the CTE.
65+
let shifted_column;
66+
let member_sql = if let Some(interval) = time_shift {
67+
shifted_column = format!(
68+
"({})",
69+
plan_templates
70+
.add_timestamp_interval(column_sql.clone(), interval.to_sql())?
71+
);
72+
shifted_column.as_str()
73+
} else {
74+
column_sql.as_str()
75+
};
5976
let ctx = FilterSqlContext {
60-
member_sql: column_sql,
77+
member_sql,
6178
query_tools,
6279
plan_templates,
6380
use_db_time_zone,
@@ -66,6 +83,8 @@ impl TypedFilter {
6683
dispatch_to_sql(self.operation(), &ctx)
6784
}
6885
FilterParamsColumn::Callback(callback) => {
86+
// A callback column is opaque SQL produced by user code, so a
87+
// time shift can't be wrapped around it; it is rendered as-is.
6988
let args = match self.operation() {
7089
// RollingWindowOffset carries [from, to, trailing, leading, offset];
7190
// only the from/to dates are filter-param args for the callback.

rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/sql_nodes/factory.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,10 @@ impl SqlNodesFactory {
5252
self.time_shifts = time_shifts;
5353
}
5454

55+
pub fn time_shifts(&self) -> &TimeShiftState {
56+
&self.time_shifts
57+
}
58+
5559
pub fn set_calendar_time_shifts(
5660
&mut self,
5761
calendar_time_shifts: HashMap<String, CalendarDimensionTimeShift>,

rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/sql_visitor.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use super::sql_nodes::SqlNode;
22
use super::CubeRefEvaluator;
33
use crate::planner::filter::Filter;
4+
use crate::planner::planners::multi_stage::TimeShiftState;
45
use crate::planner::query_tools::QueryTools;
56
use crate::planner::sql_call::CubeRef;
67
use crate::planner::sql_templates::PlanSqlTemplates;
@@ -13,6 +14,9 @@ pub struct SqlEvaluatorVisitor {
1314
query_tools: Rc<QueryTools>,
1415
cube_ref_evaluator: Rc<CubeRefEvaluator>,
1516
all_filters: Option<Filter>, //To pass to FILTER_PARAMS and FILTER_GROUP
17+
/// Active per-dimension time shifts, carried so that FILTER_PARAMS rendering
18+
/// can apply the same shift to its column as the regular filter rendering.
19+
time_shifts: TimeShiftState,
1620
ignore_tz_convert: bool,
1721
/// When `true`, the caller (typically a `SqlCall` substitution site) expects
1822
/// the rendered expression to be safe for embedding next to operators —
@@ -30,11 +34,22 @@ impl SqlEvaluatorVisitor {
3034
query_tools,
3135
cube_ref_evaluator,
3236
all_filters,
37+
time_shifts: TimeShiftState::default(),
3338
ignore_tz_convert: false,
3439
arg_needs_paren_safe: false,
3540
}
3641
}
3742

43+
pub fn with_time_shifts(&self, time_shifts: TimeShiftState) -> Self {
44+
let mut self_copy = self.clone();
45+
self_copy.time_shifts = time_shifts;
46+
self_copy
47+
}
48+
49+
pub fn time_shifts(&self) -> &TimeShiftState {
50+
&self.time_shifts
51+
}
52+
3853
pub fn with_ignore_tz_convert(&self) -> Self {
3954
let mut self_copy = self.clone();
4055
self_copy.ignore_tz_convert = true;

rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/visitor_context.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use crate::physical_plan::cube_ref_evaluator::CubeRefEvaluator;
22
use crate::physical_plan::sql_nodes::{SqlNode, SqlNodesFactory};
33
use crate::physical_plan::sql_visitor::SqlEvaluatorVisitor;
44
use crate::planner::filter::Filter;
5+
use crate::planner::planners::multi_stage::TimeShiftState;
56
use crate::planner::query_tools::QueryTools;
67
use crate::planner::sql_templates::PlanSqlTemplates;
78
use crate::planner::FiltersContext;
@@ -15,6 +16,7 @@ pub struct VisitorContext {
1516
node_processor: Rc<dyn SqlNode>,
1617
cube_ref_evaluator: Rc<CubeRefEvaluator>,
1718
all_filters: Option<Filter>, //To pass to FILTER_PARAMS and FILTER_GROUP
19+
time_shifts: TimeShiftState, //To pass to FILTER_PARAMS in time-shifted CTEs
1820
filters_context: FiltersContext,
1921
}
2022

@@ -35,6 +37,7 @@ impl VisitorContext {
3537
node_processor,
3638
cube_ref_evaluator: Rc::new(nodes_factory.cube_ref_evaluator()),
3739
all_filters,
40+
time_shifts: nodes_factory.time_shifts().clone(),
3841
filters_context,
3942
}
4043
}
@@ -43,6 +46,7 @@ impl VisitorContext {
4346
query_tools: Rc<QueryTools>,
4447
nodes_factory: &SqlNodesFactory,
4548
filter_params_columns: HashMap<String, crate::cube_bridge::member_sql::FilterParamsColumn>,
49+
time_shifts: TimeShiftState,
4650
) -> Self {
4751
let filters_context = FiltersContext {
4852
use_local_tz: nodes_factory.use_local_tz_in_date_range(),
@@ -55,6 +59,7 @@ impl VisitorContext {
5559
node_processor,
5660
cube_ref_evaluator: Rc::new(nodes_factory.cube_ref_evaluator()),
5761
all_filters: None,
62+
time_shifts,
5863
filters_context,
5964
}
6065
}
@@ -65,6 +70,7 @@ impl VisitorContext {
6570
self.cube_ref_evaluator.clone(),
6671
self.all_filters.clone(),
6772
)
73+
.with_time_shifts(self.time_shifts.clone())
6874
}
6975

7076
pub fn node_processor(&self) -> Rc<dyn SqlNode> {

rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,7 @@ impl SqlCall {
461461
query_tools.clone(),
462462
&SqlNodesFactory::new(),
463463
filter_params_columns,
464+
visitor.time_shifts().clone(),
464465
);
465466
return crate::physical_plan::filter::render_filter_item(
466467
&context, &subtree, templates,

0 commit comments

Comments
 (0)