diff --git a/packages/cubejs-schema-compiler/test/integration/postgres/multi-stage-time-shift-filter-params.test.ts b/packages/cubejs-schema-compiler/test/integration/postgres/multi-stage-time-shift-filter-params.test.ts new file mode 100644 index 0000000000000..9bdc175351c82 --- /dev/null +++ b/packages/cubejs-schema-compiler/test/integration/postgres/multi-stage-time-shift-filter-params.test.ts @@ -0,0 +1,72 @@ +import { getEnv } from '@cubejs-backend/shared'; +import { prepareYamlCompiler } from '../../unit/PrepareCompiler'; +import { dbRunner } from './PostgresDBRunner'; + +// Regression for CORE-543 (GitHub #11030): +// A multi_stage time_shift measure over a cube whose `sql` uses FILTER_PARAMS +// must render the prior-period CTE with the FILTER_PARAMS column shifted by the +// same interval as the time-shift predicate. Otherwise the current-period +// bounds on the un-shifted column contradict the shifted predicate, the +// prior-period CTE is always empty, and the YoY measure collapses to null. +describe('Multi-Stage time_shift + FILTER_PARAMS', () => { + jest.setTimeout(200000); + + const { compiler, joinGraph, cubeEvaluator } = prepareYamlCompiler(` +cubes: + - name: orders + sql: > + SELECT * FROM ( + SELECT 1 as id, '2024-03-14T00:00:00.000Z'::timestamptz as created_at, 100 as amount + union all + SELECT 2 as id, '2025-03-14T00:00:00.000Z'::timestamptz as created_at, 300 as amount + ) AS t + WHERE {FILTER_PARAMS.orders.date.filter('created_at')} + + dimensions: + - name: id + sql: id + type: number + primary_key: true + + - name: date + sql: created_at + type: time + + measures: + - name: revenue + sql: amount + type: sum + + - name: revenue_1_y_ago + sql: "{revenue}" + multi_stage: true + type: number + time_shift: + - time_dimension: date + interval: 1 year + type: prior + `); + + if (getEnv('nativeSqlPlanner')) { + it('prior-year time_shift measure is not emptied by FILTER_PARAMS', async () => dbRunner.runQueryTest({ + measures: ['orders.revenue', 'orders.revenue_1_y_ago'], + timeDimensions: [ + { + dimension: 'orders.date', + granularity: 'year', + dateRange: ['2025-01-01', '2025-12-31'], + } + ], + timezone: 'UTC', + }, [ + { + orders__date_year: '2025-01-01T00:00:00.000Z', + orders__revenue: '300', + orders__revenue_1_y_ago: '100', + }, + ], { compiler, joinGraph, cubeEvaluator })); + } else { + // This test is working only in tesseract + test.skip('prior-year time_shift measure is not emptied by FILTER_PARAMS', () => { expect(1).toBe(1); }); + } +}); 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 51e1dee0dad85..4cb5e629b3b17 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 @@ -25,8 +25,14 @@ impl ToSql for BaseFilter { .filter_params_columns .get(&symbol_to_match.full_name()) { + let time_shift = visitor + .time_shifts() + .dimensions_shifts + .get(&symbol_to_match.full_name()) + .and_then(|shift| shift.interval.as_ref()); return self.typed_filter().to_sql_for_filter_params( filter_params_column, + time_shift, &query_tools, templates, filters_ctx, 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 247fe84aa1058..e9b7f3077115e 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_templates::PlanSqlTemplates; use crate::planner::FiltersContext; +use crate::planner::SqlInterval; use cubenativeutils::CubeError; use std::rc::Rc; @@ -48,6 +49,7 @@ impl TypedFilter { pub fn to_sql_for_filter_params( &self, column: &FilterParamsColumn, + time_shift: Option<&SqlInterval>, query_tools: &Rc, plan_templates: &PlanSqlTemplates, filters_context: &FiltersContext, @@ -56,8 +58,23 @@ impl TypedFilter { match column { FilterParamsColumn::String(column_sql) => { + // Inside a time-shifted CTE the FILTER_PARAMS column must carry the + // same shift as the regular time-dimension filter, otherwise its + // current-period bounds contradict the shifted predicate and empty + // the CTE. + let shifted_column; + let member_sql = if let Some(interval) = time_shift { + shifted_column = format!( + "({})", + plan_templates + .add_timestamp_interval(column_sql.clone(), interval.to_sql())? + ); + shifted_column.as_str() + } else { + column_sql.as_str() + }; let ctx = FilterSqlContext { - member_sql: column_sql, + member_sql, query_tools, plan_templates, use_db_time_zone, @@ -66,6 +83,8 @@ impl TypedFilter { dispatch_to_sql(self.operation(), &ctx) } FilterParamsColumn::Callback(callback) => { + // A callback column is opaque SQL produced by user code, so a + // time shift can't be wrapped around it; it is rendered as-is. let args = match self.operation() { // RollingWindowOffset carries [from, to, trailing, leading, offset]; // only the from/to dates are filter-param args for the callback. 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 302aec8a5efc3..1f6b80a243645 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 @@ -52,6 +52,10 @@ impl SqlNodesFactory { self.time_shifts = time_shifts; } + pub fn time_shifts(&self) -> &TimeShiftState { + &self.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 7b0f6876b9aa2..d8e82c30ea280 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/sql_visitor.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/sql_visitor.rs @@ -1,6 +1,7 @@ use super::sql_nodes::SqlNode; use super::CubeRefEvaluator; use crate::planner::filter::Filter; +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; @@ -13,6 +14,9 @@ pub struct SqlEvaluatorVisitor { query_tools: Rc, cube_ref_evaluator: Rc, all_filters: Option, //To pass to FILTER_PARAMS and FILTER_GROUP + /// 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, 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 — @@ -30,11 +34,22 @@ impl SqlEvaluatorVisitor { query_tools, cube_ref_evaluator, all_filters, + time_shifts: TimeShiftState::default(), ignore_tz_convert: false, arg_needs_paren_safe: false, } } + pub fn with_time_shifts(&self, time_shifts: TimeShiftState) -> Self { + let mut self_copy = self.clone(); + self_copy.time_shifts = time_shifts; + self_copy + } + + pub fn time_shifts(&self) -> &TimeShiftState { + &self.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 bfbc6f7b2c2a1..47f195bdc2995 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/visitor_context.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/visitor_context.rs @@ -2,6 +2,7 @@ use crate::physical_plan::cube_ref_evaluator::CubeRefEvaluator; use crate::physical_plan::sql_nodes::{SqlNode, SqlNodesFactory}; use crate::physical_plan::sql_visitor::SqlEvaluatorVisitor; 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::FiltersContext; @@ -15,6 +16,7 @@ pub struct VisitorContext { node_processor: Rc, 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 filters_context: FiltersContext, } @@ -35,6 +37,7 @@ impl VisitorContext { node_processor, cube_ref_evaluator: Rc::new(nodes_factory.cube_ref_evaluator()), all_filters, + time_shifts: nodes_factory.time_shifts().clone(), filters_context, } } @@ -43,6 +46,7 @@ impl VisitorContext { query_tools: Rc, nodes_factory: &SqlNodesFactory, filter_params_columns: HashMap, + time_shifts: TimeShiftState, ) -> Self { let filters_context = FiltersContext { use_local_tz: nodes_factory.use_local_tz_in_date_range(), @@ -55,6 +59,7 @@ impl VisitorContext { node_processor, cube_ref_evaluator: Rc::new(nodes_factory.cube_ref_evaluator()), all_filters: None, + time_shifts, filters_context, } } @@ -65,6 +70,7 @@ impl VisitorContext { self.cube_ref_evaluator.clone(), self.all_filters.clone(), ) + .with_time_shifts(self.time_shifts.clone()) } pub fn node_processor(&self) -> Rc { diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/member_query_planner.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/member_query_planner.rs index 689d4fc95a3d2..0a3e162909e66 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/member_query_planner.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/member_query_planner.rs @@ -542,7 +542,7 @@ impl MultiStageMemberQueryPlanner { let dimensions = if let Some(exclude) = &grain.exclude { dimensions .into_iter() - .filter(|d| !exclude.iter().any(|m| d.has_member_in_reference_chain(m))) + .filter(|d| !exclude.iter().any(|m| d.matches_grain_reference(m))) .collect_vec() } else { dimensions @@ -550,7 +550,7 @@ impl MultiStageMemberQueryPlanner { let dimensions = if let Some(keep_only) = &grain.keep_only { dimensions .into_iter() - .filter(|d| keep_only.iter().any(|m| d.has_member_in_reference_chain(m))) + .filter(|d| keep_only.iter().any(|m| d.matches_grain_reference(m))) .collect_vec() } else { dimensions diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/multi_stage_query_planner.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/multi_stage_query_planner.rs index f8a9fa7c4755e..ded41d095ab1a 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/multi_stage_query_planner.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/multi_stage_query_planner.rs @@ -294,7 +294,7 @@ impl MultiStageQueryPlanner { ) -> Vec> { let dims: Vec> = if let Some(exclude) = &grain.exclude { dims.iter() - .filter(|d| !exclude.iter().any(|m| d.has_member_in_reference_chain(m))) + .filter(|d| !exclude.iter().any(|m| d.matches_grain_reference(m))) .cloned() .collect() } else { @@ -302,7 +302,7 @@ impl MultiStageQueryPlanner { }; if let Some(keep_only) = &grain.keep_only { dims.into_iter() - .filter(|d| keep_only.iter().any(|m| d.has_member_in_reference_chain(m))) + .filter(|d| keep_only.iter().any(|m| d.matches_grain_reference(m))) .collect() } else { dims diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call.rs index 4b496c4153855..de4a6f0eba293 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call.rs @@ -461,6 +461,7 @@ impl SqlCall { query_tools.clone(), &SqlNodesFactory::new(), filter_params_columns, + visitor.time_shifts().clone(), ); return crate::physical_plan::filter::render_filter_item( &context, &subtree, templates, diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/member_symbol.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/member_symbol.rs index 7e043285de13c..fe578228c11f3 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/member_symbol.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/member_symbol.rs @@ -236,6 +236,23 @@ impl MemberSymbol { false } + /// Whether this projected member is targeted by a grain reference + /// (`reduce_by` / `group_by`). Behaves like `has_member_in_reference_chain`, + /// but a time dimension also matches through its underlying base dimension: + /// a projected time dimension carries a granularity suffix in its full name + /// (`created_at_month`) while grain references point at the bare dimension + /// (`created_at`), so a grain reference removes the time dimension from the + /// grain regardless of the granularity it is queried at. + pub fn matches_grain_reference(&self, member: &Rc) -> bool { + if self.has_member_in_reference_chain(member) { + return true; + } + if let Self::TimeDimension(td) = self { + return td.base_symbol().has_member_in_reference_chain(member); + } + false + } + /// Returns a copy of this symbol with the path reduced to just the owning cube, /// stripping any join chain prefix (e.g. from views or cross-cube references). pub fn with_stripped_join_prefix(&self) -> Rc { diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/integration_multi_stage.yaml b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/integration_multi_stage.yaml index 30f379ae99f69..a35d632431675 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/integration_multi_stage.yaml +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/integration_multi_stage.yaml @@ -186,6 +186,17 @@ cubes: reduce_by: - orders.status + # reduce_by targets the time dimension itself (bare, no + # granularity). When the query selects created_at at some + # granularity, that granular dimension must be dropped from the + # partition regardless of its granularity suffix (CORE-550). + - name: amount_reduce_time + type: sum + sql: "{CUBE.total_amount}" + multi_stage: true + reduce_by: + - orders.created_at + - name: amount_reduce_all type: sum sql: "{CUBE.total_amount}" @@ -290,6 +301,20 @@ cubes: - sql: "{CUBE.total_amount}" dir: desc + # Rank reduced by the time dimension (bare) while ordered by a + # measure. With created_at selected at a granularity the partition + # must be "all outer dims except created_at" (CORE-550); the buggy + # planner left created_at_ in PARTITION BY so every partition + # held one row and all ranks collapsed to 1. + - name: amount_rank_reduce_time + type: rank + multi_stage: true + reduce_by: + - orders.created_at + order_by: + - sql: "{CUBE.total_amount}" + dir: desc + # Rank by the raw time dimension itself (order_by is a # dimension, not a measure) reduced by created_at — partition # is "all outer dims except created_at". First link in the @@ -533,3 +558,5 @@ views: - total_amount - amount_prev_month - mom_growth + - amount_reduce_time + - amount_rank_reduce_time diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/rank.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/rank.rs index 6ced7cd509567..5f29d2ad9a387 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/rank.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/rank.rs @@ -9,6 +9,66 @@ fn create_context() -> TestContext { const SEED: &str = "integration_multi_stage_tables.sql"; +/// PARTITION BY clause bodies (text between `PARTITION BY` and the following +/// `ORDER BY`) of every window function in `sql`. +fn partition_by_clauses(sql: &str) -> Vec { + sql.match_indices("PARTITION BY") + .map(|(i, _)| { + let rest = &sql[i + "PARTITION BY".len()..]; + let end = rest.find("ORDER BY").unwrap_or(rest.len()); + rest[..end].to_string() + }) + .collect() +} + +// CORE-550: a rank reduced by a bare time dimension, ordered by a measure, +// queried with that time dimension at a granularity. Partition must be +// [status] alone; the buggy planner kept created_at_month in PARTITION BY so +// each (status, month) partition held one row and every rank was 1. Correct +// ranks per status by descending monthly total (completed Mar>Feb>Jan = 1,2,3; +// pending 1,2,3; cancelled Mar=1, Jan/Feb tie at 2). +#[tokio::test(flavor = "multi_thread")] +async fn test_rank_reduce_by_time_dimension() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.total_amount + - orders.amount_rank_reduce_time + dimensions: + - orders.status + time_dimensions: + - dimension: orders.created_at + granularity: month + dateRange: + - "2024-01-01" + - "2024-03-31" + order: + - id: orders.status + - id: orders.created_at + "#}; + + let sql = ctx.build_sql(query).unwrap(); + for clause in partition_by_clauses(&sql) { + assert!( + !clause.contains("created_at"), + "reduce_by(created_at) must drop the time dimension from PARTITION BY,\n\ + partition clause was: {}\nfull SQL:\n{}", + clause, + sql, + ); + assert!( + clause.contains("status"), + "status must remain in PARTITION BY, partition clause was: {}", + clause, + ); + } + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + #[tokio::test(flavor = "multi_thread")] async fn test_basic_rank() { let ctx = create_context(); diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/reduce_by.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/reduce_by.rs index b47565f070354..218a723717959 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/reduce_by.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/reduce_by.rs @@ -27,6 +27,18 @@ fn assert_no_window(sql: &str) { ); } +/// PARTITION BY clause bodies (text between `PARTITION BY` and the following +/// `ORDER BY`) of every window function in `sql`. +fn partition_by_clauses(sql: &str) -> Vec { + sql.match_indices("PARTITION BY") + .map(|(i, _)| { + let rest = &sql[i + "PARTITION BY".len()..]; + let end = rest.find("ORDER BY").unwrap_or(rest.len()); + rest[..end].to_string() + }) + .collect() +} + // add_group_by + reduce_by together: leaf grain extends with customer_id // while partition grain shrinks by removing category. Three distinct grains: // leaf = (status, category, customer_id) ← per-customer sum(amount) @@ -317,6 +329,57 @@ async fn test_reduce_by_count_distinct() { } } +// CORE-550: reduce_by a bare time dimension must drop it from the window's +// PARTITION BY even when the query selects that dimension at a granularity +// (auto-suffixed `_month`). The buggy planner matched reduce_by references by +// full_name, so `orders.created_at` never matched the granular +// `orders.created_at_month`; created_at stayed in the partition, the window +// collapsed to plain group-by and the "reduced" value equalled total_amount. +// Correct: partition = [status] → per-status total broadcast across months +// (completed=1400, pending=650, cancelled=200). +#[tokio::test(flavor = "multi_thread")] +async fn test_reduce_by_time_dimension() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.total_amount + - orders.amount_reduce_time + dimensions: + - orders.status + time_dimensions: + - dimension: orders.created_at + granularity: month + dateRange: + - "2024-01-01" + - "2024-03-31" + order: + - id: orders.status + - id: orders.created_at + "#}; + + let sql = ctx.build_sql(query).unwrap(); + assert_uses_window(&sql); + for clause in partition_by_clauses(&sql) { + assert!( + !clause.contains("created_at"), + "reduce_by(created_at) must drop the time dimension from PARTITION BY,\n\ + partition clause was: {}\nfull SQL:\n{}", + clause, + sql, + ); + assert!( + clause.contains("status"), + "status must remain in PARTITION BY, partition clause was: {}", + clause, + ); + } + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + #[tokio::test(flavor = "multi_thread")] async fn test_reduce_by_with_time() { let ctx = create_context(); diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__rank__rank_reduce_by_time_dimension.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__rank__rank_reduce_by_time_dimension.snap new file mode 100644 index 0000000000000..23c7fb780543a --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__rank__rank_reduce_by_time_dimension.snap @@ -0,0 +1,15 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/rank.rs +expression: result +--- +orders__status | orders__created_at_month | orders__total_amount | orders__amount_rank_reduce_time +---------------+--------------------------+----------------------+-------------------------------- +cancelled | 2024-01-01 00:00:00 | 50.00 | 2 +cancelled | 2024-02-01 00:00:00 | 50.00 | 2 +cancelled | 2024-03-01 00:00:00 | 100.00 | 1 +completed | 2024-01-01 00:00:00 | 300.00 | 3 +completed | 2024-02-01 00:00:00 | 500.00 | 2 +completed | 2024-03-01 00:00:00 | 600.00 | 1 +pending | 2024-01-01 00:00:00 | 150.00 | 3 +pending | 2024-02-01 00:00:00 | 200.00 | 2 +pending | 2024-03-01 00:00:00 | 300.00 | 1 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__reduce_by__reduce_by_time_dimension.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__reduce_by__reduce_by_time_dimension.snap new file mode 100644 index 0000000000000..dfd1962f7d59b --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__reduce_by__reduce_by_time_dimension.snap @@ -0,0 +1,15 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/reduce_by.rs +expression: result +--- +orders__status | orders__created_at_month | orders__total_amount | orders__amount_reduce_time +---------------+--------------------------+----------------------+--------------------------- +cancelled | 2024-01-01 00:00:00 | 50.00 | 200.00 +cancelled | 2024-02-01 00:00:00 | 50.00 | 200.00 +cancelled | 2024-03-01 00:00:00 | 100.00 | 200.00 +completed | 2024-01-01 00:00:00 | 300.00 | 1400.00 +completed | 2024-02-01 00:00:00 | 500.00 | 1400.00 +completed | 2024-03-01 00:00:00 | 600.00 | 1400.00 +pending | 2024-01-01 00:00:00 | 150.00 | 650.00 +pending | 2024-02-01 00:00:00 | 200.00 | 650.00 +pending | 2024-03-01 00:00:00 | 300.00 | 650.00 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__views__view_reduce_by_time_dimension.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__views__view_reduce_by_time_dimension.snap new file mode 100644 index 0000000000000..ccb383a72258b --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__views__view_reduce_by_time_dimension.snap @@ -0,0 +1,15 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/views.rs +expression: result +--- +orders_ms_view__status | orders_ms_view__created_at_month | orders_ms_view__total_amount | orders_ms_view__amount_reduce_time +-----------------------+----------------------------------+------------------------------+----------------------------------- +cancelled | 2024-01-01 00:00:00 | 50.00 | 200.00 +cancelled | 2024-02-01 00:00:00 | 50.00 | 200.00 +cancelled | 2024-03-01 00:00:00 | 100.00 | 200.00 +completed | 2024-01-01 00:00:00 | 300.00 | 1400.00 +completed | 2024-02-01 00:00:00 | 500.00 | 1400.00 +completed | 2024-03-01 00:00:00 | 600.00 | 1400.00 +pending | 2024-01-01 00:00:00 | 150.00 | 650.00 +pending | 2024-02-01 00:00:00 | 200.00 | 650.00 +pending | 2024-03-01 00:00:00 | 300.00 | 650.00 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/views.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/views.rs index ecba130ba8c98..b1c20a018f154 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/views.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/views.rs @@ -9,6 +9,76 @@ fn create_context() -> TestContext { const SEED: &str = "integration_multi_stage_tables.sql"; +fn assert_uses_window(sql: &str) { + assert!( + sql.contains("OVER (PARTITION BY"), + "expected SQL to use a window function (`... OVER (PARTITION BY ...)`),\n\ + got:\n{}", + sql, + ); +} + +/// PARTITION BY clause bodies (text between `PARTITION BY` and the following +/// `ORDER BY`) of every window function in `sql`. +fn partition_by_clauses(sql: &str) -> Vec { + sql.match_indices("PARTITION BY") + .map(|(i, _)| { + let rest = &sql[i + "PARTITION BY".len()..]; + let end = rest.find("ORDER BY").unwrap_or(rest.len()); + rest[..end].to_string() + }) + .collect() +} + +// CORE-550 through a view: the query groups by view dimensions +// (orders_ms_view.status / .created_at) while the measure's reduce_by targets +// the underlying base cube member (orders.created_at). The granular view time +// dimension (created_at_month) must still be dropped from the window's +// PARTITION BY, leaving [status]. Guards that the fix resolves the reduce_by +// reference through the view→base reference chain, not just for plain cubes. +#[tokio::test(flavor = "multi_thread")] +async fn test_view_reduce_by_time_dimension() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders_ms_view.total_amount + - orders_ms_view.amount_reduce_time + dimensions: + - orders_ms_view.status + time_dimensions: + - dimension: orders_ms_view.created_at + granularity: month + dateRange: + - "2024-01-01" + - "2024-03-31" + order: + - id: orders_ms_view.status + - id: orders_ms_view.created_at + "#}; + + let sql = ctx.build_sql(query).unwrap(); + assert_uses_window(&sql); + for clause in partition_by_clauses(&sql) { + assert!( + !clause.contains("created_at"), + "reduce_by(created_at) must drop the view time dimension from PARTITION BY,\n\ + partition clause was: {}\nfull SQL:\n{}", + clause, + sql, + ); + assert!( + clause.contains("status"), + "status must remain in PARTITION BY, partition clause was: {}", + clause, + ); + } + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + #[tokio::test(flavor = "multi_thread")] async fn test_view_with_time_shift() { let ctx = create_context();