From 82154224fada1379a3976fe0e1f13fe41b720124 Mon Sep 17 00:00:00 2001 From: Aleksandr Romanenko Date: Thu, 18 Jun 2026 12:23:44 +0200 Subject: [PATCH 1/7] fix(tesseract): break Rc cycle that leaked QueryTools per query Every buildSqlAndParams built a per-query object graph hanging off Rc, and QueryTools owned the join-tree cache. The cached JoinTree's BaseCube held a strong Rc back, so QueryTools -> join_tree_cache -> JoinTree -> BaseCube -> QueryTools formed a cycle: QueryTools never dropped and the whole graph (Compiler, symbols, ~350 compiled minijinja SQL templates) leaked on every query. In the refresh worker, which generates huge volumes of pre-aggregation SQL, this grew unbounded and OOMed (Tesseract only; the JS planner was unaffected). Split the per-query state: introduce `State { query_tools, compiler, join_tree_cache }` (Deref) that owns the mutable / caching pieces, and make QueryTools a cache-free leaf. Transient planners hold Rc; cached/long-lived values (BaseCube, TypedFilter, symbols) and the render layer keep Rc, so no cached value can pin the hub. The Compiler moved into State too (its symbol cache was a latent host for the same class of cycle). Filter building needs the Compiler only to (re)compile a to_date rolling-window granularity, so BaseFilter/TypedFilterBuilder take an optional &mut Compiler; a member-only rewrite carries the precomputed FilterOp instead, avoiding the dependency. These compiler-threading points are marked FIXME for a future early-compilation pass. Adds a pure-Rust regression test (Weak-after-drop on QueryTools) that fails on the cycle and passes once it is broken. Note: member_mask_filters still lives in QueryTools and keeps a strong back-reference via TypedFilter, so the cycle remains for queries with masked members; relocating that field into State (a follow-up) closes it. --- .../pre_aggregation/dimension_matcher.rs | 2 +- .../optimizers/pre_aggregation/optimizer.rs | 11 +- .../pre_aggregations_compiler.rs | 21 +- .../cubesqlplanner/src/planner/base_query.rs | 6 +- .../src/planner/filter/base_filter.rs | 22 +- .../src/planner/filter/compiler.rs | 2 + .../src/planner/filter/typed_filter.rs | 279 ++++++++++-------- .../cubesqlplanner/src/planner/mod.rs | 2 + .../src/planner/multi_fact_join_groups.rs | 8 +- .../src/planner/planners/common_utils.rs | 18 +- .../planners/dimension_subquery_planner.rs | 10 +- .../src/planner/planners/join_planner.rs | 6 +- .../src/planner/planners/join_tree_builder.rs | 4 +- .../multi_stage/member_query_planner.rs | 8 +- .../multi_stage/multi_stage_query_planner.rs | 11 +- .../multiplied_measures_query_planner.rs | 6 +- .../src/planner/planners/query_planner.rs | 6 +- .../planner/planners/simple_query_planer.rs | 6 +- .../src/planner/query_properties.rs | 15 +- .../src/planner/query_properties_compiler.rs | 11 +- .../cubesqlplanner/src/planner/query_tools.rs | 98 ++---- .../cubesqlplanner/src/planner/state.rs | 138 +++++++++ .../planner/symbols/time_dimension_symbol.rs | 11 +- .../src/planner/top_level_planner.rs | 11 +- .../test_fixtures/test_utils/test_context.rs | 40 ++- .../src/tests/filter/tree_ops.rs | 3 +- .../src/tests/filter/use_raw_values.rs | 16 +- .../cubesqlplanner/src/tests/mod.rs | 1 + .../src/tests/no_query_tools_leak.rs | 61 ++++ .../cubesqlplanner/src/tests/utils/debug.rs | 12 +- 30 files changed, 542 insertions(+), 303 deletions(-) create mode 100644 rust/cube/cubesqlplanner/cubesqlplanner/src/planner/state.rs create mode 100644 rust/cube/cubesqlplanner/cubesqlplanner/src/tests/no_query_tools_leak.rs diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/dimension_matcher.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/dimension_matcher.rs index 1c1e0dfd390f0..29edda6ca9ef0 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/dimension_matcher.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/dimension_matcher.rs @@ -359,7 +359,7 @@ mod tests { let pre_agg = compiler.compile_pre_aggregation(&name).unwrap(); let qp = ctx.create_query_properties(query_yaml).unwrap(); - let mut matcher = DimensionMatcher::new(ctx.query_tools().clone(), &pre_agg); + let mut matcher = DimensionMatcher::new(ctx.query_tools().query_tools().clone(), &pre_agg); matcher .try_match( qp.dimensions(), diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/optimizer.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/optimizer.rs index 825094a2658f1..c7f652b4bf2bf 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/optimizer.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/optimizer.rs @@ -7,7 +7,7 @@ use crate::planner::filter::FilterOp; use crate::planner::join_hints::JoinHints; use crate::planner::multi_fact_join_groups::{MeasuresJoinHints, MultiFactJoinGroups}; use crate::planner::planners::multi_stage::TimeShiftState; -use crate::planner::query_tools::QueryTools; +use crate::planner::state::State; use crate::planner::time_dimension::QueryDateTime; use crate::planner::MemberSymbol; use cubenativeutils::CubeError; @@ -35,14 +35,14 @@ impl PreAggregationUsage { } pub struct PreAggregationOptimizer { - query_tools: Rc, + query_tools: Rc, allow_multi_stage: bool, usages: Vec, usage_counter: usize, } impl PreAggregationOptimizer { - pub fn new(query_tools: Rc, allow_multi_stage: bool) -> Self { + pub fn new(query_tools: Rc, allow_multi_stage: bool) -> Self { Self { query_tools, allow_multi_stage, @@ -431,7 +431,7 @@ impl PreAggregationOptimizer { fn extract_date_range( filter: &LogicalFilter, - query_tools: &Rc, + query_tools: &Rc, time_shifts: &TimeShiftState, external: bool, ) -> Option<(String, String)> { @@ -590,7 +590,8 @@ impl PreAggregationOptimizer { segments: &Vec, pre_aggregation: &CompiledPreAggregation, ) -> Result { - let mut matcher = DimensionMatcher::new(self.query_tools.clone(), pre_aggregation); + let mut matcher = + DimensionMatcher::new(self.query_tools.query_tools().clone(), pre_aggregation); matcher.try_match( dimensions, time_dimensions, diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/pre_aggregations_compiler.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/pre_aggregations_compiler.rs index febe485eba30a..413fe93149967 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/pre_aggregations_compiler.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/pre_aggregations_compiler.rs @@ -11,7 +11,7 @@ use crate::planner::join_hints::JoinHints; use crate::planner::multi_fact_join_groups::{MeasuresJoinHints, MultiFactJoinGroups}; use crate::planner::planners::JoinPlanner; use crate::planner::planners::ResolvedJoinItem; -use crate::planner::query_tools::QueryTools; +use crate::planner::state::State; use crate::planner::GranularityHelper; use crate::planner::MemberSymbol; use crate::planner::TimeDimensionSymbol; @@ -50,16 +50,13 @@ impl PreAggregationFullName { } pub struct PreAggregationsCompiler { - query_tools: Rc, + query_tools: Rc, descriptions: Rc)>>, compiled_cache: HashMap>, } impl PreAggregationsCompiler { - pub fn try_new( - query_tools: Rc, - cube_names: &Vec, - ) -> Result { + pub fn try_new(query_tools: Rc, cube_names: &Vec) -> Result { let mut descriptions = Vec::new(); for cube_name in cube_names.iter() { let pre_aggregations = query_tools @@ -144,7 +141,7 @@ impl PreAggregationsCompiler { resolved.push((base_symbol, td_ref.static_data().granularity.clone())); } - let evaluator_compiler_cell = self.query_tools.evaluator_compiler().clone(); + let evaluator_compiler_cell = self.query_tools.compiler().clone(); let mut evaluator_compiler = evaluator_compiler_cell.borrow_mut(); let mut result = Vec::new(); for (base_symbol, granularity) in resolved { @@ -169,7 +166,7 @@ impl PreAggregationsCompiler { Self::time_dimension_symbol_from_ref(self.query_tools.clone(), name, refs)?; if static_data.granularity.is_some() { - let evaluator_compiler_cell = self.query_tools.evaluator_compiler().clone(); + let evaluator_compiler_cell = self.query_tools.compiler().clone(); let mut evaluator_compiler = evaluator_compiler_cell.borrow_mut(); let granularity_obj = GranularityHelper::make_granularity_obj( @@ -545,12 +542,12 @@ impl PreAggregationsCompiler { } fn symbols_from_ref Result<(), CubeError>>( - query_tools: Rc, + query_tools: Rc, cube_name: &String, ref_func: Rc, check_type_fn: F, ) -> Result>, CubeError> { - let evaluator_compiler_cell = query_tools.evaluator_compiler().clone(); + let evaluator_compiler_cell = query_tools.compiler().clone(); let mut evaluator_compiler = evaluator_compiler_cell.borrow_mut(); let sql_call = evaluator_compiler.compile_sql_call(cube_name, ref_func)?; let mut res = Vec::new(); @@ -562,11 +559,11 @@ impl PreAggregationsCompiler { } fn time_dimension_symbol_from_ref( - query_tools: Rc, + query_tools: Rc, name: &PreAggregationFullName, ref_func: Rc, ) -> Result, CubeError> { - let evaluator_compiler_cell = query_tools.evaluator_compiler().clone(); + let evaluator_compiler_cell = query_tools.compiler().clone(); let mut evaluator_compiler = evaluator_compiler_cell.borrow_mut(); let sql_call = evaluator_compiler.compile_sql_call(&name.cube_name, ref_func)?; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/base_query.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/base_query.rs index 9c1b11ad3cc34..2e7004b68fa44 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/base_query.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/base_query.rs @@ -1,4 +1,4 @@ -use super::query_tools::QueryTools; +use super::state::State; use super::top_level_planner::TopLevelPlanner; use super::{QueryProperties, QueryPropertiesCompiler}; use crate::cube_bridge::base_query_options::BaseQueryOptions; @@ -32,7 +32,7 @@ struct GroupedPreAggregationInfo { pub struct BaseQuery { context: NativeContextHolder, - query_tools: Rc, + query_tools: Rc, request: Rc, cubestore_support_multistage: bool, } @@ -46,7 +46,7 @@ impl BaseQuery { .static_data() .cubestore_support_multistage .unwrap_or(false); - let query_tools = QueryTools::try_new( + let query_tools = State::try_new( options.cube_evaluator()?, options.security_context()?, options.base_tools()?, diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/base_filter.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/base_filter.rs index 57b2991ebadd9..86687f60a7621 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/base_filter.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/base_filter.rs @@ -1,5 +1,6 @@ use super::filter_operator::FilterOperator; use super::typed_filter::{resolve_base_symbol, TypedFilter}; +use crate::planner::Compiler; use crate::planner::MemberSymbol; use cubenativeutils::CubeError; use itertools::Itertools; @@ -30,12 +31,16 @@ impl PartialEq for BaseFilter { } impl BaseFilter { + // FIXME: late compilation. `compiler` is threaded through purely so a + // to_date rolling-window granularity can be (re)compiled while the filter + // is built during planning. With early compilation this disappears. pub fn try_new( query_tools: Rc, member_evaluator: Rc, filter_type: FilterType, filter_operator: FilterOperator, values: Option>>, + compiler: Option<&mut Compiler>, ) -> Result, CubeError> { let typed_filter = TypedFilter::builder() .query_tools(query_tools) @@ -43,16 +48,19 @@ impl BaseFilter { .filter_type(filter_type) .operator(filter_operator) .values(values) - .build()?; + .build(compiler)?; Ok(Rc::new(Self { typed_filter })) } + // FIXME: late compilation — see `try_new`. `compiler` only feeds the + // rolling-window granularity recompute. pub fn change_operator( &self, filter_operator: FilterOperator, values: Vec>, use_raw_values: bool, + compiler: Option<&mut Compiler>, ) -> Result, CubeError> { let typed_filter = self .typed_filter @@ -60,7 +68,7 @@ impl BaseFilter { .operator(filter_operator) .values(Some(values)) .use_raw_values(use_raw_values) - .build()?; + .build(compiler)?; Ok(Rc::new(Self { typed_filter })) } @@ -87,11 +95,19 @@ impl BaseFilter { &self, member_evaluator: Rc, ) -> Result, CubeError> { + // No compiler here (called from static-filter symbol rewriting, which + // has none). A member swap keeps the same operator/values, so the only + // branch that would need a compiler is a to_date rolling window — not + // reachable from this path. FIXME: removed once granularities are + // resolved during early compilation rather than at filter-build time. let typed_filter = self .typed_filter .to_builder() .member_evaluator(member_evaluator) - .build()?; + // A member swap leaves operator/values unchanged, so the operation + // is identical — carry it over so build() needs no Compiler. + .carry_op(self.typed_filter.operation().clone()) + .build(None)?; Ok(Rc::new(Self { typed_filter })) } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/compiler.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/compiler.rs index 92ef51ad32e5e..afa507ac07bdc 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/compiler.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/compiler.rs @@ -59,6 +59,7 @@ impl<'a> FilterCompiler<'a> { FilterType::Dimension, FilterOperator::InDateRange, Some(date_range.into_iter().map(|v| Some(v)).collect()), + None, )?; self.time_dimension_filters.push(FilterItem::Item(filter)); } @@ -120,6 +121,7 @@ impl<'a> FilterCompiler<'a> { item_type.clone(), FilterOperator::from_str(&operator)?, item.values.clone(), + Some(&mut *self.evaluator_compiler), )?)) } else { Err(CubeError::user(format!( diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/typed_filter.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/typed_filter.rs index cad84a60fd0e9..e35b449f921a7 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/typed_filter.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/typed_filter.rs @@ -1,4 +1,5 @@ use crate::planner::query_tools::QueryTools; +use crate::planner::Compiler; use crate::planner::MemberSymbol; use cubenativeutils::CubeError; use std::rc::Rc; @@ -111,6 +112,11 @@ pub struct TypedFilterBuilder { operator: Option, values: Option>>, use_raw_values: bool, + /// Pre-computed operation carried over from an existing filter. When set, + /// `build` reuses it instead of recomputing from operator/values — which is + /// what lets a member-only rewrite (`with_member_evaluator`) avoid touching + /// the Compiler for a to_date rolling-window granularity. + op: Option, } impl TypedFilterBuilder { @@ -139,6 +145,14 @@ impl TypedFilterBuilder { self } + /// Carries an already-computed `FilterOp` so `build` skips recomputation + /// (and thus the Compiler dependency). Valid only when operator and values + /// are unchanged — i.e. a pure member swap. + pub fn carry_op(mut self, op: FilterOp) -> Self { + self.op = Some(op); + self + } + pub fn values(mut self, v: Option>>) -> Self { self.values = v; self @@ -160,7 +174,12 @@ impl TypedFilterBuilder { .ok_or_else(|| CubeError::user("Expected one parameter but nothing found".to_string())) } - pub fn build(self) -> Result { + // FIXME: late compilation. `compiler` is threaded in only to (re)compile a + // custom rolling-window granularity during planning (the + // ToDateRollingWindowDateRange branch below). Once granularities are + // resolved in an early compile phase, this parameter — and the whole + // QueryTools→Compiler dependency in filter building — should go away. + pub fn build(self, compiler: Option<&mut Compiler>) -> Result { let query_tools = self .query_tools .ok_or_else(|| CubeError::internal("query_tools is required".to_string()))?; @@ -176,136 +195,146 @@ impl TypedFilterBuilder { let values = self.values.unwrap_or_default(); let values_snapshot = values.clone(); - let member_type = Self::resolve_member_type(&member_evaluator); - - let op = match operator { - FilterOperator::Equal | FilterOperator::NotEqual => { - let negated = matches!(operator, FilterOperator::NotEqual); - let has_null = values.iter().any(|v| v.is_none()); - if values.len() > 1 { - FilterOp::InList(InListOp::new(negated, values, member_type)) - } else if has_null { - // equals null → IS NULL, notEquals null → IS NOT NULL - FilterOp::Nullability(NullabilityOp::new(!negated)) - } else if let Some(Some(value)) = values.into_iter().next() { - FilterOp::Equality(EqualityOp::new(negated, value, member_type)) - } else { - return Err(CubeError::user( - "Expected at least one value for equals/notEquals filter".to_string(), - )); + let op = if let Some(op) = self.op { + op + } else { + let member_type = Self::resolve_member_type(&member_evaluator); + match operator { + FilterOperator::Equal | FilterOperator::NotEqual => { + let negated = matches!(operator, FilterOperator::NotEqual); + let has_null = values.iter().any(|v| v.is_none()); + if values.len() > 1 { + FilterOp::InList(InListOp::new(negated, values, member_type)) + } else if has_null { + // equals null → IS NULL, notEquals null → IS NOT NULL + FilterOp::Nullability(NullabilityOp::new(!negated)) + } else if let Some(Some(value)) = values.into_iter().next() { + FilterOp::Equality(EqualityOp::new(negated, value, member_type)) + } else { + return Err(CubeError::user( + "Expected at least one value for equals/notEquals filter".to_string(), + )); + } } - } - FilterOperator::In => FilterOp::InList(InListOp::new(false, values, member_type)), - FilterOperator::NotIn => FilterOp::InList(InListOp::new(true, values, member_type)), - FilterOperator::Gt | FilterOperator::Gte | FilterOperator::Lt | FilterOperator::Lte => { - let kind = match operator { - FilterOperator::Gt => ComparisonKind::Gt, - FilterOperator::Gte => ComparisonKind::Gte, - FilterOperator::Lt => ComparisonKind::Lt, - FilterOperator::Lte => ComparisonKind::Lte, - _ => unreachable!(), - }; - let value = Self::first_non_null_value(&values)?; - FilterOp::Comparison(ComparisonOp::new(kind, value, member_type)) - } - FilterOperator::Set => FilterOp::Nullability(NullabilityOp::new(false)), - FilterOperator::NotSet => FilterOp::Nullability(NullabilityOp::new(true)), - FilterOperator::InDateRange | FilterOperator::NotInDateRange => { - let from = Self::first_non_null_value(&values)?; - let to = values - .get(1) - .and_then(|v| v.as_ref().cloned()) - .ok_or_else(|| { - CubeError::user("2 arguments expected for date range".to_string()) - })?; - let kind = if matches!(operator, FilterOperator::InDateRange) { - DateRangeKind::InRange - } else { - DateRangeKind::NotInRange - }; - FilterOp::DateRange(DateRangeOp::new(kind, from, to)) - } - FilterOperator::BeforeDate => { - let value = Self::first_non_null_value(&values)?; - FilterOp::DateSingle(DateSingleOp::new(DateSingleKind::Before, value)) - } - FilterOperator::BeforeOrOnDate => { - let value = Self::first_non_null_value(&values)?; - FilterOp::DateSingle(DateSingleOp::new(DateSingleKind::BeforeOrOn, value)) - } - FilterOperator::AfterDate => { - let value = Self::first_non_null_value(&values)?; - FilterOp::DateSingle(DateSingleOp::new(DateSingleKind::After, value)) - } - FilterOperator::AfterOrOnDate => { - let value = Self::first_non_null_value(&values)?; - FilterOp::DateSingle(DateSingleOp::new(DateSingleKind::AfterOrOn, value)) - } - FilterOperator::RegularRollingWindowDateRange => { - let trailing = values.get(2).and_then(|v| v.clone()); - let leading = values.get(3).and_then(|v| v.clone()); - FilterOp::RegularRollingWindow(RegularRollingWindowOp::new(trailing, leading)) - } - FilterOperator::ToDateRollingWindowDateRange => { - let granularity_name = values - .get(2) - .and_then(|v| v.as_ref()) - .ok_or_else(|| { - CubeError::user( - "Granularity required for to_date rolling window".to_string(), + FilterOperator::In => FilterOp::InList(InListOp::new(false, values, member_type)), + FilterOperator::NotIn => FilterOp::InList(InListOp::new(true, values, member_type)), + FilterOperator::Gt + | FilterOperator::Gte + | FilterOperator::Lt + | FilterOperator::Lte => { + let kind = match operator { + FilterOperator::Gt => ComparisonKind::Gt, + FilterOperator::Gte => ComparisonKind::Gte, + FilterOperator::Lt => ComparisonKind::Lt, + FilterOperator::Lte => ComparisonKind::Lte, + _ => unreachable!(), + }; + let value = Self::first_non_null_value(&values)?; + FilterOp::Comparison(ComparisonOp::new(kind, value, member_type)) + } + FilterOperator::Set => FilterOp::Nullability(NullabilityOp::new(false)), + FilterOperator::NotSet => FilterOp::Nullability(NullabilityOp::new(true)), + FilterOperator::InDateRange | FilterOperator::NotInDateRange => { + let from = Self::first_non_null_value(&values)?; + let to = values + .get(1) + .and_then(|v| v.as_ref().cloned()) + .ok_or_else(|| { + CubeError::user("2 arguments expected for date range".to_string()) + })?; + let kind = if matches!(operator, FilterOperator::InDateRange) { + DateRangeKind::InRange + } else { + DateRangeKind::NotInRange + }; + FilterOp::DateRange(DateRangeOp::new(kind, from, to)) + } + FilterOperator::BeforeDate => { + let value = Self::first_non_null_value(&values)?; + FilterOp::DateSingle(DateSingleOp::new(DateSingleKind::Before, value)) + } + FilterOperator::BeforeOrOnDate => { + let value = Self::first_non_null_value(&values)?; + FilterOp::DateSingle(DateSingleOp::new(DateSingleKind::BeforeOrOn, value)) + } + FilterOperator::AfterDate => { + let value = Self::first_non_null_value(&values)?; + FilterOp::DateSingle(DateSingleOp::new(DateSingleKind::After, value)) + } + FilterOperator::AfterOrOnDate => { + let value = Self::first_non_null_value(&values)?; + FilterOp::DateSingle(DateSingleOp::new(DateSingleKind::AfterOrOn, value)) + } + FilterOperator::RegularRollingWindowDateRange => { + let trailing = values.get(2).and_then(|v| v.clone()); + let leading = values.get(3).and_then(|v| v.clone()); + FilterOp::RegularRollingWindow(RegularRollingWindowOp::new(trailing, leading)) + } + FilterOperator::ToDateRollingWindowDateRange => { + let granularity_name = values + .get(2) + .and_then(|v| v.as_ref()) + .ok_or_else(|| { + CubeError::user( + "Granularity required for to_date rolling window".to_string(), + ) + })? + .clone(); + + let resolved = resolve_base_symbol(&member_evaluator); + let evaluator_compiler = compiler.ok_or_else(|| { + CubeError::internal( + "Compiler is required to resolve a to_date rolling-window granularity" + .to_string(), ) - })? - .clone(); + })?; - let resolved = resolve_base_symbol(&member_evaluator); - let evaluator_compiler_cell = query_tools.evaluator_compiler().clone(); - let mut evaluator_compiler = evaluator_compiler_cell.borrow_mut(); + let granularity_obj = GranularityHelper::make_granularity_obj( + query_tools.cube_evaluator().clone(), + evaluator_compiler, + &resolved.cube_name(), + &resolved.name(), + Some(granularity_name.clone()), + )? + .ok_or_else(|| { + CubeError::internal(format!( + "Rolling window granularity '{}' is not found in time dimension '{}'", + granularity_name, + resolved.name() + )) + })?; - let granularity_obj = GranularityHelper::make_granularity_obj( - query_tools.cube_evaluator().clone(), - &mut evaluator_compiler, - &resolved.cube_name(), - &resolved.name(), - Some(granularity_name.clone()), - )? - .ok_or_else(|| { - CubeError::internal(format!( - "Rolling window granularity '{}' is not found in time dimension '{}'", - granularity_name, - resolved.name() + FilterOp::ToDateRollingWindow(ToDateRollingWindowOp::new(granularity_obj)) + } + FilterOperator::Contains + | FilterOperator::NotContains + | FilterOperator::StartsWith + | FilterOperator::NotStartsWith + | FilterOperator::EndsWith + | FilterOperator::NotEndsWith => { + let has_null = values.iter().any(|v| v.is_none()); + let non_null_values: Vec = + values.iter().filter_map(|v| v.clone()).collect(); + let (negated, start_wild, end_wild) = match operator { + FilterOperator::Contains => (false, true, true), + FilterOperator::NotContains => (true, true, true), + FilterOperator::StartsWith => (false, false, true), + FilterOperator::NotStartsWith => (true, false, true), + FilterOperator::EndsWith => (false, true, false), + FilterOperator::NotEndsWith => (true, true, false), + _ => unreachable!(), + }; + FilterOp::Like(LikeOp::new( + negated, + start_wild, + end_wild, + non_null_values, + has_null, + member_type, )) - })?; - - FilterOp::ToDateRollingWindow(ToDateRollingWindowOp::new(granularity_obj)) - } - FilterOperator::Contains - | FilterOperator::NotContains - | FilterOperator::StartsWith - | FilterOperator::NotStartsWith - | FilterOperator::EndsWith - | FilterOperator::NotEndsWith => { - let has_null = values.iter().any(|v| v.is_none()); - let non_null_values: Vec = - values.iter().filter_map(|v| v.clone()).collect(); - let (negated, start_wild, end_wild) = match operator { - FilterOperator::Contains => (false, true, true), - FilterOperator::NotContains => (true, true, true), - FilterOperator::StartsWith => (false, false, true), - FilterOperator::NotStartsWith => (true, false, true), - FilterOperator::EndsWith => (false, true, false), - FilterOperator::NotEndsWith => (true, true, false), - _ => unreachable!(), - }; - FilterOp::Like(LikeOp::new( - negated, - start_wild, - end_wild, - non_null_values, - has_null, - member_type, - )) + } + FilterOperator::MeasureFilter => FilterOp::MeasureFilter(MeasureFilterOp::new()), } - FilterOperator::MeasureFilter => FilterOp::MeasureFilter(MeasureFilterOp::new()), }; Ok(TypedFilter { diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/mod.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/mod.rs index c7a66301f4f05..8c182aab69a2e 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/mod.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/mod.rs @@ -20,6 +20,7 @@ pub mod query_properties; pub mod query_properties_compiler; pub mod query_tools; pub mod sql_templates; +pub mod state; pub mod top_level_planner; pub mod utils; pub mod visitor_context; @@ -36,6 +37,7 @@ pub use params_allocator::ParamsAllocator; pub use query_properties::{FullKeyAggregateMeasures, OrderByItem, QueryProperties}; pub use query_properties_compiler::QueryPropertiesCompiler; pub use sql_call::*; +pub use state::State; pub use symbols::*; pub use time_dimension::*; pub use top_level_planner::TopLevelPlanner; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/multi_fact_join_groups.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/multi_fact_join_groups.rs index 42bf62cda51fb..1e53d1d3bce3d 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/multi_fact_join_groups.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/multi_fact_join_groups.rs @@ -5,7 +5,7 @@ use crate::planner::filter::FilterItem; use crate::planner::join_hints::JoinHints; use crate::planner::planners::JoinTreeBuilder; use crate::planner::query_tools::JoinKey; -use crate::planner::query_tools::QueryTools; +use crate::planner::state::State; use crate::planner::JoinTree; use crate::planner::MemberSymbol; use cubenativeutils::CubeError; @@ -149,7 +149,7 @@ impl MeasuresJoinHints { /// are cheap lookups at render time. #[derive(Clone)] pub struct MultiFactJoinGroups { - query_tools: Rc, + query_tools: Rc, measures_join_hints: MeasuresJoinHints, groups: Vec<(Rc, Vec>)>, /// cube_name → join path from root, computed from the first group (shared for dimensions). @@ -162,7 +162,7 @@ impl MultiFactJoinGroups { /// Builds the join trees from `measures_join_hints` and groups /// measures by the resulting `JoinKey`. pub fn try_new( - query_tools: Rc, + query_tools: Rc, measures_join_hints: MeasuresJoinHints, ) -> Result { let groups = Self::build_groups(&query_tools, &measures_join_hints)?; @@ -184,7 +184,7 @@ impl MultiFactJoinGroups { } fn build_groups( - query_tools: &Rc, + query_tools: &Rc, hints: &MeasuresJoinHints, ) -> Result, Vec>)>, CubeError> { let join_tree_builder = JoinTreeBuilder::new(query_tools.clone()); diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/common_utils.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/common_utils.rs index f55e76ee085e8..f0b5352aa42de 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/common_utils.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/common_utils.rs @@ -1,5 +1,5 @@ use crate::cube_bridge::join_item::JoinItem; -use crate::planner::query_tools::QueryTools; +use crate::planner::state::State; use crate::planner::BaseCube; use crate::planner::MemberSymbol; use crate::planner::SqlCall; @@ -10,11 +10,11 @@ use std::rc::Rc; /// ON SQL compilation and the list of primary-key dimensions for a /// given cube. pub struct CommonUtils { - query_tools: Rc, + query_tools: Rc, } impl CommonUtils { - pub fn new(query_tools: Rc) -> Self { + pub fn new(query_tools: Rc) -> Self { Self { query_tools } } @@ -25,7 +25,7 @@ impl CommonUtils { join_item: Rc, ) -> Result, CubeError> { let definition = join_item.join()?; - let evaluator_compiler_cell = self.query_tools.evaluator_compiler().clone(); + let evaluator_compiler_cell = self.query_tools.compiler().clone(); let mut evaluator_compiler = evaluator_compiler_cell.borrow_mut(); evaluator_compiler .compile_sql_call(&join_item.static_data().original_from, definition.sql()?) @@ -33,12 +33,16 @@ impl CommonUtils { /// Resolves the planner-level `BaseCube` for the given cube path. pub fn cube_from_path(&self, cube_path: String) -> Result, CubeError> { - let evaluator_compiler_cell = self.query_tools.evaluator_compiler().clone(); + let evaluator_compiler_cell = self.query_tools.compiler().clone(); let mut evaluator_compiler = evaluator_compiler_cell.borrow_mut(); let evaluator = evaluator_compiler.add_cube_table_evaluator(cube_path.to_string(), vec![])?; - BaseCube::try_new(cube_path.to_string(), self.query_tools.clone(), evaluator) + BaseCube::try_new( + cube_path.to_string(), + self.query_tools.query_tools().clone(), + evaluator, + ) } /// Primary-key dimensions of `cube_name` as planner @@ -47,7 +51,7 @@ impl CommonUtils { &self, cube_name: &String, ) -> Result>, CubeError> { - let evaluator_compiler_cell = self.query_tools.evaluator_compiler().clone(); + let evaluator_compiler_cell = self.query_tools.compiler().clone(); let mut evaluator_compiler = evaluator_compiler_cell.borrow_mut(); let primary_keys = self .query_tools diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/dimension_subquery_planner.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/dimension_subquery_planner.rs index ff1b0eea02e02..17431107e7ced 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/dimension_subquery_planner.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/dimension_subquery_planner.rs @@ -4,7 +4,7 @@ use crate::physical_plan::QualifiedColumnName; use crate::planner::collectors::collect_sub_query_dimensions; use crate::planner::filter::FilterItem; use crate::planner::planners::multi_stage::PlanningScope; -use crate::planner::query_tools::QueryTools; +use crate::planner::state::State; use crate::planner::QueryProperties; use crate::planner::{MemberExpressionExpression, MemberExpressionSymbol, MemberSymbol}; use cubenativeutils::CubeError; @@ -18,7 +18,7 @@ use std::rc::Rc; /// gets joined back into the host query on those keys. pub struct DimensionSubqueryPlanner { utils: CommonUtils, - query_tools: Rc, + query_tools: Rc, query_properties: Rc, sub_query_dims: HashMap>>, dimensions_refs: RefCell>, @@ -27,7 +27,7 @@ pub struct DimensionSubqueryPlanner { impl DimensionSubqueryPlanner { /// Planner with no sub-query dimensions — used when the host /// query has none. - pub fn empty(query_tools: Rc, query_properties: Rc) -> Self { + pub fn empty(query_tools: Rc, query_properties: Rc) -> Self { Self { sub_query_dims: HashMap::new(), utils: CommonUtils::new(query_tools.clone()), @@ -40,7 +40,7 @@ impl DimensionSubqueryPlanner { /// by owning cube. pub fn try_new( dimensions: &Vec>, - query_tools: Rc, + query_tools: Rc, query_properties: Rc, ) -> Result { let mut sub_query_dims: HashMap>> = HashMap::new(); @@ -99,7 +99,7 @@ impl DimensionSubqueryPlanner { let cube_symbol = self .query_tools - .evaluator_compiler() + .compiler() .borrow_mut() .add_cube_table_evaluator(cube_name.clone(), vec![])?; let member_expression_symbol = MemberExpressionSymbol::try_new( diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/join_planner.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/join_planner.rs index 55bbe32b84e77..6b70c32812a6c 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/join_planner.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/join_planner.rs @@ -3,7 +3,7 @@ use crate::cube_bridge::join_definition::JoinDefinition; use crate::cube_bridge::join_item::JoinItem; use crate::logical_plan::*; use crate::planner::join_hints::JoinHints; -use crate::planner::query_tools::QueryTools; +use crate::planner::state::State; use crate::planner::JoinTree; use crate::planner::MemberSymbol; use crate::planner::SqlCall; @@ -39,11 +39,11 @@ impl ResolvedJoinItem { /// helpers for resolving the members each ON clause references. pub struct JoinPlanner { utils: CommonUtils, - query_tools: Rc, + query_tools: Rc, } impl JoinPlanner { - pub fn new(query_tools: Rc) -> Self { + pub fn new(query_tools: Rc) -> Self { Self { utils: CommonUtils::new(query_tools.clone()), query_tools, diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/join_tree_builder.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/join_tree_builder.rs index 0174a0a4b9273..c5a360ce78a3a 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/join_tree_builder.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/join_tree_builder.rs @@ -1,6 +1,6 @@ use super::CommonUtils; use crate::cube_bridge::join_definition::JoinDefinition; -use crate::planner::query_tools::QueryTools; +use crate::planner::state::State; use crate::planner::{JoinTree, JoinTreeItem}; use cubenativeutils::CubeError; use std::rc::Rc; @@ -13,7 +13,7 @@ pub struct JoinTreeBuilder { } impl JoinTreeBuilder { - pub fn new(query_tools: Rc) -> Self { + pub fn new(query_tools: Rc) -> Self { Self { utils: CommonUtils::new(query_tools), } 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 8cc25f2977676..689d4fc95a3d2 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 @@ -4,7 +4,7 @@ use super::{ }; use crate::logical_plan::*; use crate::planner::planners::{multi_stage::RollingWindowType, QueryPlanner, SimpleQueryPlanner}; -use crate::planner::query_tools::QueryTools; +use crate::planner::state::State; use crate::planner::GranularityHelper; use crate::planner::MemberSymbol; use crate::planner::MultiStageGrain; @@ -21,14 +21,14 @@ use std::vec; /// dimension / measure inode, or a leaf (base measure / /// time-series / time-series-get-range). pub struct MultiStageMemberQueryPlanner { - query_tools: Rc, + query_tools: Rc, query_properties: Rc, description: Rc, } impl MultiStageMemberQueryPlanner { pub fn new( - query_tools: Rc, + query_tools: Rc, query_properties: Rc, description: Rc, ) -> Self { @@ -146,7 +146,7 @@ impl MultiStageMemberQueryPlanner { let time_dimension = &rolling_window_desc.time_dimension; let query_granularity = to_date_rolling_window.granularity.clone(); - let evaluator_compiler_cell = self.query_tools.evaluator_compiler().clone(); + let evaluator_compiler_cell = self.query_tools.compiler().clone(); let mut evaluator_compiler = evaluator_compiler_cell.borrow_mut(); let Some(granularity_obj) = GranularityHelper::make_granularity_obj( 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 574ed82164b9b..5dfa0b0668f60 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 @@ -12,7 +12,7 @@ use crate::planner::filter::base_filter::FilterType; use crate::planner::filter::BaseFilter; use crate::planner::filter::FilterItem; use crate::planner::filter::FilterOperator; -use crate::planner::query_tools::QueryTools; +use crate::planner::state::State; use crate::planner::symbols::AggregationType; use crate::planner::Case; use crate::planner::CaseSwitchDefinition; @@ -38,7 +38,7 @@ use std::rc::Rc; /// `(member, state)` so the same multi-stage subquery isn't /// emitted twice. pub struct MultiStageQueryPlanner { - query_tools: Rc, + query_tools: Rc, query_properties: Rc, // The initial multi-stage CTE state. Shared immutably; any mutation goes // through `as_ref().clone()` on the consumer side. Used both as the entry @@ -49,7 +49,7 @@ pub struct MultiStageQueryPlanner { impl MultiStageQueryPlanner { pub fn try_new( - query_tools: Rc, + query_tools: Rc, query_properties: Rc, ) -> Result { let root_state = Self::build_root_state(&query_tools, &query_properties)?; @@ -66,7 +66,7 @@ impl MultiStageQueryPlanner { // builder skips default_order — this value is only ever used as a state // container, never planned directly. fn build_root_state( - query_tools: &Rc, + query_tools: &Rc, query_properties: &Rc, ) -> Result, CubeError> { QueryProperties::builder() @@ -424,11 +424,12 @@ impl MultiStageQueryPlanner { if let Some(values) = values { if !values.is_empty() { let filter = BaseFilter::try_new( - self.query_tools.clone(), + self.query_tools.query_tools().clone(), switch_member.clone(), FilterType::Dimension, FilterOperator::Equal, Some(values.into_iter().map(Some).collect_vec()), + None, )?; state.add_dimension_filter(FilterItem::Item(filter)); } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/multiplied_measures_query_planner.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/multiplied_measures_query_planner.rs index 3c22d2abb9163..c1c2115f3864c 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/multiplied_measures_query_planner.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/multiplied_measures_query_planner.rs @@ -5,7 +5,7 @@ use crate::planner::collectors::{ collect_sub_query_dimensions_from_members, collect_sub_query_dimensions_from_symbols, }; use crate::planner::planners::multi_stage::{EvaluationContext, PlanningScope}; -use crate::planner::query_tools::QueryTools; +use crate::planner::state::State; use crate::planner::JoinTree; use crate::planner::MemberSymbol; use crate::planner::{FullKeyAggregateMeasures, QueryProperties}; @@ -19,7 +19,7 @@ use std::rc::Rc; /// `AggregateMultipliedSubquery` CTEs (one per owning cube). Both /// kinds are registered into the shared `PlanningScope`. pub struct MultipliedMeasuresQueryPlanner { - query_tools: Rc, + query_tools: Rc, query_properties: Rc, join_planner: JoinPlanner, common_utils: CommonUtils, @@ -31,7 +31,7 @@ impl MultipliedMeasuresQueryPlanner { /// `full_key_aggregate_measures` for later use in /// `plan_queries`. pub fn try_new( - query_tools: Rc, + query_tools: Rc, query_properties: Rc, ) -> Result { let full_key_aggregate_measures = query_properties.full_key_aggregate_measures()?; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/query_planner.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/query_planner.rs index 6d14a4cfeba6e..91cd5fabe2083 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/query_planner.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/query_planner.rs @@ -4,7 +4,7 @@ use super::{ }; use crate::logical_plan::*; use crate::planner::planners::multi_stage::PlanningScope; -use crate::planner::query_tools::QueryTools; +use crate::planner::state::State; use crate::planner::QueryProperties; use cubenativeutils::CubeError; use std::rc::Rc; @@ -16,12 +16,12 @@ use std::rc::Rc; /// `MultipliedMeasuresQueryPlanner`) and stitched together by /// `FullKeyAggregateQueryPlanner`. pub struct QueryPlanner { - query_tools: Rc, + query_tools: Rc, request: Rc, } impl QueryPlanner { - pub fn new(request: Rc, query_tools: Rc) -> Self { + pub fn new(request: Rc, query_tools: Rc) -> Self { Self { request, query_tools, diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/simple_query_planer.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/simple_query_planer.rs index 548e14e2d09c0..802fe5c427266 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/simple_query_planer.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/simple_query_planer.rs @@ -2,7 +2,7 @@ use super::{DimensionSubqueryPlanner, JoinPlanner}; use crate::logical_plan::*; use crate::planner::collectors::collect_sub_query_dimensions_from_symbols; use crate::planner::planners::multi_stage::PlanningScope; -use crate::planner::query_tools::QueryTools; +use crate::planner::state::State; use crate::planner::QueryProperties; use cubenativeutils::CubeError; use std::rc::Rc; @@ -11,12 +11,12 @@ use std::rc::Rc; /// source, no multi-stage or multiplied CTEs. Sub-query dimensions /// are still woven into the join. pub struct SimpleQueryPlanner { - query_tools: Rc, + query_tools: Rc, query_properties: Rc, join_planner: JoinPlanner, } impl SimpleQueryPlanner { - pub fn new(query_tools: Rc, query_properties: Rc) -> Self { + pub fn new(query_tools: Rc, query_properties: Rc) -> Self { Self { join_planner: JoinPlanner::new(query_tools.clone()), query_properties, diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs index aa40967c234e0..d2bbeabf23ec7 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs @@ -6,7 +6,7 @@ //! For inputs that originate from `BaseQueryOptions`, see //! [`QueryPropertiesCompiler`](super::query_properties_compiler). -use super::query_tools::QueryTools; +use super::state::State; use super::MemberSymbol; use crate::planner::collectors::{collect_multiplied_measures, has_multi_stage_members}; use crate::planner::filter::tree_ops; @@ -134,7 +134,7 @@ impl FullKeyAggregateMeasures { #[derive(Clone, TypedBuilder)] #[builder(build_method(into = Result, CubeError>))] pub struct QueryProperties { - query_tools: Rc, + query_tools: Rc, #[builder(default)] measures: Vec>, #[builder(default)] @@ -913,6 +913,7 @@ impl QueryProperties { FilterOperator::BeforeOrOnDate, to_value, itm.use_raw_values(), + None, )?)); } other => new_filters.push(other.clone()), @@ -938,6 +939,7 @@ impl QueryProperties { FilterOperator::AfterOrOnDate, from_value, itm.use_raw_values(), + None, )?)); } other => new_filters.push(other.clone()), @@ -1067,7 +1069,14 @@ impl QueryProperties { }; values.extend(additional_values.iter().cloned()); let use_raw_values = use_raw_values.unwrap_or(itm.use_raw_values()); - itm.change_operator(operator.clone(), values, use_raw_values)? + itm.change_operator( + operator.clone(), + values, + use_raw_values, + // FIXME: late compilation — only needed to recompile a + // to_date rolling-window granularity here. + Some(&mut self.query_tools.compiler().borrow_mut()), + )? } else { itm.clone() }; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties_compiler.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties_compiler.rs index f6c64760938e2..9e41d7990fa8b 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties_compiler.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties_compiler.rs @@ -19,7 +19,7 @@ use super::filter::compiler::FilterCompiler; use super::filter::{BaseSegment, FilterItem}; use super::join_hints::JoinHints; use super::query_properties::{OrderByItem, QueryProperties}; -use super::query_tools::QueryTools; +use super::state::State; use super::{ Compiler, GranularityHelper, MemberExpressionExpression, MemberExpressionSymbol, MemberSymbol, TimeDimensionSymbol, @@ -28,11 +28,11 @@ use super::{ /// One-shot translator from [`BaseQueryOptions`] into a finalized /// [`QueryProperties`]. pub struct QueryPropertiesCompiler { - query_tools: Rc, + query_tools: Rc, } impl QueryPropertiesCompiler { - pub fn new(query_tools: Rc) -> Self { + pub fn new(query_tools: Rc) -> Self { Self { query_tools } } @@ -41,7 +41,7 @@ impl QueryPropertiesCompiler { options: Rc, ) -> Result, CubeError> { let options = options.as_ref(); - let evaluator_compiler_cell = self.query_tools.evaluator_compiler().clone(); + let evaluator_compiler_cell = self.query_tools.compiler().clone(); let mut evaluator_compiler = evaluator_compiler_cell.borrow_mut(); let dimensions = self.compile_dimensions(&mut evaluator_compiler, options)?; @@ -391,7 +391,8 @@ impl QueryPropertiesCompiler { time_dimensions: &[Rc], measures: &[Rc], ) -> Result<(Vec, Vec, Vec), CubeError> { - let mut filter_compiler = FilterCompiler::new(evaluator_compiler, self.query_tools.clone()); + let mut filter_compiler = + FilterCompiler::new(evaluator_compiler, self.query_tools.query_tools().clone()); if let Some(filters) = &options.static_data().filters { for filter in filters { filter_compiler.add_item(filter)?; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_tools.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_tools.rs index b2345c457f37e..e108076d23145 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_tools.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_tools.rs @@ -1,4 +1,3 @@ -use super::Compiler; use super::ParamsAllocator; use crate::cube_bridge::base_query_options::MaskedMemberItem; use crate::cube_bridge::base_tools::BaseTools; @@ -6,13 +5,10 @@ use crate::cube_bridge::evaluator::CubeEvaluator; use crate::cube_bridge::join_definition::JoinDefinition; use crate::cube_bridge::join_graph::JoinGraph; use crate::cube_bridge::join_item::JoinItemStatic; -use crate::cube_bridge::security_context::SecurityContext; use crate::cube_bridge::sql_templates_render::SqlTemplatesRender; -use crate::planner::filter::compiler::FilterCompiler; -use crate::planner::filter::{FilterGroup, FilterGroupOperator, FilterItem}; +use crate::planner::filter::FilterItem; use crate::planner::join_hints::JoinHints; use crate::planner::sql_templates::PlanSqlTemplates; -use crate::planner::JoinTreeCache; use chrono_tz::Tz; use cubenativeutils::CubeError; use itertools::Itertools; @@ -32,28 +28,39 @@ pub struct QueryTools { join_graph: Rc, templates_render: Rc, params_allocator: Rc>, - evaluator_compiler: Rc>, timezone: Tz, convert_tz_for_raw_time_dimension: bool, masked_members: HashSet, - // Compiled mask filters keyed by member full path. Populated in try_new - // after the QueryTools Rc is constructed (FilterCompiler requires it), + // Compiled mask filters keyed by member full path. Populated once by + // `State` after the Rc exists (FilterCompiler requires it), // then never mutated again — RefCell only carries the construction phase. + // + // KNOWN REMAINING CYCLE (only when masked members are present): the stored + // FilterItem -> BaseFilter -> TypedFilter holds a strong `Rc`, + // so QueryTools -> member_mask_filters -> ... -> QueryTools never drops. + // This field still lives here only because moving it is deferred to a + // follow-up; relocating it into `State` (like the Compiler and join-tree + // cache) closes the cycle, since `State` is not pointed back to by any + // cached value. The leak regression test covers the join-cache cycle, not + // this one. member_mask_filters: RefCell>, - join_tree_cache: JoinTreeCache, } impl QueryTools { + /// Builds the immutable, cache-free per-query leaf. The mutable + /// per-query state (the `Compiler`, the join-tree cache, the + /// compiled mask filters) lives in `State`, which owns this and + /// fills `member_mask_filters` once it has a `Compiler`. Keeping + /// `QueryTools` free of any cache that can hold an `Rc` + /// is what prevents the planner's reference-cycle leaks. pub fn try_new( cube_evaluator: Rc, - security_context: Rc, base_tools: Rc, join_graph: Rc, timezone_name: Option, export_annotated_sql: bool, convert_tz_for_raw_time_dimension: bool, masked_members: Option>, - member_to_alias: Option>, ) -> Result, CubeError> { let templates_render = base_tools.sql_templates()?; let timezone = if let Some(timezone) = timezone_name { @@ -63,17 +70,7 @@ impl QueryTools { } else { Tz::UTC }; - let evaluator_compiler = Rc::new(RefCell::new(Compiler::new( - cube_evaluator.clone(), - base_tools.clone(), - security_context.clone(), - timezone.clone(), - member_to_alias, - ))); - // Phase 1: collect masked member names eagerly; mask filters are - // compiled in Phase 2 below (FilterCompiler requires Rc, - // which doesn't exist yet at this point). let mut masked_set = HashSet::new(); if let Some(items) = &masked_members { for item in items { @@ -81,62 +78,23 @@ impl QueryTools { } } - let result = Rc::new(Self { + Ok(Rc::new(Self { cube_evaluator, base_tools, join_graph, templates_render, params_allocator: Rc::new(RefCell::new(ParamsAllocator::new(export_annotated_sql))), - evaluator_compiler: evaluator_compiler.clone(), timezone, convert_tz_for_raw_time_dimension, masked_members: masked_set, member_mask_filters: RefCell::new(HashMap::new()), - join_tree_cache: JoinTreeCache::default(), - }); - - evaluator_compiler - .borrow_mut() - .set_query_tools(Rc::downgrade(&result)); - - // Phase 2: compile mask filters once now that Rc exists. - // After this, member_mask_filters is treated as immutable for the - // lifetime of QueryTools. - if let Some(items) = masked_members { - Self::compile_mask_filters(&result, items)?; - } - - Ok(result) + })) } - fn compile_mask_filters( - this: &Rc, - items: Vec, - ) -> Result<(), CubeError> { - let mut compiled = HashMap::new(); - for item in items { - let Some(native_filter) = item.filter else { - continue; - }; - let mut compiler = this.evaluator_compiler.borrow_mut(); - let mut filter_compiler = FilterCompiler::new(&mut compiler, this.clone()); - filter_compiler.add_item(&native_filter)?; - let (dimension_filters, _, _) = filter_compiler.extract_result(); - if dimension_filters.is_empty() { - continue; - } - let filter_item = if dimension_filters.len() == 1 { - dimension_filters.into_iter().next().unwrap() - } else { - FilterItem::Group(Rc::new(FilterGroup::new( - FilterGroupOperator::And, - dimension_filters, - ))) - }; - compiled.insert(item.member, filter_item); - } - *this.member_mask_filters.borrow_mut() = compiled; - Ok(()) + /// Installs the compiled mask filters. Called once by `State` after it + /// has compiled them (it owns the `Compiler` needed to do so). + pub(crate) fn set_member_mask_filters(&self, filters: HashMap) { + *self.member_mask_filters.borrow_mut() = filters; } pub fn is_member_masked(&self, member_path: &str) -> bool { @@ -190,14 +148,6 @@ impl QueryTools { Ok((join_key, join)) } - pub fn evaluator_compiler(&self) -> &Rc> { - &self.evaluator_compiler - } - - pub fn join_tree_cache(&self) -> &JoinTreeCache { - &self.join_tree_cache - } - pub fn alias_name(&self, name: &str) -> String { PlanSqlTemplates::alias_name(name) } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/state.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/state.rs new file mode 100644 index 0000000000000..fb411b94b6658 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/state.rs @@ -0,0 +1,138 @@ +use super::query_tools::QueryTools; +use super::Compiler; +use super::JoinTreeCache; +use crate::cube_bridge::base_query_options::MaskedMemberItem; +use crate::cube_bridge::base_tools::BaseTools; +use crate::cube_bridge::evaluator::CubeEvaluator; +use crate::cube_bridge::join_graph::JoinGraph; +use crate::cube_bridge::security_context::SecurityContext; +use crate::planner::filter::compiler::FilterCompiler; +use crate::planner::filter::{FilterGroup, FilterGroupOperator, FilterItem}; +use cubenativeutils::CubeError; +use std::cell::RefCell; +use std::collections::HashMap; +use std::ops::Deref; +use std::rc::Rc; + +/// Mutable, per-query planning state. Owns everything that mutates or +/// caches during planning — the symbol `Compiler` and the join-tree +/// cache — and holds the immutable leaf `QueryTools`. +/// +/// This split is what keeps the planner free of `Rc` cycles: cached +/// values (`BaseCube`, `TypedFilter`, symbols, …) only ever hold an +/// `Rc`, and `QueryTools` owns no cache that could point +/// back at them. `State` is the cache owner, and nothing stored in a +/// cache may hold an `Rc` — so the per-query graph drops cleanly +/// when the query finishes. Transient planners hold `Rc`; +/// long-lived/cached values must not. +/// +/// `Deref` lets callers reach the immutable leaf +/// API directly (`state.cube_evaluator()`, `state.alias_name(..)`, …); +/// the mutable state is reached via `compiler()` / `join_tree_cache()`. +pub struct State { + query_tools: Rc, + compiler: Rc>, + join_tree_cache: JoinTreeCache, +} + +impl State { + #[allow(clippy::too_many_arguments)] + pub fn try_new( + cube_evaluator: Rc, + security_context: Rc, + base_tools: Rc, + join_graph: Rc, + timezone_name: Option, + export_annotated_sql: bool, + convert_tz_for_raw_time_dimension: bool, + masked_members: Option>, + member_to_alias: Option>, + ) -> Result, CubeError> { + let query_tools = QueryTools::try_new( + cube_evaluator.clone(), + base_tools.clone(), + join_graph, + timezone_name, + export_annotated_sql, + convert_tz_for_raw_time_dimension, + masked_members.clone(), + )?; + + let compiler = Rc::new(RefCell::new(Compiler::new( + cube_evaluator, + base_tools, + security_context, + query_tools.timezone(), + member_to_alias, + ))); + + let result = Rc::new(Self { + query_tools, + compiler, + join_tree_cache: JoinTreeCache::default(), + }); + + result + .compiler + .borrow_mut() + .set_query_tools(Rc::downgrade(&result.query_tools)); + + // Compile mask filters now that both the Compiler and the + // Rc exist; the result is stored back into QueryTools. + if let Some(items) = masked_members { + Self::compile_mask_filters(&result, items)?; + } + + Ok(result) + } + + pub fn query_tools(&self) -> &Rc { + &self.query_tools + } + + pub fn compiler(&self) -> &Rc> { + &self.compiler + } + + pub fn join_tree_cache(&self) -> &JoinTreeCache { + &self.join_tree_cache + } + + fn compile_mask_filters( + this: &Rc, + items: Vec, + ) -> Result<(), CubeError> { + let mut compiled = HashMap::new(); + for item in items { + let Some(native_filter) = item.filter else { + continue; + }; + let mut compiler = this.compiler.borrow_mut(); + let mut filter_compiler = FilterCompiler::new(&mut compiler, this.query_tools.clone()); + filter_compiler.add_item(&native_filter)?; + let (dimension_filters, _, _) = filter_compiler.extract_result(); + if dimension_filters.is_empty() { + continue; + } + let filter_item = if dimension_filters.len() == 1 { + dimension_filters.into_iter().next().unwrap() + } else { + FilterItem::Group(Rc::new(FilterGroup::new( + FilterGroupOperator::And, + dimension_filters, + ))) + }; + compiled.insert(item.member, filter_item); + } + this.query_tools.set_member_mask_filters(compiled); + Ok(()) + } +} + +impl Deref for State { + type Target = QueryTools; + + fn deref(&self) -> &Self::Target { + &self.query_tools + } +} diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/time_dimension_symbol.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/time_dimension_symbol.rs index da568383668dd..efdccc541eeee 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/time_dimension_symbol.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/time_dimension_symbol.rs @@ -1,6 +1,7 @@ use super::common::CompiledMemberPath; use super::MemberSymbol; use crate::planner::query_tools::QueryTools; +use crate::planner::state::State; use crate::planner::time_dimension::Granularity; use crate::planner::CubeRef; use crate::planner::{GranularityHelper, QueryDateTime, QueryDateTimeHelper}; @@ -83,14 +84,18 @@ impl TimeDimensionSymbol { /// the base symbol and date range are preserved. pub fn change_granularity( &self, - query_tools: Rc, + // FIXME: late compilation. `State` is threaded here only to reach the + // `Compiler` and (re)compile a granularity during planning. Once + // compilation happens in an early phase, this should take the + // already-resolved granularity and drop the `State`/`Compiler` dependency. + state: Rc, new_granularity: Option, ) -> Result, CubeError> { - let evaluator_compiler_cell = query_tools.evaluator_compiler().clone(); + let evaluator_compiler_cell = state.compiler().clone(); let mut evaluator_compiler = evaluator_compiler_cell.borrow_mut(); let new_granularity_obj = GranularityHelper::make_granularity_obj( - query_tools.cube_evaluator().clone(), + state.cube_evaluator().clone(), &mut evaluator_compiler, &&self.base_symbol.cube_name(), &self.base_symbol.name(), diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/top_level_planner.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/top_level_planner.rs index 5e72715a1b3d9..f7e3ff0d091fd 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/top_level_planner.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/top_level_planner.rs @@ -1,6 +1,6 @@ use super::planners::multi_stage::PlanningScope; use super::planners::QueryPlanner; -use super::query_tools::QueryTools; +use super::state::State; use super::QueryProperties; use crate::logical_plan::OriginalSqlCollector; use crate::logical_plan::PreAggregationOptimizer; @@ -12,7 +12,7 @@ use std::collections::HashMap; use std::rc::Rc; pub struct TopLevelPlanner { - query_tools: Rc, + query_tools: Rc, request: Rc, cubestore_support_multistage: bool, } @@ -20,7 +20,7 @@ pub struct TopLevelPlanner { impl TopLevelPlanner { pub fn new( request: Rc, - query_tools: Rc, + query_tools: Rc, cubestore_support_multistage: bool, ) -> Self { Self { @@ -52,9 +52,10 @@ impl TopLevelPlanner { let templates = self.query_tools.plan_sql_templates(is_external)?; let physical_plan_builder = - PhysicalPlanBuilder::new(self.query_tools.clone(), templates.clone()); + PhysicalPlanBuilder::new(self.query_tools.query_tools().clone(), templates.clone()); let original_sql_pre_aggregations = if !self.request.is_pre_aggregation_query() { - OriginalSqlCollector::new(self.query_tools.clone()).collect(&optimized_plan)? + OriginalSqlCollector::new(self.query_tools.query_tools().clone()) + .collect(&optimized_plan)? } else { HashMap::new() }; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/test_utils/test_context.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/test_utils/test_context.rs index 53600665cf461..8e87988639238 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/test_utils/test_context.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/test_utils/test_context.rs @@ -8,8 +8,8 @@ use crate::physical_plan::sql_nodes::SqlNodesFactory; use crate::physical_plan::{SqlEvaluatorVisitor, VisitorContext}; use crate::planner::filter::base_segment::BaseSegment; use crate::planner::filter::Filter; -use crate::planner::query_tools::QueryTools; use crate::planner::sql_templates::PlanSqlTemplates; +use crate::planner::state::State; use crate::planner::top_level_planner::TopLevelPlanner; use crate::planner::{GranularityHelper, QueryProperties, QueryPropertiesCompiler}; use crate::planner::{MemberSymbol, TimeDimensionSymbol}; @@ -24,7 +24,7 @@ use std::rc::Rc; /// Test context providing query tools and symbol creation helpers pub struct TestContext { schema: MockSchema, - query_tools: Rc, + query_tools: Rc, security_context: Rc, /// Custom SQL templates carried over through `for_options` so that /// timezone-driven `MockBaseTools` rebuilds (e.g. when the caller @@ -50,7 +50,7 @@ impl TestContext { let security_context: Rc = Rc::new(MockSecurityContext); - let query_tools = QueryTools::try_new( + let query_tools = State::try_new( evaluator, security_context.clone(), Rc::new(base_tools), @@ -170,7 +170,7 @@ impl TestContext { let security_context: Rc = Rc::new(MockSecurityContext); - let query_tools = QueryTools::try_new( + let query_tools = State::try_new( evaluator, security_context.clone(), Rc::new(base_tools), @@ -191,7 +191,7 @@ impl TestContext { } #[allow(dead_code)] - pub fn query_tools(&self) -> &Rc { + pub fn query_tools(&self) -> &Rc { &self.query_tools } @@ -205,21 +205,21 @@ impl TestContext { #[allow(dead_code)] pub fn create_symbol(&self, member_path: &str) -> Result, CubeError> { self.query_tools - .evaluator_compiler() + .compiler() .borrow_mut() .add_auto_resolved_member_evaluator(member_path.to_string()) } pub fn create_dimension(&self, path: &str) -> Result, CubeError> { self.query_tools - .evaluator_compiler() + .compiler() .borrow_mut() .add_dimension_evaluator(path.to_string()) } pub fn create_measure(&self, path: &str) -> Result, CubeError> { self.query_tools - .evaluator_compiler() + .compiler() .borrow_mut() .add_measure_evaluator(path.to_string()) } @@ -236,7 +236,7 @@ impl TestContext { .query_tools .cube_evaluator() .segment_by_path(path.to_string())?; - let mut compiler = self.query_tools.evaluator_compiler().borrow_mut(); + let mut compiler = self.query_tools.compiler().borrow_mut(); let expression = compiler.compile_sql_call(&cube_name, definition.sql()?)?; let cube_symbol = compiler.add_cube_table_evaluator(cube_name.clone(), vec![])?; drop(compiler); @@ -249,7 +249,7 @@ impl TestContext { path: &str, granularity: Option<&str>, ) -> Result, CubeError> { - let mut compiler = self.query_tools.evaluator_compiler().borrow_mut(); + let mut compiler = self.query_tools.compiler().borrow_mut(); let base_symbol = compiler.add_dimension_evaluator(path.to_string())?; let granularity = granularity.map(|g| g.to_string()); let granularity_obj = GranularityHelper::make_granularity_obj( @@ -270,7 +270,11 @@ impl TestContext { pub fn evaluate_symbol(&self, symbol: &Rc) -> Result { let nodes_factory = SqlNodesFactory::default(); let cube_ref_evaluator = Rc::new(nodes_factory.cube_ref_evaluator()); - let visitor = SqlEvaluatorVisitor::new(self.query_tools.clone(), cube_ref_evaluator, None); + let visitor = SqlEvaluatorVisitor::new( + self.query_tools.query_tools().clone(), + cube_ref_evaluator, + None, + ); let base_tools = self.query_tools.base_tools(); let driver_tools = base_tools.driver_tools(false)?; let templates = PlanSqlTemplates::try_new(driver_tools, false)?; @@ -292,7 +296,11 @@ impl TestContext { nodes_factory.set_ungrouped(false); nodes_factory.set_group_by_members(group_by_members.into_iter().collect()); let cube_ref_evaluator = Rc::new(nodes_factory.cube_ref_evaluator()); - let visitor = SqlEvaluatorVisitor::new(self.query_tools.clone(), cube_ref_evaluator, None); + let visitor = SqlEvaluatorVisitor::new( + self.query_tools.query_tools().clone(), + cube_ref_evaluator, + None, + ); let base_tools = self.query_tools.base_tools(); let driver_tools = base_tools.driver_tools(false)?; let templates = PlanSqlTemplates::try_new(driver_tools, false)?; @@ -713,7 +721,7 @@ impl TestContext { let nodes_factory = SqlNodesFactory::default(); let context = Rc::new(VisitorContext::new( - self.query_tools.clone(), + self.query_tools.query_tools().clone(), &nodes_factory, None, )); @@ -732,7 +740,7 @@ impl TestContext { ) -> Result<(String, Vec), CubeError> { let nodes_factory = SqlNodesFactory::default(); let context = Rc::new(VisitorContext::new( - self.query_tools.clone(), + self.query_tools.query_tools().clone(), &nodes_factory, None, )); @@ -740,11 +748,11 @@ impl TestContext { let driver_tools = base_tools.driver_tools(false)?; let templates = PlanSqlTemplates::try_new(driver_tools, false)?; - let visitor = context.make_visitor(self.query_tools.clone()); + let visitor = context.make_visitor(self.query_tools.query_tools().clone()); let sql = base_filter.to_sql( &visitor, context.node_processor(), - self.query_tools.clone(), + self.query_tools.query_tools().clone(), &templates, context.filters_context(), )?; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/tree_ops.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/tree_ops.rs index a71941a2247c5..6fa881ff2fda1 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/tree_ops.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/tree_ops.rs @@ -14,11 +14,12 @@ fn ctx() -> TestContext { fn make_dim_filter(ctx: &TestContext, member_path: &str, value: &str) -> Rc { let symbol = ctx.create_symbol(member_path).unwrap(); BaseFilter::try_new( - ctx.query_tools().clone(), + ctx.query_tools().query_tools().clone(), symbol, FilterType::Dimension, FilterOperator::Equal, Some(vec![Some(value.to_string())]), + None, ) .unwrap() } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/use_raw_values.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/use_raw_values.rs index ac4ebec1df8bd..24acfc526e106 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/use_raw_values.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/use_raw_values.rs @@ -23,7 +23,7 @@ fn test_in_date_range_use_raw_values() { let symbol = ctx.create_symbol("visitors.created_at").unwrap(); let filter = BaseFilter::try_new( - ctx.query_tools().clone(), + ctx.query_tools().query_tools().clone(), symbol, FilterType::Dimension, FilterOperator::InDateRange, @@ -31,6 +31,7 @@ fn test_in_date_range_use_raw_values() { Some("2024-01-01".to_string()), Some("2024-12-31".to_string()), ]), + None, ) .unwrap(); @@ -42,6 +43,7 @@ fn test_in_date_range_use_raw_values() { Some("(SELECT max(dt) FROM cte)".to_string()), ], true, + None, ) .unwrap(); @@ -58,7 +60,7 @@ fn test_not_in_date_range_use_raw_values() { let symbol = ctx.create_symbol("visitors.created_at").unwrap(); let filter = BaseFilter::try_new( - ctx.query_tools().clone(), + ctx.query_tools().query_tools().clone(), symbol, FilterType::Dimension, FilterOperator::NotInDateRange, @@ -66,6 +68,7 @@ fn test_not_in_date_range_use_raw_values() { Some("2024-01-01".to_string()), Some("2024-12-31".to_string()), ]), + None, ) .unwrap(); @@ -77,6 +80,7 @@ fn test_not_in_date_range_use_raw_values() { Some("(SELECT max(dt) FROM cte)".to_string()), ], true, + None, ) .unwrap(); @@ -93,7 +97,7 @@ fn test_before_or_on_date_use_raw_values() { let symbol = ctx.create_symbol("visitors.created_at").unwrap(); let filter = BaseFilter::try_new( - ctx.query_tools().clone(), + ctx.query_tools().query_tools().clone(), symbol, FilterType::Dimension, FilterOperator::InDateRange, @@ -101,6 +105,7 @@ fn test_before_or_on_date_use_raw_values() { Some("2024-01-01".to_string()), Some("2024-12-31".to_string()), ]), + None, ) .unwrap(); @@ -109,6 +114,7 @@ fn test_before_or_on_date_use_raw_values() { FilterOperator::BeforeOrOnDate, vec![Some("(SELECT max(dt) FROM cte)".to_string())], true, + None, ) .unwrap(); @@ -125,7 +131,7 @@ fn test_after_or_on_date_use_raw_values() { let symbol = ctx.create_symbol("visitors.created_at").unwrap(); let filter = BaseFilter::try_new( - ctx.query_tools().clone(), + ctx.query_tools().query_tools().clone(), symbol, FilterType::Dimension, FilterOperator::InDateRange, @@ -133,6 +139,7 @@ fn test_after_or_on_date_use_raw_values() { Some("2024-01-01".to_string()), Some("2024-12-31".to_string()), ]), + None, ) .unwrap(); @@ -141,6 +148,7 @@ fn test_after_or_on_date_use_raw_values() { FilterOperator::AfterOrOnDate, vec![Some("(SELECT min(df) FROM cte)".to_string())], true, + None, ) .unwrap(); diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs index 46ec50e758885..fdb0df6ccf57b 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs @@ -9,6 +9,7 @@ mod filter; mod join_hints_collector; mod measure_symbol; mod member_expressions_on_views; +mod no_query_tools_leak; mod string_measures; mod subquery_dimensions; mod utils; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/no_query_tools_leak.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/no_query_tools_leak.rs new file mode 100644 index 0000000000000..732454b2db213 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/no_query_tools_leak.rs @@ -0,0 +1,61 @@ +//! Regression guard against Rc-cycle leaks in the planner. +//! +//! The whole per-query object graph hangs off `Rc`. If anything +//! reachable from `QueryTools` holds a strong `Rc` back (e.g. a +//! value cached inside `QueryTools` that, in turn, points at it), the refcount +//! never reaches zero and the entire graph — including the compiled minijinja +//! SQL templates — leaks on every query. That is what OOMs the refresh worker. +//! +//! There is no general "is anything still alive?" reflection in Rust, but for +//! an `Rc`-managed graph a `Weak` is exactly that probe: after every legitimate +//! owner is dropped, a still-upgradable `Weak` means a cycle kept the value +//! alive. `assert_released` encodes that check and is reusable for any `Rc`. + +use crate::test_fixtures::cube_bridge::MockSchema; +use crate::test_fixtures::test_utils::TestContext; +use indoc::indoc; +use std::rc::{Rc, Weak}; + +/// Assert that `weak` has no strong owners left. Use after dropping everything +/// that should own the value: a surviving strong ref means an Rc cycle leaked it. +#[track_caller] +fn assert_released(weak: &Weak, what: &str) { + let alive = weak.strong_count(); + assert_eq!( + alive, 0, + "{what} leaked: {alive} strong Rc ref(s) still alive after all owners were dropped \ + (Rc cycle — the per-query graph cannot be freed)" + ); +} + +/// Building SQL for a query that spans a join populates the join-tree cache, +/// which historically created the cycle +/// `QueryTools -> join_tree_cache -> JoinTree -> BaseCube -> Rc`. +/// After planning and dropping the context, `QueryTools` must be fully released. +#[test] +fn query_tools_released_after_join_query() { + let schema = MockSchema::from_yaml_file("common/multi_fact.yaml"); + let ctx = TestContext::new(schema).unwrap(); + + // Probe the leaf `QueryTools` (the hub that owns the compiled minijinja + // templates), NOT the `State` wrapper — `State` is held only by transient + // planners and is released trivially, so downgrading it would prove nothing. + let weak = Rc::downgrade(ctx.query_tools().query_tools()); + + // orders.count + customers.name spans the orders<->customers join, so the + // planner builds and caches a JoinTree (whose BaseCube points back). + let query_yaml = indoc! {" + measures: + - orders.count + dimensions: + - customers.name + "}; + let options = ctx.create_query_options_from_yaml(query_yaml); + let _sql = ctx + .build_sql_from_options(options) + .expect("planning should succeed"); + + drop(ctx); + + assert_released(&weak, "QueryTools"); +} diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/utils/debug.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/utils/debug.rs index 0d17d2eb25fd5..39fd5e2af67ca 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/utils/debug.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/utils/debug.rs @@ -135,11 +135,12 @@ fn test_filter_simple_collapsed() { let symbol = ctx.create_symbol("visitors.source").unwrap(); let filter = BaseFilter::try_new( - ctx.query_tools().clone(), + ctx.query_tools().query_tools().clone(), symbol, crate::planner::filter::base_filter::FilterType::Dimension, FilterOperator::Equal, Some(vec![Some("google".to_string())]), + None, ) .unwrap(); @@ -154,11 +155,12 @@ fn test_filter_simple_expanded() { let symbol = ctx.create_symbol("visitors.source").unwrap(); let filter = BaseFilter::try_new( - ctx.query_tools().clone(), + ctx.query_tools().query_tools().clone(), symbol, crate::planner::filter::base_filter::FilterType::Dimension, FilterOperator::Equal, Some(vec![Some("google".to_string())]), + None, ) .unwrap(); @@ -174,20 +176,22 @@ fn test_filter_group_and_collapsed() { let id_symbol = ctx.create_symbol("visitors.id").unwrap(); let filter1 = BaseFilter::try_new( - ctx.query_tools().clone(), + ctx.query_tools().query_tools().clone(), source_symbol, crate::planner::filter::base_filter::FilterType::Dimension, FilterOperator::Equal, Some(vec![Some("google".to_string())]), + None, ) .unwrap(); let filter2 = BaseFilter::try_new( - ctx.query_tools().clone(), + ctx.query_tools().query_tools().clone(), id_symbol, crate::planner::filter::base_filter::FilterType::Dimension, FilterOperator::Gt, Some(vec![Some("100".to_string())]), + None, ) .unwrap(); From e18d53a7bf43f20f82a738ebe6287c5f7f5535f8 Mon Sep 17 00:00:00 2001 From: Aleksandr Romanenko Date: Thu, 18 Jun 2026 15:40:56 +0200 Subject: [PATCH 2/7] fix(tesseract): compile member sql in JS to stop the native proxy leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Member `sql` functions were compiled by building recording proxies in Rust via neon and invoking the JS function through them. Every property access created neon `JsFunction`s (the get-trap and to-string closures); their backing napi function objects were never reclaimed even after the JS side was GC'd, so the refresh worker leaked unboundedly while generating pre-aggregation SQL (Tesseract only). Move the recording to a stateless JS module (MemberSqlTemplateCompiler): it builds the `{CUBE}`, FILTER_PARAMS, FILTER_GROUP, SECURITY_CONTEXT and SQL_UTILS proxies in JS — where V8 garbage-collects them normally — runs the member function, and returns the produced template plus the recorded dependencies as plain data. BaseQuery exposes it via `compileMemberSql`, reached from Rust through a new `BaseTools::compile_member_sql`; the planner consumes the result exactly as before. The proxy machinery, `ProxyState`, and `MemberSql::compile_template_sql` are removed from the native side. The only JS reference that still crosses back into Rust is a `FILTER_PARAMS.x.filter(fn)` column callback, which is deferred (invoked at render time with the resolved column). It is an existing JS function held by a `Root` — not a Rust-created `JsFunction` — and is released with the query graph, so it is bounded per query and does not reintroduce the leak. The recording logic is now unit-testable in isolation (member-sql-template-compiler.test.ts), which was impractical while it lived behind the Rust↔JS bridge. --- .../src/adapter/BaseQuery.js | 10 + .../src/adapter/MemberSqlTemplateCompiler.js | 268 ++++++++ .../unit/member-sql-template-compiler.test.ts | 166 +++++ .../src/cube_bridge/base_tools.rs | 9 + .../src/cube_bridge/member_sql.rs | 643 ++---------------- .../src/planner/sql_call_builder.rs | 8 +- .../cube_bridge/mock_base_tools.rs | 17 +- .../cube_bridge/mock_evaluator.rs | 7 +- .../cube_bridge/mock_member_sql.rs | 47 +- .../test_fixtures/cube_bridge/mock_schema.rs | 34 +- .../src/test_fixtures/cube_bridge/mod.rs | 2 +- .../test_fixtures/cube_bridge/yaml/schema.rs | 25 +- 12 files changed, 567 insertions(+), 669 deletions(-) create mode 100644 packages/cubejs-schema-compiler/src/adapter/MemberSqlTemplateCompiler.js create mode 100644 packages/cubejs-schema-compiler/test/unit/member-sql-template-compiler.test.ts diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index 3251d51f34ca6..b328685321a60 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -5119,6 +5119,16 @@ export class BaseQuery { }; } + // Invoked from the native planner to compile a member's `sql` function: runs + // it under recording proxies and returns the produced template plus the + // dependencies it touched. The recording logic is a standalone, stateless + // module so it can be unit-tested in isolation. + compileMemberSql(sqlFn, securityContext, argNames) { + // eslint-disable-next-line global-require + const { compileMemberSql } = require('./MemberSqlTemplateCompiler'); + return compileMemberSql(sqlFn, argNames, securityContext, this.sqlUtilsForRust()); + } + contextSymbolsProxy(symbols) { return CubeSymbols.contextSymbolsProxyFrom(symbols, this.paramAllocator.allocateParam.bind(this.paramAllocator)); } diff --git a/packages/cubejs-schema-compiler/src/adapter/MemberSqlTemplateCompiler.js b/packages/cubejs-schema-compiler/src/adapter/MemberSqlTemplateCompiler.js new file mode 100644 index 0000000000000..2fdeb7573ed86 --- /dev/null +++ b/packages/cubejs-schema-compiler/src/adapter/MemberSqlTemplateCompiler.js @@ -0,0 +1,268 @@ +/* eslint-disable no-use-before-define */ + +/** + * Stateless compiler for a member's user-written `sql` function. + * + * It invokes the function with recording proxies (the `{CUBE}`, + * `FILTER_PARAMS`, `FILTER_GROUP`, `SECURITY_CONTEXT`, `SQL_UTILS` arguments the + * data model expects) and returns the produced SQL template together with the + * dependencies the function touched, as plain data: + * + * { + * template: string | string[], + * symbolPaths: string[][], // {arg:N} + * filterParams: [{ cube_name, name, column }], // {fp:N}, column = fn|string + * filterGroups: [{ filterParams: [...] }], // {fg:N} + * securityContextValues: string[] // {sv:N} + * } + * + * Member references are returned as recorded paths — the caller resolves them + * to symbols. FILTER_PARAMS column callbacks are deferred (returned as the raw + * JS function for the caller to invoke at render time); SECURITY_CONTEXT is + * resolved eagerly here against the provided context. + * + * The module holds no planner state — `securityContext` and `sqlUtils` are + * passed in — so it can be unit-tested in isolation. + */ + +const ARG_PREFIX = 'arg'; +const FILTER_PARAM_PREFIX = 'fp'; +const FILTER_GROUP_PREFIX = 'fg'; +const SECURITY_VALUE_PREFIX = 'sv'; + +function placeholder(prefix, index) { + return `{${prefix}:${index}}`; +} + +// Returns the index of an equal path if it already exists, otherwise appends +// and returns the new index. Paths are short arrays of strings, compared by +// value, so repeated references collapse to a single placeholder. +function uniqueInsertPath(paths, path) { + const key = JSON.stringify(path); + for (let i = 0; i < paths.length; i++) { + if (JSON.stringify(paths[i]) === key) { + return i; + } + } + paths.push(path); + return paths.length - 1; +} + +function uniqueInsertString(values, value) { + const i = values.indexOf(value); + if (i !== -1) { + return i; + } + values.push(value); + return values.length - 1; +} + +// ---- member reference (`{CUBE}` / nested paths / `.sql`) -------------------- + +function memberReferenceProxy(path, state) { + return new Proxy({}, { + get(_target, prop) { + if (typeof prop !== 'string') { + // Symbol access (e.g. Symbol.toPrimitive) — return undefined so JS + // falls back to toString/valueOf for string coercion. + return undefined; + } + if (prop === 'sql') { + const index = uniqueInsertPath(state.symbolPaths, [...path, '__sql_fn']); + const ph = placeholder(ARG_PREFIX, index); + return () => ph; + } + if (prop === 'toString' || prop === 'valueOf') { + const index = uniqueInsertPath(state.symbolPaths, path); + const ph = placeholder(ARG_PREFIX, index); + return () => ph; + } + return memberReferenceProxy([...path, prop], state); + }, + }); +} + +// ---- FILTER_PARAMS / FILTER_GROUP ------------------------------------------ + +function filterParamsItemProxy(cubeName, name, state) { + return { + filter(column) { + const item = { cube_name: cubeName, name, column }; + const toString = () => { + const index = state.filterParams.length; + state.filterParams.push(item); + return placeholder(FILTER_PARAM_PREFIX, index); + }; + // `__member` lets FILTER_GROUP recover the item; `toString` records and + // yields the {fp:N} placeholder on coercion. + return { __member: item, toString }; + }, + }; +} + +function filterParamsProxy(state) { + return new Proxy({}, { + get(_t, cubeName) { + return new Proxy({}, { + get(_t2, memberName) { + return filterParamsItemProxy(cubeName, memberName, state); + }, + }); + }, + }); +} + +function filterGroupFn(state) { + return (...args) => { + const filterParams = args.map(arg => { + if (!arg || typeof arg.__member === 'undefined') { + throw new Error('FILTER_GROUP expects FILTER_PARAMS args to be passed.'); + } + return arg.__member; + }); + const index = state.filterGroups.length; + state.filterGroups.push({ filterParams }); + return placeholder(FILTER_GROUP_PREFIX, index); + }; +} + +// ---- SECURITY_CONTEXT ------------------------------------------------------ + +function coerceScalarToString(value) { + if (typeof value === 'string') return value; + if (typeof value === 'number') return `${value}`; + if (typeof value === 'boolean') return `${value}`; + throw new Error('Invalid param for security context'); +} + +// Coercion used by `.filter()` — falsy scalars collapse to "no value". +function coerceFilterValue(value) { + if (value === undefined || value === null) return { kind: 'none' }; + if (Array.isArray(value)) { + return { kind: 'vec', values: value.map(coerceScalarToString) }; + } + if (typeof value === 'string') { + return value === '' ? { kind: 'none' } : { kind: 'string', value }; + } + if (typeof value === 'number') { + return value === 0 || Number.isNaN(value) ? { kind: 'none' } : { kind: 'string', value: `${value}` }; + } + if (typeof value === 'boolean') { + return value ? { kind: 'string', value: 'true' } : { kind: 'none' }; + } + throw new Error('Invalid param for security context'); +} + +// Coercion used by toString/valueOf — keeps all non-null values. +function coerceToStringValue(value) { + if (value === undefined || value === null) return null; + if (Array.isArray(value)) return value.map(coerceScalarToString); + if (typeof value === 'string') return [value]; + if (typeof value === 'number') return [`${value}`]; + if (typeof value === 'boolean') return [`${value}`]; + return null; +} + +function recordSecurityValue(value, state) { + return placeholder(SECURITY_VALUE_PREFIX, uniqueInsertString(state.securityContextValues, value)); +} + +function securityFilterFn(value, required, state) { + const param = coerceFilterValue(value); + return (column) => { + if (param.kind === 'string') { + const ph = recordSecurityValue(param.value, state); + if (typeof column === 'function') return column(ph); + if (typeof column === 'string') return `${column} = ${ph}`; + return ''; + } + if (param.kind === 'vec') { + if (param.values.length === 0) { + if (typeof column === 'function') return column([]); + return '1 = 0'; + } + const phs = param.values.map(v => recordSecurityValue(v, state)); + if (typeof column === 'function') return column(phs); + if (typeof column === 'string') return `${column} IN (${phs.join(', ')})`; + return ''; + } + // none + if (required) { + throw new Error(`Filter for ${column} is required`); + } + return '1 = 1'; + }; +} + +function securityToStringFn(value, state) { + const values = coerceToStringValue(value); + return () => { + if (values === null) return ''; + return values.map(v => recordSecurityValue(v, state)).join(','); + }; +} + +function securityContextProxy(value, state) { + return new Proxy({}, { + get(_t, prop) { + if (typeof prop !== 'string') return undefined; + // Methods coerce the current value lazily — only on access, so reading a + // nested object property does not attempt to coerce the object itself. + if (prop === 'filter') return securityFilterFn(value, false, state); + if (prop === 'requiredFilter') return securityFilterFn(value, true, state); + if (prop === 'unsafeValue') return () => value; + if (prop === 'toString' || prop === 'valueOf') return securityToStringFn(value, state); + const propertyValue = value != null && typeof value === 'object' ? value[prop] : undefined; + return securityContextProxy(propertyValue, state); + }, + }); +} + +// ---- entry point ----------------------------------------------------------- + +function buildArg(argName, state, securityContext, sqlUtils) { + if (argName === 'FILTER_PARAMS') return filterParamsProxy(state); + if (argName === 'FILTER_GROUP') return filterGroupFn(state); + if (argName === 'SECURITY_CONTEXT' || argName === 'security_context' || argName === 'securityContext') { + return securityContextProxy(securityContext, state); + } + if (argName === 'SQL_UTILS') return sqlUtils; + return memberReferenceProxy([argName], state); +} + +function parseTemplateResult(result) { + if (Array.isArray(result)) { + return result.map(r => `${r}`); + } + return `${result}`; +} + +/** + * @param {Function} sqlFn the member's `sql` function from the data model + * @param {string[]} argNames its argument names (CUBE, FILTER_PARAMS, ...) + * @param {object} securityContext the request security context object + * @param {object} sqlUtils the SQL_UTILS object passed through to the template + */ +function compileMemberSql(sqlFn, argNames, securityContext, sqlUtils) { + const state = { + symbolPaths: [], + filterParams: [], + filterGroups: [], + securityContextValues: [], + }; + + const args = argNames.map(name => buildArg(name, state, securityContext, sqlUtils)); + const template = parseTemplateResult(sqlFn(...args)); + + return { + template, + symbolPaths: state.symbolPaths, + filterParams: state.filterParams, + filterGroups: state.filterGroups, + securityContextValues: state.securityContextValues, + }; +} + +exports.compileMemberSql = compileMemberSql; +exports.uniqueInsertPath = uniqueInsertPath; +exports.uniqueInsertString = uniqueInsertString; diff --git a/packages/cubejs-schema-compiler/test/unit/member-sql-template-compiler.test.ts b/packages/cubejs-schema-compiler/test/unit/member-sql-template-compiler.test.ts new file mode 100644 index 0000000000000..ab7eb8165247a --- /dev/null +++ b/packages/cubejs-schema-compiler/test/unit/member-sql-template-compiler.test.ts @@ -0,0 +1,166 @@ +// eslint-disable-next-line @typescript-eslint/no-var-requires +const { compileMemberSql, uniqueInsertPath } = require('../../src/adapter/MemberSqlTemplateCompiler'); + +describe('MemberSqlTemplateCompiler — member reference path', () => { + it('records a single member reference via string coercion', () => { + const res = compileMemberSql((orders) => `${orders.amount}`, ['orders']); + expect(res.template).toBe('{arg:0}'); + expect(res.symbolPaths).toEqual([['orders', 'amount']]); + expect(res.filterParams).toEqual([]); + }); + + it('records nested member paths', () => { + const res = compileMemberSql((cube) => `${cube.a.b.c}`, ['cube']); + expect(res.template).toBe('{arg:0}'); + expect(res.symbolPaths).toEqual([['cube', 'a', 'b', 'c']]); + }); + + it('records the .sql() function call form with the __sql_fn suffix', () => { + const res = compileMemberSql((orders) => `${orders.amount.sql()}`, ['orders']); + expect(res.template).toBe('{arg:0}'); + expect(res.symbolPaths).toEqual([['orders', 'amount', '__sql_fn']]); + }); + + it('dedups repeated identical references to the same {arg:N}', () => { + const res = compileMemberSql((o) => `${o.amount} + ${o.amount}`, ['o']); + expect(res.template).toBe('{arg:0} + {arg:0}'); + expect(res.symbolPaths).toEqual([['o', 'amount']]); + }); + + it('assigns distinct indices to distinct references', () => { + const res = compileMemberSql((o) => `${o.a} - ${o.b}`, ['o']); + expect(res.template).toBe('{arg:0} - {arg:1}'); + expect(res.symbolPaths).toEqual([['o', 'a'], ['o', 'b']]); + }); + + it('handles multiple cube args', () => { + const res = compileMemberSql((a, b) => `${a.x} = ${b.y}`, ['a', 'b']); + expect(res.template).toBe('{arg:0} = {arg:1}'); + expect(res.symbolPaths).toEqual([['a', 'x'], ['b', 'y']]); + }); + + it('supports an array template result (member sql returning an array)', () => { + const res = compileMemberSql((o) => [o.a, o.b], ['o']); + expect(res.template).toEqual(['{arg:0}', '{arg:1}']); + expect(res.symbolPaths).toEqual([['o', 'a'], ['o', 'b']]); + }); +}); + +describe('MemberSqlTemplateCompiler — FILTER_PARAMS / FILTER_GROUP', () => { + it('records a filter param with a string column and yields {fp:0}', () => { + const res = compileMemberSql( + (FILTER_PARAMS) => `${FILTER_PARAMS.orders.status.filter('t.status')}`, + ['FILTER_PARAMS'] + ); + expect(res.template).toBe('{fp:0}'); + expect(res.filterParams).toEqual([{ cube_name: 'orders', name: 'status', column: 't.status' }]); + }); + + it('keeps the column callback as a function (deferred) and records {fp:N}', () => { + let captured; + const res = compileMemberSql( + (FILTER_PARAMS) => `${FILTER_PARAMS.orders.status.filter((c) => `${c} > 0`)}`, + ['FILTER_PARAMS'] + ); + expect(res.template).toBe('{fp:0}'); + expect(res.filterParams).toHaveLength(1); + expect(res.filterParams[0].cube_name).toBe('orders'); + expect(res.filterParams[0].name).toBe('status'); + expect(typeof res.filterParams[0].column).toBe('function'); + captured = res.filterParams[0].column; + expect(captured('X')).toBe('X > 0'); + }); + + it('records a filter group from filter-param args and yields {fg:0}', () => { + const res = compileMemberSql( + (FILTER_GROUP, FILTER_PARAMS) => `${FILTER_GROUP( + FILTER_PARAMS.orders.a.filter('a'), + FILTER_PARAMS.orders.b.filter('b') + )}`, + ['FILTER_GROUP', 'FILTER_PARAMS'] + ); + expect(res.template).toBe('{fg:0}'); + expect(res.filterGroups).toHaveLength(1); + expect(res.filterGroups[0].filterParams.map((p) => p.name)).toEqual(['a', 'b']); + }); +}); + +describe('MemberSqlTemplateCompiler — SECURITY_CONTEXT', () => { + it('filter() with string column records the value and emits col = {sv:0}', () => { + const res = compileMemberSql( + (SECURITY_CONTEXT) => `${SECURITY_CONTEXT.tenantId.filter('t.id')}`, + ['SECURITY_CONTEXT'], + { tenantId: 'acme' } + ); + expect(res.template).toBe('t.id = {sv:0}'); + expect(res.securityContextValues).toEqual(['acme']); + }); + + it('filter() with a callback passes the {sv:N} placeholder into the callback', () => { + const res = compileMemberSql( + (SECURITY_CONTEXT) => `${SECURITY_CONTEXT.tenantId.filter((c) => `${c} IN (sub)`)}`, + ['SECURITY_CONTEXT'], + { tenantId: 'acme' } + ); + expect(res.template).toBe('{sv:0} IN (sub)'); + expect(res.securityContextValues).toEqual(['acme']); + }); + + it('array value emits IN (...) with one {sv:N} per element', () => { + const res = compileMemberSql( + (SECURITY_CONTEXT) => `${SECURITY_CONTEXT.roles.filter('r')}`, + ['SECURITY_CONTEXT'], + { roles: ['a', 'b'] } + ); + expect(res.template).toBe('r IN ({sv:0}, {sv:1})'); + expect(res.securityContextValues).toEqual(['a', 'b']); + }); + + it('missing value: filter() emits 1 = 1, requiredFilter() throws', () => { + const ok = compileMemberSql( + (SECURITY_CONTEXT) => `${SECURITY_CONTEXT.missing.filter('x')}`, + ['SECURITY_CONTEXT'], + {} + ); + expect(ok.template).toBe('1 = 1'); + expect(ok.securityContextValues).toEqual([]); + + expect(() => compileMemberSql( + (SECURITY_CONTEXT) => `${SECURITY_CONTEXT.missing.requiredFilter('x')}`, + ['SECURITY_CONTEXT'], + {} + )).toThrow(); + }); + + it('toString coercion records the value as {sv:0}; unsafeValue returns raw', () => { + const res = compileMemberSql( + (SECURITY_CONTEXT) => `${SECURITY_CONTEXT.tenantId}|${SECURITY_CONTEXT.tenantId.unsafeValue()}`, + ['SECURITY_CONTEXT'], + { tenantId: 'acme' } + ); + expect(res.template).toBe('{sv:0}|acme'); + expect(res.securityContextValues).toEqual(['acme']); + }); +}); + +describe('MemberSqlTemplateCompiler — SQL_UTILS', () => { + it('passes the provided sqlUtils through to the template', () => { + const res = compileMemberSql( + (SQL_UTILS) => `${SQL_UTILS.convertTz('x')}`, + ['SQL_UTILS'], + undefined, + { convertTz: (c) => `TZ(${c})` } + ); + expect(res.template).toBe('TZ(x)'); + }); +}); + +describe('uniqueInsertPath', () => { + it('returns existing index for an equal path and appends new ones', () => { + const paths = []; + expect(uniqueInsertPath(paths, ['a', 'b'])).toBe(0); + expect(uniqueInsertPath(paths, ['a', 'c'])).toBe(1); + expect(uniqueInsertPath(paths, ['a', 'b'])).toBe(0); + expect(paths).toEqual([['a', 'b'], ['a', 'c']]); + }); +}); diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/base_tools.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/base_tools.rs index 208899f5c49a1..4d87e30c40b94 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/base_tools.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/base_tools.rs @@ -1,6 +1,8 @@ use super::driver_tools::{DriverTools, NativeDriverTools}; use super::join_definition::{JoinDefinition, NativeJoinDefinition}; +use super::member_sql::{CompiledMemberTemplate, MemberSql, NativeMemberSql}; use super::pre_aggregation_obj::{NativePreAggregationObj, PreAggregationObj}; +use super::security_context::{NativeSecurityContext, SecurityContext}; use super::sql_templates_render::{NativeSqlTemplatesRender, SqlTemplatesRender}; use super::sql_utils::{NativeSqlUtils, SqlUtils}; use crate::cube_bridge::join_hints::JoinHintItem; @@ -40,4 +42,11 @@ pub trait BaseTools { &self, hints: Vec, ) -> Result, CubeError>; + + fn compile_member_sql( + &self, + member_sql: Rc, + security_context: Rc, + arg_names: Vec, + ) -> Result; } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/member_sql.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/member_sql.rs index 97fcc5545c2eb..ac744baf5a063 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/member_sql.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/member_sql.rs @@ -1,20 +1,14 @@ use super::filter_params_callback::{FilterParamsCallback, NativeFilterParamsCallback}; -use super::{ - security_context::{NativeSecurityContext, SecurityContext}, - sql_utils::NativeSqlUtils, -}; -use crate::cube_bridge::base_tools::BaseTools; -use crate::planner::SqlCallArg; use crate::utils::UniqueVector; +use cubenativeutils::wrappers::inner_types::InnerTypes; use cubenativeutils::wrappers::object::{NativeFunction, NativeStruct, NativeType}; use cubenativeutils::wrappers::serializer::{NativeDeserialize, NativeSerialize}; use cubenativeutils::wrappers::NativeContextHolderRef; use cubenativeutils::wrappers::NativeObjectHandle; -use cubenativeutils::wrappers::{inner_types::InnerTypes, NativeString}; use cubenativeutils::wrappers::{NativeArray, NativeContextHolder}; use cubenativeutils::CubeError; +use std::any::Any; use std::rc::Rc; -use std::{any::Any, cell::RefCell, rc::Weak}; /// Result of evaluating a member's `sql` JS function: a single SQL /// string, or — for pre-aggregation `dimensions:` / `measures:` @@ -180,11 +174,63 @@ impl FilterGroupItem { } } +fn deserialize_filter_params_vec( + handle: NativeObjectHandle, +) -> Result, CubeError> { + handle + .to_array()? + .to_vec()? + .into_iter() + .map(FilterParamsItem::from_native) + .collect() +} + +impl NativeDeserialize for FilterGroupItem { + fn from_native(native_object: NativeObjectHandle) -> Result { + let object = native_object.to_struct()?; + let filter_params = deserialize_filter_params_vec(object.get_field("filterParams")?)?; + Ok(Self { filter_params }) + } +} + #[derive(Default, Clone, Debug)] pub struct SecutityContextProps { pub values: Vec, } +/// Result of compiling a member `sql` function on the JS side: the produced +/// template plus the dependencies it recorded. +pub struct CompiledMemberTemplate { + pub template: SqlTemplate, + pub args: SqlTemplateArgs, +} + +impl NativeDeserialize for CompiledMemberTemplate { + fn from_native(native_object: NativeObjectHandle) -> Result { + let object = native_object.to_struct()?; + let template = SqlTemplate::from_native(object.get_field("template")?)?; + let symbol_paths = Vec::>::from_native(object.get_field("symbolPaths")?)?; + let filter_params = deserialize_filter_params_vec(object.get_field("filterParams")?)?; + let filter_groups = object + .get_field("filterGroups")? + .to_array()? + .to_vec()? + .into_iter() + .map(FilterGroupItem::from_native) + .collect::, _>>()?; + let values = Vec::::from_native(object.get_field("securityContextValues")?)?; + Ok(Self { + template, + args: SqlTemplateArgs { + symbol_paths, + filter_params, + filter_groups, + security_context: SecutityContextProps { values }, + }, + }) + } +} + /// Dependencies collected while compiling a member `sql` function. /// Each `{arg:N}` / `{fp:N}` / `{fg:N}` / `{sv:N}` placeholder in /// the produced `SqlTemplate` indexes into one of these vectors. @@ -241,70 +287,6 @@ impl SqlTemplateArgs { } } -struct ProxyState { - state: RefCell, -} - -impl ProxyState { - fn new() -> Rc { - Rc::new(Self { - state: RefCell::new(SqlTemplateArgs::default()), - }) - } - - fn get_args(self: &Rc) -> Result { - self.with_state(|state| state.clone()) - } - - fn weak(self: &Rc) -> ProxyStateWeak { - ProxyStateWeak { - state: Rc::downgrade(self), - } - } - - fn with_state(&self, f: F) -> Result - where - F: FnOnce(&SqlTemplateArgs) -> T, - { - let state = self - .state - .try_borrow() - .map_err(|_| CubeError::internal(format!("Cant borrow dependency parsing state")))?; - Ok(f(&state)) - } - fn with_state_mut(&self, f: F) -> Result - where - F: FnOnce(&mut SqlTemplateArgs) -> T, - { - let mut state = self - .state - .try_borrow_mut() - .map_err(|_| CubeError::internal(format!("Cant borrow dependency parsing state")))?; - Ok(f(&mut state)) - } -} - -#[derive(Clone)] -struct ProxyStateWeak { - state: Weak, -} - -impl ProxyStateWeak { - fn with_state_mut(&self, f: F) -> Result - where - F: FnOnce(&mut SqlTemplateArgs) -> T, - { - let state = self.state.upgrade().ok_or(CubeError::internal(format!( - "Cant upgrade dependency parsing state" - )))?; - state.with_state_mut(f) - } - - fn insert_symbol_path(&self, path: &Vec) -> Result { - self.with_state_mut(|state| state.insert_symbol_path(path.clone())) - } -} - /// A member's `sql:` function as provided by the JS schema compiler. /// `compile_template_sql` invokes the function under proxied /// arguments (`{CUBE}`, `FILTER_PARAMS`, `FILTER_GROUP`, @@ -313,11 +295,6 @@ impl ProxyStateWeak { pub trait MemberSql { fn args_names(&self) -> &Vec; fn as_any(self: Rc) -> Rc; - fn compile_template_sql( - &self, - base_tools: Rc, - security_context: Rc, - ) -> Result<(SqlTemplate, SqlTemplateArgs), CubeError>; } /// Neon-backed implementation of `MemberSql`. `compile_template_sql` @@ -339,462 +316,6 @@ impl NativeMemberSql { args_names, }) } - - fn property_proxy( - context_holder: NativeContextHolder, - proxy_state: ProxyStateWeak, - path: Vec, - ) -> Result, CubeError> { - context_holder.make_proxy(None, move |inner_context, _, prop| { - if prop == "sql" { - let mut path_with_sql = path.clone(); - path_with_sql.push("__sql_fn".to_string()); - let index = proxy_state.insert_symbol_path(&path_with_sql)?; - let str = SqlCallArg::dependency(index); - let result = inner_context.to_string_fn(str)?; - let result = NativeObjectHandle::new(result.into_object()); - return Ok(Some(result)); - } - if prop == "toString" || prop == "valueOf" { - let index = proxy_state.insert_symbol_path(&path)?; - let str = SqlCallArg::dependency(index); - let result = inner_context.to_string_fn(str)?; - let result = NativeObjectHandle::new(result.into_object()); - return Ok(Some(result)); - } - - let mut new_path = path.clone(); - new_path.push(prop.clone()); - let result = - Self::property_proxy(inner_context.clone(), proxy_state.clone(), new_path)?; - - Ok(Some(result)) - }) - } - - fn process_secutity_context_value( - proxy_state: &ProxyStateWeak, - value: &String, - ) -> Result { - let index = proxy_state.with_state_mut(|state| { - let i = state.security_context.values.len(); - state.security_context.values.push(value.clone()); - i - })?; - Ok(SqlCallArg::security_value(index)) - } - - /// Invokes a user-provided `column` callback (e.g. - /// `col => \`col IN (${groups.join(', ')})\``) passed to - /// `SECURITY_CONTEXT.…filter()`, passing the prepared `arg` value and - /// returning the resulting SQL string. - /// - /// Returns an empty string if the callback result cannot be converted to - /// a string, matching the pre-existing behavior. - fn invoke_filter_column_callback( - column: &::Function, - arg: NativeObjectHandle, - ) -> Result { - let result = column.call(vec![arg])?; - if let Ok(result) = result.to_string() { - Ok(result.value()?) - } else { - Ok("".to_string()) - } - } - - fn coerce_scalar_to_string( - handle: NativeObjectHandle, - ) -> Result { - if let Ok(s) = String::from_native(handle.clone()) { - return Ok(s); - } - if let Ok(n) = f64::from_native(handle.clone()) { - return Ok(n.to_string()); - } - if let Ok(b) = bool::from_native(handle.clone()) { - return Ok(b.to_string()); - } - Err(CubeError::user( - "Invalid param for security context".to_string(), - )) - } - - fn security_context_filter_fn( - context_holder: NativeContextHolder, - property_value: NativeObjectHandle, - required: bool, - proxy_state: ProxyStateWeak, - ) -> Result, CubeError> { - enum ParamValue { - String(String), - StringVec(Vec), - None, - } - // Falsy scalars (undefined / null / "" / 0 / NaN / false) collapse to - // `ParamValue::None` and emit `1 = 1`. Empty arrays stay as an empty - // `StringVec` and emit `1 = 0` separately below — `IN ()` is invalid - // SQL in most dialects. - let param_value = if property_value.is_undefined()? || property_value.is_null()? { - ParamValue::None - } else if let Ok(arr) = property_value.to_array() { - let values = arr - .to_vec()? - .into_iter() - .map(Self::coerce_scalar_to_string) - .collect::, _>>()?; - ParamValue::StringVec(values) - } else if let Ok(prop) = String::from_native(property_value.clone()) { - if prop.is_empty() { - ParamValue::None - } else { - ParamValue::String(prop) - } - } else if let Ok(prop) = f64::from_native(property_value.clone()) { - if prop == 0.0 || prop.is_nan() { - ParamValue::None - } else { - ParamValue::String(prop.to_string()) - } - } else if let Ok(prop) = bool::from_native(property_value.clone()) { - if prop { - ParamValue::String("true".to_string()) - } else { - ParamValue::None - } - } else { - return Err(CubeError::user( - "Invalid param for security context".to_string(), - )); - }; - let result = - context_holder.make_vararg_function(move |context, args| -> Result<_, CubeError> { - if args.is_empty() { - return Ok("".to_string()); - } - - let column = args[0].clone(); - - let res = match ¶m_value { - ParamValue::String(value) => { - let value = Self::process_secutity_context_value(&proxy_state, value)?; - if let Ok(column) = column.to_function() { - let native_value = value.to_native(context.clone())?; - Self::invoke_filter_column_callback(&column, native_value)? - } else if let Ok(column) = column.to_string() { - let column_value = column.value()?; - format!("{} = {}", column_value, value) - } else { - "".to_string() - } - } - ParamValue::StringVec(items) => { - // An empty array means the user passed a filter value - // but nothing matches (e.g. `groups: []`). Emitting - // `col IN ()` produces invalid SQL in Postgres and - // other dialects, so fall back to `1 = 0` (or the - // function callback variant) to make the filter - // match nothing explicitly. - if items.is_empty() { - if let Ok(column) = column.to_function() { - let empty: Vec = vec![]; - let native_values = empty.to_native(context)?; - Self::invoke_filter_column_callback(&column, native_values)? - } else { - "1 = 0".to_string() - } - } else { - let values = items - .iter() - .map(|v| Self::process_secutity_context_value(&proxy_state, &v)) - .collect::, _>>()?; - - if let Ok(column) = column.to_function() { - let native_values = values.to_native(context)?; - Self::invoke_filter_column_callback(&column, native_values)? - } else if let Ok(column) = column.to_string() { - let column_value = column.value()?; - format!("{} IN ({})", column_value, values.join(", ")) - } else { - "".to_string() - } - } - } - ParamValue::None => { - if required { - let column_name = String::from_native(column).unwrap_or_default(); - return Err(CubeError::user(format!( - "Filter for {} is required", - column_name - ))); - } - "1 = 1".to_string() - } - }; - - Ok(res) - })?; - Ok(NativeObjectHandle::new(result.into_object())) - } - - fn security_context_unsafe_value_fn( - context_holder: NativeContextHolder, - property_value: NativeObjectHandle, - ) -> Result, CubeError> { - let result = context_holder.make_vararg_function( - move |context, _| -> Result, CubeError> { - property_value.clone_to_function_context_ref(context.as_holder_ref()) - }, - )?; - Ok(NativeObjectHandle::new(result.into_object())) - } - - fn security_context_to_string_fn( - context_holder: NativeContextHolder, - property_value: NativeObjectHandle, - proxy_state: ProxyStateWeak, - ) -> Result, CubeError> { - // Type extraction is read-only and runs eagerly. Placeholder - // allocation happens lazily inside the returned function so it only - // fires when the proxy is actually coerced via `${...}` — otherwise - // every property access would register a placeholder. - let str_value: Option> = - if property_value.is_undefined()? || property_value.is_null()? { - None - } else if let Ok(arr) = property_value.to_array() { - let elements = arr.to_vec()?; - let mut values = Vec::with_capacity(elements.len()); - for el in elements { - values.push(Self::coerce_scalar_to_string(el)?); - } - Some(values) - } else if let Ok(prop) = String::from_native(property_value.clone()) { - Some(vec![prop]) - } else if let Ok(prop) = f64::from_native(property_value.clone()) { - Some(vec![prop.to_string()]) - } else if let Ok(prop) = bool::from_native(property_value.clone()) { - Some(vec![prop.to_string()]) - } else { - None - }; - let result = - context_holder.make_vararg_function(move |_, _| -> Result { - match &str_value { - Some(values) => Ok(values - .iter() - .map(|v| Self::process_secutity_context_value(&proxy_state, v)) - .collect::, _>>()? - .join(",")), - None => Ok(String::new()), - } - })?; - Ok(NativeObjectHandle::new(result.into_object())) - } - - fn security_context_proxy( - context_holder: NativeContextHolder, - proxy_state: ProxyStateWeak, - base_object: NativeObjectHandle, - ) -> Result, CubeError> { - context_holder.make_proxy(Some(base_object), move |inner_context, target, prop| { - if &prop == "filter" { - return Ok(Some(Self::security_context_filter_fn( - inner_context.clone(), - target.clone(), - false, - proxy_state.clone(), - )?)); - } - if &prop == "requiredFilter" { - return Ok(Some(Self::security_context_filter_fn( - inner_context.clone(), - target.clone(), - true, - proxy_state.clone(), - )?)); - } - if &prop == "unsafeValue" { - return Ok(Some(Self::security_context_unsafe_value_fn( - inner_context.clone(), - target.clone(), - )?)); - } - if &prop == "toString" || &prop == "valueOf" { - return Ok(Some(Self::security_context_to_string_fn( - inner_context.clone(), - target.clone(), - proxy_state.clone(), - )?)); - } - let target_obj = target.to_struct()?; - let property_value = target_obj.get_field(&prop)?; - if property_value.to_struct().is_ok() { - return Ok(Some(Self::security_context_proxy( - inner_context, - proxy_state.clone(), - property_value, - )?)); - } - Ok(Some(Self::security_context_leaf_proxy( - inner_context, - proxy_state.clone(), - property_value, - )?)) - }) - } - - /// Creates a chainable proxy for leaf (non-object) security context values. - /// The proxy target is a struct with method properties (filter, unsafeValue, - /// etc.). Unknown property access returns another chainable proxy, enabling - /// deeply nested paths like `SECURITY_CONTEXT.cubeCloud.tenantId.filter(...)`. - fn security_context_leaf_proxy( - context_holder: NativeContextHolder, - proxy_state: ProxyStateWeak, - property_value: NativeObjectHandle, - ) -> Result, CubeError> { - let result = context_holder.empty_struct()?; - result.set_field( - "filter", - Self::security_context_filter_fn( - context_holder.clone(), - property_value.clone(), - false, - proxy_state.clone(), - )?, - )?; - result.set_field( - "requiredFilter", - Self::security_context_filter_fn( - context_holder.clone(), - property_value.clone(), - true, - proxy_state.clone(), - )?, - )?; - result.set_field( - "unsafeValue", - Self::security_context_unsafe_value_fn(context_holder.clone(), property_value.clone())?, - )?; - result.set_field( - "toString", - Self::security_context_to_string_fn( - context_holder.clone(), - property_value.clone(), - proxy_state.clone(), - )?, - )?; - let methods_handle = NativeObjectHandle::new(result.into_object()); - context_holder.make_proxy(Some(methods_handle), move |inner_context, target, prop| { - if let Ok(target_obj) = target.to_struct() { - if let Ok(true) = target_obj.has_field(&prop) { - return Ok(Some(target_obj.get_field(&prop)?)); - } - } - let undef = inner_context.undefined()?; - Ok(Some(Self::security_context_leaf_proxy( - inner_context, - proxy_state.clone(), - undef, - )?)) - }) - } - - fn filter_group_fn( - context_holder: NativeContextHolder, - proxy_state: ProxyStateWeak, - ) -> Result, CubeError> { - let proxy_state = proxy_state.clone(); - let result = context_holder.make_vararg_function(move |_, args| { - let filter_params = args - .iter() - .map(|arg| -> Result<_, CubeError> { - let member = arg.to_struct()?.get_field("__member")?; - FilterParamsItem::from_native(member.clone()) - }) - .collect::, _>>() - .map_err(|_| { - CubeError::user( - "FILTER_GROUP expects FILTER_PARAMS args to be passed.".to_string(), - ) - })?; - let filter_group = FilterGroupItem { filter_params }; - let index = - proxy_state.with_state_mut(|state| state.insert_filter_group(filter_group))?; - - let str = SqlCallArg::filter_group(index); - Ok(str) - })?; - Ok(NativeObjectHandle::new(result.into_object())) - } - - fn filter_params_filter( - context_holder: NativeContextHolder, - proxy_state: ProxyStateWeak, - cube_name: String, - name: String, - column: FilterParamsColumn, - ) -> Result, CubeError> { - let item = Rc::new(FilterParamsItem { - cube_name: cube_name.clone(), - name: name.clone(), - column, - }); - let item_native = item.to_native(context_holder.clone())?; - let to_string_fn = context_holder.make_function(move |_| { - let index = proxy_state - .with_state_mut(|state| state.insert_filter_params(item.as_ref().clone()))?; - - let str = SqlCallArg::filter_param(index); - Ok(str) - })?; - let result = context_holder.empty_struct()?; - result.set_field("__member", item_native)?; - result.set_field( - "toString", - NativeObjectHandle::new(to_string_fn.into_object()), - )?; - Ok(NativeObjectHandle::new(result.into_object())) - } - - fn filter_params_cube_proxy( - context_holder: NativeContextHolder, - proxy_state: ProxyStateWeak, - cube_name: String, - ) -> Result, CubeError> { - context_holder.make_proxy(None, move |inner_context, _, prop| { - let name = prop.clone(); - let cube_name_to_move = Rc::new(cube_name.clone()); - let proxy_state = proxy_state.clone(); - let filter_func = inner_context.make_function( - move |filter_context, column: FilterParamsColumn| { - Self::filter_params_filter( - filter_context, - proxy_state.clone(), - cube_name_to_move.as_ref().clone(), - name.clone(), - column.clone(), - ) - }, - )?; - let filter_func = NativeObjectHandle::new(filter_func.into_object()); - let result_struct = inner_context.empty_struct()?; - result_struct.set_field("filter", filter_func)?; - Ok(Some(NativeObjectHandle::new(result_struct.into_object()))) - }) - } - fn filter_params_proxy( - context_holder: NativeContextHolder, - proxy_state: ProxyStateWeak, - ) -> Result, CubeError> { - context_holder.make_proxy(None, move |inner_context, _, prop| { - let cube_name = prop; - Ok(Some(Self::filter_params_cube_proxy( - inner_context, - proxy_state.clone(), - cube_name, - )?)) - }) - } } impl MemberSql for NativeMemberSql { @@ -804,62 +325,6 @@ impl MemberSql for NativeMemberSql { fn args_names(&self) -> &Vec { &self.args_names } - - fn compile_template_sql( - &self, - base_tools: Rc, - security_context: Rc, - ) -> Result<(SqlTemplate, SqlTemplateArgs), CubeError> { - let state = ProxyState::new(); - let weak_state = state.weak(); - let context_holder = NativeContextHolder::::new(self.native_object.get_context()); - let mut proxy_args = vec![]; - for arg in self.args_names.iter().cloned() { - let proxy_arg = if arg == "FILTER_PARAMS" { - Self::filter_params_proxy(context_holder.clone(), weak_state.clone())? - } else if arg == "FILTER_GROUP" { - Self::filter_group_fn(context_holder.clone(), weak_state.clone())? - } else if arg == "SECURITY_CONTEXT" - || arg == "security_context" - || arg == "securityContext" - { - let context_obj = if let Some(security_context) = security_context - .clone() - .as_any() - .downcast_ref::>( - ) { - security_context.to_native(context_holder.clone())? - } else { - return Err(CubeError::internal(format!( - "Cannot dowcast security_context to native type" - ))); - }; - Self::security_context_proxy( - context_holder.clone(), - weak_state.clone(), - context_obj, - )? - } else if arg == "SQL_UTILS" { - base_tools - .sql_utils_for_rust()? - .as_any() - .downcast::>() - .unwrap() - .to_native(context_holder.clone())? - } else { - let path = vec![arg]; - Self::property_proxy(context_holder.clone(), weak_state.clone(), path.clone())? - }; - proxy_args.push(proxy_arg); - } - let native_func = self.native_object.to_function()?; - let evaluation_result = native_func.call(proxy_args)?; - let template = SqlTemplate::from_native(evaluation_result)?; - let context_ref = context_holder.as_holder_ref(); - let sql_args = state.get_args()?.clone_to_context(context_ref)?; - - Ok((template, sql_args)) - } } impl NativeSerialize for NativeMemberSql { diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call_builder.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call_builder.rs index cc27a56af3fee..67fe0133df560 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call_builder.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call_builder.rs @@ -41,8 +41,12 @@ impl<'a> SqlCallBuilder<'a> { cube_name: &String, member_sql: Rc, ) -> Result { - let (template, template_args) = member_sql - .compile_template_sql(self.base_tools.clone(), self.security_context.clone())?; + let compiled = self.base_tools.compile_member_sql( + member_sql.clone(), + self.security_context.clone(), + member_sql.args_names().clone(), + )?; + let (template, template_args) = (compiled.template, compiled.args); let deps = template_args .symbol_paths diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_base_tools.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_base_tools.rs index 4140d81bf1a06..36fa441a71614 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_base_tools.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_base_tools.rs @@ -2,12 +2,14 @@ use crate::cube_bridge::base_tools::BaseTools; use crate::cube_bridge::driver_tools::DriverTools; use crate::cube_bridge::join_definition::JoinDefinition; use crate::cube_bridge::join_hints::JoinHintItem; +use crate::cube_bridge::member_sql::{CompiledMemberTemplate, MemberSql}; use crate::cube_bridge::pre_aggregation_obj::PreAggregationObj; +use crate::cube_bridge::security_context::SecurityContext; use crate::cube_bridge::sql_templates_render::SqlTemplatesRender; use crate::cube_bridge::sql_utils::SqlUtils; use crate::planner::sql_templates::PlanSqlTemplates; use crate::test_fixtures::cube_bridge::{ - MockDriverTools, MockJoinGraph, MockSqlTemplatesRender, MockSqlUtils, + MockDriverTools, MockJoinGraph, MockMemberSql, MockSqlTemplatesRender, MockSqlUtils, }; use cubenativeutils::CubeError; use std::any::Any; @@ -104,4 +106,17 @@ impl BaseTools for MockBaseTools { let result = self.join_graph.build_join(hints)?; Ok(result as Rc) } + + fn compile_member_sql( + &self, + member_sql: Rc, + _security_context: Rc, + _arg_names: Vec, + ) -> Result { + let mock = member_sql + .as_any() + .downcast::() + .map_err(|_| CubeError::internal("MockBaseTools expects MockMemberSql".to_string()))?; + Ok(mock.compiled()) + } } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_evaluator.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_evaluator.rs index 6629edc45ad8e..ffd4d8e1e3fcd 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_evaluator.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_evaluator.rs @@ -8,7 +8,7 @@ use crate::cube_bridge::pre_aggregation_description::PreAggregationDescription; use crate::cube_bridge::segment_definition::SegmentDefinition; use crate::impl_static_data; use crate::test_fixtures::cube_bridge::mock_schema::MockSchema; -use crate::test_fixtures::cube_bridge::{MockBaseTools, MockJoinGraph, MockSecurityContext}; +use crate::test_fixtures::cube_bridge::{mock_compiled, MockJoinGraph}; use cubenativeutils::CubeError; use std::any::Any; use std::collections::HashMap; @@ -297,10 +297,7 @@ impl CubeEvaluator for MockCubeEvaluator { // Simple implementation for mock: extract symbol paths from compiled template // For YAML schemas, rollups are already provided as strings like "visitors.for_join" // which MockMemberSql parses into symbol_paths like [["visitors", "for_join"]] - let (_template, args) = sql.compile_template_sql( - Rc::new(MockBaseTools::default()), - Rc::new(MockSecurityContext), - )?; + let (_template, args) = mock_compiled(sql); // Convert symbol paths back to dot-separated strings Ok(args 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 f6c500cf832dd..95fb8fb92a975 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 @@ -1,10 +1,20 @@ -use crate::cube_bridge::base_tools::BaseTools; -use crate::cube_bridge::member_sql::{MemberSql, SqlTemplate, SqlTemplateArgs}; -use crate::cube_bridge::security_context::SecurityContext; +use crate::cube_bridge::member_sql::{ + CompiledMemberTemplate, MemberSql, SqlTemplate, SqlTemplateArgs, +}; use cubenativeutils::CubeError; use std::any::Any; use std::rc::Rc; +/// Test helper: extract the compiled `(template, args)` from a mock member sql. +pub fn mock_compiled(sql: Rc) -> (SqlTemplate, SqlTemplateArgs) { + let c = sql + .as_any() + .downcast::() + .expect("expected MockMemberSql") + .compiled(); + (c.template, c.args) +} + /// Mock implementation of MemberSql for testing /// Parses template strings like "{CUBE.field} / {other_cube.field} + {revenue}" /// and converts them to "{arg:0} / {arg:1} + {arg:2}" with extracted symbol paths @@ -164,6 +174,15 @@ impl MockMemberSql { } } +impl MockMemberSql { + pub fn compiled(&self) -> CompiledMemberTemplate { + CompiledMemberTemplate { + template: self.template.clone(), + args: self.args.clone(), + } + } +} + impl MemberSql for MockMemberSql { fn args_names(&self) -> &Vec { &self.args_names @@ -172,14 +191,6 @@ impl MemberSql for MockMemberSql { fn as_any(self: Rc) -> Rc { self } - - fn compile_template_sql( - &self, - _base_tools: Rc, - _security_context: Rc, - ) -> Result<(SqlTemplate, SqlTemplateArgs), CubeError> { - Ok((self.template.clone(), self.args.clone())) - } } #[cfg(test)] @@ -314,12 +325,7 @@ mod tests { #[test] fn test_compile_template_sql() { let mock = Rc::new(MockMemberSql::new("{CUBE.field} / {other.field}").unwrap()); - let (template, args) = mock - .compile_template_sql( - Rc::new(crate::test_fixtures::cube_bridge::MockBaseTools::default()), - Rc::new(crate::test_fixtures::cube_bridge::MockSecurityContext), - ) - .unwrap(); + let (template, args) = mock_compiled(mock); match template { SqlTemplate::String(s) => { @@ -425,12 +431,7 @@ mod tests { MockMemberSql::pre_agg_array_refs(vec!["orders.status", "line_items.product_id"]) .unwrap(); - let (template, args) = mock - .compile_template_sql( - Rc::new(crate::test_fixtures::cube_bridge::MockBaseTools::default()), - Rc::new(crate::test_fixtures::cube_bridge::MockSecurityContext), - ) - .unwrap(); + let (template, args) = mock_compiled(mock); match template { SqlTemplate::StringVec(vec) => { diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_schema.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_schema.rs index d79f3e80ff148..e8622eb637682 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_schema.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_schema.rs @@ -664,7 +664,8 @@ mod tests { use crate::cube_bridge::dimension_definition::DimensionDefinition; use crate::cube_bridge::measure_definition::MeasureDefinition; use crate::cube_bridge::segment_definition::SegmentDefinition; - use crate::test_fixtures::cube_bridge::MockBaseTools; + + use crate::test_fixtures::cube_bridge::mock_compiled; #[test] fn test_basic_schema() { @@ -1201,9 +1202,6 @@ mod tests { #[test] fn test_view_with_multiple_long_join_paths() { - use crate::test_fixtures::cube_bridge::MockSecurityContext; - use std::rc::Rc; - let schema = MockSchemaBuilder::new() .add_cube("visitors") .add_dimension( @@ -1260,12 +1258,7 @@ mod tests { let checkin_id_sql = checkin_id_dim.sql().unwrap().unwrap(); // Compile template and check symbol_paths structure - let (_template, args) = checkin_id_sql - .compile_template_sql( - Rc::new(MockBaseTools::default()), - Rc::new(MockSecurityContext), - ) - .unwrap(); + let (_template, args) = mock_compiled(checkin_id_sql); // Should have exactly one symbol path assert_eq!( @@ -1286,12 +1279,7 @@ mod tests { .unwrap(); let checkin_count_sql = checkin_count_measure.sql().unwrap().unwrap(); - let (_template, args) = checkin_count_sql - .compile_template_sql( - Rc::new(MockBaseTools::default()), - Rc::new(MockSecurityContext), - ) - .unwrap(); + let (_template, args) = mock_compiled(checkin_count_sql); assert_eq!( args.symbol_paths.len(), @@ -1309,12 +1297,7 @@ mod tests { let id_dim = schema.get_dimension("multi_path_view", "id").unwrap(); let id_sql = id_dim.sql().unwrap().unwrap(); - let (_template, args) = id_sql - .compile_template_sql( - Rc::new(MockBaseTools::default()), - Rc::new(MockSecurityContext), - ) - .unwrap(); + let (_template, args) = mock_compiled(id_sql); assert_eq!( args.symbol_paths.len(), @@ -1330,12 +1313,7 @@ mod tests { let count_measure = schema.get_measure("multi_path_view", "count").unwrap(); let count_sql = count_measure.sql().unwrap().unwrap(); - let (_template, args) = count_sql - .compile_template_sql( - Rc::new(MockBaseTools::default()), - Rc::new(MockSecurityContext), - ) - .unwrap(); + let (_template, args) = mock_compiled(count_sql); assert_eq!( args.symbol_paths.len(), diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mod.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mod.rs index e3329aab14ee3..d201c77b6395f 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mod.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mod.rs @@ -62,7 +62,7 @@ pub use mock_join_item_definition::MockJoinItemDefinition; pub use mock_measure_definition::MockMeasureDefinition; pub use mock_member_expression_definition::MockMemberExpressionDefinition; pub use mock_member_order_by::MockMemberOrderBy; -pub use mock_member_sql::MockMemberSql; +pub use mock_member_sql::{mock_compiled, MockMemberSql}; pub use mock_multi_stage_filter::MockMultiStageFilterReferences; pub use mock_multi_stage_grain::MockMultiStageGrainReferences; pub use mock_pre_aggregation_description::MockPreAggregationDescription; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/yaml/schema.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/yaml/schema.rs index b7d92ad94ef96..994f2dc6fc40a 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/yaml/schema.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/yaml/schema.rs @@ -210,9 +210,8 @@ mod tests { use crate::cube_bridge::measure_definition::MeasureDefinition; use crate::cube_bridge::member_sql::SqlTemplate; use crate::cube_bridge::pre_aggregation_description::PreAggregationDescription; - use crate::test_fixtures::cube_bridge::{MockBaseTools, MockSecurityContext}; + use crate::test_fixtures::cube_bridge::mock_compiled; use indoc::indoc; - use std::rc::Rc; #[test] fn test_parse_basic_cube() { @@ -550,18 +549,9 @@ mod tests { assert_eq!(dim_refs.args_names(), &vec!["orders"]); assert_eq!(time_dim_ref.args_names(), &vec!["orders"]); - let base_tools = Rc::new(MockBaseTools::default()); - let sec_ctx = Rc::new(MockSecurityContext); - - let (measure_template, measure_args) = measure_refs - .compile_template_sql(base_tools.clone(), sec_ctx.clone()) - .unwrap(); - let (dim_template, dim_args) = dim_refs - .compile_template_sql(base_tools.clone(), sec_ctx.clone()) - .unwrap(); - let (time_template, time_args) = time_dim_ref - .compile_template_sql(base_tools.clone(), sec_ctx.clone()) - .unwrap(); + let (measure_template, measure_args) = mock_compiled(measure_refs); + let (dim_template, dim_args) = mock_compiled(dim_refs); + let (time_template, time_args) = mock_compiled(time_dim_ref); match measure_template { SqlTemplate::StringVec(vec) => { @@ -651,12 +641,7 @@ mod tests { assert_eq!(dim_refs.args_names(), &vec!["orders", "line_items"]); assert_eq!(time_dim_ref.args_names(), &vec!["orders"]); - let base_tools = Rc::new(MockBaseTools::default()); - let sec_ctx = Rc::new(MockSecurityContext); - - let (dim_template, dim_args) = dim_refs - .compile_template_sql(base_tools.clone(), sec_ctx.clone()) - .unwrap(); + let (dim_template, dim_args) = mock_compiled(dim_refs); match dim_template { SqlTemplate::StringVec(vec) => { From 8c5879403b5b27034ca2f800af37e778b9fa6343 Mon Sep 17 00:00:00 2001 From: Aleksandr Romanenko Date: Thu, 18 Jun 2026 16:02:48 +0200 Subject: [PATCH 3/7] fix(tesseract): use const in member-sql compiler test to satisfy lint --- .../test/unit/member-sql-template-compiler.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/cubejs-schema-compiler/test/unit/member-sql-template-compiler.test.ts b/packages/cubejs-schema-compiler/test/unit/member-sql-template-compiler.test.ts index ab7eb8165247a..b2c9c6cdc4d29 100644 --- a/packages/cubejs-schema-compiler/test/unit/member-sql-template-compiler.test.ts +++ b/packages/cubejs-schema-compiler/test/unit/member-sql-template-compiler.test.ts @@ -57,7 +57,6 @@ describe('MemberSqlTemplateCompiler — FILTER_PARAMS / FILTER_GROUP', () => { }); it('keeps the column callback as a function (deferred) and records {fp:N}', () => { - let captured; const res = compileMemberSql( (FILTER_PARAMS) => `${FILTER_PARAMS.orders.status.filter((c) => `${c} > 0`)}`, ['FILTER_PARAMS'] @@ -67,7 +66,7 @@ describe('MemberSqlTemplateCompiler — FILTER_PARAMS / FILTER_GROUP', () => { expect(res.filterParams[0].cube_name).toBe('orders'); expect(res.filterParams[0].name).toBe('status'); expect(typeof res.filterParams[0].column).toBe('function'); - captured = res.filterParams[0].column; + const captured = res.filterParams[0].column; expect(captured('X')).toBe('X > 0'); }); From 98b3aaede5d7a3f5312e7898dd38e8fab5ce5ff1 Mon Sep 17 00:00:00 2001 From: Aleksandr Romanenko Date: Thu, 18 Jun 2026 16:44:21 +0200 Subject: [PATCH 4/7] fix(tesseract): return empty SQL for null/undefined member sql result --- .../src/adapter/MemberSqlTemplateCompiler.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/cubejs-schema-compiler/src/adapter/MemberSqlTemplateCompiler.js b/packages/cubejs-schema-compiler/src/adapter/MemberSqlTemplateCompiler.js index 2fdeb7573ed86..09e5c5fcb5554 100644 --- a/packages/cubejs-schema-compiler/src/adapter/MemberSqlTemplateCompiler.js +++ b/packages/cubejs-schema-compiler/src/adapter/MemberSqlTemplateCompiler.js @@ -234,6 +234,9 @@ function parseTemplateResult(result) { if (Array.isArray(result)) { return result.map(r => `${r}`); } + if (result === null || result === undefined) { + return ''; + } return `${result}`; } From 48f83e3ab482c03ad4b0617d64fce34004d1cb0e Mon Sep 17 00:00:00 2001 From: Aleksandr Romanenko Date: Thu, 18 Jun 2026 16:44:29 +0200 Subject: [PATCH 5/7] test(tesseract): cover member-sql classes in JS, drop obsolete Rust bridge tests --- .../src/bridge_test_exports.rs | 188 +------------ .../test/bridge/bridge-fixtures.ts | 7 + .../test/bridge/filter-group.test.ts | 87 ------ .../test/bridge/filter-params.test.ts | 74 ----- .../test/bridge/helpers.ts | 37 +-- .../test/bridge/multi-arg.test.ts | 32 --- .../bridge/object-bridges-coverage.test.ts | 1 + .../test/bridge/result-shape.test.ts | 51 ---- .../test/bridge/security-context.test.ts | 262 ------------------ .../test/bridge/symbol-paths.test.ts | 176 ------------ .../unit/member-sql-template-compiler.test.ts | 237 ++++++++++++++++ 11 files changed, 254 insertions(+), 898 deletions(-) delete mode 100644 packages/cubejs-backend-native/test/bridge/filter-group.test.ts delete mode 100644 packages/cubejs-backend-native/test/bridge/filter-params.test.ts delete mode 100644 packages/cubejs-backend-native/test/bridge/multi-arg.test.ts delete mode 100644 packages/cubejs-backend-native/test/bridge/result-shape.test.ts delete mode 100644 packages/cubejs-backend-native/test/bridge/security-context.test.ts delete mode 100644 packages/cubejs-backend-native/test/bridge/symbol-paths.test.ts diff --git a/packages/cubejs-backend-native/src/bridge_test_exports.rs b/packages/cubejs-backend-native/src/bridge_test_exports.rs index 8760e8fd7766e..44fe1ab5a6eef 100644 --- a/packages/cubejs-backend-native/src/bridge_test_exports.rs +++ b/packages/cubejs-backend-native/src/bridge_test_exports.rs @@ -1,13 +1,10 @@ //! Test endpoints for the Tesseract bridge layer. //! //! These functions are exported on the native module under names prefixed with -//! `__testBridge` (e.g. `__testBridgeCompileMemberSql`). They drive real V8 +//! `__testBridge` (e.g. `__testBridgeParseArgsNames`). They drive real V8 //! through the production bridge code so //! that bridge logic can be regression-tested at the unit level rather than //! only via end-to-end JS planner tests. -//! -//! Stub implementations for trait dependencies (e.g. `BaseTools`) live in this -//! module; they should fail loudly when an unsupported code path is exercised. use cubenativeutils::wrappers::bridge_meta::{BridgeFieldKind, BridgeFieldMeta}; use cubenativeutils::wrappers::neon::neon_guarded_funcion_call; @@ -49,7 +46,6 @@ use cubesqlplanner::cube_bridge::{ }, join_definition::{join_definition_bridge_fields_meta, JoinDefinition, NativeJoinDefinition}, join_graph::{join_graph_bridge_fields_meta, JoinGraph, NativeJoinGraph}, - join_hints::JoinHintItem, join_item::{join_item_bridge_fields_meta, JoinItem, NativeJoinItem}, join_item_definition::{ join_item_definition_bridge_fields_meta, JoinItemDefinition, NativeJoinItemDefinition, @@ -66,9 +62,6 @@ use cubesqlplanner::cube_bridge::{ NativeMemberExpressionDefinition, }, member_order_by::{member_order_by_bridge_fields_meta, MemberOrderBy, NativeMemberOrderBy}, - member_sql::{ - FilterGroupItem, FilterParamsItem, MemberSql, NativeMemberSql, SqlTemplate, SqlTemplateArgs, - }, multi_stage_filter::{ multi_stage_filter_references_bridge_fields_meta, NativeMultiStageFilterReferences, }, @@ -79,21 +72,16 @@ use cubesqlplanner::cube_bridge::{ pre_aggregation_description_bridge_fields_meta, NativePreAggregationDescription, PreAggregationDescription, }, - pre_aggregation_obj::{ - pre_aggregation_obj_bridge_fields_meta, NativePreAggregationObj, PreAggregationObj, - }, + pre_aggregation_obj::{pre_aggregation_obj_bridge_fields_meta, NativePreAggregationObj}, pre_aggregation_time_dimension::{ pre_aggregation_time_dimension_bridge_fields_meta, NativePreAggregationTimeDimension, PreAggregationTimeDimension, }, - security_context::{ - security_context_bridge_fields_meta, NativeSecurityContext, SecurityContext, - }, + security_context::{security_context_bridge_fields_meta, NativeSecurityContext}, segment_definition::{ segment_definition_bridge_fields_meta, NativeSegmentDefinition, SegmentDefinition, }, - sql_templates_render::SqlTemplatesRender, - sql_utils::{sql_utils_bridge_fields_meta, NativeSqlUtils, SqlUtils}, + sql_utils::{sql_utils_bridge_fields_meta, NativeSqlUtils}, struct_with_sql_member::{ struct_with_sql_member_bridge_fields_meta, NativeStructWithSqlMember, StructWithSqlMember, }, @@ -105,9 +93,7 @@ use cubesqlplanner::cube_bridge::{ }, }; use neon::prelude::*; -use std::any::Any; use std::collections::HashSet; -use std::rc::Rc; enum InvokeStatus { Ok, @@ -169,167 +155,6 @@ impl InvokeResult { } } -struct StubBaseTools; - -fn stub_err(method: &str) -> CubeError { - CubeError::internal(format!( - "StubBaseTools::{} called from bridge test harness — \ - this test path requires a real BaseTools implementation", - method - )) -} - -impl BaseTools for StubBaseTools { - fn as_any(self: Rc) -> Rc { - self - } - fn driver_tools(&self, _external: bool) -> Result, CubeError> { - Err(stub_err("driver_tools")) - } - fn sql_templates(&self) -> Result, CubeError> { - Err(stub_err("sql_templates")) - } - fn sql_utils_for_rust(&self) -> Result, CubeError> { - Err(stub_err("sql_utils_for_rust")) - } - fn get_allocated_params(&self) -> Result, CubeError> { - Err(stub_err("get_allocated_params")) - } - fn all_cube_members(&self, _path: String) -> Result, CubeError> { - Err(stub_err("all_cube_members")) - } - fn interval_and_minimal_time_unit(&self, _interval: String) -> Result, CubeError> { - Err(stub_err("interval_and_minimal_time_unit")) - } - fn get_pre_aggregation_by_name( - &self, - _cube_name: String, - _name: String, - ) -> Result, CubeError> { - Err(stub_err("get_pre_aggregation_by_name")) - } - fn pre_aggregation_table_name( - &self, - _cube_name: String, - _name: String, - ) -> Result { - Err(stub_err("pre_aggregation_table_name")) - } - fn join_tree_for_hints( - &self, - _hints: Vec, - ) -> Result, CubeError> { - Err(stub_err("join_tree_for_hints")) - } -} - -fn handles_to_array( - items: Vec>, - context: NativeContextHolder, -) -> Result, CubeError> { - let arr = context.empty_array()?; - for (i, item) in items.into_iter().enumerate() { - arr.set(i as u32, item)?; - } - Ok(NativeObjectHandle::new(arr.into_object())) -} - -fn template_to_native( - template: &SqlTemplate, - context: NativeContextHolder, -) -> Result, CubeError> { - match template { - SqlTemplate::String(s) => s.to_native(context), - SqlTemplate::StringVec(strings) => strings.to_native(context), - } -} - -fn filter_params_to_native( - items: &[FilterParamsItem], - context: NativeContextHolder, -) -> Result, CubeError> { - let serialized = items - .iter() - .map(|itm| itm.to_native(context.clone())) - .collect::, _>>()?; - handles_to_array(serialized, context) -} - -fn filter_group_to_native( - group: &FilterGroupItem, - context: NativeContextHolder, -) -> Result, CubeError> { - let result = context.empty_struct()?; - result.set_field( - "filter_params", - filter_params_to_native(&group.filter_params, context.clone())?, - )?; - Ok(NativeObjectHandle::new(result.into_object())) -} - -fn args_to_native( - args: &SqlTemplateArgs, - context: NativeContextHolder, -) -> Result, CubeError> { - let result = context.empty_struct()?; - result.set_field( - "symbol_paths", - args.symbol_paths.to_native(context.clone())?, - )?; - result.set_field( - "filter_params", - filter_params_to_native(&args.filter_params, context.clone())?, - )?; - let groups = args - .filter_groups - .iter() - .map(|g| filter_group_to_native(g, context.clone())) - .collect::, _>>()?; - result.set_field("filter_groups", handles_to_array(groups, context.clone())?)?; - let security_context = context.empty_struct()?; - security_context.set_field( - "values", - args.security_context.values.to_native(context.clone())?, - )?; - result.set_field( - "security_context", - NativeObjectHandle::new(security_context.into_object()), - )?; - Ok(NativeObjectHandle::new(result.into_object())) -} - -fn compile_member_sql_inner( - context_holder: NativeContextHolder, - js_fn: NativeObjectHandle, - security_context_obj: NativeObjectHandle, -) -> Result, CubeError> { - let member_sql = NativeMemberSql::try_new(js_fn)?; - let security_context: Rc = - Rc::new(NativeSecurityContext::try_new(security_context_obj)?); - let base_tools: Rc = Rc::new(StubBaseTools); - - let (template, args) = member_sql.compile_template_sql(base_tools, security_context)?; - - let result = context_holder.empty_struct()?; - result.set_field( - "template", - template_to_native(&template, context_holder.clone())?, - )?; - result.set_field("args", args_to_native(&args, context_holder.clone())?)?; - Ok(NativeObjectHandle::new(result.into_object())) -} - -fn compile_member_sql(cx: FunctionContext) -> JsResult { - neon_guarded_funcion_call( - cx, - |context_holder: NativeContextHolder<_>, - js_fn: NativeObjectHandle<_>, - security_context_obj: NativeObjectHandle<_>| { - compile_member_sql_inner(context_holder, js_fn, security_context_obj) - }, - ) -} - fn parse_args_names_inner( context_holder: NativeContextHolder, js_fn: NativeObjectHandle, @@ -824,6 +649,10 @@ fn invoke_base_tools(b: &NativeBaseTools) -> InvokeResult { b.pre_aggregation_table_name("Orders".to_string(), "main".to_string()), ); r.record("join_tree_for_hints", b.join_tree_for_hints(vec![])); + r.skip( + "compile_member_sql", + "Rc argument has no auto-default in Rust", + ); r } @@ -969,7 +798,6 @@ fn rust_box_unwrap(cx: FunctionContext) -> JsResult { } pub fn register_module(cx: &mut ModuleContext) -> NeonResult<()> { - cx.export_function("__testBridgeCompileMemberSql", compile_member_sql)?; cx.export_function("__testBridgeParseArgsNames", parse_args_names)?; cx.export_function( "__testBridgeInvokeFilterParamsCallback", diff --git a/packages/cubejs-backend-native/test/bridge/bridge-fixtures.ts b/packages/cubejs-backend-native/test/bridge/bridge-fixtures.ts index b4d3e0d30deb5..3828f518d4186 100644 --- a/packages/cubejs-backend-native/test/bridge/bridge-fixtures.ts +++ b/packages/cubejs-backend-native/test/bridge/bridge-fixtures.ts @@ -248,6 +248,13 @@ export const baseToolsFixture = (): unknown => ({ getPreAggregationByName: () => preAggregationObjFixture(), preAggregationTableName: () => 'pre_aggr_table', joinTreeForHints: () => joinDefinitionFixture(), + compileMemberSql: () => ({ + template: '', + symbolPaths: [], + filterParams: [], + filterGroups: [], + securityContextValues: [], + }), }); export const baseQueryOptionsFixture = (): unknown => ({ diff --git a/packages/cubejs-backend-native/test/bridge/filter-group.test.ts b/packages/cubejs-backend-native/test/bridge/filter-group.test.ts deleted file mode 100644 index f59ed09221961..0000000000000 --- a/packages/cubejs-backend-native/test/bridge/filter-group.test.ts +++ /dev/null @@ -1,87 +0,0 @@ -import { bridgeHarnessAvailable, compileMemberSql } from './helpers'; - -const describeBridge = bridgeHarnessAvailable ? describe : describe.skip; - -describeBridge('bridge: FILTER_GROUP', () => { - it('wraps a single FILTER_PARAMS arg into a group', () => { - const result = compileMemberSql( - (FILTER_PARAMS: any, FILTER_GROUP: any) => FILTER_GROUP(FILTER_PARAMS.orders.status.filter('col')) - ); - - expect(result.template).toBe('{fg:0}'); - expect(result.args.filter_groups).toHaveLength(1); - expect(result.args.filter_groups[0].filter_params).toHaveLength(1); - expect(result.args.filter_groups[0].filter_params[0]).toEqual({ - cube_name: 'orders', - name: 'status', - column: 'col', - }); - // Filter params used only inside FILTER_GROUP are not promoted to the - // top-level filter_params list. - expect(result.args.filter_params).toEqual([]); - }); - - it('preserves member identity across multiple grouped FILTER_PARAMS', () => { - const result = compileMemberSql( - (FILTER_PARAMS: any, FILTER_GROUP: any) => FILTER_GROUP( - FILTER_PARAMS.orders.status.filter('a'), - FILTER_PARAMS.users.tier.filter('b') - ) - ); - - expect(result.template).toBe('{fg:0}'); - expect(result.args.filter_groups[0].filter_params).toEqual([ - { cube_name: 'orders', name: 'status', column: 'a' }, - { cube_name: 'users', name: 'tier', column: 'b' }, - ]); - }); - - it('coexists with a top-level FILTER_PARAMS reference in the same template', () => { - const result = compileMemberSql( - (FILTER_PARAMS: any, FILTER_GROUP: any) => `${FILTER_PARAMS.orders.region.filter('r')} AND ${FILTER_GROUP( - FILTER_PARAMS.orders.status.filter('s') - )}` - ); - - expect(result.template).toBe('{fp:0} AND {fg:0}'); - expect(result.args.filter_params).toHaveLength(1); - expect(result.args.filter_params[0]).toEqual({ - cube_name: 'orders', - name: 'region', - column: 'r', - }); - expect(result.args.filter_groups[0].filter_params).toEqual([ - { cube_name: 'orders', name: 'status', column: 's' }, - ]); - }); - - it('throws a user error when FILTER_GROUP receives a non-FILTER_PARAMS arg', () => { - expect(() => compileMemberSql((FILTER_GROUP: any) => FILTER_GROUP('not a filter'))).toThrow(/FILTER_GROUP expects FILTER_PARAMS args to be passed/); - }); - - it('produces an empty group with no filter_params when FILTER_GROUP() is called with no args', () => { - const result = compileMemberSql((FILTER_GROUP: any) => FILTER_GROUP()); - - expect(result.template).toBe('{fg:0}'); - expect(result.args.filter_groups).toHaveLength(1); - expect(result.args.filter_groups[0].filter_params).toEqual([]); - }); - - it('does NOT dedup the same FILTER_PARAMS across two FILTER_GROUP() calls', () => { - // Mirrors filter-params anti-dedup: each FILTER_GROUP gets its own copy - // of the items via push, no UniqueVector. - const result = compileMemberSql( - (FILTER_PARAMS: any, FILTER_GROUP: any) => `${FILTER_GROUP(FILTER_PARAMS.orders.status.filter('s'))} OR ` + - `${FILTER_GROUP(FILTER_PARAMS.orders.status.filter('s'))}` - ); - - expect(result.template).toBe('{fg:0} OR {fg:1}'); - expect(result.args.filter_groups).toHaveLength(2); - expect(result.args.filter_groups[0].filter_params).toEqual([ - { cube_name: 'orders', name: 'status', column: 's' }, - ]); - expect(result.args.filter_groups[1].filter_params).toEqual([ - { cube_name: 'orders', name: 'status', column: 's' }, - ]); - }); -}); diff --git a/packages/cubejs-backend-native/test/bridge/filter-params.test.ts b/packages/cubejs-backend-native/test/bridge/filter-params.test.ts deleted file mode 100644 index 059fba2de6498..0000000000000 --- a/packages/cubejs-backend-native/test/bridge/filter-params.test.ts +++ /dev/null @@ -1,74 +0,0 @@ -import { bridgeHarnessAvailable, compileMemberSql } from './helpers'; - -const describeBridge = bridgeHarnessAvailable ? describe : describe.skip; - -describeBridge('bridge: FILTER_PARAMS', () => { - it('records cube_name, name, and string column for a single filter', () => { - const result = compileMemberSql( - (FILTER_PARAMS: any) => FILTER_PARAMS.orders.status.filter('col') - ); - - expect(result.template).toBe('{fp:0}'); - expect(result.args.filter_params).toHaveLength(1); - expect(result.args.filter_params[0]).toEqual({ - cube_name: 'orders', - name: 'status', - column: 'col', - }); - }); - - it('exposes a callback column as a JS function and forwards the prepared arg to it', () => { - const result = compileMemberSql( - (FILTER_PARAMS: any) => FILTER_PARAMS.orders.status.filter((c: string) => `${c} IN (1,2)`) - ); - - expect(result.template).toBe('{fp:0}'); - expect(result.args.filter_params).toHaveLength(1); - - const fp = result.args.filter_params[0]; - expect(fp.cube_name).toBe('orders'); - expect(fp.name).toBe('status'); - expect(typeof fp.column).toBe('function'); - expect((fp.column as Function)('orders.status')).toBe( - 'orders.status IN (1,2)' - ); - }); - - it('records distinct filter params for different cubes/members without dedup', () => { - const result = compileMemberSql( - (FILTER_PARAMS: any) => `${FILTER_PARAMS.orders.status.filter('a')} AND ` + - `${FILTER_PARAMS.orders.region.filter('b')} AND ` + - `${FILTER_PARAMS.users.tier.filter('c')}` - ); - - expect(result.template).toBe('{fp:0} AND {fp:1} AND {fp:2}'); - expect(result.args.filter_params).toEqual([ - { cube_name: 'orders', name: 'status', column: 'a' }, - { cube_name: 'orders', name: 'region', column: 'b' }, - { cube_name: 'users', name: 'tier', column: 'c' }, - ]); - }); - - it('does NOT dedup identical filter params — each .filter() call adds a new entry', () => { - // Capturing this on purpose: filter_params uses Vec::push, not - // unique_insert, unlike symbol_paths. If this changes, the test will - // fail and force a deliberate decision. - const result = compileMemberSql( - (FILTER_PARAMS: any) => `${FILTER_PARAMS.orders.status.filter('col')} OR ` + - `${FILTER_PARAMS.orders.status.filter('col')}` - ); - - expect(result.template).toBe('{fp:0} OR {fp:1}'); - expect(result.args.filter_params).toHaveLength(2); - expect(result.args.filter_params[0]).toEqual({ - cube_name: 'orders', - name: 'status', - column: 'col', - }); - expect(result.args.filter_params[1]).toEqual({ - cube_name: 'orders', - name: 'status', - column: 'col', - }); - }); -}); diff --git a/packages/cubejs-backend-native/test/bridge/helpers.ts b/packages/cubejs-backend-native/test/bridge/helpers.ts index 17b4969cd032c..f5f743e14ae19 100644 --- a/packages/cubejs-backend-native/test/bridge/helpers.ts +++ b/packages/cubejs-backend-native/test/bridge/helpers.ts @@ -7,45 +7,10 @@ const native = loadNative(); // Test suites should gate themselves on this flag rather than calling the // helpers blindly, so a regular debug build doesn't blow up the unit run. export const bridgeHarnessAvailable: boolean = - typeof native.__testBridgeCompileMemberSql === 'function'; + typeof native.__testBridgeParseArgsNames === 'function'; export type SqlTemplate = string | string[]; -export interface FilterParamsItem { - cube_name: string; - name: string; - // String column → string. Callback column → JS function (call to inspect). - column: string | Function; -} - -export interface FilterGroupItem { - filter_params: FilterParamsItem[]; -} - -export interface SqlTemplateArgs { - symbol_paths: string[][]; - filter_params: FilterParamsItem[]; - filter_groups: FilterGroupItem[]; - security_context: { values: string[] }; -} - -export interface CompiledMemberSql { - template: SqlTemplate; - args: SqlTemplateArgs; -} - -export function compileMemberSql( - fn: Function, - securityContext: object = {} -): CompiledMemberSql { - if (!bridgeHarnessAvailable) { - throw new Error( - 'Bridge test harness is not built. Rebuild with `yarn native:build-debug-bridge-tests`.' - ); - } - return native.__testBridgeCompileMemberSql(fn, securityContext); -} - export function parseArgsNames(fn: Function): string[] { return native.__testBridgeParseArgsNames(fn); } diff --git a/packages/cubejs-backend-native/test/bridge/multi-arg.test.ts b/packages/cubejs-backend-native/test/bridge/multi-arg.test.ts deleted file mode 100644 index 22e63e4fbfc39..0000000000000 --- a/packages/cubejs-backend-native/test/bridge/multi-arg.test.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { bridgeHarnessAvailable, compileMemberSql } from './helpers'; - -const describeBridge = bridgeHarnessAvailable ? describe : describe.skip; - -describeBridge('bridge: multi-arg dispatch', () => { - it('wires CUBE, FILTER_PARAMS, and SECURITY_CONTEXT independently in one function', () => { - const result = compileMemberSql( - (CUBE: any, FILTER_PARAMS: any, SECURITY_CONTEXT: any) => `SUM(${CUBE.amount}) WHERE ` + - `${FILTER_PARAMS.orders.status.filter('col')} AND ` + - `${SECURITY_CONTEXT.tenant.filter('t')}`, - { tenant: 'acme' } - ); - - expect(result.template).toBe( - 'SUM({arg:0}) WHERE {fp:0} AND t = {sv:0}' - ); - expect(result.args.symbol_paths).toEqual([['CUBE', 'amount']]); - expect(result.args.filter_params).toEqual([ - { cube_name: 'orders', name: 'status', column: 'col' }, - ]); - expect(result.args.security_context.values).toEqual(['acme']); - }); - - it('throws an internal error from the StubBaseTools when SQL_UTILS is referenced', () => { - // The bridge harness deliberately does not provide a real BaseTools. - // Tests that exercise SQL_UTILS need a richer stub; this test pins - // current behavior so we notice if/when we add one. - expect(() => compileMemberSql( - (SQL_UTILS: any) => `${SQL_UTILS.someHelper()}` - )).toThrow(/StubBaseTools::sql_utils_for_rust/); - }); -}); diff --git a/packages/cubejs-backend-native/test/bridge/object-bridges-coverage.test.ts b/packages/cubejs-backend-native/test/bridge/object-bridges-coverage.test.ts index 8824b465fcbf7..ea902c5658809 100644 --- a/packages/cubejs-backend-native/test/bridge/object-bridges-coverage.test.ts +++ b/packages/cubejs-backend-native/test/bridge/object-bridges-coverage.test.ts @@ -67,6 +67,7 @@ const BRIDGES: BridgeSpec[] = [ name: 'baseTools', expected: [ 'all_cube_members', + 'compile_member_sql', 'driver_tools', 'get_allocated_params', 'get_pre_aggregation_by_name', diff --git a/packages/cubejs-backend-native/test/bridge/result-shape.test.ts b/packages/cubejs-backend-native/test/bridge/result-shape.test.ts deleted file mode 100644 index 38bdcab7b1418..0000000000000 --- a/packages/cubejs-backend-native/test/bridge/result-shape.test.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { bridgeHarnessAvailable, compileMemberSql } from './helpers'; - -const describeBridge = bridgeHarnessAvailable ? describe : describe.skip; - -describeBridge('bridge: result shape', () => { - it('returns a string-vec template when the user function returns an array', () => { - // Pre-aggregation refs use this path. - const result = compileMemberSql( - // eslint-disable-next-line camelcase - (orders: any, line_items: any) => [orders.status, line_items.id] - ); - - expect(result.template).toEqual(['{arg:0}', '{arg:1}']); - expect(result.args.symbol_paths).toEqual([ - ['orders', 'status'], - ['line_items', 'id'], - ]); - }); - - it('returns a string template for any non-array return (including objects coerced via toString)', () => { - // If the user returns a single proxy node, convert_to_string falls back - // to its toString interceptor, which in turn registers the path. - const result = compileMemberSql((orders: any) => orders.status); - - expect(result.template).toBe('{arg:0}'); - expect(result.args.symbol_paths).toEqual([['orders', 'status']]); - }); - - it('coerces a numeric return to its string form', () => { - const integerResult = compileMemberSql(() => 42 as any); - const decimalResult = compileMemberSql(() => 1.5 as any); - - expect(integerResult.template).toBe('42'); - expect(decimalResult.template).toBe('1.5'); - }); - - it('coerces a boolean return to its string form', () => { - const trueResult = compileMemberSql(() => true as any); - const falseResult = compileMemberSql(() => false as any); - - expect(trueResult.template).toBe('true'); - expect(falseResult.template).toBe('false'); - }); - - it('returns an empty string template when the user function returns null', () => { - const result = compileMemberSql(() => null as any); - - expect(result.template).toBe(''); - expect(result.args.symbol_paths).toEqual([]); - }); -}); diff --git a/packages/cubejs-backend-native/test/bridge/security-context.test.ts b/packages/cubejs-backend-native/test/bridge/security-context.test.ts deleted file mode 100644 index 480852c4ba68d..0000000000000 --- a/packages/cubejs-backend-native/test/bridge/security-context.test.ts +++ /dev/null @@ -1,262 +0,0 @@ -import { bridgeHarnessAvailable, compileMemberSql } from './helpers'; - -const describeBridge = bridgeHarnessAvailable ? describe : describe.skip; - -describeBridge('bridge: SECURITY_CONTEXT — filter input shapes', () => { - it('handles a string filter value as col = {sv:0}', () => { - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.tenant.filter('col')}`, - { tenant: 'acme' } - ); - - expect(result.template).toBe('col = {sv:0}'); - expect(result.args.security_context.values).toEqual(['acme']); - }); - - it('handles a string array as col IN (sv0, sv1, sv2)', () => { - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.groups.filter('col')}`, - { groups: ['a', 'b', 'c'] } - ); - - expect(result.template).toBe('col IN ({sv:0}, {sv:1}, {sv:2})'); - expect(result.args.security_context.values).toEqual(['a', 'b', 'c']); - }); - - it('handles a numeric array by stringifying each element', () => { - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.ids.filter('id')}`, - { ids: [1, 2, 3] } - ); - - expect(result.template).toBe('id IN ({sv:0}, {sv:1}, {sv:2})'); - expect(result.args.security_context.values).toEqual(['1', '2', '3']); - }); - - it('renders an empty string array as 1 = 0 with no values registered for the filter', () => { - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.groups.filter('col')}`, - { groups: [] } - ); - - expect(result.template).toBe('1 = 0'); - expect(result.args.security_context.values).toEqual([]); - }); - - it('passes an empty array to a callback column for the empty-array case', () => { - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.groups.filter( - (vals: string[]) => `received:${vals.length}` - )}`, - { groups: [] } - ); - - expect(result.template).toBe('received:0'); - }); - - it('formats an integer-valued number without a decimal point', () => { - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.user_id.filter('uid')}`, - { user_id: 42 } - ); - - expect(result.template).toBe('uid = {sv:0}'); - expect(result.args.security_context.values).toEqual(['42']); - }); - - it('formats a non-integer number as a decimal string', () => { - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.factor.filter('f')}`, - { factor: 1.5 } - ); - - expect(result.template).toBe('f = {sv:0}'); - expect(result.args.security_context.values).toEqual(['1.5']); - }); - - it('formats a truthy boolean as the string "true"', () => { - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.flag.filter('f')}`, - { flag: true } - ); - - expect(result.template).toBe('f = {sv:0}'); - expect(result.args.security_context.values).toEqual(['true']); - }); - - it.each([ - ['missing', undefined], - ['null', null], - ['empty string', ''], - ['zero', 0], - ['false', false], - ])('returns 1 = 1 when filter value is %s', (_, value) => { - const ctx = value === undefined ? {} : { tenant: value }; - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.tenant.filter('col')}`, - ctx - ); - - expect(result.template).toBe('1 = 1'); - expect(result.args.security_context.values).toEqual([]); - }); - - it.each([ - ['missing', undefined], - ['null', null], - ['empty string', ''], - ['zero', 0], - ['false', false], - ])('throws when requiredFilter value is %s', (_, value) => { - const ctx = value === undefined ? {} : { tenant: value }; - - expect(() => compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.tenant.requiredFilter('col')}`, - ctx - )).toThrow(/Filter for col is required/); - }); - - it('rejects an unsupported value type with a user error', () => { - expect(() => compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.bad.filter('col')}`, - { bad: { nested: 'object' } } - )).toThrow(/Invalid param for security context/); - }); -}); - -describeBridge('bridge: SECURITY_CONTEXT — proxy structure', () => { - it('navigates nested struct values through the recursive proxy', () => { - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.tenant.id.filter('col')}`, - { tenant: { id: '123' } } - ); - - expect(result.template).toBe('col = {sv:0}'); - expect(result.args.security_context.values).toEqual(['123']); - }); - - it('does not crash on a deep leaf-proxy path that does not exist in the context', () => { - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.a.b.c.filter('col')}`, - {} - ); - - // Filter on a fully-undefined leaf path: the value resolves to None and - // the filter falls back to "1 = 1" (the path is treated as an absent - // optional filter, not an error). - expect(result.template).toBe('1 = 1'); - expect(result.args.security_context.values).toEqual([]); - }); - - it('exposes unsafeValue() that returns the raw value without registering a placeholder', () => { - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `prefix-${SECURITY_CONTEXT.tenant.unsafeValue()}-suffix`, - { tenant: 'acme' } - ); - - expect(result.template).toBe('prefix-acme-suffix'); - expect(result.args.security_context.values).toEqual([]); - }); - - it('lets the user branch the template at compile time via unsafeValue()', () => { - // Real prod pattern: unsafeValue() returns the actual JS value, so a - // ternary in the template literal picks one branch at compile time - // and the resulting template is just the picked literal — no - // placeholders registered. - const adminResult = compileMemberSql( - (SECURITY_CONTEXT: any) => `SELECT * FROM ${ - SECURITY_CONTEXT.cubeCloud.groups.unsafeValue() === 'admin' - ? 'admin_orders' - : 'public_orders' - }`, - { cubeCloud: { groups: 'admin' } } - ); - const viewerResult = compileMemberSql( - (SECURITY_CONTEXT: any) => `SELECT * FROM ${ - SECURITY_CONTEXT.cubeCloud.groups.unsafeValue() === 'admin' - ? 'admin_orders' - : 'public_orders' - }`, - { cubeCloud: { groups: 'viewer' } } - ); - - expect(adminResult.template).toBe('SELECT * FROM admin_orders'); - expect(viewerResult.template).toBe('SELECT * FROM public_orders'); - expect(adminResult.args.security_context.values).toEqual([]); - expect(viewerResult.args.security_context.values).toEqual([]); - }); - - it('renders a scalar leaf used directly in a template as a single placeholder', () => { - // `tenant_id = ${SECURITY_CONTEXT.cubeCloud.tenantId}` — common in prod. - // Coerce-time toString fires once and registers a single placeholder. - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `tenant_id = ${SECURITY_CONTEXT.cubeCloud.tenantId}`, - { cubeCloud: { tenantId: '123' } } - ); - - expect(result.template).toBe('tenant_id = {sv:0}'); - expect(result.args.security_context.values).toEqual(['123']); - }); - - it('renders an array leaf directly in a template as comma-joined placeholders', () => { - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.groups}`, - { groups: ['a', 'b'] } - ); - - expect(result.template).toBe('{sv:0},{sv:1}'); - expect(result.args.security_context.values).toEqual(['a', 'b']); - }); - - it('renders an empty array leaf directly in a template as an empty string', () => { - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `[${SECURITY_CONTEXT.groups}]`, - { groups: [] } - ); - - expect(result.template).toBe('[]'); - expect(result.args.security_context.values).toEqual([]); - }); - - it('allocates a fresh placeholder on every coercion of the same leaf proxy', () => { - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => { - const t = SECURITY_CONTEXT.tenant; - return `${t} | ${t}`; - }, - { tenant: 'acme' } - ); - - expect(result.template).toBe('{sv:0} | {sv:1}'); - expect(result.args.security_context.values).toEqual(['acme', 'acme']); - }); - - it('supports the canonical array-filter callback pattern with groups.join(...)', () => { - // Canonical prod pattern. The callback receives the prepared - // placeholder strings; join glues them into the SQL fragment. - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.cubeCloud.groups.filter( - (groups: string[]) => `source IN (${groups.join(',')})` - )}`, - { cubeCloud: { groups: ['a', 'b'] } } - ); - - expect(result.template).toBe('source IN ({sv:0},{sv:1})'); - expect(result.args.security_context.values).toEqual(['a', 'b']); - }); - - it('accepts both camelCase securityContext and snake_case security_context arg names', () => { - const camel = compileMemberSql( - (securityContext: any) => `${securityContext.tenant.filter('col')}`, - { tenant: 'acme' } - ); - const snake = compileMemberSql( - // eslint-disable-next-line camelcase - (security_context: any) => `${security_context.tenant.filter('col')}`, - { tenant: 'acme' } - ); - - expect(camel.template).toBe('col = {sv:0}'); - expect(snake.template).toBe('col = {sv:0}'); - }); -}); diff --git a/packages/cubejs-backend-native/test/bridge/symbol-paths.test.ts b/packages/cubejs-backend-native/test/bridge/symbol-paths.test.ts deleted file mode 100644 index 8e82b8fdb3a30..0000000000000 --- a/packages/cubejs-backend-native/test/bridge/symbol-paths.test.ts +++ /dev/null @@ -1,176 +0,0 @@ -import { bridgeHarnessAvailable, compileMemberSql } from './helpers'; - -const describeBridge = bridgeHarnessAvailable ? describe : describe.skip; - -describeBridge('bridge: symbol paths via property_proxy', () => { - describe('basic capture', () => { - it('captures a simple cube ref via toString interception', () => { - const result = compileMemberSql((CUBE: any) => `${CUBE.amount}`); - - expect(result.template).toBe('{arg:0}'); - expect(result.args.symbol_paths).toEqual([['CUBE', 'amount']]); - }); - - it('captures a nested path', () => { - const result = compileMemberSql((users: any) => `${users.address.city}`); - - expect(result.template).toBe('{arg:0}'); - expect(result.args.symbol_paths).toEqual([['users', 'address', 'city']]); - }); - - it('handles direct return without template literal (convert_to_string fallback)', () => { - const result = compileMemberSql((CUBE: any) => CUBE.amount); - - expect(result.template).toBe('{arg:0}'); - expect(result.args.symbol_paths).toEqual([['CUBE', 'amount']]); - }); - - it('exposes a constant template when the function returns a string literal', () => { - const result = compileMemberSql(() => 'NOW()'); - - expect(result.template).toBe('NOW()'); - expect(result.args.symbol_paths).toEqual([]); - }); - }); - - describe('coercion paths', () => { - it('captures via String(x) explicit coercion', () => { - const result = compileMemberSql((CUBE: any) => `value=${String(CUBE.x)}`); - - expect(result.template).toBe('value={arg:0}'); - expect(result.args.symbol_paths).toEqual([['CUBE', 'x']]); - }); - - it('captures via + "" concatenation (valueOf path)', () => { - // eslint-disable-next-line prefer-template - const result = compileMemberSql((CUBE: any) => `${'' + CUBE.x}`); - - expect(result.template).toBe('{arg:0}'); - expect(result.args.symbol_paths).toEqual([['CUBE', 'x']]); - }); - }); - - describe('.sql() accessor', () => { - // Canonical user-facing usage: `.sql()` for subquery references — - // e.g. `select v.* from ${visitors.sql()} as v` (see prod schemas in - // packages/cubejs-schema-compiler/test/integration/postgres/...). - // .sql is itself a function — JS does NOT auto-invoke it like - // toString/valueOf, so user code must call it explicitly. - - it('records {arg:N} with __sql_fn suffix for cube.sql() at the cube root', () => { - const result = compileMemberSql( - (visitors: any) => `select v.* from ${visitors.sql()} v where v.source = 'google'` - ); - - expect(result.template).toBe( - 'select v.* from {arg:0} v where v.source = \'google\'' - ); - expect(result.args.symbol_paths).toEqual([['visitors', '__sql_fn']]); - }); - - it('keeps cube.sql() and a member ref as distinct paths', () => { - const result = compileMemberSql( - (orders: any) => `SELECT * FROM ${orders.sql()} WHERE created = ${orders.createdAt}` - ); - - expect(result.template).toBe( - 'SELECT * FROM {arg:0} WHERE created = {arg:1}' - ); - expect(result.args.symbol_paths).toEqual([ - ['orders', '__sql_fn'], - ['orders', 'createdAt'], - ]); - }); - }); - - describe('dedup behavior', () => { - it('dedups identical paths in the same template', () => { - const result = compileMemberSql((CUBE: any) => `${CUBE.x} + ${CUBE.x}`); - - expect(result.template).toBe('{arg:0} + {arg:0}'); - expect(result.args.symbol_paths).toEqual([['CUBE', 'x']]); - }); - - it('dedups same path across different surrounding SQL contexts', () => { - // Bare reference and the same reference wrapped in ceil() share one - // {arg:N}. Parenthesizing/safety is handled downstream by SqlCall; - // the bridge only records the path once via UniqueVector. - const result = compileMemberSql( - (cube: any) => `${cube.a} + ceil(${cube.a})` - ); - - expect(result.template).toBe('{arg:0} + ceil({arg:0})'); - expect(result.args.symbol_paths).toEqual([['cube', 'a']]); - }); - - it('treats different leaves under the same top-level as distinct paths', () => { - const result = compileMemberSql( - (CUBE: any) => `${CUBE.a} + ${CUBE.b}` - ); - - expect(result.template).toBe('{arg:0} + {arg:1}'); - expect(result.args.symbol_paths).toEqual([ - ['CUBE', 'a'], - ['CUBE', 'b'], - ]); - }); - }); - - describe('view chains', () => { - it('captures sibling view chains as distinct paths sharing a top-level', () => { - const result = compileMemberSql( - (view: any) => `${view.v1.field} + ${view.v2.field}` - ); - - expect(result.template).toBe('{arg:0} + {arg:1}'); - expect(result.args.symbol_paths).toEqual([ - ['view', 'v1', 'field'], - ['view', 'v2', 'field'], - ]); - }); - - it('dedups identical view chains', () => { - const result = compileMemberSql( - (view: any) => `${view.v1.field} + ${view.v1.field}` - ); - - expect(result.template).toBe('{arg:0} + {arg:0}'); - expect(result.args.symbol_paths).toEqual([['view', 'v1', 'field']]); - }); - - it('keeps different leaves under a shared chain prefix as distinct', () => { - const result = compileMemberSql( - (view: any) => `${view.v1.a} + ${view.v1.b}` - ); - - expect(result.template).toBe('{arg:0} + {arg:1}'); - expect(result.args.symbol_paths).toEqual([ - ['view', 'v1', 'a'], - ['view', 'v1', 'b'], - ]); - }); - - it('captures deeper view chains (4+ levels)', () => { - const result = compileMemberSql( - (view: any) => `${view.v1.sub.field}` - ); - - expect(result.template).toBe('{arg:0}'); - expect(result.args.symbol_paths).toEqual([ - ['view', 'v1', 'sub', 'field'], - ]); - }); - - it('handles mixed-depth view chains in one template', () => { - const result = compileMemberSql( - (view: any) => `${view.v1.field} + ${view.v2.sub.field}` - ); - - expect(result.template).toBe('{arg:0} + {arg:1}'); - expect(result.args.symbol_paths).toEqual([ - ['view', 'v1', 'field'], - ['view', 'v2', 'sub', 'field'], - ]); - }); - }); -}); diff --git a/packages/cubejs-schema-compiler/test/unit/member-sql-template-compiler.test.ts b/packages/cubejs-schema-compiler/test/unit/member-sql-template-compiler.test.ts index b2c9c6cdc4d29..ebe37b975c398 100644 --- a/packages/cubejs-schema-compiler/test/unit/member-sql-template-compiler.test.ts +++ b/packages/cubejs-schema-compiler/test/unit/member-sql-template-compiler.test.ts @@ -44,6 +44,37 @@ describe('MemberSqlTemplateCompiler — member reference path', () => { expect(res.template).toEqual(['{arg:0}', '{arg:1}']); expect(res.symbolPaths).toEqual([['o', 'a'], ['o', 'b']]); }); + + it('captures a reference via String() coercion', () => { + const res = compileMemberSql((o) => String(o.amount), ['o']); + expect(res.template).toBe('{arg:0}'); + expect(res.symbolPaths).toEqual([['o', 'amount']]); + }); + + it('captures a reference via valueOf (concatenation) coercion', () => { + // eslint-disable-next-line prefer-template + const res = compileMemberSql((o) => o.amount + '', ['o']); + expect(res.template).toBe('{arg:0}'); + expect(res.symbolPaths).toEqual([['o', 'amount']]); + }); + + it('captures a direct (non-template) member return', () => { + const res = compileMemberSql((o) => o.amount, ['o']); + expect(res.template).toBe('{arg:0}'); + expect(res.symbolPaths).toEqual([['o', 'amount']]); + }); + + it('keeps .sql() and a plain member ref as distinct paths', () => { + const res = compileMemberSql((o) => `${o.amount.sql()} ${o.amount}`, ['o']); + expect(res.template).toBe('{arg:0} {arg:1}'); + expect(res.symbolPaths).toEqual([['o', 'amount', '__sql_fn'], ['o', 'amount']]); + }); + + it('keeps different leaves under a shared prefix distinct', () => { + const res = compileMemberSql((o) => `${o.a.x} ${o.a.y}`, ['o']); + expect(res.template).toBe('{arg:0} {arg:1}'); + expect(res.symbolPaths).toEqual([['o', 'a', 'x'], ['o', 'a', 'y']]); + }); }); describe('MemberSqlTemplateCompiler — FILTER_PARAMS / FILTER_GROUP', () => { @@ -82,6 +113,49 @@ describe('MemberSqlTemplateCompiler — FILTER_PARAMS / FILTER_GROUP', () => { expect(res.filterGroups).toHaveLength(1); expect(res.filterGroups[0].filterParams.map((p) => p.name)).toEqual(['a', 'b']); }); + + it('does not dedup distinct filter params', () => { + const res = compileMemberSql( + (FILTER_PARAMS) => `${FILTER_PARAMS.orders.a.filter('a')} ${FILTER_PARAMS.line.b.filter('b')}`, + ['FILTER_PARAMS'] + ); + expect(res.template).toBe('{fp:0} {fp:1}'); + expect(res.filterParams.map((p) => [p.cube_name, p.name])).toEqual([['orders', 'a'], ['line', 'b']]); + }); + + it('does not dedup identical filter params', () => { + const res = compileMemberSql( + (FILTER_PARAMS) => `${FILTER_PARAMS.orders.a.filter('a')} ${FILTER_PARAMS.orders.a.filter('a')}`, + ['FILTER_PARAMS'] + ); + expect(res.template).toBe('{fp:0} {fp:1}'); + expect(res.filterParams).toHaveLength(2); + }); + + it('produces an empty group when FILTER_GROUP() is called with no args', () => { + const res = compileMemberSql((FILTER_GROUP) => `${FILTER_GROUP()}`, ['FILTER_GROUP']); + expect(res.template).toBe('{fg:0}'); + expect(res.filterGroups).toEqual([{ filterParams: [] }]); + }); + + it('throws when FILTER_GROUP receives a non-FILTER_PARAMS arg', () => { + expect(() => compileMemberSql( + (FILTER_GROUP) => `${FILTER_GROUP('x')}`, + ['FILTER_GROUP'] + )).toThrow(); + }); + + it('coexists with a top-level FILTER_PARAMS reference in the same template', () => { + const res = compileMemberSql( + (FILTER_GROUP, FILTER_PARAMS) => `${FILTER_PARAMS.orders.a.filter('a')} ${FILTER_GROUP( + FILTER_PARAMS.orders.b.filter('b') + )}`, + ['FILTER_GROUP', 'FILTER_PARAMS'] + ); + expect(res.template).toBe('{fp:0} {fg:0}'); + expect(res.filterParams).toHaveLength(1); + expect(res.filterGroups[0].filterParams.map((p) => p.name)).toEqual(['b']); + }); }); describe('MemberSqlTemplateCompiler — SECURITY_CONTEXT', () => { @@ -140,6 +214,169 @@ describe('MemberSqlTemplateCompiler — SECURITY_CONTEXT', () => { expect(res.template).toBe('{sv:0}|acme'); expect(res.securityContextValues).toEqual(['acme']); }); + + it('stringifies each element of a numeric array into IN (...)', () => { + const res = compileMemberSql( + (SECURITY_CONTEXT) => `${SECURITY_CONTEXT.ids.filter('i')}`, + ['SECURITY_CONTEXT'], + { ids: [1, 2] } + ); + expect(res.template).toBe('i IN ({sv:0}, {sv:1})'); + expect(res.securityContextValues).toEqual(['1', '2']); + }); + + it('emits 1 = 0 for an empty array with a string column', () => { + const res = compileMemberSql( + (SECURITY_CONTEXT) => `${SECURITY_CONTEXT.ids.filter('i')}`, + ['SECURITY_CONTEXT'], + { ids: [] } + ); + expect(res.template).toBe('1 = 0'); + expect(res.securityContextValues).toEqual([]); + }); + + it('passes an empty array to a callback column', () => { + const res = compileMemberSql( + (SECURITY_CONTEXT) => `${SECURITY_CONTEXT.ids.filter((vs) => `len=${vs.length}`)}`, + ['SECURITY_CONTEXT'], + { ids: [] } + ); + expect(res.template).toBe('len=0'); + }); + + it('formats an integer without a decimal point and a non-integer with one', () => { + const int = compileMemberSql( + (SECURITY_CONTEXT) => `${SECURITY_CONTEXT.n.filter('c')}`, + ['SECURITY_CONTEXT'], + { n: 42 } + ); + const dec = compileMemberSql( + (SECURITY_CONTEXT) => `${SECURITY_CONTEXT.n.filter('c')}`, + ['SECURITY_CONTEXT'], + { n: 1.5 } + ); + expect(int.securityContextValues).toEqual(['42']); + expect(dec.securityContextValues).toEqual(['1.5']); + }); + + it('formats a truthy boolean as the string "true"', () => { + const res = compileMemberSql( + (SECURITY_CONTEXT) => `${SECURITY_CONTEXT.flag.filter('c')}`, + ['SECURITY_CONTEXT'], + { flag: true } + ); + expect(res.template).toBe('c = {sv:0}'); + expect(res.securityContextValues).toEqual(['true']); + }); + + it('rejects an unsupported value type', () => { + expect(() => compileMemberSql( + (SECURITY_CONTEXT) => `${SECURITY_CONTEXT.obj.filter('c')}`, + ['SECURITY_CONTEXT'], + { obj: { nested: 1 } } + )).toThrow(); + }); + + it('navigates nested struct values through the recursive proxy', () => { + const res = compileMemberSql( + (SECURITY_CONTEXT) => `${SECURITY_CONTEXT.a.b.filter('c')}`, + ['SECURITY_CONTEXT'], + { a: { b: 'v' } } + ); + expect(res.template).toBe('c = {sv:0}'); + expect(res.securityContextValues).toEqual(['v']); + }); + + it('does not crash on a missing deep leaf path', () => { + const res = compileMemberSql( + (SECURITY_CONTEXT) => `x=${SECURITY_CONTEXT.a.b.c}`, + ['SECURITY_CONTEXT'], + {} + ); + expect(res.template).toBe('x='); + expect(res.securityContextValues).toEqual([]); + }); + + it('renders a scalar leaf used directly as a single placeholder', () => { + const res = compileMemberSql( + (SECURITY_CONTEXT) => `tenant = ${SECURITY_CONTEXT.cubeCloud.tenantId}`, + ['SECURITY_CONTEXT'], + { cubeCloud: { tenantId: '123' } } + ); + expect(res.template).toBe('tenant = {sv:0}'); + expect(res.securityContextValues).toEqual(['123']); + }); + + it('renders an array leaf directly as comma-joined placeholders', () => { + const res = compileMemberSql( + (SECURITY_CONTEXT) => `${SECURITY_CONTEXT.groups}`, + ['SECURITY_CONTEXT'], + { groups: ['a', 'b'] } + ); + expect(res.template).toBe('{sv:0},{sv:1}'); + expect(res.securityContextValues).toEqual(['a', 'b']); + }); + + it('renders an empty array leaf directly as an empty string', () => { + const res = compileMemberSql( + (SECURITY_CONTEXT) => `[${SECURITY_CONTEXT.groups}]`, + ['SECURITY_CONTEXT'], + { groups: [] } + ); + expect(res.template).toBe('[]'); + expect(res.securityContextValues).toEqual([]); + }); + + it('dedups identical security values across repeated coercions', () => { + const res = compileMemberSql( + (SECURITY_CONTEXT) => { + const t = SECURITY_CONTEXT.tenant; + return `${t} | ${t}`; + }, + ['SECURITY_CONTEXT'], + { tenant: 'acme' } + ); + expect(res.template).toBe('{sv:0} | {sv:0}'); + expect(res.securityContextValues).toEqual(['acme']); + }); + + it('accepts camelCase and snake_case security-context arg names', () => { + const camel = compileMemberSql( + (ctx) => `${ctx.tenant.filter('c')}`, + ['securityContext'], + { tenant: 'acme' } + ); + const snake = compileMemberSql( + (ctx) => `${ctx.tenant.filter('c')}`, + ['security_context'], + { tenant: 'acme' } + ); + expect(camel.template).toBe('c = {sv:0}'); + expect(snake.template).toBe('c = {sv:0}'); + }); +}); + +describe('MemberSqlTemplateCompiler — result coercion', () => { + it('coerces a number return to its string form', () => { + expect(compileMemberSql(() => 42, []).template).toBe('42'); + expect(compileMemberSql(() => 1.5, []).template).toBe('1.5'); + }); + + it('coerces a boolean return to its string form', () => { + expect(compileMemberSql(() => true, []).template).toBe('true'); + expect(compileMemberSql(() => false, []).template).toBe('false'); + }); + + it('returns an empty string for a null or undefined return', () => { + expect(compileMemberSql(() => null, []).template).toBe(''); + expect(compileMemberSql(() => undefined, []).template).toBe(''); + }); + + it('returns a constant template with no recorded paths for a string-literal return', () => { + const res = compileMemberSql(() => 'CONST', []); + expect(res.template).toBe('CONST'); + expect(res.symbolPaths).toEqual([]); + }); }); describe('MemberSqlTemplateCompiler — SQL_UTILS', () => { From 1e3244e5940925a52717265cbd59882db3b32868 Mon Sep 17 00:00:00 2001 From: Aleksandr Romanenko Date: Thu, 18 Jun 2026 17:02:07 +0200 Subject: [PATCH 6/7] refactor(tesseract): drop QueryTools ref from TypedFilter to remove latent Rc cycle --- .../src/physical_plan/filter/base_filter.rs | 1 + .../src/physical_plan/filter/typed_filter.rs | 9 ++++--- .../src/planner/filter/base_filter.rs | 2 ++ .../src/planner/filter/typed_filter.rs | 27 +++++++++---------- .../src/planner/query_properties.rs | 3 +++ .../src/tests/filter/use_raw_values.rs | 4 +++ 6 files changed, 27 insertions(+), 19 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 adf806d12e43b..51e1dee0dad85 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 @@ -27,6 +27,7 @@ impl ToSql for BaseFilter { { return self.typed_filter().to_sql_for_filter_params( filter_params_column, + &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 e1a407c95a7a1..ae7d5676454d0 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 @@ -34,7 +34,7 @@ impl ToSql for TypedFilter { let ctx = FilterSqlContext { member_sql: &member_sql, - query_tools: self.query_tools(), + query_tools: &query_tools, plan_templates: templates, use_db_time_zone: !filters_ctx.use_local_tz, use_raw_values: self.use_raw_values(), @@ -48,6 +48,7 @@ impl TypedFilter { pub fn to_sql_for_filter_params( &self, column: &FilterParamsColumn, + query_tools: &Rc, plan_templates: &PlanSqlTemplates, filters_context: &FiltersContext, ) -> Result { @@ -57,7 +58,7 @@ impl TypedFilter { FilterParamsColumn::String(column_sql) => { let ctx = FilterSqlContext { member_sql: column_sql, - query_tools: self.query_tools(), + query_tools, plan_templates, use_db_time_zone, use_raw_values: self.use_raw_values(), @@ -69,7 +70,7 @@ impl TypedFilter { FilterOp::DateRange(_) | FilterOp::DateSingle(_) => { let ctx = FilterSqlContext { member_sql: "", - query_tools: self.query_tools(), + query_tools, plan_templates, use_db_time_zone, use_raw_values: self.use_raw_values(), @@ -91,7 +92,7 @@ impl TypedFilter { _ => self .values() .iter() - .filter_map(|v| v.as_ref().map(|v| self.query_tools().allocate_param(v))) + .filter_map(|v| v.as_ref().map(|v| query_tools.allocate_param(v))) .collect::>(), }; callback.call(&args) diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/base_filter.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/base_filter.rs index 86687f60a7621..dec5bcd485bd8 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/base_filter.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/base_filter.rs @@ -60,11 +60,13 @@ impl BaseFilter { filter_operator: FilterOperator, values: Vec>, use_raw_values: bool, + query_tools: Rc, compiler: Option<&mut Compiler>, ) -> Result, CubeError> { let typed_filter = self .typed_filter .to_builder() + .query_tools(query_tools) .operator(filter_operator) .values(Some(values)) .use_raw_values(use_raw_values) diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/typed_filter.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/typed_filter.rs index e35b449f921a7..2ea52d096983b 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/typed_filter.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/typed_filter.rs @@ -51,7 +51,6 @@ pub enum FilterOp { /// whichever view they need. #[derive(Clone)] pub struct TypedFilter { - query_tools: Rc, member_evaluator: Rc, filter_type: FilterType, operator: FilterOperator, @@ -67,7 +66,6 @@ impl TypedFilter { pub fn to_builder(&self) -> TypedFilterBuilder { TypedFilter::builder() - .query_tools(self.query_tools.clone()) .member_evaluator(self.member_evaluator.clone()) .filter_type(self.filter_type.clone()) .operator(self.operator.clone()) @@ -75,10 +73,6 @@ impl TypedFilter { .use_raw_values(self.use_raw_values) } - pub fn query_tools(&self) -> &Rc { - &self.query_tools - } - pub fn member_evaluator(&self) -> &Rc { &self.member_evaluator } @@ -174,15 +168,13 @@ impl TypedFilterBuilder { .ok_or_else(|| CubeError::user("Expected one parameter but nothing found".to_string())) } - // FIXME: late compilation. `compiler` is threaded in only to (re)compile a - // custom rolling-window granularity during planning (the - // ToDateRollingWindowDateRange branch below). Once granularities are - // resolved in an early compile phase, this parameter — and the whole - // QueryTools→Compiler dependency in filter building — should go away. + // FIXME: late compilation. `compiler` and the builder's `query_tools` are + // consumed only to (re)compile a custom rolling-window granularity during + // planning (the ToDateRollingWindowDateRange branch below); neither is + // stored on the built filter. Once granularities are resolved in an early + // compile phase, both inputs should go away. pub fn build(self, compiler: Option<&mut Compiler>) -> Result { - let query_tools = self - .query_tools - .ok_or_else(|| CubeError::internal("query_tools is required".to_string()))?; + let query_tools = self.query_tools; let member_evaluator = self .member_evaluator .ok_or_else(|| CubeError::internal("member_evaluator is required".to_string()))?; @@ -288,6 +280,12 @@ impl TypedFilterBuilder { .to_string(), ) })?; + let query_tools = query_tools.as_ref().ok_or_else(|| { + CubeError::internal( + "query_tools is required to resolve a to_date rolling-window granularity" + .to_string(), + ) + })?; let granularity_obj = GranularityHelper::make_granularity_obj( query_tools.cube_evaluator().clone(), @@ -338,7 +336,6 @@ impl TypedFilterBuilder { }; Ok(TypedFilter { - query_tools, member_evaluator, filter_type, operator, diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs index d2bbeabf23ec7..561fb74f8b734 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs @@ -913,6 +913,7 @@ impl QueryProperties { FilterOperator::BeforeOrOnDate, to_value, itm.use_raw_values(), + self.query_tools.query_tools().clone(), None, )?)); } @@ -939,6 +940,7 @@ impl QueryProperties { FilterOperator::AfterOrOnDate, from_value, itm.use_raw_values(), + self.query_tools.query_tools().clone(), None, )?)); } @@ -1073,6 +1075,7 @@ impl QueryProperties { operator.clone(), values, use_raw_values, + self.query_tools.query_tools().clone(), // FIXME: late compilation — only needed to recompile a // to_date rolling-window granularity here. Some(&mut self.query_tools.compiler().borrow_mut()), diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/use_raw_values.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/use_raw_values.rs index 24acfc526e106..f68f4b00676a7 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/use_raw_values.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/use_raw_values.rs @@ -43,6 +43,7 @@ fn test_in_date_range_use_raw_values() { Some("(SELECT max(dt) FROM cte)".to_string()), ], true, + ctx.query_tools().query_tools().clone(), None, ) .unwrap(); @@ -80,6 +81,7 @@ fn test_not_in_date_range_use_raw_values() { Some("(SELECT max(dt) FROM cte)".to_string()), ], true, + ctx.query_tools().query_tools().clone(), None, ) .unwrap(); @@ -114,6 +116,7 @@ fn test_before_or_on_date_use_raw_values() { FilterOperator::BeforeOrOnDate, vec![Some("(SELECT max(dt) FROM cte)".to_string())], true, + ctx.query_tools().query_tools().clone(), None, ) .unwrap(); @@ -148,6 +151,7 @@ fn test_after_or_on_date_use_raw_values() { FilterOperator::AfterOrOnDate, vec![Some("(SELECT min(df) FROM cte)".to_string())], true, + ctx.query_tools().query_tools().clone(), None, ) .unwrap(); From 3cf86700444e013e6ca1e5cfb572b144be4acedb Mon Sep 17 00:00:00 2001 From: Aleksandr Romanenko Date: Thu, 18 Jun 2026 17:21:40 +0200 Subject: [PATCH 7/7] =?UTF-8?q?docs(tesseract):=20correct=20member=5Fmask?= =?UTF-8?q?=5Ffilters=20comment=20=E2=80=94=20cycle=20closed=20by=20TypedF?= =?UTF-8?q?ilter=20change?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../cubesqlplanner/src/planner/query_tools.rs | 27 +++++++------------ 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_tools.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_tools.rs index e108076d23145..c746c48364815 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_tools.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_tools.rs @@ -32,27 +32,20 @@ pub struct QueryTools { convert_tz_for_raw_time_dimension: bool, masked_members: HashSet, // Compiled mask filters keyed by member full path. Populated once by - // `State` after the Rc exists (FilterCompiler requires it), - // then never mutated again — RefCell only carries the construction phase. - // - // KNOWN REMAINING CYCLE (only when masked members are present): the stored - // FilterItem -> BaseFilter -> TypedFilter holds a strong `Rc`, - // so QueryTools -> member_mask_filters -> ... -> QueryTools never drops. - // This field still lives here only because moving it is deferred to a - // follow-up; relocating it into `State` (like the Compiler and join-tree - // cache) closes the cycle, since `State` is not pointed back to by any - // cached value. The leak regression test covers the join-cache cycle, not - // this one. + // `State` after the `Rc` exists (the FilterCompiler needs it), + // then read-only — the `RefCell` only carries the construction phase. + // Nothing reachable from a stored `FilterItem` holds an `Rc`, + // so this cache forms no reference cycle. It stays here only until the + // early-compilation refactor resolves mask filters up front. member_mask_filters: RefCell>, } impl QueryTools { - /// Builds the immutable, cache-free per-query leaf. The mutable - /// per-query state (the `Compiler`, the join-tree cache, the - /// compiled mask filters) lives in `State`, which owns this and - /// fills `member_mask_filters` once it has a `Compiler`. Keeping - /// `QueryTools` free of any cache that can hold an `Rc` - /// is what prevents the planner's reference-cycle leaks. + /// Builds the per-query leaf. The mutable per-query state (the + /// `Compiler` and the join-tree cache) lives in `State`, which owns + /// this and fills `member_mask_filters` once it has a `Compiler`. + /// Keeping `QueryTools` free of any cache that can hold an + /// `Rc` is what prevents the planner's reference-cycle leaks. pub fn try_new( cube_evaluator: Rc, base_tools: Rc,