diff --git a/docs-mintlify/reference/data-modeling/context-variables.mdx b/docs-mintlify/reference/data-modeling/context-variables.mdx index 34919d971bab8..2c607b67035d1 100644 --- a/docs-mintlify/reference/data-modeling/context-variables.mdx +++ b/docs-mintlify/reference/data-modeling/context-variables.mdx @@ -332,6 +332,66 @@ cube(`multi_filter`, { }) ``` +### Example with segment + +A `FILTER_PARAMS` argument can also name a [segment][ref-ref-segments]. A segment +is not compared to a value, so the expression passed to `filter()` is the whole +predicate rather than a column: it is rendered as-is when the query selects that +segment, and as `1 = 1` when it does not. + + + +```yaml title="YAML" +cubes: + - name: events + sql: | + SELECT * + FROM events + WHERE {FILTER_PARAMS.events.start_load.filter( + "evid = 115 AND action_group = 'load'" + )} + + segments: + - name: start_load + sql: "{CUBE}.evid = 115 AND {CUBE}.action_group = 'load'" +``` + +```javascript title="JavaScript" +cube(`events`, { + sql: ` + SELECT * + FROM events + WHERE ${FILTER_PARAMS.events.start_load.filter( + `evid = 115 AND action_group = 'load'` + )} + `, + + segments: { + start_load: { + sql: `${CUBE}.evid = 115 AND ${CUBE}.action_group = 'load'` + } + } +}) +``` + + + +The segment's own `sql` prefixes its columns with the cube, which is not in +scope inside the `sql` that builds that cube, so the pushed-down predicate is +restated in `filter()` — the same way a dimension's column is. + +A function may be passed instead of a string, as long as it takes no arguments: +a segment carries no filter values to pass to it. A function that does take +arguments renders as `1 = 1`. + + + +Naming a segment is only supported by the default SQL planner. With +[`CUBEJS_TESSERACT_SQL_PLANNER`][ref-env-tesseract] set to `false`, such a +binding always renders as `1 = 1`. + + + ## `FILTER_GROUP` If you use `FILTER_PARAMS` in your query more than once, you must wrap them @@ -817,4 +877,6 @@ cube(`orders`, { [ref-query-filter]: /reference/core-data-apis/rest-api/query-format#query-properties [ref-dynamic-jinja]: /docs/data-modeling/dynamic/jinja [ref-filter-boolean]: /reference/core-data-apis/rest-api/query-format#boolean-logical-operators -[ref-links]: /reference/data-modeling/dimensions#links \ No newline at end of file +[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 diff --git a/packages/cubejs-schema-compiler/test/integration/postgres/filter-params-segment.test.ts b/packages/cubejs-schema-compiler/test/integration/postgres/filter-params-segment.test.ts new file mode 100644 index 0000000000000..9060a811e4994 --- /dev/null +++ b/packages/cubejs-schema-compiler/test/integration/postgres/filter-params-segment.test.ts @@ -0,0 +1,184 @@ +import { getEnv } from '@cubejs-backend/shared'; +import { PostgresQuery } from '../../../src/adapter/PostgresQuery'; +import { prepareJsCompiler } from '../../unit/PrepareCompiler'; +import { dbRunner } from './PostgresDBRunner'; + +// A cube wrapping a large scan pushes predicates into its own `sql` through +// FILTER_PARAMS. A segment can be named there like any other member: its +// binding renders the column it was given whenever the query selects that +// segment, and `1 = 1` when it does not. +describe('FILTER_PARAMS referencing a segment', () => { + jest.setTimeout(200000); + + const events = ` + SELECT * FROM ( + SELECT 1 as id, 115 as evid, 'load' as action_group, 'us' as region + union all + SELECT 2 as id, 115 as evid, 'load' as action_group, 'eu' as region + union all + SELECT 3 as id, 200 as evid, 'click' as action_group, 'us' as region + ) AS t + `; + + const compilers = prepareJsCompiler(` + cube('events', { + sql: \`${events} WHERE \${FILTER_PARAMS.events.start_load.filter("evid = 115 AND action_group = 'load'")} + AND \${FILTER_PARAMS.events.region.filter('region')}\`, + measures: { + count: { type: 'count' }, + }, + dimensions: { + id: { sql: 'id', type: 'number', primaryKey: true }, + region: { sql: 'region', type: 'string' }, + }, + segments: { + start_load: { sql: \`\${CUBE}.evid = 115 AND \${CUBE}.action_group = 'load'\` }, + us_only: { sql: \`\${CUBE}.region = 'us'\` }, + }, + }); + + cube('grouped_events', { + sql: \`${events} WHERE \${FILTER_GROUP( + FILTER_PARAMS.grouped_events.start_load.filter("evid = 115 AND action_group = 'load'"), + FILTER_PARAMS.grouped_events.region.filter('region') + )}\`, + measures: { + count: { type: 'count' }, + }, + dimensions: { + id: { sql: 'id', type: 'number', primaryKey: true }, + region: { sql: 'region', type: 'string' }, + }, + segments: { + start_load: { sql: \`\${CUBE}.evid = 115 AND \${CUBE}.action_group = 'load'\` }, + }, + }); + + cube('callback_events', { + sql: \`${events} WHERE \${FILTER_PARAMS.callback_events.start_load.filter(() => "evid = 115")} + AND \${FILTER_PARAMS.callback_events.needs_value.filter((v) => 'evid = ' + v)}\`, + measures: { + count: { type: 'count' }, + }, + dimensions: { + id: { sql: 'id', type: 'number', primaryKey: true }, + }, + segments: { + start_load: { sql: \`\${CUBE}.evid = 115\` }, + needs_value: { sql: \`\${CUBE}.evid = 115\` }, + }, + }); + `); + + // The cube's `sql` is a subquery aliased as the cube, so everything before + // that alias is what the pushdown produced. + const baseSql = (sql: string, cube: string) => { + const alias = sql.indexOf(`AS "${cube}"`); + // Without the alias the slice would be the whole query, and the negative + // assertions would silently stop testing the pushed-down part. + expect(alias).toBeGreaterThan(-1); + return sql.slice(0, alias); + }; + + const buildSql = async (query: any) => { + await compilers.compiler.compile(); + return new PostgresQuery(compilers, { timezone: 'UTC', ...query }).buildSqlAndParams()[0]; + }; + + if (getEnv('nativeSqlPlanner')) { + it('pushes the segment predicate into the cube sql when the segment is selected', async () => { + const sql = await buildSql({ + measures: ['events.count'], + segments: ['events.start_load'], + }); + + expect(baseSql(sql, 'events')).toMatch(/evid = 115 AND action_group = 'load'/); + }); + + it('leaves the binding always-true when the segment is not selected', async () => { + const sql = await buildSql({ measures: ['events.count'] }); + + expect(baseSql(sql, 'events')).not.toMatch(/evid = 115/); + expect(baseSql(sql, 'events')).toMatch(/1\s*=\s*1/); + }); + + it('does not activate the binding for a different segment', async () => { + const sql = await buildSql({ + measures: ['events.count'], + segments: ['events.us_only'], + }); + + expect(baseSql(sql, 'events')).not.toMatch(/evid = 115/); + }); + + it('pushes the segment down alongside a dimension filter', async () => { + const sql = await buildSql({ + measures: ['events.count'], + segments: ['events.start_load'], + filters: [{ member: 'events.region', operator: 'equals', values: ['us'] }], + }); + + const base = baseSql(sql, 'events'); + expect(base).toMatch(/evid = 115 AND action_group = 'load'/); + expect(base).toMatch(/region = \$\d/); + expect(base).not.toMatch(/1\s*=\s*1/); + }); + + it('renders the segment as one member of a FILTER_GROUP', async () => { + const sql = await buildSql({ + measures: ['grouped_events.count'], + segments: ['grouped_events.start_load'], + }); + + expect(baseSql(sql, 'grouped_events')).toMatch(/evid = 115 AND action_group = 'load'/); + }); + + it('renders a callback column that takes no filter values', async () => { + const sql = await buildSql({ + measures: ['callback_events.count'], + segments: ['callback_events.start_load'], + }); + + // Nothing else in this cube's sql states the predicate, so it can only + // have come from the callback the binding compiled. + expect(baseSql(sql, 'callback_events')).toMatch(/WHERE \(evid = 115\)/); + }); + + // A segment supplies no values, so a column that takes one cannot render. + // Dropping only its restatement is narrower than binding a value the + // segment never gave — the segment still filters the query on its own. + it('leaves a value-taking callback column always-true', async () => { + const sql = await buildSql({ + measures: ['callback_events.count'], + segments: ['callback_events.needs_value'], + }); + + // The parentheses are what the filter renderer adds around a binding it + // reached, so they tell an activated-then-dropped column apart from the + // bare `1 = 1` of a binding whose segment was never selected. + expect(baseSql(sql, 'callback_events')).toMatch(/AND \(1\s*=\s*1\)/); + expect(baseSql(sql, 'callback_events')).not.toMatch(/undefined|\{fpv:/); + }); + + // The predicate now applies both inside the cube's sql and in the outer + // WHERE the segment always produced. Both restrict the same rows, so the + // result must be what the segment alone selected. + it('counts the segment rows once', async () => dbRunner.runQueryTest({ + measures: ['events.count'], + segments: ['events.start_load'], + timezone: 'UTC', + }, [ + { events__count: '2' }, + ], compilers)); + + it('counts every row when no segment is selected', async () => dbRunner.runQueryTest({ + measures: ['events.count'], + timezone: 'UTC', + }, [ + { events__count: '3' }, + ], compilers)); + } else { + // Segment pushdown is implemented in the Tesseract planner only. + test.skip('FILTER_PARAMS referencing a segment', () => { expect(1).toBe(1); }); + } +}); diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/base_segment.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/base_segment.rs index 9f09873ceb519..ab38106ad1e23 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/base_segment.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/base_segment.rs @@ -1,8 +1,10 @@ use super::ToSql; +use crate::cube_bridge::member_sql::FilterParamsColumn; use crate::physical_plan::sql_nodes::SqlNode; use crate::physical_plan::SqlEvaluatorVisitor; use crate::planner::filter::BaseSegment; use crate::planner::query_tools::QueryTools; +use crate::planner::sql_call::SqlCallFilterParamsItem; use crate::planner::sql_templates::PlanSqlTemplates; use crate::planner::FiltersContext; use cubenativeutils::CubeError; @@ -13,10 +15,20 @@ impl ToSql for BaseSegment { &self, visitor: &SqlEvaluatorVisitor, node_processor: Rc, - _query_tools: Rc, + query_tools: Rc, templates: &PlanSqlTemplates, filters_ctx: &FiltersContext, ) -> Result { + if let Some(item) = self.matching_filter_params_column(filters_ctx) { + return self.filter_params_column_sql( + item, + visitor, + node_processor, + query_tools, + templates, + ); + } + let sql = visitor.apply(&self.member_evaluator(), node_processor, templates)?; if filters_ctx.reading_pre_aggregation { // The segment is a stored pre-aggregation column; compare it to its @@ -27,3 +39,57 @@ impl ToSql for BaseSegment { } } } + +impl BaseSegment { + // The binding named by the path the query asked for is the one the model + // meant. A view re-exporting a segment leaves only the underlying cube's + // path to match, which takes a scan; the name breaks the tie when a group + // binds both paths, since the map is unordered. + fn matching_filter_params_column<'a>( + &self, + filters_ctx: &'a FiltersContext, + ) -> Option<&'a SqlCallFilterParamsItem> { + if let Some(item) = filters_ctx.filter_params_columns.get(&self.full_name()) { + return Some(item); + } + filters_ctx + .filter_params_columns + .iter() + .filter(|(name, _)| self.matches_member_name(name)) + .min_by_key(|(name, _)| *name) + .map(|(_, item)| item) + } + + // A segment restricts the rows without comparing anything to a filter value, + // so its `FILTER_PARAMS` column is the whole predicate rather than the left + // side of one. + fn filter_params_column_sql( + &self, + item: &SqlCallFilterParamsItem, + visitor: &SqlEvaluatorVisitor, + node_processor: Rc, + query_tools: Rc, + templates: &PlanSqlTemplates, + ) -> Result { + match &item.column { + FilterParamsColumn::String(column_sql) => Ok(column_sql.clone()), + FilterParamsColumn::Compiled(compiled) if compiled.value_params_count == 0 => { + let Some(call) = &item.compiled_call else { + return Err(CubeError::internal(format!( + "Compiled filter params column for `{}` has no call", + item.filter_symbol_name + ))); + }; + call.eval(visitor, node_processor, query_tools, templates) + } + // A column that takes filter values cannot render for a segment, + // which supplies none — a rest parameter consumes as many as the + // query happens to give, and a parameter list that could not be read + // is assumed to take some. Only the restatement inside this SQL is + // dropped; the segment still reaches the query on its own. + FilterParamsColumn::Compiled(_) | FilterParamsColumn::Callback(_) => { + templates.always_true() + } + } + } +} diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/base_segment.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/base_segment.rs index d8003789a9d7b..e7520a254ce27 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/base_segment.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/base_segment.rs @@ -57,6 +57,36 @@ impl BaseSegment { pub fn is_member_expression(&self) -> bool { self.is_member_expression } + + /// Whether `member` names this segment, as a `FILTER_PARAMS` binding or a + /// filter-tree target does. A view exposes a segment under its own path + /// while a binding in the underlying cube's sql names the cube's, so every + /// segment in the reference chain counts, not only the name the query asked + /// for. The chain stops at the first non-segment: a segment whose sql is a + /// bare reference resolves on to that dimension, whose own binding states a + /// column to compare a value against rather than a predicate. + pub fn matches_member_name(&self, member: &str) -> bool { + if self.is_member_expression { + return false; + } + if self.full_name == member { + return true; + } + let mut current = Some(self.member_evaluator.clone()); + while let Some(symbol) = current { + if symbol.as_member_expression().is_err() { + return false; + } + // A segment symbol lives in the `expr:` namespace, so the path is + // reassembled from the cube and member names it was compiled under. + if format!("{}.{}", symbol.cube_name(), symbol.name()) == member { + return true; + } + current = symbol.reference_member(); + } + false + } + pub fn full_name(&self) -> String { self.full_name.clone() } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/tree.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/tree.rs index 5c5d792cc75a4..43effff4e6e13 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/tree.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/tree.rs @@ -115,8 +115,9 @@ impl FilterItem { /// Partial matching is only supported for AND groups. OR groups are /// preserved only when all of their children match the target members. /// - /// `Segment` nodes are skipped during extraction — they do not prevent - /// sibling member filters from being collected in AND groups. + /// A `Segment` node matches when `target_members` names it, and is + /// otherwise skipped — skipping does not prevent sibling member filters + /// from being collected in AND groups. pub fn find_subtree_for_members(&self, target_members: &[&String]) -> Option { self.find_subtree_for_members_inner(target_members) .map(|(filter_item, _)| filter_item) @@ -198,7 +199,16 @@ impl FilterItem { None } } - FilterItem::Segment(_) => None, + FilterItem::Segment(segment) => { + if target_members + .iter() + .any(|target| segment.matches_member_name(target)) + { + Some((self.clone(), true)) + } else { + None + } + } } } 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 a5befaa8c1136..845bbe7cba12b 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 @@ -14,6 +14,7 @@ use crate::planner::filter::BaseFilter; use crate::planner::filter::FilterItem; use crate::planner::filter::FilterOperator; use crate::planner::state::State; +use crate::planner::symbols::deps::{collect_cube_refs, collect_deps, SymbolDeps}; use crate::planner::symbols::transforms; use crate::planner::symbols::AggregationType; use crate::planner::Case; @@ -221,11 +222,16 @@ impl MultiStageQueryPlanner { &self, member: Rc, new_state: Rc, + parent_state: &Rc, result: &mut Vec>, descriptions: &mut Vec>, resolved_multi_stage_dimensions: &mut HashSet, scope: &mut PlanningScope, ) -> Result<(), CubeError> { + // The CASE-SWITCH path plans every branch dependency as its own CTE, + // dimensions included, so each one is a column of the source rather than + // something the stage grain has to carry. It deliberately skips the + // reachability check `default_make_childs` applies. if let Some(Case::CaseSwitch(case_switch)) = member.case() { if self.try_make_childs_for_case_switch( case_switch, @@ -241,6 +247,7 @@ impl MultiStageQueryPlanner { self.default_make_childs( member, new_state, + parent_state, result, descriptions, resolved_multi_stage_dimensions, @@ -330,11 +337,21 @@ impl MultiStageQueryPlanner { &self, member: Rc, new_state: Rc, + parent_state: &Rc, result: &mut Vec>, descriptions: &mut Vec>, resolved_multi_stage_dimensions: &mut HashSet, scope: &mut PlanningScope, ) -> Result<(), CubeError> { + let is_masked = |m: &Rc| { + self.query_tools + .query_tools() + .is_member_masked(&m.full_name()) + }; + let rendered: HashSet = rendered_dependencies(&member, &is_masked) + .into_iter() + .map(|d| d.resolve_reference_chain().full_name()) + .collect(); let mut has_inputs = false; for dep in member.get_dependencies() { let dep = &dep.resolve_reference_chain(); @@ -350,6 +367,8 @@ impl MultiStageQueryPlanner { if !description.is_multi_stage_dimension() || member.as_dimension().is_ok() { result.push(description); } + } else if dep.is_dimension() && rendered.contains(&dep.full_name()) { + self.check_dimension_is_reachable(&member, dep, &new_state, parent_state)?; } } if !has_inputs { @@ -374,6 +393,36 @@ impl MultiStageQueryPlanner { Ok(()) } + /// A dimension read by a multi-stage member's SQL is rendered against the + /// CTE that computes the member, so it has to be a column of it. Two grains + /// can supply one: the stage's own (`grain_state`) and, when the assembly + /// broadcasts a narrowed measure back onto the query grid, the keys side + /// built from `parent_state`. Beyond those the dimension has no rendering at + /// all — the fallback is its own cube alias, and no cube is part of a CTE + /// built from subqueries. + fn check_dimension_is_reachable( + &self, + member: &Rc, + dimension: &Rc, + grain_state: &QueryProperties, + parent_state: &QueryProperties, + ) -> Result<(), CubeError> { + if dimension_is_reachable(dimension, grain_state, parent_state, &|m| { + self.query_tools + .query_tools() + .is_member_masked(&m.full_name()) + }) { + return Ok(()); + } + Err(CubeError::user(format!( + "Multi-stage member {member} reads dimension {dimension}, which is not part of the \ + grain it is computed at. Add {dimension} to `grain.include` of {member}, or remove \ + it from the member's sql.", + member = member.full_name(), + dimension = dimension.full_name(), + ))) + } + /// Plans CASE-SWITCH dependencies: collects, per dependency, the /// union of switch values it covers and renders each dependency /// under a state with an equality filter on the switch member @@ -640,6 +689,7 @@ impl MultiStageQueryPlanner { self.make_childs( member.clone(), new_state.clone(), + &state, &mut input, descriptions, resolved_multi_stage_dimensions, @@ -678,6 +728,7 @@ impl MultiStageQueryPlanner { self.make_childs( member.clone(), state.clone(), + &state, &mut keys_input, descriptions, resolved_multi_stage_dimensions, @@ -1103,6 +1154,119 @@ impl MultiStageQueryPlanner { } } +// Mirrors how references are resolved when the CTE is rendered: a member the +// source exposes as a column stops the walk, and anything else is reachable only +// if every member its own SQL reads is. A dimension that also reads a raw cube +// column is unreachable whatever its member deps resolve to — that column renders +// against the cube alias, which such a CTE never has in scope. +// +// A leaf outside both grains is reported as well. `sql: category` and a constant +// `sql: "'x'"` are indistinguishable here — neither carries a cube ref — so the +// constant is reported too, and declaring it in `grain.include` resolves that the +// same way. The same ambiguity runs the other way: a bare identifier inside a +// larger expression (`UPPER({CUBE.status}) || category`) carries no cube ref +// either, so it passes and fails at the database instead. +fn dimension_is_reachable( + dimension: &Rc, + grain_state: &QueryProperties, + parent_state: &QueryProperties, + is_masked: &dyn Fn(&Rc) -> bool, +) -> bool { + let target = dimension.full_name(); + let carries = |state: &QueryProperties| { + state + .dimensions() + .iter() + .chain(state.time_dimensions().iter()) + .any(|d| d.clone().resolve_reference_chain().full_name() == target) + }; + if carries(grain_state) || carries(parent_state) { + return true; + } + let deps = rendered_dependencies(dimension, is_masked); + !deps.is_empty() + && !reads_raw_cube_column(dimension, is_masked) + && deps.iter().all(|dep| { + dimension_is_reachable( + &dep.clone().resolve_reference_chain(), + grain_state, + parent_state, + is_masked, + ) + }) +} + +// The slots of a member that reach its rendered SQL. `drill_filters` are carried +// by the symbol but never emitted, so they never put a column requirement on the +// CTE. A `mask` is emitted only for the members it applies to, so it is included +// for those and excluded otherwise. Both the member deps and the cube refs have to +// be read through this, or an excluded slot leaks back in through the side that +// isn't filtered. +// +// `iter_sql_calls` is the neighbouring accessor and is deliberately not reused: +// on the measure side it covers `kind` and `case` only, missing `measure_filters` +// and `measure_order_by`. +fn visit_rendered_slots( + member: &Rc, + is_masked: &dyn Fn(&Rc) -> bool, + visit: &mut dyn FnMut(&dyn SymbolDeps), +) { + // A mask reaches the SQL exactly for the members it is applied to, so it + // counts as rendered only for those. The member's own slots below stay + // required even then: an unconditional mask replaces the original render + // rather than wrapping it, so strictly they are not emitted for a masked + // member — but the same model is broken for every unmasked one, and the + // narrower rule would only move where that surfaces. + if is_masked(member) { + if let Some(mask) = member.mask_sql() { + visit(mask); + } + } + if let Ok(measure) = member.as_measure() { + visit(measure.kind()); + for filter in measure.measure_filters() { + visit(filter); + } + for order_by in measure.measure_order_by() { + visit(order_by); + } + if let Some(case) = measure.case() { + visit(case); + } + } else if let Ok(dimension) = member.as_dimension() { + visit(dimension.kind()); + } else if let Ok(time_dimension) = member.as_time_dimension() { + // A time dimension is a view of its base: the granularity renders around + // whatever the base renders, so both contribute. + visit(time_dimension.granularity_obj()); + visit_rendered_slots(time_dimension.base_symbol(), is_masked, visit); + } else { + visit(member.as_ref()); + } +} + +fn rendered_dependencies( + member: &Rc, + is_masked: &dyn Fn(&Rc) -> bool, +) -> Vec> { + let mut result = vec![]; + visit_rendered_slots(member, is_masked, &mut |slot| { + result.extend(collect_deps(slot)) + }); + result +} + +fn reads_raw_cube_column( + member: &Rc, + is_masked: &dyn Fn(&Rc) -> bool, +) -> bool { + let mut found = false; + visit_rendered_slots(member, is_masked, &mut |slot| { + found = found || !collect_cube_refs(slot).is_empty() + }); + found +} + fn multi_stage_filter_directive(member: &Rc) -> Option { if let Ok(measure) = member.as_measure() { return measure.multi_stage().and_then(|m| m.filter.clone()); diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_member_sql.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_member_sql.rs index 6aa0d1836aae5..0e9546017538a 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_member_sql.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_member_sql.rs @@ -150,6 +150,20 @@ impl MockMemberSql { continue; } + // `{FILTER_PARAMS_COLUMN:.:}` records a + // FILTER_PARAMS binding whose column is a plain string, the way + // `.filter('created_at')` is written in the data model. + if let Some(body) = path.strip_prefix("FILTER_PARAMS_COLUMN:") { + let (cube_name, name, column) = Self::parse_filter_params_body(body)?; + let index = args.insert_filter_params(FilterParamsItem { + cube_name, + name, + column: FilterParamsColumn::String(column), + }); + result.push_str(&format!("{{fp:{}}}", index)); + continue; + } + // `{FILTER_PARAMS:.:}` records a FILTER_PARAMS // binding with a callback column and yields `{fp:N}`. Inside // ``, `[path]` is a member reference — recorded into this @@ -157,32 +171,12 @@ impl MockMemberSql { // dependency list — and `%N` stands for the Nth filter value the // planner passes at render time. if let Some(body) = path.strip_prefix("FILTER_PARAMS:") { - let (member, column) = body.split_once(':').ok_or_else(|| { - CubeError::user(format!( - "FILTER_PARAMS needs a `.:` body: {}", - body - )) - })?; - // The scanner above stops at the first `}`, so a column carrying - // one would have been cut short here. - if column.is_empty() || column.contains('{') { - return Err(CubeError::user(format!( - "FILTER_PARAMS column must be non-empty and reference members as `[path]`: {}", - column - ))); - } - let member_parts = member.split('.').collect::>(); - if member_parts.len() != 2 || member_parts.iter().any(|p| p.is_empty()) { - return Err(CubeError::user(format!( - "FILTER_PARAMS member must be `.`: {}", - member - ))); - } - let (cube_name, name) = (member_parts[0], member_parts[1]); - let column = Self::parse_column_references(column, &mut args, &mut args_names)?; + let (cube_name, name, column) = Self::parse_filter_params_body(body)?; + let column = + Self::parse_column_references(&column, &mut args, &mut args_names)?; let index = args.insert_filter_params(FilterParamsItem { - cube_name: cube_name.to_string(), - name: name.to_string(), + cube_name, + name, column: FilterParamsColumn::Callback(Rc::new( MockFilterParamsCallback::new(column), )), @@ -225,6 +219,36 @@ impl MockMemberSql { Ok((result, args, args_names)) } + // Splits a `.:` FILTER_PARAMS body. + fn parse_filter_params_body(body: &str) -> Result<(String, String, String), CubeError> { + let (member, column) = body.split_once(':').ok_or_else(|| { + CubeError::user(format!( + "FILTER_PARAMS needs a `.:` body: {}", + body + )) + })?; + // The scanner above stops at the first `}`, so a column carrying one + // would have been cut short here. + if column.is_empty() || column.contains('{') { + return Err(CubeError::user(format!( + "FILTER_PARAMS column must be non-empty and reference members as `[path]`: {}", + column + ))); + } + let member_parts = member.split('.').collect::>(); + if member_parts.len() != 2 || member_parts.iter().any(|p| p.is_empty()) { + return Err(CubeError::user(format!( + "FILTER_PARAMS member must be `.`: {}", + member + ))); + } + Ok(( + member_parts[0].to_string(), + member_parts[1].to_string(), + column.to_string(), + )) + } + // Replaces every `[path.to.member]` in a callback column with the `{arg:N}` // placeholder of its recorded path. fn parse_column_references( 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 cfae8627de248..43f33edd26866 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 @@ -131,6 +131,30 @@ cubes: type: string sql: "NULLIF(category, 'books')" + # Reads a member and a raw cube column in one expression: the member + # half can resolve against a CTE column, the raw half never can. + - name: status_and_category + type: string + sql: "UPPER({CUBE.status}) || {CUBE}.category" + + # Clean sql, with the raw cube column confined to the mask. Reading the + # cube refs unfiltered would treat it as a raw read of the dimension. + - name: status_upper_masked_by_column + type: string + sql: "UPPER({CUBE.status})" + mask: + sql: "{CUBE}.category" + + # Multi-stage time dimension carrying a mask: the mask is not rendered + # unless the member is masked, so the dimension it names must not be + # required of the CTE. + - name: created_at_masked_multi_stage + type: time + sql: "{CUBE.created_at}" + multi_stage: true + mask: + sql: "{CUBE.category}" + - name: customer_name type: string sql: "{customers.name}" @@ -371,6 +395,117 @@ cubes: type: prior timeDimension: orders.created_at + # Reads a plain dimension of its own cube alongside an aggregated + # dependency, without declaring it. At month grain the raw + # `created_at` value is no column of the CTE this measure is computed + # from, and the only rendering left would be the cube alias, which + # that CTE's FROM does not contain — the planner rejects it. + - name: amount_first_half_of_month + type: sum + sql: "CASE WHEN EXTRACT(DAY FROM {CUBE.created_at}) <= 15 THEN {CUBE.total_amount} ELSE 0 END" + multi_stage: true + + # The same read, with the grain the sql needs declared. + - name: amount_first_half_of_month_with_grain + type: sum + sql: "CASE WHEN EXTRACT(DAY FROM {CUBE.created_at}) <= 15 THEN {CUBE.total_amount} ELSE 0 END" + multi_stage: true + grain: + include: + - orders.created_at + + # Undeclared read one stage further out: the dimension is read by a + # measure whose aggregated dependency is itself multi-stage. + - name: amount_prev_month_first_half + type: sum + sql: "CASE WHEN EXTRACT(DAY FROM {CUBE.created_at}) <= 15 THEN {CUBE.amount_prev_month} ELSE 0 END" + multi_stage: true + + # Undeclared read of a non-time dimension. + - name: completed_amount_multi_stage + type: sum + sql: "CASE WHEN {CUBE.status} = 'completed' THEN {CUBE.total_amount} ELSE 0 END" + multi_stage: true + + # Reads the very dimension its own `reduce_by` drops from the stage + # grain. The dimension still reaches the CTE from the keys side the + # JOIN-model builds on the parent grain, so this one plans. + - name: amount_reduce_status_reading_status + type: sum + sql: "CASE WHEN {CUBE.status} = 'completed' THEN {CUBE.total_amount} ELSE 0 END" + multi_stage: true + reduce_by: + - orders.status + + # Reads a dimension built out of another one. It is no column of the + # CTE itself, but renders from the column its own sql reads. + - name: amount_by_category_label + type: sum + sql: "CASE WHEN {CUBE.category_label} = 'Literature' THEN {CUBE.total_amount} ELSE 0 END" + multi_stage: true + + # `drill_filters` are held on the symbol and never rendered, so the + # dimension this one names puts no column requirement on the CTE. + - name: amount_with_drill_filters + type: sum + sql: "{CUBE.total_amount}" + multi_stage: true + drill_filters: + - sql: "{CUBE.category} = 'books'" + + # A mask is rendered only for members the security context masks, so + # the dimension it reads puts no column requirement on the CTE either. + - name: amount_with_masked_dimension_read + type: sum + sql: "{CUBE.total_amount}" + multi_stage: true + mask: + sql: "CASE WHEN {CUBE.category} = 'books' THEN 0 ELSE 1 END" + + # The read resolves through a member, but the same expression also + # reaches a raw cube column that no CTE column can stand in for. + - name: amount_by_status_and_category + type: sum + sql: "CASE WHEN {CUBE.status_and_category} = 'X' THEN {CUBE.total_amount} ELSE 0 END" + multi_stage: true + + # The dimension read here is clean; only its unrendered mask names a + # raw cube column. + - name: amount_by_status_upper_masked + type: sum + sql: "CASE WHEN {CUBE.status_upper_masked_by_column} = 'COMPLETED' THEN {CUBE.total_amount} ELSE 0 END" + multi_stage: true + + # Masked, with the dimension the mask reads declared. Pins that an + # applied mask is rendered inside the multi-stage CTE and therefore + # reads its dimension as a column of it. + - name: amount_masked_by_category_in_grain + type: sum + sql: "{CUBE.total_amount}" + multi_stage: true + grain: + include: + - orders.category + mask: + sql: "CASE WHEN {CUBE.category} = 'books' THEN -1 ELSE 0 END" + + # Declares a grain its own leaf is computed at. + - name: amount_with_status_in_leaf_grain + type: sum + sql: "{CUBE.total_amount}" + multi_stage: true + grain: + include: + - orders.status + + # Reads the dimension the measure below it declared. A declared grain + # widens that measure's own leaf, not the columns its CTE projects, so + # this read still has nowhere to come from. + - name: amount_reading_a_child_leaf_grain + type: sum + sql: "CASE WHEN {CUBE.status} = 'completed' THEN {CUBE.amount_with_status_in_leaf_grain} ELSE 0 END" + multi_stage: true + - name: amount_mom_diff type: number sql: "{CUBE.total_amount} - {CUBE.amount_prev_month}" diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter_params_segment.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter_params_segment.rs new file mode 100644 index 0000000000000..d15de326db095 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter_params_segment.rs @@ -0,0 +1,169 @@ +use crate::test_fixtures::cube_bridge::MockSchema; +use crate::test_fixtures::test_utils::TestContext; +use indoc::indoc; + +// A cube whose `sql` pushes a segment down through FILTER_PARAMS. The pushed +// predicate is stated as the binding's column, the way a dimension binding +// states its column: the segment's own `sql` prefixes the columns with the cube, +// and no cube is in scope inside the sql that builds it. +fn schema() -> MockSchema { + MockSchema::from_yaml(indoc! {" + cubes: + - name: orders + sql: \"SELECT * FROM orders WHERE {FILTER_PARAMS_COLUMN:orders.completed:status = 'completed'}\" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: status + type: string + sql: status + - name: created_at + type: time + sql: created_at + measures: + - name: count + type: count + segments: + - name: completed + sql: \"{CUBE}.status = 'completed'\" + - name: recent + sql: \"{CUBE}.created_at > '2024-01-01'\" + + views: + - name: orders_view + cubes: + - join_path: orders + includes: + - count + - completed + "}) + .unwrap() +} + +#[test] +fn segment_in_query_pushes_its_filter_params_column_into_the_cube_sql() { + let ctx = TestContext::new(schema()).unwrap(); + + let (sql, _) = ctx + .build_sql_and_params(indoc! {" + measures: + - orders.count + segments: + - orders.completed + "}) + .unwrap(); + + assert!( + sql.contains("SELECT * FROM orders WHERE (status = 'completed')"), + "the segment's column must render inside the cube's sql\nsql: {}", + sql + ); + assert!( + !sql.contains("1 = 1"), + "the binding must not fall back to always-true\nsql: {}", + sql + ); +} + +#[test] +fn segment_absent_from_query_leaves_the_binding_always_true() { + let ctx = TestContext::new(schema()).unwrap(); + + let (sql, _) = ctx + .build_sql_and_params(indoc! {" + measures: + - orders.count + "}) + .unwrap(); + + assert!( + sql.contains("SELECT * FROM orders WHERE 1 = 1"), + "an unselected segment must not restrict the cube's sql\nsql: {}", + sql + ); +} + +#[test] +fn another_segment_in_query_does_not_activate_the_binding() { + let ctx = TestContext::new(schema()).unwrap(); + + let (sql, _) = ctx + .build_sql_and_params(indoc! {" + measures: + - orders.count + segments: + - orders.recent + "}) + .unwrap(); + + assert!( + sql.contains("SELECT * FROM orders WHERE 1 = 1"), + "only the segment the binding names may activate it\nsql: {}", + sql + ); +} + +#[test] +fn segment_selected_through_a_view_activates_the_cube_binding() { + let ctx = TestContext::new(schema()).unwrap(); + + let (sql, _) = ctx + .build_sql_and_params(indoc! {" + measures: + - orders_view.count + segments: + - orders_view.completed + "}) + .unwrap(); + + assert!( + sql.contains("SELECT * FROM orders WHERE (status = 'completed')"), + "a view re-exports the cube's segment, so the cube's binding still applies\nsql: {}", + sql + ); +} + +// A segment whose sql is a bare member reference resolves to that dimension. +// The dimension's own binding states a column to compare a value against, which +// is not a predicate, so selecting the segment must not activate it. +#[test] +fn segment_referencing_a_dimension_does_not_activate_that_dimensions_binding() { + let schema = MockSchema::from_yaml(indoc! {" + cubes: + - name: orders + sql: \"SELECT * FROM orders WHERE {FILTER_PARAMS_COLUMN:orders.status:LOWER(status)}\" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: status + type: string + sql: \"{CUBE}.status\" + measures: + - name: count + type: count + segments: + - name: bare_status + sql: \"{CUBE.status}\" + "}) + .unwrap(); + let ctx = TestContext::new(schema).unwrap(); + + let (sql, _) = ctx + .build_sql_and_params(indoc! {" + measures: + - orders.count + segments: + - orders.bare_status + "}) + .unwrap(); + + assert!( + sql.contains("SELECT * FROM orders WHERE 1 = 1"), + "a dimension binding must not be activated by a segment\nsql: {}", + sql + ); +} diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/dimension_deps.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/dimension_deps.rs new file mode 100644 index 0000000000000..c5383476b0e58 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/dimension_deps.rs @@ -0,0 +1,482 @@ +//! Multi-stage members whose SQL reads a plain dimension of their own cube. +//! +//! Such a member is rendered against the CTE that computes its aggregated +//! dependency, so the dimension has to be one of that CTE's columns. When no +//! grain supplies it, rendering would fall back to the dimension's own cube +//! alias — a table name nothing in the CTE's FROM brings into scope — and the +//! planner reports the member instead. + +use crate::test_fixtures::cube_bridge::MockSchema; +use crate::test_fixtures::test_utils::TestContext; +use indoc::indoc; + +fn create_context() -> TestContext { + let schema = MockSchema::from_yaml_file("common/integration_multi_stage.yaml"); + TestContext::new(schema).unwrap() +} + +const SEED: &str = "integration_multi_stage_tables.sql"; + +fn month_query(measure: &str) -> String { + format!( + indoc! {r#" + measures: + - orders.{} + time_dimensions: + - dimension: orders.created_at + granularity: month + dateRange: + - "2024-01-01" + - "2024-03-31" + "#}, + measure + ) +} + +fn expect_error(measure: &str) -> String { + let ctx = create_context(); + match ctx.build_sql(&month_query(measure)) { + Ok(sql) => panic!("Expected a planning error for orders.{measure}, got SQL:\n{sql}"), + Err(e) => e.to_string(), + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_undeclared_time_dimension_read_is_reported() { + let message = expect_error("amount_first_half_of_month"); + + assert!( + message.contains("orders.amount_first_half_of_month") + && message.contains("orders.created_at"), + "The error must name both the member and the dimension it reads:\n{}", + message + ); + assert!( + message.contains("grain.include"), + "The error must point at the declaration that fixes the model:\n{}", + message + ); +} + +/// The reading member consumes a time-shifted multi-stage measure, so its own +/// grain is settled one stage above the leaf that would have to carry the +/// dimension. +#[tokio::test(flavor = "multi_thread")] +async fn test_undeclared_read_by_a_consumer_of_a_shifted_measure_is_reported() { + let message = expect_error("amount_prev_month_first_half"); + + assert!( + message.contains("orders.amount_prev_month_first_half") + && message.contains("orders.created_at"), + "The error must name both the member and the dimension it reads:\n{}", + message + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_undeclared_string_dimension_read_is_reported() { + let message = expect_error("completed_amount_multi_stage"); + + assert!( + message.contains("orders.completed_amount_multi_stage") + && message.contains("orders.status"), + "The error must name both the member and the dimension it reads:\n{}", + message + ); +} + +/// Declaring the grain the sql needs is what makes the model plan: the leaf +/// carries the raw dimension, and the measure reads it as a CTE column. +#[tokio::test(flavor = "multi_thread")] +async fn test_declared_grain_plans_and_reads_the_cte_column() { + let ctx = create_context(); + + let query = month_query("amount_first_half_of_month_with_grain"); + let sql = ctx.build_sql(&query).unwrap(); + + // The trailing quote is what keeps this from matching `orders__created_at_month`. + assert!( + sql.contains("\"orders__created_at\""), + "Expected the raw time dimension to be materialized as a CTE column:\n{}", + sql + ); + assert!( + !sql.contains("EXTRACT(DAY FROM \"orders\".created_at)"), + "The measure must not read the dimension off the cube alias:\n{}", + sql + ); + + if let Some(result) = ctx.try_execute_pg(&query, SEED).await { + insta::assert_snapshot!(result); + } +} + +/// The declared grain widens the leaf only — the query still reports months. +#[tokio::test(flavor = "multi_thread")] +async fn test_declared_grain_keeps_the_query_grain() { + let ctx = create_context(); + + let sql = ctx + .build_sql(&month_query("amount_first_half_of_month_with_grain")) + .unwrap(); + + let (_, final_select) = sql + .rsplit_once("\nSELECT") + .expect("the plan must end in a top-level SELECT"); + assert!( + !final_select.contains("\"orders__created_at\""), + "The raw time dimension must not reach the query projection:\n{}", + sql + ); + assert!( + final_select.contains("orders__created_at_month"), + "Expected the month grain in the query projection:\n{}", + sql + ); +} + +/// A dimension the stage's own `reduce_by` drops is still reachable from the +/// keys side, so reading it must not be reported. +/// +/// The values are the whole month against the `completed` row rather than the +/// `completed` total: `reduce_by` collapses the measure to a grain without +/// `status`, and the CASE then reads the status of the broadcast row. That is +/// what this shape means, not a defect in the reachability check. +#[tokio::test(flavor = "multi_thread")] +async fn test_dimension_reachable_from_the_keys_side_is_not_reported() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.amount_reduce_status_reading_status + 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!( + sql.contains("\"fk_aggregate_keys\".\"orders__status\" = 'completed'"), + "Expected the measure to read the dimension off the keys side:\n{}", + sql + ); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + +/// A `grain.include` on the measure below widens *that* measure's leaf, not the +/// columns its own CTE projects, so a member reading the dimension one stage up +/// still has nowhere to read it from. Pinned because the difference between +/// widening a leaf and projecting a column is easy to mistake for an oversight. +#[tokio::test(flavor = "multi_thread")] +async fn test_grain_declared_by_a_child_does_not_satisfy_the_parent() { + let message = expect_error("amount_reading_a_child_leaf_grain"); + + assert!( + message.contains("orders.amount_reading_a_child_leaf_grain") + && message.contains("orders.status"), + "The error must name both the member and the dimension it reads:\n{}", + message + ); +} + +/// A dimension the query itself groups by is part of the stage grain, so it +/// resolves without any declaration. +#[tokio::test(flavor = "multi_thread")] +async fn test_dimension_in_the_query_grain_is_not_reported() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.completed_amount_multi_stage + 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!( + sql.contains("\"fk_aggregate\".\"orders__status\" = 'completed'"), + "Expected the measure to read the dimension off the stage grain:\n{}", + sql + ); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + +/// A dimension built out of another one is no column of the CTE itself, but +/// renders from the column its own sql reads — the reachability walk has to +/// follow that, the way reference resolution does. +#[tokio::test(flavor = "multi_thread")] +async fn test_dimension_derived_from_a_grain_dimension_is_not_reported() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.amount_by_category_label + dimensions: + - orders.category + time_dimensions: + - dimension: orders.created_at + granularity: month + dateRange: + - "2024-01-01" + - "2024-03-31" + order: + - id: orders.category + - id: orders.created_at + "#}; + + let sql = ctx.build_sql(query).unwrap(); + + // The label's own branch condition, which only the derived dimension emits — + // the bare column appears in the projection either way. + assert!( + sql.contains("\"fk_aggregate\".\"orders__category\" = 'books'"), + "Expected the derived dimension to render from the grain column:\n{}", + sql + ); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + +/// The read resolves through a member, but the same expression also reaches a +/// raw cube column, and no CTE column can stand in for that. +#[tokio::test(flavor = "multi_thread")] +async fn test_read_mixing_a_member_and_a_raw_column_is_reported() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.amount_by_status_and_category + 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 + "#}; + + match ctx.build_sql(query) { + Ok(sql) => panic!("Expected a planning error, got SQL:\n{sql}"), + Err(e) => assert!( + e.to_string().contains("orders.status_and_category"), + "The error must name the dimension that reads the raw column:\n{}", + e + ), + } +} + +/// `drill_filters` never reach the rendered SQL, so a dimension named only +/// there puts no column requirement on the CTE. +#[tokio::test(flavor = "multi_thread")] +async fn test_dimension_read_only_by_drill_filters_is_not_reported() { + let ctx = create_context(); + + let query = month_query("amount_with_drill_filters"); + let sql = ctx.build_sql(&query).unwrap(); + + assert!( + !sql.contains("\"orders\".category"), + "The drill filter must not put a cube-qualified column into the CTE:\n{}", + sql + ); + + if let Some(result) = ctx.try_execute_pg(&query, SEED).await { + insta::assert_snapshot!(result); + } +} + +/// A mask is rendered only for members the security context masks, so a +/// dimension read from an inactive mask puts no column requirement either. +#[tokio::test(flavor = "multi_thread")] +async fn test_dimension_read_only_by_an_inactive_mask_is_not_reported() { + let ctx = create_context(); + + let query = month_query("amount_with_masked_dimension_read"); + let sql = ctx.build_sql(&query).unwrap(); + + assert!( + !sql.contains("\"orders\".category"), + "The inactive mask must not put a cube-qualified column into the CTE:\n{}", + sql + ); +} + +/// The exclusion has to hold for the cube refs of a mask as well: a dimension +/// whose own sql is clean must not be rejected because its mask names a raw +/// column. +#[tokio::test(flavor = "multi_thread")] +async fn test_raw_column_read_only_by_a_mask_is_not_reported() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.amount_by_status_upper_masked + 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!( + sql.contains("UPPER(\"fk_aggregate\".\"orders__status\")"), + "Expected the dimension to render from the grain column:\n{}", + sql + ); + assert!( + !sql.contains("\"orders\".category"), + "The inactive mask must not put a cube-qualified column into the CTE:\n{}", + sql + ); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + +/// The exclusion is conditional on the mask not being applied. Once the query +/// masks the member, the mask does reach the SQL, and the dimension it reads has +/// to be reachable like any other. +#[tokio::test(flavor = "multi_thread")] +async fn test_active_mask_reading_an_out_of_grain_dimension_is_reported() { + let ctx = create_context(); + + let query = format!( + "{}{}", + month_query("amount_with_masked_dimension_read"), + indoc! {" + maskedMembers: + - member: orders.amount_with_masked_dimension_read + "}, + ); + + match ctx.build_sql(&query) { + Ok(sql) => panic!("Expected a planning error, got SQL:\n{sql}"), + Err(e) => assert!( + e.to_string().contains("orders.category"), + "The error must name the dimension the mask reads:\n{}", + e + ), + } +} + +/// The accept side of the same rule, and the premise the mask branch rests on: +/// an applied mask is rendered inside the multi-stage CTE, so its dimension read +/// resolves against a column of that CTE rather than the cube alias. +/// +/// The dimension is grouped by in the query, not merely declared in +/// `grain.include`. An unconditional mask replaces the member's aggregate, so its +/// read sits outside any aggregate and has to be in the stage's own GROUP BY; a +/// declared leaf grain puts the column in the source but not in that GROUP BY, +/// and the database rejects the result. The reachability check does not tell the +/// two apart — it asks only whether a column exists — so that shape still plans +/// and fails at the database. +#[tokio::test(flavor = "multi_thread")] +async fn test_active_mask_reading_a_grouped_dimension_plans() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.amount_masked_by_category_in_grain + dimensions: + - orders.category + time_dimensions: + - dimension: orders.created_at + granularity: month + dateRange: + - "2024-01-01" + - "2024-03-31" + order: + - id: orders.category + - id: orders.created_at + maskedMembers: + - member: orders.amount_masked_by_category_in_grain + "#}; + + let sql = ctx.build_sql(query).unwrap(); + + assert!( + sql.contains("\"fk_aggregate\".\"orders__category\" = 'books'"), + "Expected the mask to render against the CTE column:\n{}", + sql + ); + assert!( + !sql.contains("\"orders\".category = 'books'"), + "The mask must not read the dimension off the cube alias:\n{}", + sql + ); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + +/// The same exclusion has to hold when the masked member is a multi-stage time +/// dimension, which reaches its slots through the base symbol it is a view of. +#[tokio::test(flavor = "multi_thread")] +async fn test_masked_multi_stage_time_dimension_is_not_reported() { + let ctx = create_context(); + + // `orders.created_at` is grouped by so that the dimension's own sql dep is + // reachable — otherwise this would report for that reason and stop guarding + // the mask. + let query = indoc! {r#" + measures: + - orders.total_amount + dimensions: + - orders.created_at + time_dimensions: + - dimension: orders.created_at_masked_multi_stage + granularity: month + dateRange: + - "2024-01-01" + - "2024-03-31" + "#}; + + let sql = ctx.build_sql(query).unwrap(); + + assert!( + !sql.contains("\"orders\".category"), + "The inactive mask must not put a cube-qualified column into the CTE:\n{}", + sql + ); +} diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/mod.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/mod.rs index 97ca662f5aff8..dd58d6c66a19e 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/mod.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/mod.rs @@ -2,6 +2,7 @@ mod add_group_by; mod bucketing; mod calculated; mod case_switch; +mod dimension_deps; mod dimensions; mod edge_cases; mod filter_directive; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__active_mask_reading_a_grouped_dimension_plans.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__active_mask_reading_a_grouped_dimension_plans.snap new file mode 100644 index 0000000000000..01d8c47b2950a --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__active_mask_reading_a_grouped_dimension_plans.snap @@ -0,0 +1,15 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/dimension_deps.rs +expression: result +--- +orders__category | orders__created_at_month | orders__amount_masked_by_category_in_grain +-----------------+--------------------------+------------------------------------------- +books | 2024-01-01 00:00:00 | -1 +books | 2024-02-01 00:00:00 | -1 +books | 2024-03-01 00:00:00 | -1 +clothing | 2024-01-01 00:00:00 | 0 +clothing | 2024-02-01 00:00:00 | 0 +clothing | 2024-03-01 00:00:00 | 0 +electronics | 2024-01-01 00:00:00 | 0 +electronics | 2024-02-01 00:00:00 | 0 +electronics | 2024-03-01 00:00:00 | 0 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__declared_grain_plans_and_reads_the_cte_column.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__declared_grain_plans_and_reads_the_cte_column.snap new file mode 100644 index 0000000000000..42941337b67fc --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__declared_grain_plans_and_reads_the_cte_column.snap @@ -0,0 +1,9 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/dimension_deps.rs +expression: result +--- +orders__created_at_month | orders__amount_first_half_of_month_with_grain +-------------------------+---------------------------------------------- +2024-01-01 00:00:00 | 420.00 +2024-02-01 00:00:00 | 650.00 +2024-03-01 00:00:00 | 850.00 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__dimension_derived_from_a_grain_dimension_is_not_reported.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__dimension_derived_from_a_grain_dimension_is_not_reported.snap new file mode 100644 index 0000000000000..79a21938d7c91 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__dimension_derived_from_a_grain_dimension_is_not_reported.snap @@ -0,0 +1,15 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/dimension_deps.rs +expression: result +--- +orders__category | orders__created_at_month | orders__amount_by_category_label +-----------------+--------------------------+--------------------------------- +books | 2024-01-01 00:00:00 | 230.00 +books | 2024-02-01 00:00:00 | 150.00 +books | 2024-03-01 00:00:00 | 500.00 +clothing | 2024-01-01 00:00:00 | 0 +clothing | 2024-02-01 00:00:00 | 0 +clothing | 2024-03-01 00:00:00 | 0 +electronics | 2024-01-01 00:00:00 | 0 +electronics | 2024-02-01 00:00:00 | 0 +electronics | 2024-03-01 00:00:00 | 0 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__dimension_in_the_query_grain_is_not_reported.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__dimension_in_the_query_grain_is_not_reported.snap new file mode 100644 index 0000000000000..a90df82dca2b1 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__dimension_in_the_query_grain_is_not_reported.snap @@ -0,0 +1,15 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/dimension_deps.rs +expression: result +--- +orders__status | orders__created_at_month | orders__completed_amount_multi_stage +---------------+--------------------------+------------------------------------- +cancelled | 2024-01-01 00:00:00 | 0 +cancelled | 2024-02-01 00:00:00 | 0 +cancelled | 2024-03-01 00:00:00 | 0 +completed | 2024-01-01 00:00:00 | 300.00 +completed | 2024-02-01 00:00:00 | 500.00 +completed | 2024-03-01 00:00:00 | 600.00 +pending | 2024-01-01 00:00:00 | 0 +pending | 2024-02-01 00:00:00 | 0 +pending | 2024-03-01 00:00:00 | 0 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__dimension_reachable_from_the_keys_side_is_not_reported.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__dimension_reachable_from_the_keys_side_is_not_reported.snap new file mode 100644 index 0000000000000..251d03ad103fc --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__dimension_reachable_from_the_keys_side_is_not_reported.snap @@ -0,0 +1,15 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/dimension_deps.rs +expression: result +--- +orders__status | orders__created_at_month | orders__amount_reduce_status_reading_status +---------------+--------------------------+-------------------------------------------- +cancelled | 2024-01-01 00:00:00 | 0 +cancelled | 2024-02-01 00:00:00 | 0 +cancelled | 2024-03-01 00:00:00 | 0 +completed | 2024-01-01 00:00:00 | 500.00 +completed | 2024-02-01 00:00:00 | 750.00 +completed | 2024-03-01 00:00:00 | 1000.00 +pending | 2024-01-01 00:00:00 | 0 +pending | 2024-02-01 00:00:00 | 0 +pending | 2024-03-01 00:00:00 | 0 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__dimension_read_only_by_drill_filters_is_not_reported.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__dimension_read_only_by_drill_filters_is_not_reported.snap new file mode 100644 index 0000000000000..8e29f8bcfd595 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__dimension_read_only_by_drill_filters_is_not_reported.snap @@ -0,0 +1,9 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/dimension_deps.rs +expression: result +--- +orders__created_at_month | orders__amount_with_drill_filters +-------------------------+---------------------------------- +2024-01-01 00:00:00 | 500.00 +2024-02-01 00:00:00 | 750.00 +2024-03-01 00:00:00 | 1000.00 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__raw_column_read_only_by_a_mask_is_not_reported.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__raw_column_read_only_by_a_mask_is_not_reported.snap new file mode 100644 index 0000000000000..78a614c4af420 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__dimension_deps__raw_column_read_only_by_a_mask_is_not_reported.snap @@ -0,0 +1,15 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/dimension_deps.rs +expression: result +--- +orders__status | orders__created_at_month | orders__amount_by_status_upper_masked +---------------+--------------------------+-------------------------------------- +cancelled | 2024-01-01 00:00:00 | 0 +cancelled | 2024-02-01 00:00:00 | 0 +cancelled | 2024-03-01 00:00:00 | 0 +completed | 2024-01-01 00:00:00 | 300.00 +completed | 2024-02-01 00:00:00 | 500.00 +completed | 2024-03-01 00:00:00 | 600.00 +pending | 2024-01-01 00:00:00 | 0 +pending | 2024-02-01 00:00:00 | 0 +pending | 2024-03-01 00:00:00 | 0 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs index fef9974441c84..2788c6a90d863 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs @@ -7,6 +7,7 @@ mod date_filters; mod dimension_symbol; mod filter; mod filter_params_callback_column; +mod filter_params_segment; mod join_hints_collector; mod measure_symbol; mod member_expressions_on_views;