diff --git a/docs-mintlify/reference/data-modeling/context-variables.mdx b/docs-mintlify/reference/data-modeling/context-variables.mdx index 2c607b67035d1..87f0a0b164b99 100644 --- a/docs-mintlify/reference/data-modeling/context-variables.mdx +++ b/docs-mintlify/reference/data-modeling/context-variables.mdx @@ -392,6 +392,85 @@ binding always renders as `1 = 1`. +### Time-shifted measures + +A [`multi_stage` measure with a `time_shift`][ref-measures-time-shift] plans each +shift as its own query stage, and every stage re-reads the cube's `sql`. The +pushed-down column has to carry the same shift as the stage around it, otherwise +it filters the source rows by the unshifted range while the stage reads the +shifted one, and the stage comes back empty. + +A shift declared as an `interval` is applied to the column for you. A shift +declared on a [calendar cube][ref-calendar-cubes] with its own `sql` cannot be: +it maps through a column on the calendar rather than offsetting the date, and +that column is not in scope inside the cube's `sql`. Passing the column as a +string in that case fails with a planning error naming the shift. + +Pass a function instead. It receives the query's own filter values, so it can +widen the pushed-down range to cover the periods the shifted stages read. + +`weekly_margin` joins a calendar cube whose `calendar_d` declares the +`prior_fiscal_year` shift, so the shifted stage reaches fact rows through that +mapping — which is why the band has to be widened by hand. The bracketing is +what carries across data sources; the date arithmetic is BigQuery syntax, so +adjust it for yours — here a fiscal prior year, bracketed at 371 and 364 days: + + + +```yaml title="YAML" +cubes: + - name: weekly_margin + sql: | + SELECT * + FROM weekly_margin + WHERE {FILTER_PARAMS.retail_calendar.calendar_d.filter( + lambda x, y: f""" + ((week_end_d >= DATE(TIMESTAMP({x})) + AND week_end_d <= DATE(TIMESTAMP({y}))) + OR (week_end_d >= DATE_SUB(DATE(TIMESTAMP({x})), INTERVAL 371 DAY) + AND week_end_d <= DATE_SUB(DATE(TIMESTAMP({y})), INTERVAL 364 DAY))) + """ + )} + + joins: + - name: retail_calendar + sql: "{CUBE}.week_end_d = {retail_calendar.calendar_d}" + relationship: many_to_one +``` + +```javascript title="JavaScript" +cube(`weekly_margin`, { + sql: ` + SELECT * + FROM weekly_margin + WHERE ${FILTER_PARAMS.retail_calendar.calendar_d.filter( + (x, y) => ` + ((week_end_d >= DATE(TIMESTAMP(${x})) + AND week_end_d <= DATE(TIMESTAMP(${y}))) + OR (week_end_d >= DATE_SUB(DATE(TIMESTAMP(${x})), INTERVAL 371 DAY) + AND week_end_d <= DATE_SUB(DATE(TIMESTAMP(${y})), INTERVAL 364 DAY))) + ` + )} + `, + + joins: { + retail_calendar: { + sql: `${CUBE}.week_end_d = ${retail_calendar.calendar_d}`, + relationship: `many_to_one` + } + } +}) +``` + + + +The values arrive as bare query parameters, so they are cast before use; and the +band is parenthesised as a whole because it is an `OR`, without which appending +another condition to the `WHERE` would bind it to the second branch alone. + +Every stage then scans every band, so keep the bands as narrow as the calendar +actually requires. + ## `FILTER_GROUP` If you use `FILTER_PARAMS` in your query more than once, you must wrap them @@ -879,4 +958,6 @@ cube(`orders`, { [ref-filter-boolean]: /reference/core-data-apis/rest-api/query-format#boolean-logical-operators [ref-links]: /reference/data-modeling/dimensions#links [ref-ref-segments]: /reference/data-modeling/segments -[ref-env-tesseract]: /reference/configuration/environment-variables#cubejs_tesseract_sql_planner \ No newline at end of file +[ref-env-tesseract]: /reference/configuration/environment-variables#cubejs_tesseract_sql_planner +[ref-measures-time-shift]: /reference/data-modeling/measures#time_shift +[ref-calendar-cubes]: /docs/data-modeling/concepts/calendar-cubes \ No newline at end of file diff --git a/packages/cubejs-schema-compiler/test/unit/filter-params-calendar-time-shift.test.ts b/packages/cubejs-schema-compiler/test/unit/filter-params-calendar-time-shift.test.ts new file mode 100644 index 0000000000000..012273a82cab8 --- /dev/null +++ b/packages/cubejs-schema-compiler/test/unit/filter-params-calendar-time-shift.test.ts @@ -0,0 +1,118 @@ +import { PostgresQuery } from '../../src/adapter/PostgresQuery'; +import { prepareYamlCompiler } from './PrepareCompiler'; + +// A calendar shift declared with `sql` maps through a column on the calendar +// (`prior_fiscal_year` -> `next_fiscal_year_d`) rather than offsetting the date, +// so a string FILTER_PARAMS column is rejected and a callback — handed the +// query's own bounds — is not. Interval-declared shifts are offset onto the +// column instead; that shape is pinned in the planner suite. +const model = (filterParams: string) => ` +cubes: + - name: fpc_calendar + calendar: true + sql: > + SELECT '2026-06-20'::date AS calendar_d, + '2025-06-21'::date AS next_fiscal_year_d, + '2024-06-22'::date AS next_two_fiscal_year_d + dimensions: + - name: calendar_d + sql: calendar_d + type: time + primary_key: true + time_shift: + - name: prior_fiscal_year + sql: "{CUBE}.next_fiscal_year_d" + - name: prior_two_fiscal_year + sql: "{CUBE}.next_two_fiscal_year_d" + + - name: fpc_margin + sql: > + SELECT * FROM fpc_margin WHERE ${filterParams} + joins: + - name: fpc_calendar + sql: "{CUBE}.week_end_d = {fpc_calendar.calendar_d}" + relationship: many_to_one + dimensions: + - name: id + sql: id + type: number + primary_key: true + - name: week_end_d + sql: week_end_d + type: time + measures: + - name: net_sales + sql: net_sales_a + type: sum + - name: net_sales_ly + multi_stage: true + sql: "{net_sales}" + type: number + time_shift: + - name: prior_fiscal_year + - name: net_sales_ly2 + multi_stage: true + sql: "{net_sales}" + type: number + time_shift: + - name: prior_two_fiscal_year +`; + +const STRING_COLUMN = '{FILTER_PARAMS.fpc_calendar.calendar_d.filter(\'week_end_d\')}'; + +// The band a model writes by hand once it knows the shifted periods it has to +// cover. 371/364 days back brackets the fiscal prior year. YAML `.filter()` +// bodies are Python, so this is a lambda — the same form the reporting models +// that hit this use. +const CALLBACK_COLUMN = '{FILTER_PARAMS.fpc_calendar.calendar_d.filter(' + + 'lambda x, y: f"(week_end_d >= {x} AND week_end_d <= {y}) ' + + 'OR (week_end_d >= {x}::timestamptz - interval \'371 day\' ' + + 'AND week_end_d <= {y}::timestamptz - interval \'364 day\')")}'; + +async function buildSql(filterParams: string): Promise<[string, unknown[]]> { + const { compiler, joinGraph, cubeEvaluator } = prepareYamlCompiler(model(filterParams)); + await compiler.compile(); + + return new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['fpc_margin.net_sales_ly', 'fpc_margin.net_sales_ly2'], + timeDimensions: [{ + dimension: 'fpc_calendar.calendar_d', + dateRange: ['2026-06-20', '2026-06-20'], + }], + timezone: 'UTC', + // Named calendar time shifts are planned by Tesseract only. + useNativeSqlPlanner: true, + }).buildSqlAndParams(); +} + +describe('FILTER_PARAMS under a named calendar time shift', () => { + // Before this was rejected the column rendered bare and was bound to the + // unshifted reporting bounds in every stage. Each stage joins the calendar on + // its own mapping column, so the pushed-down predicate contradicted the stage + // around it and the stage came back empty — the same failure CORE-543 fixed + // for interval shifts, reached by a different route. + it('rejects a string column', async () => { + await expect(buildSql(STRING_COLUMN)).rejects.toThrow( + /fpc_calendar\.calendar_d.*prior_fiscal_year.*callback/s + ); + }); + + // A callback column is handed the query's bounds and decides the range + // itself, so it is left alone. This is what a model widened by hand relies + // on, and it must keep working. + it('pushes a callback column into every shifted stage', async () => { + const [sql] = await buildSql(CALLBACK_COLUMN); + + expect(sql.match(/week_end_d >=/g)).toHaveLength(4); + expect(sql).not.toContain('FROM fpc_margin WHERE (1 = 1)'); + }); + + // The stage predicate is what carries the shift: each stage compares the + // calendar's own mapping column, not `calendar_d`. + it('filters each stage on its own mapping column', async () => { + const [sql] = await buildSql(CALLBACK_COLUMN); + + expect(sql).toContain('next_fiscal_year_d >='); + expect(sql).toContain('next_two_fiscal_year_d >='); + }); +}); diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/base_filter.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/base_filter.rs index 6a7f307b78446..cf6f1c5176b55 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/base_filter.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/base_filter.rs @@ -1,4 +1,5 @@ use super::ToSql; +use crate::physical_plan::filter::typed_filter::FilterParamsTimeShift; use crate::physical_plan::sql_nodes::SqlNode; use crate::physical_plan::SqlEvaluatorVisitor; use crate::planner::filter::typed_filter::resolve_base_symbol; @@ -25,10 +26,37 @@ impl ToSql for BaseFilter { .filter_params_columns .get(&symbol_to_match.full_name()) { + // Both shift kinds: `extract_time_shifts` routes calendar + // shifts to their own map, so `time_shifts` alone misses them. let time_shift = visitor .time_shifts() .get_for_symbol(&symbol_to_match) - .and_then(|shift| shift.interval.as_ref()); + .and_then(|shift| shift.interval.as_ref()) + .map(FilterParamsTimeShift::Interval) + .or_else(|| { + // Keyed by the calendar cube's PK, so a filter on a + // non-PK dimension of that calendar misses the bare + // name and needs the PK probe. A miss renders the + // column bare, which is the silent failure this whole + // path exists to avoid. + // + // A filter on a NON-calendar cube's dimension is out of + // reach either way: `time_shift_pk_full_name` is only + // populated for calendar-cube dimensions, and a named + // shift on such a dimension is dropped by + // `extract_time_shifts` before it gets here. + let calendar_shifts = visitor.calendar_time_shifts(); + calendar_shifts + .get(&symbol_to_match.full_name()) + .or_else(|| { + let pk = symbol_to_match + .as_dimension() + .ok()? + .time_shift_pk_full_name()?; + calendar_shifts.get(&pk) + }) + .map(FilterParamsTimeShift::Calendar) + }); return self.typed_filter().to_sql_for_filter_params( filter_params_item, time_shift, diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/typed_filter.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/typed_filter.rs index 237608a105a94..3d914997a1765 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/typed_filter.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/typed_filter.rs @@ -7,6 +7,7 @@ use crate::planner::filter::typed_filter::{resolve_base_symbol, FilterOp, TypedF use crate::planner::query_tools::QueryTools; use crate::planner::sql_call::SqlCallFilterParamsItem; use crate::planner::sql_templates::PlanSqlTemplates; +use crate::planner::symbols::CalendarDimensionTimeShift; use crate::planner::FiltersContext; use crate::planner::SqlInterval; use cubenativeutils::CubeError; @@ -46,11 +47,21 @@ impl ToSql for TypedFilter { } } +/// The time shift active on the stage a FILTER_PARAMS column is rendered into. +/// An interval is arithmetic the column can carry. A calendar shift can be +/// either: declared as an interval it is arithmetic too, but declared with +/// `sql` it resolves through a mapping column on the calendar cube, which has +/// no expression over the fact's own column. +pub enum FilterParamsTimeShift<'a> { + Interval(&'a SqlInterval), + Calendar(&'a CalendarDimensionTimeShift), +} + impl TypedFilter { pub fn to_sql_for_filter_params( &self, item: &SqlCallFilterParamsItem, - time_shift: Option<&SqlInterval>, + time_shift: Option>, visitor: &SqlEvaluatorVisitor, node_processor: Rc, query_tools: &Rc, @@ -66,15 +77,55 @@ impl TypedFilter { // current-period bounds contradict the shifted predicate and empty // the CTE. let shifted_column; - let member_sql = if let Some(interval) = time_shift { - shifted_column = format!( - "({})", - plan_templates - .add_timestamp_interval(column_sql.clone(), interval.to_sql())? - ); - shifted_column.as_str() - } else { - column_sql.as_str() + let member_sql = match &time_shift { + Some(FilterParamsTimeShift::Interval(interval)) => { + shifted_column = format!( + "({})", + plan_templates + .add_timestamp_interval(column_sql.clone(), interval.to_sql())? + ); + shifted_column.as_str() + } + // The inverse is not symmetric with the arm above: + // `extract_time_shifts` inverts before storing into + // `TimeShiftState`, while the calendar map keeps the + // declaration as written and the calendar node inverts at + // render. Taking `to_sql()` directly here would shift the + // pushed-down bounds the wrong way. + Some(FilterParamsTimeShift::Calendar(shift)) if shift.sql.is_none() => { + match &shift.interval { + Some(interval) => { + shifted_column = format!( + "({})", + plan_templates.add_timestamp_interval( + column_sql.clone(), + interval.inverse().to_sql() + )? + ); + shifted_column.as_str() + } + // The calendar node renders the dimension unshifted + // too, so there is nothing to carry. + None => column_sql.as_str(), + } + } + Some(FilterParamsTimeShift::Calendar(shift)) => { + return Err(CubeError::user(format!( + concat!( + "FILTER_PARAMS column for `{}` is a string, so it cannot carry ", + "the `{}` calendar time shift this query applies: that shift ", + "resolves through a mapping column on the calendar cube rather ", + "than offsetting the column, and no expression over `{}` stands ", + "for it. Pass the column as a callback instead - it receives the ", + "query's own bounds and can widen the pushed-down range to cover ", + "the shifted periods." + ), + item.filter_symbol_name, + shift.name.as_deref().unwrap_or(""), + column_sql, + ))); + } + None => column_sql.as_str(), }; let ctx = FilterSqlContext::new( member_sql, @@ -86,7 +137,12 @@ impl TypedFilter { dispatch_to_sql(self.operation(), &ctx) } FilterParamsColumn::Compiled(compiled) => { - if time_shift.is_some() { + // An INTERVAL shift is carried by offsetting the column, which + // only a string column allows. A named calendar shift is carried + // by the callback itself: it is handed the query's own bounds and + // decides what range around them to push down, which is the only + // place that mapping can be expressed. + if matches!(time_shift, Some(FilterParamsTimeShift::Interval(_))) { return Err(CubeError::user(format!( "FILTER_PARAMS column for `{}` is a callback, which cannot carry the time \ shift the surrounding query applies; pass the column as a string instead", diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/sql_nodes/factory.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/sql_nodes/factory.rs index 92d22405403cd..8cd1a0937fe59 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/sql_nodes/factory.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/sql_nodes/factory.rs @@ -49,6 +49,10 @@ impl SqlNodesFactory { &self.time_shifts } + pub fn calendar_time_shifts(&self) -> &HashMap { + &self.calendar_time_shifts + } + pub fn set_calendar_time_shifts( &mut self, calendar_time_shifts: HashMap, diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/sql_visitor.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/sql_visitor.rs index d8e82c30ea280..76c04a9e2e567 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/sql_visitor.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/sql_visitor.rs @@ -5,8 +5,10 @@ use crate::planner::planners::multi_stage::TimeShiftState; use crate::planner::query_tools::QueryTools; use crate::planner::sql_call::CubeRef; use crate::planner::sql_templates::PlanSqlTemplates; +use crate::planner::symbols::CalendarDimensionTimeShift; use crate::planner::MemberSymbol; use cubenativeutils::CubeError; +use std::collections::HashMap; use std::rc::Rc; #[derive(Clone)] @@ -17,6 +19,11 @@ pub struct SqlEvaluatorVisitor { /// Active per-dimension time shifts, carried so that FILTER_PARAMS rendering /// can apply the same shift to its column as the regular filter rendering. time_shifts: TimeShiftState, + /// Active calendar-cube time shifts, keyed by the shifted dimension's full + /// name. Carried for the same reason as `time_shifts`, but these cannot be + /// applied to a FILTER_PARAMS column - they resolve through the calendar + /// rather than offsetting - so the rendering path rejects them instead. + calendar_time_shifts: HashMap, ignore_tz_convert: bool, /// When `true`, the caller (typically a `SqlCall` substitution site) expects /// the rendered expression to be safe for embedding next to operators — @@ -35,6 +42,7 @@ impl SqlEvaluatorVisitor { cube_ref_evaluator, all_filters, time_shifts: TimeShiftState::default(), + calendar_time_shifts: HashMap::new(), ignore_tz_convert: false, arg_needs_paren_safe: false, } @@ -46,10 +54,23 @@ impl SqlEvaluatorVisitor { self_copy } + pub fn with_calendar_time_shifts( + &self, + calendar_time_shifts: HashMap, + ) -> Self { + let mut self_copy = self.clone(); + self_copy.calendar_time_shifts = calendar_time_shifts; + self_copy + } + pub fn time_shifts(&self) -> &TimeShiftState { &self.time_shifts } + pub fn calendar_time_shifts(&self) -> &HashMap { + &self.calendar_time_shifts + } + pub fn with_ignore_tz_convert(&self) -> Self { let mut self_copy = self.clone(); self_copy.ignore_tz_convert = true; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/visitor_context.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/visitor_context.rs index 1a76613fa0cef..bf8b03d983864 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/visitor_context.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/visitor_context.rs @@ -5,6 +5,7 @@ use crate::planner::filter::Filter; use crate::planner::planners::multi_stage::TimeShiftState; use crate::planner::query_tools::QueryTools; use crate::planner::sql_templates::PlanSqlTemplates; +use crate::planner::symbols::CalendarDimensionTimeShift; use crate::planner::FiltersContext; use crate::planner::{MemberSymbol, SqlCall}; use cubenativeutils::CubeError; @@ -17,6 +18,10 @@ pub struct VisitorContext { cube_ref_evaluator: Rc, all_filters: Option, //To pass to FILTER_PARAMS and FILTER_GROUP time_shifts: TimeShiftState, //To pass to FILTER_PARAMS in time-shifted CTEs + /// Calendar-cube shifts active on this CTE, carried alongside `time_shifts` + /// so FILTER_PARAMS rendering can tell a shift it cannot express from no + /// shift at all. + calendar_time_shifts: HashMap, filters_context: FiltersContext, } @@ -38,6 +43,7 @@ impl VisitorContext { cube_ref_evaluator: Rc::new(nodes_factory.cube_ref_evaluator()), all_filters, time_shifts: nodes_factory.time_shifts().clone(), + calendar_time_shifts: nodes_factory.calendar_time_shifts().clone(), filters_context, } } @@ -47,6 +53,7 @@ impl VisitorContext { nodes_factory: &SqlNodesFactory, filter_params_columns: HashMap, time_shifts: TimeShiftState, + calendar_time_shifts: HashMap, ) -> Self { let filters_context = FiltersContext { use_local_tz: nodes_factory.use_local_tz_in_date_range(), @@ -60,6 +67,7 @@ impl VisitorContext { cube_ref_evaluator: Rc::new(nodes_factory.cube_ref_evaluator()), all_filters: None, time_shifts, + calendar_time_shifts, filters_context, } } @@ -71,6 +79,7 @@ impl VisitorContext { self.all_filters.clone(), ) .with_time_shifts(self.time_shifts.clone()) + .with_calendar_time_shifts(self.calendar_time_shifts.clone()) } pub fn node_processor(&self) -> Rc { diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call.rs index af0478eca5500..0b3ac1e788fad 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call.rs @@ -532,6 +532,7 @@ impl SqlCall { &SqlNodesFactory::new(), filter_params_columns, visitor.time_shifts().clone(), + visitor.calendar_time_shifts().clone(), ); return crate::physical_plan::filter::render_filter_item( &context, &subtree, templates, diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter_params_calendar_time_shift.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter_params_calendar_time_shift.rs new file mode 100644 index 0000000000000..0d2d0799e112f --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter_params_calendar_time_shift.rs @@ -0,0 +1,313 @@ +use crate::test_fixtures::cube_bridge::MockSchema; +use crate::test_fixtures::test_utils::TestContext; +use indoc::indoc; + +// A calendar shift declared with `sql` resolves through a mapping column on the +// calendar (`prior_fiscal_year` -> `next_fiscal_year_d`), not an offset, so a +// string FILTER_PARAMS column is rejected and a callback - handed the query's +// own bounds - is not. Interval-declared shifts are arithmetic and are offset +// onto the column; see `interval_declared_calendar_shift_offsets_the_column`. +fn schema(margin_sql: &str) -> MockSchema { + MockSchema::from_yaml(&format!( + indoc! {" + cubes: + - name: fpc_calendar + calendar: true + sql: \"SELECT * FROM fpc_calendar\" + dimensions: + - name: calendar_d + type: time + sql: calendar_d + primary_key: true + time_shift: + - name: prior_fiscal_year + sql: \"{{CUBE}}.next_fiscal_year_d\" + - name: prior_two_fiscal_year + sql: \"{{CUBE}}.next_two_fiscal_year_d\" + + - name: fpc_margin + sql: \"{margin_sql}\" + joins: + - name: fpc_calendar + relationship: many_to_one + sql: \"{{fpc_margin}}.week_end_d = {{fpc_calendar.calendar_d}}\" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: week_end_d + type: time + sql: week_end_d + measures: + - name: net_sales + type: sum + sql: net_sales_a + + - name: net_sales_ly + type: number + sql: \"{{CUBE.net_sales}}\" + multi_stage: true + time_shift: + - name: prior_fiscal_year + + - name: net_sales_ly2 + type: number + sql: \"{{CUBE.net_sales}}\" + multi_stage: true + time_shift: + - name: prior_two_fiscal_year + "}, + margin_sql = margin_sql + )) + .unwrap() +} + +const STRING_COLUMN: &str = + "SELECT * FROM fpc_margin WHERE {FILTER_PARAMS_COLUMN:fpc_calendar.calendar_d:week_end_d}"; + +// The band a model writes by hand once it knows the shifted periods it has to +// cover - the callback form, which receives the query's own bounds as %0/%1. +const CALLBACK_COLUMN: &str = concat!( + "SELECT * FROM fpc_margin WHERE ", + "{FILTER_PARAMS:fpc_calendar.calendar_d:", + "(week_end_d >= %0 AND week_end_d <= %1) OR ", + "(week_end_d >= %0 - interval '371 day' AND week_end_d <= %1 - interval '364 day')}" +); + +fn build(margin_sql: &str) -> Result { + let ctx = TestContext::new(schema(margin_sql)).unwrap(); + + ctx.build_sql_and_params(indoc! {" + measures: + - fpc_margin.net_sales_ly + - fpc_margin.net_sales_ly2 + time_dimensions: + - dimension: fpc_calendar.calendar_d + dateRange: + - \"2026-06-20\" + - \"2026-06-20\" + "}) + .map(|(sql, _)| sql) +} + +// Before this was rejected, the column rendered bare and was bound to the +// unshifted reporting bounds in every stage. Each stage joins the calendar on +// its own mapping column, so the pushed-down predicate contradicted the stage +// around it and the stage came back empty - the same failure CORE-543 fixed for +// interval shifts, reached by a different route. +#[test] +fn string_column_is_rejected_under_a_named_calendar_shift() { + let err = build(STRING_COLUMN).expect_err("a string column cannot carry a calendar shift"); + let message = err.to_string(); + + assert!( + message.contains("fpc_calendar.calendar_d") && message.contains("prior_fiscal_year"), + "the error must name the binding and the shift it cannot carry\nerror: {}", + message + ); + assert!( + message.contains("callback"), + "the error must point at the form that can carry it\nerror: {}", + message + ); +} + +// A callback column is handed the query's bounds and decides the range itself, +// so it is left alone. This is what a model widened by hand relies on, and it +// must keep working. +#[test] +fn callback_column_is_pushed_into_every_shifted_stage() { + let sql = build(CALLBACK_COLUMN).expect("a callback column carries its own band"); + + assert_eq!( + sql.matches("week_end_d >=").count(), + 4, + "both bands must render in both shifted stages\nsql: {}", + sql + ); + assert!( + !sql.contains("FROM fpc_margin WHERE (1 = 1)"), + "no stage may be left scanning the fact unfiltered\nsql: {}", + sql + ); +} + +// The stage predicate is what carries the shift: each stage compares the +// calendar's own mapping column, not `calendar_d`. +#[test] +fn stage_predicate_uses_the_named_mapping_column() { + let sql = build(CALLBACK_COLUMN).unwrap(); + + assert!( + sql.contains("next_fiscal_year_d"), + "the prior-fiscal-year stage must filter on its mapping column\nsql: {}", + sql + ); + assert!( + sql.contains("next_two_fiscal_year_d"), + "the prior-two-fiscal-year stage must filter on its mapping column\nsql: {}", + sql + ); +} + +// A calendar cube may declare a shift as a plain interval, with no mapping +// `sql`. That shape is arithmetic, so the column carries it rather than being +// rejected - and the sign has to match what `CalendarTimeShiftSqlNode` renders: +// the calendar map holds the declaration as written and inverts at render, +// unlike `TimeShiftState`, which is stored already inverted. +fn interval_declared_schema() -> MockSchema { + MockSchema::from_yaml(indoc! {" + cubes: + - name: fpi_calendar + calendar: true + sql: \"SELECT * FROM fpi_calendar\" + dimensions: + - name: calendar_d + type: time + sql: calendar_d + primary_key: true + time_shift: + - name: prior_year + interval: \"1 year\" + type: prior + + - name: fpi_margin + sql: \"SELECT * FROM fpi_margin WHERE {FILTER_PARAMS_COLUMN:fpi_calendar.calendar_d:week_end_d}\" + joins: + - name: fpi_calendar + relationship: many_to_one + sql: \"{fpi_margin}.week_end_d = {fpi_calendar.calendar_d}\" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: week_end_d + type: time + sql: week_end_d + measures: + - name: net_sales + type: sum + sql: net_sales_a + + - name: net_sales_ly + type: number + sql: \"{CUBE.net_sales}\" + multi_stage: true + time_shift: + - name: prior_year + "}) + .unwrap() +} + +#[test] +fn interval_declared_calendar_shift_offsets_the_column() { + let ctx = TestContext::new(interval_declared_schema()).unwrap(); + + let (sql, _) = ctx + .build_sql_and_params(indoc! {" + measures: + - fpi_margin.net_sales_ly + time_dimensions: + - dimension: fpi_calendar.calendar_d + dateRange: + - \"2026-06-20\" + - \"2026-06-20\" + "}) + .expect("an interval-declared calendar shift is arithmetic the column can carry"); + + assert!( + sql.contains("(week_end_d + interval '-1 year')"), + "the column must be offset by the inverted declaration, the same way \ + CalendarTimeShiftSqlNode renders the dimension\nsql: {}", + sql + ); +} + +// A calendar cube whose shifted time dimension is NOT its primary key. The +// shift is registered under the calendar's PK (`calendar_time_shift_for_*` +// returns `time_shift_pk_full_name`), while FILTER_PARAMS binds the shifted +// dimension itself - so a lookup on the bound symbol's own name misses and the +// column would render bare against unshifted bounds. +fn non_pk_binding_schema() -> MockSchema { + MockSchema::from_yaml(indoc! {" + cubes: + - name: fpk_calendar + calendar: true + sql: \"SELECT * FROM fpk_calendar\" + dimensions: + - name: date_key + type: time + sql: date_key + primary_key: true + - name: retail_d + type: time + sql: retail_d + time_shift: + - name: prior_fiscal_year + sql: \"{CUBE}.prior_retail_d\" + + - name: fpk_margin + sql: \"SELECT * FROM fpk_margin WHERE {FILTER_PARAMS_COLUMN:fpk_calendar.retail_d:week_end_d}\" + joins: + - name: fpk_calendar + relationship: many_to_one + sql: \"{fpk_margin}.week_end_d = {fpk_calendar.date_key}\" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: week_end_d + type: time + sql: week_end_d + measures: + - name: net_sales + type: sum + sql: net_sales_a + + - name: net_sales_ly + type: number + sql: \"{CUBE.net_sales}\" + multi_stage: true + time_shift: + - name: prior_fiscal_year + "}) + .unwrap() +} + +#[test] +fn shift_is_found_when_the_binding_is_not_the_calendar_pk() { + let ctx = TestContext::new(non_pk_binding_schema()).unwrap(); + + let result = ctx.build_sql_and_params(indoc! {" + measures: + - fpk_margin.net_sales_ly + time_dimensions: + - dimension: fpk_calendar.retail_d + dateRange: + - \"2026-06-20\" + - \"2026-06-20\" + "}); + + match result { + Err(err) => { + let message = err.to_string(); + // Both halves matter: naming the shift alone would also be + // satisfied by a "time shift not found" or a join-resolution + // failure, so pin that this is the rejection and not some other + // planner error that happens to mention it. + assert!( + message.contains("cannot carry") && message.contains("prior_fiscal_year"), + "the shift must be found through the calendar PK and rejected here, \ + not silently missed\nerror: {}", + message + ); + } + Ok((sql, _)) => panic!( + "the binding's shift was missed and the column rendered bare\nsql: {}", + sql + ), + } +} diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs index 5e5ef25d6ad5d..774067aa1650d 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs @@ -6,6 +6,7 @@ mod cube_names_collector; mod date_filters; mod dimension_symbol; mod filter; +mod filter_params_calendar_time_shift; mod filter_params_callback_column; mod filter_params_segment; mod filter_params_time_shift;