From ebdc76418270408198be91dc222a5134ef7a5063 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 19:48:00 +0000 Subject: [PATCH 01/16] test(tesseract): pin FILTER_PARAMS behaviour under named calendar time shifts filter_params_time_shift.rs covers an INTERVAL time shift and asserts the pushed-down FILTER_PARAMS column is offset by the same interval, so a shifted stage scans the rows it groups by. A NAMED calendar shift - the form a retail 4-5-4 calendar forces, where "one fiscal year back" is a mapping column rather than an interval - gets no such treatment. base_filter.rs resolves the shift as `time_shifts().get_for_symbol(sym).and_then(|s| s.interval.as_ref())`, and a named calendar shift carries `interval: None`, so `to_sql_for_filter_params` receives `None` and renders the column bare against the unshifted reporting bounds. The binding still matches in every stage (the shift substitutes the column only where the stage's own predicate renders), so the fact scan is never left unfiltered. But each stage joins the calendar on its mapping column while the pushed-down predicate restricts the same scan to the reporting period, so the stage is empty unless the model widens the band by hand - and once it does, every stage scans every band. These tests characterise the current behaviour so the gap is visible and a fix has something to invert. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EwtxWoLUp7ULnsQJjVLkZt --- .../filter_params_calendar_time_shift.rs | 173 ++++++++++++++++++ .../cubesqlplanner/src/tests/mod.rs | 1 + 2 files changed, 174 insertions(+) create mode 100644 rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter_params_calendar_time_shift.rs 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..07a824f31ab18 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter_params_calendar_time_shift.rs @@ -0,0 +1,173 @@ +use crate::cube_bridge::base_query_options::FilterValue; +use crate::test_fixtures::cube_bridge::MockSchema; +use crate::test_fixtures::test_utils::TestContext; +use indoc::indoc; + +// A fact whose `sql` pushes a joined CALENDAR cube's date down through +// FILTER_PARAMS, with multi-stage measures that shift that date by NAME rather +// than by interval - the shape a retail 4-5-4 calendar forces, where "one +// fiscal year back" is a mapping column on the calendar and not an interval any +// arithmetic can express. +// +// Contrast with filter_params_time_shift.rs, which covers the interval form. +// The two are handled by the same call site but only one of them can produce a +// shift: see `base_filter.rs`, where the shift handed to +// `to_sql_for_filter_params` is `…get_for_symbol(sym).and_then(|s| s.interval.as_ref())`. +// A named calendar shift carries `interval: None`, so that resolves to `None` +// and the pushed-down column is rendered bare. +fn schema() -> MockSchema { + MockSchema::from_yaml(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: \"SELECT * FROM fpc_margin WHERE {FILTER_PARAMS_COLUMN:fpc_calendar.calendar_d:week_end_d}\" + 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 + "}) + .unwrap() +} + +fn shifted_stages() -> (String, Vec) { + let ctx = TestContext::new(schema()).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\" + "}) + .unwrap() +} + +fn shifted_stages_sql() -> String { + shifted_stages().0 +} + +// Each named shift builds its own stage, and each stage rescans the fact. The +// binding stays attributed to `fpc_calendar.calendar_d` - the shift substitutes +// the column only where the stage's own predicate renders - so FILTER_PARAMS +// keeps matching and the fact scan is never left unfiltered. +#[test] +fn pushed_down_column_reaches_every_named_shift_stage() { + let sql = shifted_stages_sql(); + + assert_eq!( + sql.matches("FROM fpc_margin WHERE (week_end_d >=").count(), + 2, + "both named-shift stages must push the column down\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 mapping column, not `calendar_d`. +#[test] +fn stage_predicate_uses_the_named_mapping_column() { + let sql = shifted_stages_sql(); + + 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 + ); +} + +// The gap this file exists to pin down. +// +// `filter_params_time_shift.rs` asserts that an INTERVAL shift offsets the +// pushed-down column, so the rows a shifted stage scans line up with the bounds +// that stage groups by. A NAMED calendar shift gets no such treatment: the +// column is rendered bare and bound to the UNSHIFTED reporting bounds, in every +// stage. +// +// That is a contradiction inside each stage. `cte_0` joins the calendar on +// `week_end_d = next_fiscal_year_d`, so it reads PRIOR-year fact rows - while +// the pushed-down predicate restricts the same scan to the REPORTING week. The +// stage is empty unless the model widens the pushed-down band by hand to cover +// the shifted periods, and once it does, every stage scans every band. +#[test] +fn named_shift_binds_the_pushed_down_column_to_unshifted_bounds() { + let (sql, params) = shifted_stages(); + + assert!( + !sql.contains("week_end_d + interval"), + "a named calendar shift currently cannot offset the pushed-down column; \ + if this now passes, base_filter.rs learned to carry non-interval shifts \ + and this test should be inverted\nsql: {}", + sql + ); + + // Eight bounds: a pushed-down pair and a stage pair per shifted stage. Every + // one of them is the reporting day, so nothing distinguishes the scan of the + // prior-fiscal-year stage from the scan of the two-year one. + let bounds: Vec = params + .iter() + .map(|p| match p { + FilterValue::Str(s) => s.clone(), + other => panic!("unexpected bound {:?}", other), + }) + .collect(); + assert_eq!(bounds.len(), 8, "sql: {}", sql); + assert!( + bounds.iter().all(|b| b.starts_with("2026-06-20")), + "every bound stays on the reporting day - none is shifted back a fiscal \ + year or two\nbounds: {:?}\nsql: {}", + bounds, + 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; From ec9e3a7f62c0e84ef1d64ac13c636a877ae4efac Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 19:58:03 +0000 Subject: [PATCH 02/16] test(schema-compiler): FILTER_PARAMS push-down under a named calendar time shift The YAML-level counterpart to the planner test added in ebdc764, reproducing the shape a retail 4-5-4 calendar forces: a fact whose `sql` pushes a joined calendar cube's date down through FILTER_PARAMS, with multi_stage measures that shift that date by NAME rather than by interval. Three tests characterise what Tesseract emits today, and a fourth pins the interval form beside it so the asymmetry is visible in one file: - the column reaches every shifted stage (no stage scans the fact unfiltered) - each stage filters on its own calendar mapping column - an INTERVAL shift offsets the pushed-down column (CORE-543 / #11030) - a NAMED shift does not - it binds the bare column to unshifted bounds The last one is the gap. base_filter.rs resolves the shift as `time_shifts().get_for_symbol(sym).and_then(|s| s.interval.as_ref())`, and a named calendar shift carries `interval: None`, so the offsetting branch in `to_sql_for_filter_params` is never reached. Each stage then joins the calendar on its mapping column while the pushed-down predicate restricts the same scan to the reporting period, so the stage is empty unless the model widens the band by hand - and once it does, every stage scans every band. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EwtxWoLUp7ULnsQJjVLkZt --- .../filter-params-calendar-time-shift.test.ts | 178 ++++++++++++++++++ 1 file changed, 178 insertions(+) create mode 100644 packages/cubejs-schema-compiler/test/unit/filter-params-calendar-time-shift.test.ts 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..2895070f8c5e0 --- /dev/null +++ b/packages/cubejs-schema-compiler/test/unit/filter-params-calendar-time-shift.test.ts @@ -0,0 +1,178 @@ +import { PostgresQuery } from '../../src/adapter/PostgresQuery'; +import { prepareYamlCompiler } from './PrepareCompiler'; + +// A fact whose `sql` pushes a joined CALENDAR cube's date down through +// FILTER_PARAMS, with multi_stage measures that shift that date by NAME. +// +// Named shifts are what a retail 4-5-4 calendar forces: "one fiscal year back" +// is a mapping column on the calendar, not an interval any arithmetic can +// express, so the shift is declared as `time_shift: [{ name, sql }]` on the +// calendar dimension and referenced by name from the measure. +// +// Compare `multi-stage-time-shift-filter-params.test.ts` (CORE-543 / #11030), +// which covers the INTERVAL form: there the pushed-down column is offset by the +// same interval as the stage predicate, so a shifted stage scans the rows it +// groups by. A named shift gets no such treatment — see the last test here. +const CALENDAR_SHIFT_MODEL = ` +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 {FILTER_PARAMS.fpc_calendar.calendar_d.filter('week_end_d')} + 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 +`; + +// The same push-down, shifted by an interval instead of by name. +const INTERVAL_SHIFT_MODEL = ` +cubes: + - name: fpi_margin + sql: > + SELECT * FROM fpi_margin + WHERE {FILTER_PARAMS.fpi_margin.week_end_d.filter('week_end_d')} + 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: + - time_dimension: week_end_d + interval: 1 year + type: prior +`; + +async function buildSql(model: string, query: any): Promise<[string, unknown[]]> { + const { compiler, joinGraph, cubeEvaluator } = prepareYamlCompiler(model); + await compiler.compile(); + + return new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, { + timezone: 'UTC', + // Named calendar time shifts are planned by Tesseract only. + useNativeSqlPlanner: true, + ...query, + }).buildSqlAndParams(); +} + +const calendarShiftSql = () => buildSql(CALENDAR_SHIFT_MODEL, { + measures: ['fpc_margin.net_sales_ly', 'fpc_margin.net_sales_ly2'], + timeDimensions: [{ + dimension: 'fpc_calendar.calendar_d', + dateRange: ['2026-06-20', '2026-06-20'], + }], +}); + +describe('FILTER_PARAMS under a named calendar time shift', () => { + // Each named shift builds its own stage, and each stage rescans the fact. The + // binding stays attributed to `fpc_calendar.calendar_d` — the shift + // substitutes the column only where the stage's own predicate renders — so + // FILTER_PARAMS keeps matching and no stage scans the fact unfiltered. + it('pushes the column into every shifted stage', async () => { + const [sql] = await calendarShiftSql(); + + expect(sql.match(/FROM fpc_margin WHERE \(week_end_d >=/g)).toHaveLength(2); + expect(sql).not.toContain('FROM fpc_margin WHERE (1 = 1)'); + }); + + // The stage predicate is what carries the shift: each stage compares the + // calendar's mapping column, not `calendar_d`. + it('filters each stage on its own mapping column', async () => { + const [sql] = await calendarShiftSql(); + + expect(sql).toContain('"fpc_calendar".next_fiscal_year_d >='); + expect(sql).toContain('"fpc_calendar".next_two_fiscal_year_d >='); + }); + + // The interval form, for contrast: the pushed-down column is offset so the + // rows the shifted stage scans line up with the bounds it groups by. + it('offsets the pushed-down column for an interval shift', async () => { + const [sql] = await buildSql(INTERVAL_SHIFT_MODEL, { + measures: ['fpi_margin.net_sales', 'fpi_margin.net_sales_ly'], + timeDimensions: [{ + dimension: 'fpi_margin.week_end_d', + dateRange: ['2026-06-20', '2026-06-20'], + }], + }); + + expect(sql).toContain('(week_end_d + interval \'1 year\')'); + }); + + // The gap. + // + // A named shift never reaches the offsetting branch above. In + // `base_filter.rs` the shift handed to `to_sql_for_filter_params` is + // `time_shifts().get_for_symbol(sym).and_then(|s| s.interval.as_ref())`, and a + // named calendar shift carries `interval: None` — so the column is rendered + // bare and bound to the UNSHIFTED reporting bounds, in every stage. + // + // That contradicts the stage around it. `cte_0` joins the calendar on + // `week_end_d = next_fiscal_year_d`, so it reads PRIOR-year fact rows, while + // the pushed-down predicate restricts the same scan to the REPORTING week. + // The stage comes back empty unless the model widens the pushed-down band by + // hand to cover the shifted periods — and once it does, every stage scans + // every band. + it('binds the pushed-down column to unshifted bounds', async () => { + const [sql, params] = await calendarShiftSql(); + + expect(sql).not.toContain('week_end_d + interval'); + + // Eight bounds: a pushed-down pair and a stage pair per shifted stage. + // Every one of them is the reporting day, so nothing distinguishes the scan + // of the prior-fiscal-year stage from the scan of the two-year one. + expect(params).toHaveLength(8); + expect(params.every((p) => String(p).startsWith('2026-06-20'))).toBe(true); + }); +}); From be026dd698960e5ff6b471cff640b8aa89630af0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 21:34:11 +0000 Subject: [PATCH 03/16] fix(tesseract): reject FILTER_PARAMS string columns under a calendar time shift A multi_stage measure shifted by NAME (`time_shift: [{ name: prior_fiscal_year }]`, resolved through a calendar cube's own `time_shift` declaration) used to render its cube's FILTER_PARAMS push-down as a bare column bound to the query's UNSHIFTED reporting bounds. That contradicts the stage around it. The stage joins the calendar on its mapping column and reads prior-period fact rows, while the pushed-down predicate restricts the same scan to the reporting period, so the stage comes back empty - the failure CORE-543 fixed for interval shifts, reached by a different route. The cause was two levels down from where the shift is read. `PushDownBuilderContext::make_sql_nodes_factory` splits shifts with `extract_time_shifts`: interval shifts go to `TimeShiftState`, calendar shifts to a separate map consumed only by `CalendarTimeShiftSqlNode`. `base_filter.rs` read `TimeShiftState` alone, so a calendar shift looked like no shift at all. Carry the calendar shifts to the filter-params path as well (`SqlNodesFactory` -> `VisitorContext` -> `SqlEvaluatorVisitor`), and have `to_sql_for_filter_params` distinguish the two kinds via `FilterParamsTimeShift`: - a string column under an INTERVAL shift is offset, as before - a string column under a CALENDAR shift is rejected, naming the binding, the shift and the remedy - the mapping is data, not arithmetic, so there is no expression over that column that stands for it and no bound the planner can widen to without reading the calendar - a callback column is unchanged: it receives the query's own bounds and widens the pushed-down range itself, which is the only place the mapping can be expressed BEHAVIOUR CHANGE: a model combining a string FILTER_PARAMS column with a named calendar shift now fails to plan where it previously returned silently empty or under-filtered stages. The remedy is in the message. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EwtxWoLUp7ULnsQJjVLkZt --- .../filter-params-calendar-time-shift.test.ts | 137 ++++++----------- .../src/physical_plan/filter/base_filter.rs | 15 +- .../src/physical_plan/filter/typed_filter.rs | 64 ++++++-- .../src/physical_plan/sql_nodes/factory.rs | 4 + .../src/physical_plan/sql_visitor.rs | 21 +++ .../src/physical_plan/visitor_context.rs | 9 ++ .../cubesqlplanner/src/planner/sql_call.rs | 1 + .../filter_params_calendar_time_shift.rs | 143 ++++++++---------- 8 files changed, 212 insertions(+), 182 deletions(-) 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 index 2895070f8c5e0..b31c86cb9bc83 100644 --- 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 @@ -12,8 +12,10 @@ import { prepareYamlCompiler } from './PrepareCompiler'; // Compare `multi-stage-time-shift-filter-params.test.ts` (CORE-543 / #11030), // which covers the INTERVAL form: there the pushed-down column is offset by the // same interval as the stage predicate, so a shifted stage scans the rows it -// groups by. A named shift gets no such treatment — see the last test here. -const CALENDAR_SHIFT_MODEL = ` +// groups by. A calendar shift has no such offset, so a string column cannot +// carry it and is rejected; a callback column receives the query's own bounds +// and can widen the pushed-down range itself. +const model = (filterParams: string) => ` cubes: - name: fpc_calendar calendar: true @@ -34,8 +36,7 @@ cubes: - name: fpc_margin sql: > - SELECT * FROM fpc_margin - WHERE {FILTER_PARAMS.fpc_calendar.calendar_d.filter('week_end_d')} + SELECT * FROM fpc_margin WHERE ${filterParams} joins: - name: fpc_calendar sql: "{CUBE}.week_end_d = {fpc_calendar.calendar_d}" @@ -66,113 +67,61 @@ cubes: - name: prior_two_fiscal_year `; -// The same push-down, shifted by an interval instead of by name. -const INTERVAL_SHIFT_MODEL = ` -cubes: - - name: fpi_margin - sql: > - SELECT * FROM fpi_margin - WHERE {FILTER_PARAMS.fpi_margin.week_end_d.filter('week_end_d')} - 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: - - time_dimension: week_end_d - interval: 1 year - type: prior -`; +const STRING_COLUMN = '{FILTER_PARAMS.fpc_calendar.calendar_d.filter(\'week_end_d\')}'; -async function buildSql(model: string, query: any): Promise<[string, unknown[]]> { - const { compiler, joinGraph, cubeEvaluator } = prepareYamlCompiler(model); +// 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, - ...query, }).buildSqlAndParams(); } -const calendarShiftSql = () => buildSql(CALENDAR_SHIFT_MODEL, { - measures: ['fpc_margin.net_sales_ly', 'fpc_margin.net_sales_ly2'], - timeDimensions: [{ - dimension: 'fpc_calendar.calendar_d', - dateRange: ['2026-06-20', '2026-06-20'], - }], -}); - describe('FILTER_PARAMS under a named calendar time shift', () => { - // Each named shift builds its own stage, and each stage rescans the fact. The - // binding stays attributed to `fpc_calendar.calendar_d` — the shift - // substitutes the column only where the stage's own predicate renders — so - // FILTER_PARAMS keeps matching and no stage scans the fact unfiltered. - it('pushes the column into every shifted stage', async () => { - const [sql] = await calendarShiftSql(); + // 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 + ); + }); - expect(sql.match(/FROM fpc_margin WHERE \(week_end_d >=/g)).toHaveLength(2); + // 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 mapping column, not `calendar_d`. + // calendar's own mapping column, not `calendar_d`. it('filters each stage on its own mapping column', async () => { - const [sql] = await calendarShiftSql(); - - expect(sql).toContain('"fpc_calendar".next_fiscal_year_d >='); - expect(sql).toContain('"fpc_calendar".next_two_fiscal_year_d >='); - }); - - // The interval form, for contrast: the pushed-down column is offset so the - // rows the shifted stage scans line up with the bounds it groups by. - it('offsets the pushed-down column for an interval shift', async () => { - const [sql] = await buildSql(INTERVAL_SHIFT_MODEL, { - measures: ['fpi_margin.net_sales', 'fpi_margin.net_sales_ly'], - timeDimensions: [{ - dimension: 'fpi_margin.week_end_d', - dateRange: ['2026-06-20', '2026-06-20'], - }], - }); - - expect(sql).toContain('(week_end_d + interval \'1 year\')'); - }); - - // The gap. - // - // A named shift never reaches the offsetting branch above. In - // `base_filter.rs` the shift handed to `to_sql_for_filter_params` is - // `time_shifts().get_for_symbol(sym).and_then(|s| s.interval.as_ref())`, and a - // named calendar shift carries `interval: None` — so the column is rendered - // bare and bound to the UNSHIFTED reporting bounds, in every stage. - // - // That contradicts the stage around it. `cte_0` joins the calendar on - // `week_end_d = next_fiscal_year_d`, so it reads PRIOR-year fact rows, while - // the pushed-down predicate restricts the same scan to the REPORTING week. - // The stage comes back empty unless the model widens the pushed-down band by - // hand to cover the shifted periods — and once it does, every stage scans - // every band. - it('binds the pushed-down column to unshifted bounds', async () => { - const [sql, params] = await calendarShiftSql(); - - expect(sql).not.toContain('week_end_d + interval'); + const [sql] = await buildSql(CALLBACK_COLUMN); - // Eight bounds: a pushed-down pair and a stage pair per shifted stage. - // Every one of them is the reporting day, so nothing distinguishes the scan - // of the prior-fiscal-year stage from the scan of the two-year one. - expect(params).toHaveLength(8); - expect(params.every((p) => String(p).startsWith('2026-06-20'))).toBe(true); + 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..1e3f1aeac94f2 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,22 @@ impl ToSql for BaseFilter { .filter_params_columns .get(&symbol_to_match.full_name()) { + // Both shift kinds, not just the interval one. A calendar shift + // never reaches `time_shifts` - `extract_time_shifts` routes it + // to its own map - and reading only that map is what used to + // leave the pushed-down column bare against unshifted bounds + // inside a calendar-shifted stage. 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(|| { + visitor + .calendar_time_shifts() + .get(&symbol_to_match.full_name()) + .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..92818cc2d9a43 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,20 @@ impl ToSql for TypedFilter { } } +/// The time shift active on the stage a FILTER_PARAMS column is rendered into. +/// The two kinds are not interchangeable: an interval is arithmetic the column +/// can carry, while a calendar shift resolves through a mapping column on the +/// calendar cube and 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, @@ -65,16 +75,43 @@ impl TypedFilter { // same shift as the regular time-dimension filter, otherwise its // current-period bounds contradict the shifted predicate and empty // the CTE. + // + // An interval shift is arithmetic, so the column can carry it by + // being offset. A NAMED calendar shift is not: it resolves through + // a mapping column on the calendar cube (`prior_fiscal_year` -> + // `next_fiscal_year_d`, and so on), which is data, not an offset, + // and is not in scope inside the cube's own `sql`. There is no + // expression over this column that stands for it, and no bound the + // planner can widen to without knowing the calendar's contents - so + // say that, rather than emit a predicate that silently empties the + // stage. 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() + } + 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 +123,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 index 07a824f31ab18..d6ca6f50d305b 100644 --- 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 @@ -1,4 +1,3 @@ -use crate::cube_bridge::base_query_options::FilterValue; use crate::test_fixtures::cube_bridge::MockSchema; use crate::test_fixtures::test_utils::TestContext; use indoc::indoc; @@ -10,13 +9,19 @@ use indoc::indoc; // arithmetic can express. // // Contrast with filter_params_time_shift.rs, which covers the interval form. -// The two are handled by the same call site but only one of them can produce a -// shift: see `base_filter.rs`, where the shift handed to -// `to_sql_for_filter_params` is `…get_for_symbol(sym).and_then(|s| s.interval.as_ref())`. -// A named calendar shift carries `interval: None`, so that resolves to `None` -// and the pushed-down column is rendered bare. -fn schema() -> MockSchema { - MockSchema::from_yaml(indoc! {" +// There the pushed-down column is offset by the same interval as the stage +// predicate, so the rows a shifted stage scans line up with the bounds it +// groups by. A calendar shift has no such offset: `prior_fiscal_year` resolves +// to `next_fiscal_year_d` on the calendar cube, which is data rather than +// arithmetic and is not in scope inside the fact's own `sql`. +// +// So a STRING column is rejected - it would otherwise be bound to the +// unshifted reporting bounds and empty the stage - while a CALLBACK column is +// accepted, because it is handed the query's own bounds and can widen the +// pushed-down range itself. +fn schema(margin_sql: &str) -> MockSchema { + MockSchema::from_yaml(&format!( + indoc! {" cubes: - name: fpc_calendar calendar: true @@ -28,16 +33,16 @@ fn schema() -> MockSchema { primary_key: true time_shift: - name: prior_fiscal_year - sql: \"{CUBE}.next_fiscal_year_d\" + sql: \"{{CUBE}}.next_fiscal_year_d\" - name: prior_two_fiscal_year - sql: \"{CUBE}.next_two_fiscal_year_d\" + sql: \"{{CUBE}}.next_two_fiscal_year_d\" - name: fpc_margin - sql: \"SELECT * FROM fpc_margin WHERE {FILTER_PARAMS_COLUMN:fpc_calendar.calendar_d:week_end_d}\" + sql: \"{margin_sql}\" joins: - name: fpc_calendar relationship: many_to_one - sql: \"{fpc_margin}.week_end_d = {fpc_calendar.calendar_d}\" + sql: \"{{fpc_margin}}.week_end_d = {{fpc_calendar.calendar_d}}\" dimensions: - name: id type: number @@ -53,23 +58,37 @@ fn schema() -> MockSchema { - name: net_sales_ly type: number - sql: \"{CUBE.net_sales}\" + sql: \"{{CUBE.net_sales}}\" multi_stage: true time_shift: - name: prior_fiscal_year - name: net_sales_ly2 type: number - sql: \"{CUBE.net_sales}\" + sql: \"{{CUBE.net_sales}}\" multi_stage: true time_shift: - name: prior_two_fiscal_year - "}) + "}, + margin_sql = margin_sql + )) .unwrap() } -fn shifted_stages() -> (String, Vec) { - let ctx = TestContext::new(schema()).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: @@ -81,25 +100,42 @@ fn shifted_stages() -> (String, Vec) { - \"2026-06-20\" - \"2026-06-20\" "}) - .unwrap() + .map(|(sql, _)| sql) } -fn shifted_stages_sql() -> String { - shifted_stages().0 +// 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 + ); } -// Each named shift builds its own stage, and each stage rescans the fact. The -// binding stays attributed to `fpc_calendar.calendar_d` - the shift substitutes -// the column only where the stage's own predicate renders - so FILTER_PARAMS -// keeps matching and the fact scan is never left unfiltered. +// 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 pushed_down_column_reaches_every_named_shift_stage() { - let sql = shifted_stages_sql(); +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("FROM fpc_margin WHERE (week_end_d >=").count(), - 2, - "both named-shift stages must push the column down\nsql: {}", + sql.matches("week_end_d >=").count(), + 4, + "both bands must render in both shifted stages\nsql: {}", sql ); assert!( @@ -110,10 +146,10 @@ fn pushed_down_column_reaches_every_named_shift_stage() { } // The stage predicate is what carries the shift: each stage compares the -// calendar's mapping column, not `calendar_d`. +// calendar's own mapping column, not `calendar_d`. #[test] fn stage_predicate_uses_the_named_mapping_column() { - let sql = shifted_stages_sql(); + let sql = build(CALLBACK_COLUMN).unwrap(); assert!( sql.contains("next_fiscal_year_d"), @@ -126,48 +162,3 @@ fn stage_predicate_uses_the_named_mapping_column() { sql ); } - -// The gap this file exists to pin down. -// -// `filter_params_time_shift.rs` asserts that an INTERVAL shift offsets the -// pushed-down column, so the rows a shifted stage scans line up with the bounds -// that stage groups by. A NAMED calendar shift gets no such treatment: the -// column is rendered bare and bound to the UNSHIFTED reporting bounds, in every -// stage. -// -// That is a contradiction inside each stage. `cte_0` joins the calendar on -// `week_end_d = next_fiscal_year_d`, so it reads PRIOR-year fact rows - while -// the pushed-down predicate restricts the same scan to the REPORTING week. The -// stage is empty unless the model widens the pushed-down band by hand to cover -// the shifted periods, and once it does, every stage scans every band. -#[test] -fn named_shift_binds_the_pushed_down_column_to_unshifted_bounds() { - let (sql, params) = shifted_stages(); - - assert!( - !sql.contains("week_end_d + interval"), - "a named calendar shift currently cannot offset the pushed-down column; \ - if this now passes, base_filter.rs learned to carry non-interval shifts \ - and this test should be inverted\nsql: {}", - sql - ); - - // Eight bounds: a pushed-down pair and a stage pair per shifted stage. Every - // one of them is the reporting day, so nothing distinguishes the scan of the - // prior-fiscal-year stage from the scan of the two-year one. - let bounds: Vec = params - .iter() - .map(|p| match p { - FilterValue::Str(s) => s.clone(), - other => panic!("unexpected bound {:?}", other), - }) - .collect(); - assert_eq!(bounds.len(), 8, "sql: {}", sql); - assert!( - bounds.iter().all(|b| b.starts_with("2026-06-20")), - "every bound stays on the reporting day - none is shifted back a fiscal \ - year or two\nbounds: {:?}\nsql: {}", - bounds, - sql - ); -} From 6aa097b4443cf298a3f0c3ddc524db29ba36b754 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 21:40:16 +0000 Subject: [PATCH 04/16] fix(tesseract): carry interval-declared calendar shifts, probe the PK key Two review findings on the previous commit. 1. Not every calendar shift is a mapping. `CalendarDimensionTimeShift` carries both `interval` and `sql`, and a calendar cube may declare a shift as `{ name: prior_year, interval: 1 year, type: prior }` with no `sql`. `calendar_time_shift_for_interval` routes that into the calendar map and `CalendarTimeShiftSqlNode` renders it as plain arithmetic, so the column can carry it after all - rejecting it was wrong, and the `` in the error message was the tell, since the interval-matched path leaves `name` unset. Reject only when `sql` is declared; offset when only an interval is. The offset is inverted here because the calendar map keeps the declaration as written and inverts at render, unlike `TimeShiftState`. A shift with neither renders bare, matching the calendar node's own fallthrough. 2. The calendar lookup was an exact-name hit while the interval path resolves the symbol. The map is keyed by the calendar cube's PK, so a binding on the fact's own time dimension missed and the column rendered bare against unshifted bounds - the same silent failure this PR fixes, one binding away. Probe the dimension's `time_shift_pk_full_name` as well. Adds a planner test pinning the offset and its sign for an interval-declared calendar shift, the shape neither test covered. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EwtxWoLUp7ULnsQJjVLkZt --- .../src/physical_plan/filter/base_filter.rs | 19 ++++- .../src/physical_plan/filter/typed_filter.rs | 32 ++++++++ .../filter_params_calendar_time_shift.rs | 74 +++++++++++++++++++ 3 files changed, 123 insertions(+), 2 deletions(-) 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 1e3f1aeac94f2..44b0a190fd54d 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 @@ -37,9 +37,24 @@ impl ToSql for BaseFilter { .and_then(|shift| shift.interval.as_ref()) .map(FilterParamsTimeShift::Interval) .or_else(|| { - visitor - .calendar_time_shifts() + // The calendar map is keyed by the calendar cube's PK, + // which is the filtered symbol only when FILTER_PARAMS + // binds the calendar dimension itself. A binding on the + // fact's own time dimension reaches the same shift + // through that dimension's `time_shift_pk_full_name`, so + // probe both - matching how `get_for_symbol` probes more + // than the bare name. Missing here is silent: the column + // would render bare against unshifted bounds. + 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( 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 92818cc2d9a43..f5a462b428684 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 @@ -95,6 +95,38 @@ impl TypedFilter { ); shifted_column.as_str() } + // A calendar shift is only a mapping when it declares + // `sql`. Declared as a plain interval it is arithmetic after + // all - `CalendarTimeShiftSqlNode` renders exactly this + // offset - so the column can carry it like any other + // interval. + // + // 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() && shift.interval.is_some() => + { + let interval = shift.interval.as_ref().unwrap(); + shifted_column = format!( + "({})", + plan_templates.add_timestamp_interval( + column_sql.clone(), + interval.inverse().to_sql() + )? + ); + shifted_column.as_str() + } + // Neither a mapping nor an offset: the calendar node renders + // the dimension unshifted too, so there is nothing to carry. + Some(FilterParamsTimeShift::Calendar(shift)) + if shift.sql.is_none() && shift.interval.is_none() => + { + column_sql.as_str() + } Some(FilterParamsTimeShift::Calendar(shift)) => { return Err(CubeError::user(format!( concat!( 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 index d6ca6f50d305b..fa35b4b6b40dd 100644 --- 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 @@ -162,3 +162,77 @@ fn stage_predicate_uses_the_named_mapping_column() { 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 + ); +} From f259ace498e1f7cb3534c4ad4ddd3f9eac29b7cc Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 21:41:05 +0000 Subject: [PATCH 05/16] refactor(tesseract): trim filter-params shift comments to what the code doesn't say Review nit: the block above the Calendar arm restated the CubeError::user message ten lines below it, and the base_filter.rs comment narrated the routing rather than guarding against reintroducing the bug. Keep the load-bearing part - why the column must carry the shift at all, and why both maps are probed - and let the error string carry the rest. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EwtxWoLUp7ULnsQJjVLkZt --- .../src/physical_plan/filter/base_filter.rs | 20 +++++++------------ .../src/physical_plan/filter/typed_filter.rs | 10 ---------- 2 files changed, 7 insertions(+), 23 deletions(-) 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 44b0a190fd54d..30c8f584dc500 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 @@ -26,25 +26,19 @@ impl ToSql for BaseFilter { .filter_params_columns .get(&symbol_to_match.full_name()) { - // Both shift kinds, not just the interval one. A calendar shift - // never reaches `time_shifts` - `extract_time_shifts` routes it - // to its own map - and reading only that map is what used to - // leave the pushed-down column bare against unshifted bounds - // inside a calendar-shifted stage. + // 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()) .map(FilterParamsTimeShift::Interval) .or_else(|| { - // The calendar map is keyed by the calendar cube's PK, - // which is the filtered symbol only when FILTER_PARAMS - // binds the calendar dimension itself. A binding on the - // fact's own time dimension reaches the same shift - // through that dimension's `time_shift_pk_full_name`, so - // probe both - matching how `get_for_symbol` probes more - // than the bare name. Missing here is silent: the column - // would render bare against unshifted bounds. + // Keyed by the calendar cube's PK, which is the filtered + // symbol only when FILTER_PARAMS binds the calendar + // dimension itself; a binding on the fact's own time + // dimension reaches it via `time_shift_pk_full_name`. + // Probe both - a miss here renders the column bare. let calendar_shifts = visitor.calendar_time_shifts(); calendar_shifts .get(&symbol_to_match.full_name()) 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 f5a462b428684..b33270576711a 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 @@ -75,16 +75,6 @@ impl TypedFilter { // same shift as the regular time-dimension filter, otherwise its // current-period bounds contradict the shifted predicate and empty // the CTE. - // - // An interval shift is arithmetic, so the column can carry it by - // being offset. A NAMED calendar shift is not: it resolves through - // a mapping column on the calendar cube (`prior_fiscal_year` -> - // `next_fiscal_year_d`, and so on), which is data, not an offset, - // and is not in scope inside the cube's own `sql`. There is no - // expression over this column that stands for it, and no bound the - // planner can widen to without knowing the calendar's contents - so - // say that, rather than emit a predicate that silently empties the - // stage. let shifted_column; let member_sql = match &time_shift { Some(FilterParamsTimeShift::Interval(interval)) => { From f6905857cff34fe667e58b8838410ccf80e1db86 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 21:45:20 +0000 Subject: [PATCH 06/16] test(tesseract): cover the non-PK calendar binding; docs: FILTER_PARAMS under time shift Closes the two remaining review findings. The PK probe added in 6aa097b had no test. `shift_is_found_when_the_binding_is_not_the_calendar_pk` covers it: a calendar cube whose shifted dimension is not its primary key, so the shift is registered under the PK while FILTER_PARAMS binds the dimension itself. Verified meaningful - reverting the probe makes it fail with the column rendered bare. Note the reviewer's framing was slightly off and the test reflects what the probe actually reaches: `time_shift_pk_full_name` is only populated for dimensions on a calendar cube (dimension_symbol.rs:422), so a binding on the FACT's own time dimension has no PK to probe and cannot reach the calendar map at all - a named shift on such a dimension is dropped by `extract_time_shifts` before this point. That is a separate, pre-existing gap. Docs: a "Time-shifted measures" subsection under FILTER_PARAMS in context-variables.mdx, covering why the column must carry the stage's shift, which shift kinds can be carried, and the callback form as the remedy - the planning error was until now the only place that remedy was written down. Also fixes three assert messages that carried a literal backslash-n. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EwtxWoLUp7ULnsQJjVLkZt --- .../data-modeling/context-variables.mdx | 38 ++++++++- .../filter_params_calendar_time_shift.rs | 81 ++++++++++++++++++- 2 files changed, 117 insertions(+), 2 deletions(-) diff --git a/docs-mintlify/reference/data-modeling/context-variables.mdx b/docs-mintlify/reference/data-modeling/context-variables.mdx index 2c607b67035d1..fdce88454020a 100644 --- a/docs-mintlify/reference/data-modeling/context-variables.mdx +++ b/docs-mintlify/reference/data-modeling/context-variables.mdx @@ -392,6 +392,40 @@ 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 — here +a fiscal prior year, bracketed at 371 and 364 days: + +```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 >= {x} AND week_end_d <= {y}) " + f"OR (week_end_d >= DATE_SUB({x}, INTERVAL 371 DAY) " + f"AND week_end_d <= DATE_SUB({y}, INTERVAL 364 DAY))" + )} +``` + +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 +913,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/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 index fa35b4b6b40dd..3db5125277dae 100644 --- 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 @@ -232,7 +232,86 @@ fn interval_declared_calendar_shift_offsets_the_column() { 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: {}", + 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) => assert!( + err.to_string().contains("prior_fiscal_year"), + "the shift must be found through the calendar PK, not silently missed\nerror: {}", + err + ), + Ok((sql, _)) => panic!( + "the binding's shift was missed and the column rendered bare\nsql: {}", + sql + ), + } +} From 1c57118379d128f2014d0bded294d6651b6a4e9e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 21:47:15 +0000 Subject: [PATCH 07/16] refactor(tesseract): fold the calendar arms, correct the PK-probe comment Three review nits on the follow-ups. The PK-probe comment claimed the fallback covers a FILTER_PARAMS binding on the fact's own time dimension. It cannot: `time_shift_pk_full_name` is populated only for calendar-cube dimensions, so a fact dimension has no PK to probe with, and a named shift on one is dropped by `extract_time_shifts` before this point. Now describes what the arm actually reaches - a filter on a non-PK dimension of the calendar - and names the fact-dimension shape as out of reach either way. `shift_is_found_when_the_binding_is_not_the_calendar_pk` (f690585) covers it. The two guarded calendar arms differed only in whether `interval` was set, and the first paid for that with an `unwrap()` the guard had already proved. Folded into one arm with an inner match on `shift.interval`. Dropped the paragraph that restated the guard; kept the one recording the inverse asymmetry, since a later "consistency fix" there would silently reverse the pushed-down bounds. The `FilterParamsTimeShift` doc still said a calendar shift has no expression over the fact's own column, which stopped being true when the interval-declared case started being offset. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EwtxWoLUp7ULnsQJjVLkZt --- .../src/physical_plan/filter/base_filter.rs | 16 +++++-- .../src/physical_plan/filter/typed_filter.rs | 48 ++++++++----------- 2 files changed, 31 insertions(+), 33 deletions(-) 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 30c8f584dc500..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 @@ -34,11 +34,17 @@ impl ToSql for BaseFilter { .and_then(|shift| shift.interval.as_ref()) .map(FilterParamsTimeShift::Interval) .or_else(|| { - // Keyed by the calendar cube's PK, which is the filtered - // symbol only when FILTER_PARAMS binds the calendar - // dimension itself; a binding on the fact's own time - // dimension reaches it via `time_shift_pk_full_name`. - // Probe both - a miss here renders the column bare. + // 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()) 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 b33270576711a..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 @@ -48,9 +48,10 @@ impl ToSql for TypedFilter { } /// The time shift active on the stage a FILTER_PARAMS column is rendered into. -/// The two kinds are not interchangeable: an interval is arithmetic the column -/// can carry, while a calendar shift resolves through a mapping column on the -/// calendar cube and has no expression over the fact's own column. +/// 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), @@ -85,37 +86,28 @@ impl TypedFilter { ); shifted_column.as_str() } - // A calendar shift is only a mapping when it declares - // `sql`. Declared as a plain interval it is arithmetic after - // all - `CalendarTimeShiftSqlNode` renders exactly this - // offset - so the column can carry it like any other - // interval. - // // 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() && shift.interval.is_some() => - { - let interval = shift.interval.as_ref().unwrap(); - shifted_column = format!( - "({})", - plan_templates.add_timestamp_interval( - column_sql.clone(), - interval.inverse().to_sql() - )? - ); - shifted_column.as_str() - } - // Neither a mapping nor an offset: the calendar node renders - // the dimension unshifted too, so there is nothing to carry. - Some(FilterParamsTimeShift::Calendar(shift)) - if shift.sql.is_none() && shift.interval.is_none() => - { - column_sql.as_str() + 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!( From bda197c68b6f0a6daa76229f324230b0846259a4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 21:48:21 +0000 Subject: [PATCH 08/16] docs(tesseract): refresh the stale header on the calendar time-shift tests The file header still said a calendar shift has no offset and that string columns are rejected - true of the first commit, but not since the rejection was narrowed to sql-declared shifts, with interval_declared_calendar_shift_offsets_the_column sitting in the same file as the counterexample. Restated around the distinction that survived: sql-declared vs interval-declared. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EwtxWoLUp7ULnsQJjVLkZt --- .../filter_params_calendar_time_shift.rs | 22 +++++-------------- 1 file changed, 5 insertions(+), 17 deletions(-) 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 index 3db5125277dae..6fe2b13e3b3f4 100644 --- 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 @@ -2,23 +2,11 @@ use crate::test_fixtures::cube_bridge::MockSchema; use crate::test_fixtures::test_utils::TestContext; use indoc::indoc; -// A fact whose `sql` pushes a joined CALENDAR cube's date down through -// FILTER_PARAMS, with multi-stage measures that shift that date by NAME rather -// than by interval - the shape a retail 4-5-4 calendar forces, where "one -// fiscal year back" is a mapping column on the calendar and not an interval any -// arithmetic can express. -// -// Contrast with filter_params_time_shift.rs, which covers the interval form. -// There the pushed-down column is offset by the same interval as the stage -// predicate, so the rows a shifted stage scans line up with the bounds it -// groups by. A calendar shift has no such offset: `prior_fiscal_year` resolves -// to `next_fiscal_year_d` on the calendar cube, which is data rather than -// arithmetic and is not in scope inside the fact's own `sql`. -// -// So a STRING column is rejected - it would otherwise be bound to the -// unshifted reporting bounds and empty the stage - while a CALLBACK column is -// accepted, because it is handed the query's own bounds and can widen the -// pushed-down range itself. +// 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! {" From 08f34950bbd2848561a579b24935f6eca51f765f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 21:49:58 +0000 Subject: [PATCH 09/16] test(tesseract): pin the non-PK probe test to this rejection, not any error `contains("prior_fiscal_year")` alone was satisfied by any error naming the shift - a "time shift not found" or a join-resolution failure on the non-PK dimension would have passed it, so a regression turning the probe into a different error went unnoticed. Also assert "cannot carry", matching the sibling test one screen up. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EwtxWoLUp7ULnsQJjVLkZt --- .../tests/filter_params_calendar_time_shift.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) 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 index 6fe2b13e3b3f4..0d2d0799e112f 100644 --- 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 @@ -292,11 +292,19 @@ fn shift_is_found_when_the_binding_is_not_the_calendar_pk() { "}); match result { - Err(err) => assert!( - err.to_string().contains("prior_fiscal_year"), - "the shift must be found through the calendar PK, not silently missed\nerror: {}", - err - ), + 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 From 70ba23e6d5b7411e830407fbaa79806243688443 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 21:52:57 +0000 Subject: [PATCH 10/16] docs(schema-compiler): refresh the stale header on the YAML calendar shift tests bda197c fixed this staleness on the Rust side and missed the twin here: the header still claimed a calendar shift has no offset and that string columns are rejected, which since 6aa097b is only true of sql-declared shifts. Restated around that distinction and let the fixture speak for the rest. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EwtxWoLUp7ULnsQJjVLkZt --- .../filter-params-calendar-time-shift.test.ts | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) 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 index b31c86cb9bc83..012273a82cab8 100644 --- 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 @@ -1,20 +1,11 @@ import { PostgresQuery } from '../../src/adapter/PostgresQuery'; import { prepareYamlCompiler } from './PrepareCompiler'; -// A fact whose `sql` pushes a joined CALENDAR cube's date down through -// FILTER_PARAMS, with multi_stage measures that shift that date by NAME. -// -// Named shifts are what a retail 4-5-4 calendar forces: "one fiscal year back" -// is a mapping column on the calendar, not an interval any arithmetic can -// express, so the shift is declared as `time_shift: [{ name, sql }]` on the -// calendar dimension and referenced by name from the measure. -// -// Compare `multi-stage-time-shift-filter-params.test.ts` (CORE-543 / #11030), -// which covers the INTERVAL form: there the pushed-down column is offset by the -// same interval as the stage predicate, so a shifted stage scans the rows it -// groups by. A calendar shift has no such offset, so a string column cannot -// carry it and is rejected; a callback column receives the query's own bounds -// and can widen the pushed-down range itself. +// 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 From ce09d2e6de8acade4fee07e3ae338dbb607599b0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 21:53:53 +0000 Subject: [PATCH 11/16] docs: pair the time-shift FILTER_PARAMS example with a JavaScript twin The surrounding subsections all use a with YAML and JavaScript; this one was a bare yaml fence. Matches the shape of the BigQuery-shard example above it, including the triple-quoted f-string / template-literal forms. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EwtxWoLUp7ULnsQJjVLkZt --- .../data-modeling/context-variables.mdx | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/docs-mintlify/reference/data-modeling/context-variables.mdx b/docs-mintlify/reference/data-modeling/context-variables.mdx index fdce88454020a..f71fee5eeb273 100644 --- a/docs-mintlify/reference/data-modeling/context-variables.mdx +++ b/docs-mintlify/reference/data-modeling/context-variables.mdx @@ -410,19 +410,41 @@ 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 — here a fiscal prior year, bracketed at 371 and 364 days: -```yaml + + +```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 >= {x} AND week_end_d <= {y}) " - f"OR (week_end_d >= DATE_SUB({x}, INTERVAL 371 DAY) " - f"AND week_end_d <= DATE_SUB({y}, INTERVAL 364 DAY))" + lambda x, y: f""" + (week_end_d >= {x} AND week_end_d <= {y}) + OR (week_end_d >= DATE_SUB({x}, INTERVAL 371 DAY) + AND week_end_d <= DATE_SUB({y}, INTERVAL 364 DAY)) + """ )} ``` +```javascript title="JavaScript" +cube(`weekly_margin`, { + sql: ` + SELECT * + FROM weekly_margin + WHERE ${FILTER_PARAMS.retail_calendar.calendar_d.filter( + (x, y) => ` + (week_end_d >= ${x} AND week_end_d <= ${y}) + OR (week_end_d >= DATE_SUB(${x}, INTERVAL 371 DAY) + AND week_end_d <= DATE_SUB(${y}, INTERVAL 364 DAY)) + ` + )} + ` +}) +``` + + + Every stage then scans every band, so keep the bands as narrow as the calendar actually requires. From 94d3a6032a97027a8c6769554fb3d0c513da2e77 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 21:57:02 +0000 Subject: [PATCH 12/16] docs: name the dialect in the time-shift FILTER_PARAMS example The band uses DATE_SUB(..., INTERVAL n DAY), which is BigQuery/MySQL syntax, so a reader on Postgres would copy something that does not parse. Say so, the way the BigQuery-shard example one section up already does, and point at the 371/364 bracketing as the part worth carrying across dialects. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EwtxWoLUp7ULnsQJjVLkZt --- docs-mintlify/reference/data-modeling/context-variables.mdx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs-mintlify/reference/data-modeling/context-variables.mdx b/docs-mintlify/reference/data-modeling/context-variables.mdx index f71fee5eeb273..e2dc26a757622 100644 --- a/docs-mintlify/reference/data-modeling/context-variables.mdx +++ b/docs-mintlify/reference/data-modeling/context-variables.mdx @@ -408,7 +408,9 @@ 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 — here -a fiscal prior year, bracketed at 371 and 364 days: +a fiscal prior year, bracketed at 371 and 364 days. The bracketing is the +load-bearing part; the date arithmetic below is BigQuery syntax, so adjust it +for your data source: From c8a9739e3c52fb465d96d861a4f685f2b159840c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 22:00:33 +0000 Subject: [PATCH 13/16] docs: cast the filter values in the time-shift FILTER_PARAMS example The band passed the raw {x}/{y} straight to DATE_SUB and to the comparison, but callback values arrive as bare query parameters - filter_param_values allocates them through format_and_allocate_*_date_no_cast - so the example needed the same DATE(TIMESTAMP(...)) wrapping the shard example one section up already uses. As written it would not have run on BigQuery. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EwtxWoLUp7ULnsQJjVLkZt --- .../data-modeling/context-variables.mdx | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/docs-mintlify/reference/data-modeling/context-variables.mdx b/docs-mintlify/reference/data-modeling/context-variables.mdx index e2dc26a757622..cefbf6ca7a301 100644 --- a/docs-mintlify/reference/data-modeling/context-variables.mdx +++ b/docs-mintlify/reference/data-modeling/context-variables.mdx @@ -410,7 +410,8 @@ 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 — here a fiscal prior year, bracketed at 371 and 364 days. The bracketing is the load-bearing part; the date arithmetic below is BigQuery syntax, so adjust it -for your data source: +for your data source. Note that the values arrive as bare query parameters, so +they are cast before use — the same as the shard example above: @@ -422,9 +423,10 @@ cubes: FROM weekly_margin WHERE {FILTER_PARAMS.retail_calendar.calendar_d.filter( lambda x, y: f""" - (week_end_d >= {x} AND week_end_d <= {y}) - OR (week_end_d >= DATE_SUB({x}, INTERVAL 371 DAY) - AND week_end_d <= DATE_SUB({y}, INTERVAL 364 DAY)) + (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)) """ )} ``` @@ -436,9 +438,10 @@ cube(`weekly_margin`, { FROM weekly_margin WHERE ${FILTER_PARAMS.retail_calendar.calendar_d.filter( (x, y) => ` - (week_end_d >= ${x} AND week_end_d <= ${y}) - OR (week_end_d >= DATE_SUB(${x}, INTERVAL 371 DAY) - AND week_end_d <= DATE_SUB(${y}, INTERVAL 364 DAY)) + (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)) ` )} ` From 1fb4b70c564dbbfa63a3c3218e4133b50699fd0c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 22:02:12 +0000 Subject: [PATCH 14/16] docs: parenthesise the OR band in the time-shift FILTER_PARAMS example The band is the page's first filter() callback with a top-level OR, thirty lines above the FILTER_GROUP warning about exactly that. Correct as written, since the substitution is the whole WHERE - but a reader appending "AND tenant_id = 1" to the copied sql would bind it to the second branch alone. One outer paren makes the snippet safe to extend, matching what production models using this pattern already do. Prose reworked so the two non-obvious details (bare params, the paren) are stated before the example rather than trailing it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EwtxWoLUp7ULnsQJjVLkZt --- .../data-modeling/context-variables.mdx | 32 +++++++++++-------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/docs-mintlify/reference/data-modeling/context-variables.mdx b/docs-mintlify/reference/data-modeling/context-variables.mdx index cefbf6ca7a301..f74f2caff346f 100644 --- a/docs-mintlify/reference/data-modeling/context-variables.mdx +++ b/docs-mintlify/reference/data-modeling/context-variables.mdx @@ -407,11 +407,17 @@ 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 — here -a fiscal prior year, bracketed at 371 and 364 days. The bracketing is the -load-bearing part; the date arithmetic below is BigQuery syntax, so adjust it -for your data source. Note that the values arrive as bare query parameters, so -they are cast before use — the same as the shard example above: +widen the pushed-down range to cover the periods the shifted stages read. + +Two details in the example below are easy to miss. The values arrive as bare +query parameters, so they are cast before use, the same as the shard example +above. And the band is parenthesised as a whole because it is an `OR` — without +that, appending another condition to the `WHERE` would bind it to the second +branch alone. + +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: @@ -423,10 +429,10 @@ cubes: 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)) + ((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))) """ )} ``` @@ -438,10 +444,10 @@ cube(`weekly_margin`, { 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)) + ((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))) ` )} ` From 30a9b6acd3838f403ee2a0c660a00fc9e2f051b3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 22:05:59 +0000 Subject: [PATCH 15/16] docs: show the calendar join in the time-shift FILTER_PARAMS example The binding crosses cubes - weekly_margin.sql pushes down a filter on retail_calendar.calendar_d - but the snippet showed neither the join nor any mention of the calendar cube, so unlike every other example on the page a reader could not run it as written. Adds the joins entry to both arms and a sentence naming what the calendar declares, since that join is also why the band needs widening at all. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EwtxWoLUp7ULnsQJjVLkZt --- .../data-modeling/context-variables.mdx | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/docs-mintlify/reference/data-modeling/context-variables.mdx b/docs-mintlify/reference/data-modeling/context-variables.mdx index f74f2caff346f..793205f2592f1 100644 --- a/docs-mintlify/reference/data-modeling/context-variables.mdx +++ b/docs-mintlify/reference/data-modeling/context-variables.mdx @@ -415,9 +415,11 @@ above. And the band is parenthesised as a whole because it is an `OR` — withou that, appending another condition to the `WHERE` would bind it to the second branch alone. -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: +`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: @@ -435,6 +437,11 @@ cubes: 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" @@ -450,7 +457,14 @@ cube(`weekly_margin`, { 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` + } + } }) ``` From 3dea13a8ec67c6197f4ab021ce1bc837a0ed745a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 22:09:08 +0000 Subject: [PATCH 16/16] docs: move the cast and paren notes below the fence The preamble had accumulated into four paragraphs, three of them caveats stacked ahead of the example - a gotcha list rather than an explanation. Both the cast and the paren are visible in the snippet, so they read better as a note after it; the join sentence stays above, since it is the one thing the snippet cannot show. Net reduction, no content dropped. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EwtxWoLUp7ULnsQJjVLkZt --- .../reference/data-modeling/context-variables.mdx | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/docs-mintlify/reference/data-modeling/context-variables.mdx b/docs-mintlify/reference/data-modeling/context-variables.mdx index 793205f2592f1..87f0a0b164b99 100644 --- a/docs-mintlify/reference/data-modeling/context-variables.mdx +++ b/docs-mintlify/reference/data-modeling/context-variables.mdx @@ -409,12 +409,6 @@ 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. -Two details in the example below are easy to miss. The values arrive as bare -query parameters, so they are cast before use, the same as the shard example -above. And the band is parenthesised as a whole because it is an `OR` — without -that, appending another condition to the `WHERE` would bind it to the second -branch alone. - `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 @@ -470,6 +464,10 @@ cube(`weekly_margin`, { +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.