diff --git a/rust/cube/cubenativeutils/src/wrappers/serializer/deserializer.rs b/rust/cube/cubenativeutils/src/wrappers/serializer/deserializer.rs index 2dddddbb483b9..e97f53c65e231 100644 --- a/rust/cube/cubenativeutils/src/wrappers/serializer/deserializer.rs +++ b/rust/cube/cubenativeutils/src/wrappers/serializer/deserializer.rs @@ -40,8 +40,15 @@ impl<'de, IT: InnerTypes> Deserializer<'de> for NativeSerdeDeserializer { } else if let Ok(val) = self.input.to_string() { visitor.visit_string(val.value().unwrap()) } else if let Ok(val) = self.input.to_number() { - visitor.visit_i64(val.value().unwrap() as i64) //We deserialize float value in - //different methods + let num = val.value().unwrap(); + // Preserve fractional numbers as floats; only integral values are + // narrowed to i64 (whole-number JS values are the common case, and + // self-describing consumers like FilterValue expect them as ints). + if num.fract() == 0.0 && num.is_finite() { + visitor.visit_i64(num as i64) + } else { + visitor.visit_f64(num) + } } else if let Ok(val) = self.input.to_array() { let deserializer = NativeSeqDeserializer::::new(val); visitor.visit_seq(deserializer) @@ -67,14 +74,21 @@ impl<'de, IT: InnerTypes> Deserializer<'de> for NativeSerdeDeserializer { fn deserialize_enum( self, - _name: &'static str, + name: &'static str, _variants: &'static [&'static str], _visitor: V, ) -> Result where V: Visitor<'de>, { - todo!() + // The native deserializer has no notion of serde's tagged enum + // representation, so a derived `Deserialize` on an enum can't work here. + // Implement `Deserialize` manually via `deserialize_any` instead (see + // `FilterValue` in cubesqlplanner for an example). + Err(NativeObjSerializerError::Message(format!( + "Deserializing enum `{name}` is not supported by the native deserializer; \ + implement Deserialize manually via deserialize_any" + ))) } fn deserialize_bytes(self, _visitor: V) -> Result diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/base_query_options.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/base_query_options.rs index ac95eaa39baec..593b9311de6eb 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/base_query_options.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/base_query_options.rs @@ -9,11 +9,144 @@ use cubenativeutils::wrappers::serializer::{ }; use cubenativeutils::wrappers::{NativeArray, NativeContextHolder, NativeObjectHandle}; use cubenativeutils::CubeError; -use serde::{Deserialize, Serialize}; +use serde::de::Visitor; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; use std::any::Any; use std::collections::HashMap; +use std::fmt; use std::rc::Rc; +/// A single value of a filter (`equals`, `in`, `gt`, …). +#[derive(Debug, Clone, PartialEq)] +pub enum FilterValue { + Str(String), + Bool(bool), + Num(f64), + Null, +} + +impl FilterValue { + pub fn is_null(&self) -> bool { + matches!(self, FilterValue::Null) + } + + /// Canonical string representation bound as a SQL parameter. `Null` yields + /// `None` (the value is dropped / handled as `IS NULL`). Whole numbers are + /// rendered without a trailing `.0` (`42.0` → `"42"`). + pub fn to_param_string(&self) -> Option { + match self { + FilterValue::Str(s) => Some(s.clone()), + FilterValue::Bool(b) => Some(b.to_string()), + FilterValue::Num(n) => Some(Self::format_number(*n)), + FilterValue::Null => None, + } + } + + pub fn to_debug_string(&self) -> String { + match self { + FilterValue::Str(s) => format!("'{}'", s), + FilterValue::Bool(b) => b.to_string(), + FilterValue::Num(n) => Self::format_number(*n), + FilterValue::Null => "NULL".to_string(), + } + } + + fn format_number(n: f64) -> String { + if n.is_finite() && n.fract() == 0.0 && n.abs() < 1e15 { + format!("{}", n as i64) + } else { + format!("{}", n) + } + } +} + +impl From> for FilterValue { + fn from(value: Option) -> Self { + match value { + Some(s) => FilterValue::Str(s), + None => FilterValue::Null, + } + } +} + +impl From for FilterValue { + fn from(value: String) -> Self { + FilterValue::Str(value) + } +} + +impl Serialize for FilterValue { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + match self { + FilterValue::Str(s) => serializer.serialize_str(s), + FilterValue::Bool(b) => serializer.serialize_bool(*b), + FilterValue::Num(n) => serializer.serialize_f64(*n), + FilterValue::Null => serializer.serialize_none(), + } + } +} + +impl<'de> Deserialize<'de> for FilterValue { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + struct FilterValueVisitor; + + impl<'de> Visitor<'de> for FilterValueVisitor { + type Value = FilterValue; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("a string, boolean, number, or null") + } + + fn visit_bool(self, v: bool) -> Result { + Ok(FilterValue::Bool(v)) + } + + fn visit_i64(self, v: i64) -> Result { + Ok(FilterValue::Num(v as f64)) + } + + fn visit_u64(self, v: u64) -> Result { + Ok(FilterValue::Num(v as f64)) + } + + fn visit_f64(self, v: f64) -> Result { + Ok(FilterValue::Num(v)) + } + + fn visit_str(self, v: &str) -> Result { + Ok(FilterValue::Str(v.to_string())) + } + + fn visit_string(self, v: String) -> Result { + Ok(FilterValue::Str(v)) + } + + fn visit_unit(self) -> Result { + Ok(FilterValue::Null) + } + + fn visit_none(self) -> Result { + Ok(FilterValue::Null) + } + + fn visit_some(self, deserializer: D) -> Result + where + D: Deserializer<'de>, + { + deserializer.deserialize_any(self) + } + } + + deserializer.deserialize_any(FilterValueVisitor) + } +} + #[derive(Serialize, Deserialize, Debug, Clone)] pub struct MaskedMemberItem { pub member: String, @@ -35,7 +168,7 @@ pub struct FilterItem { pub member: Option, pub dimension: Option, pub operator: Option, - pub values: Option>>, + pub values: Option>, } #[derive(Serialize, Deserialize, Debug, Clone)] @@ -107,3 +240,78 @@ pub trait BaseQueryOptions { #[nbridge(field, optional, vec)] fn join_hints(&self) -> Result>, CubeError>; } + +#[cfg(test)] +mod tests { + use super::FilterValue; + + #[test] + fn deserializes_native_boolean() { + let values: Vec = serde_json::from_str("[true, false]").unwrap(); + assert_eq!( + values, + vec![FilterValue::Bool(true), FilterValue::Bool(false)] + ); + } + + #[test] + fn deserializes_native_integer() { + let values: Vec = serde_json::from_str("[42]").unwrap(); + assert_eq!(values, vec![FilterValue::Num(42.0)]); + } + + #[test] + fn deserializes_native_fractional_number() { + let values: Vec = serde_json::from_str("[42.5]").unwrap(); + assert_eq!(values, vec![FilterValue::Num(42.5)]); + } + + #[test] + fn deserializes_quoted_value_as_string() { + let values: Vec = serde_json::from_str(r#"["42"]"#).unwrap(); + assert_eq!(values, vec![FilterValue::Str("42".to_string())]); + } + + #[test] + fn deserializes_null() { + let values: Vec = serde_json::from_str("[null]").unwrap(); + assert_eq!(values, vec![FilterValue::Null]); + } + + #[test] + fn deserializes_mixed_value_array() { + let values: Vec = serde_json::from_str(r#"[true, 42, "x", null]"#).unwrap(); + assert_eq!( + values, + vec![ + FilterValue::Bool(true), + FilterValue::Num(42.0), + FilterValue::Str("x".to_string()), + FilterValue::Null, + ] + ); + } + + #[test] + fn to_param_string_renders_canonical_form() { + assert_eq!( + FilterValue::Bool(true).to_param_string().as_deref(), + Some("true") + ); + assert_eq!( + FilterValue::Num(42.0).to_param_string().as_deref(), + Some("42") + ); + assert_eq!( + FilterValue::Num(42.5).to_param_string().as_deref(), + Some("42.5") + ); + assert_eq!( + FilterValue::Str("42".to_string()) + .to_param_string() + .as_deref(), + Some("42") + ); + assert_eq!(FilterValue::Null.to_param_string(), None); + } +} 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 4d87e30c40b94..cf34d7921c963 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/base_tools.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/base_tools.rs @@ -5,6 +5,7 @@ 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::base_query_options::FilterValue; use crate::cube_bridge::join_hints::JoinHintItem; use cubenativeutils::wrappers::serializer::{ NativeDeserialize, NativeDeserializer, NativeSerialize, @@ -24,7 +25,7 @@ pub trait BaseTools { fn driver_tools(&self, external: bool) -> Result, CubeError>; fn sql_templates(&self) -> Result, CubeError>; fn sql_utils_for_rust(&self) -> Result, CubeError>; - fn get_allocated_params(&self) -> Result, CubeError>; + fn get_allocated_params(&self) -> Result, CubeError>; fn all_cube_members(&self, path: String) -> Result, CubeError>; fn interval_and_minimal_time_unit(&self, interval: String) -> Result, CubeError>; fn get_pre_aggregation_by_name( diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/pretty_print.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/pretty_print.rs index 4c3d82d1de3db..8e9fad8b728b4 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/pretty_print.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/pretty_print.rs @@ -106,7 +106,7 @@ pub fn pretty_print_filter_item( item.filter_operator().to_string(), item.values() .iter() - .map(|v| v.clone().unwrap_or("null".to_string())) + .map(|v| v.to_param_string().unwrap_or_else(|| "null".to_string())) .collect::>() .join(", ") ), diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/operators/filter_sql_context.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/operators/filter_sql_context.rs index 3ac92edff0a0e..b93cafe3b6393 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/operators/filter_sql_context.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/operators/filter_sql_context.rs @@ -1,3 +1,4 @@ +use crate::cube_bridge::base_query_options::FilterValue; use crate::planner::query_tools::QueryTools; use crate::planner::sql_templates::{PlanSqlTemplates, TemplateProjectionColumn}; use crate::planner::QueryDateTimeHelper; @@ -33,6 +34,17 @@ impl<'a> FilterSqlContext<'a> { } pub fn allocate_and_cast( + &self, + value: &FilterValue, + member_type: &Option, + ) -> Result { + let value = value.to_param_string().ok_or_else(|| { + CubeError::internal("Unexpected null value for a single-value filter".to_string()) + })?; + self.allocate_and_cast_str(&value, member_type) + } + + pub fn allocate_and_cast_str( &self, value: &str, member_type: &Option, @@ -43,12 +55,12 @@ impl<'a> FilterSqlContext<'a> { pub fn allocate_and_cast_values( &self, - values: &[Option], + values: &[FilterValue], member_type: &Option, ) -> Result, CubeError> { values .iter() - .filter_map(|v| v.as_ref()) + .filter(|v| !v.is_null()) .map(|v| self.allocate_and_cast(v, member_type)) .collect() } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/operators/in_list.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/operators/in_list.rs index 4532fbada975f..bf75934571463 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/operators/in_list.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/operators/in_list.rs @@ -4,7 +4,7 @@ use cubenativeutils::CubeError; impl FilterOperationSql for InListOp { fn to_sql(&self, ctx: &FilterSqlContext) -> Result { - let has_null = self.values.iter().any(|v| v.is_none()); + let has_null = self.values.iter().any(|v| v.is_null()); let need_null_check = if self.negated { !has_null } else { has_null }; let allocated = ctx.allocate_and_cast_values(&self.values, &self.member_type)?; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/operators/like.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/operators/like.rs index 1a1d84fe0a41e..1d867a57f7ec7 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/operators/like.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/operators/like.rs @@ -4,14 +4,11 @@ use cubenativeutils::CubeError; impl FilterOperationSql for LikeOp { fn to_sql(&self, ctx: &FilterSqlContext) -> Result { - let allocated = ctx.allocate_and_cast_values( - &self - .values - .iter() - .map(|v| Some(v.clone())) - .collect::>(), - &self.member_type, - )?; + let allocated = self + .values + .iter() + .map(|v| ctx.allocate_and_cast_str(v, &self.member_type)) + .collect::, _>>()?; let like_parts = allocated .into_iter() 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 ae7d5676454d0..fe62d0013dfa0 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 @@ -78,21 +78,22 @@ impl TypedFilter { let from = self .values() .first() - .and_then(|v| v.as_ref()) - .map(|v| ctx.format_and_allocate_from_date(v)) + .and_then(|v| v.to_param_string()) + .map(|v| ctx.format_and_allocate_from_date(&v)) .transpose()?; let to = self .values() .get(1) - .and_then(|v| v.as_ref()) - .map(|v| ctx.format_and_allocate_to_date(v)) + .and_then(|v| v.to_param_string()) + .map(|v| ctx.format_and_allocate_to_date(&v)) .transpose()?; [from, to].into_iter().flatten().collect() } _ => self .values() .iter() - .filter_map(|v| v.as_ref().map(|v| query_tools.allocate_param(v))) + .filter_map(|v| v.to_param_string()) + .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 dec5bcd485bd8..8548c9b3db5f3 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::cube_bridge::base_query_options::FilterValue; use crate::planner::Compiler; use crate::planner::MemberSymbol; use cubenativeutils::CubeError; @@ -39,7 +40,7 @@ impl BaseFilter { member_evaluator: Rc, filter_type: FilterType, filter_operator: FilterOperator, - values: Option>>, + values: Option>, compiler: Option<&mut Compiler>, ) -> Result, CubeError> { let typed_filter = TypedFilter::builder() @@ -58,7 +59,7 @@ impl BaseFilter { pub fn change_operator( &self, filter_operator: FilterOperator, - values: Vec>, + values: Vec, use_raw_values: bool, query_tools: Rc, compiler: Option<&mut Compiler>, @@ -129,7 +130,7 @@ impl BaseFilter { } } - pub fn values(&self) -> &Vec> { + pub fn values(&self) -> &Vec { self.typed_filter.values() } @@ -175,8 +176,7 @@ impl BaseFilter { self.typed_filter .values() .iter() - .cloned() - .filter_map(|v| v) + .filter_map(|v| v.to_param_string()) .collect_vec(), ) } else { diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/compiler.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/compiler.rs index afa507ac07bdc..aa8fd4d2e14fc 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/compiler.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/compiler.rs @@ -1,6 +1,6 @@ use super::base_filter::{BaseFilter, FilterType}; use super::FilterOperator; -use crate::cube_bridge::base_query_options::FilterItem as NativeFilterItem; +use crate::cube_bridge::base_query_options::{FilterItem as NativeFilterItem, FilterValue}; use crate::planner::filter::{FilterGroup, FilterGroupOperator, FilterItem}; use crate::planner::query_tools::QueryTools; use crate::planner::{Compiler, MemberSymbol}; @@ -58,7 +58,7 @@ impl<'a> FilterCompiler<'a> { item.clone(), FilterType::Dimension, FilterOperator::InDateRange, - Some(date_range.into_iter().map(|v| Some(v)).collect()), + Some(date_range.into_iter().map(FilterValue::Str).collect()), None, )?; self.time_dimension_filters.push(FilterItem::Item(filter)); diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/filter_debug.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/filter_debug.rs index 4aee5e40afa98..366d182b301c6 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/filter_debug.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/filter_debug.rs @@ -12,10 +12,7 @@ impl DebugSql for BaseFilter { let values_str = self .values() .iter() - .map(|v| match v { - Some(val) => format!("'{}'", val), - None => "NULL".to_string(), - }) + .map(|v| v.to_debug_string()) .collect::>() .join(", "); diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/operators/comparison.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/operators/comparison.rs index 76a7fdf40a32b..8a1b5046f185f 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/operators/comparison.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/operators/comparison.rs @@ -1,3 +1,5 @@ +use crate::cube_bridge::base_query_options::FilterValue; + #[derive(Clone, Debug)] pub enum ComparisonKind { Gt, @@ -11,12 +13,12 @@ pub enum ComparisonKind { #[derive(Clone, Debug)] pub struct ComparisonOp { pub(crate) kind: ComparisonKind, - pub(crate) value: String, + pub(crate) value: FilterValue, pub(crate) member_type: Option, } impl ComparisonOp { - pub fn new(kind: ComparisonKind, value: String, member_type: Option) -> Self { + pub fn new(kind: ComparisonKind, value: FilterValue, member_type: Option) -> Self { Self { kind, value, diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/operators/equality.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/operators/equality.rs index 93924b3843a75..08061844aae80 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/operators/equality.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/operators/equality.rs @@ -1,14 +1,16 @@ +use crate::cube_bridge::base_query_options::FilterValue; + /// `Equality` filter operation: tests the member for equality, or /// inequality when `negated`, against a single value. #[derive(Clone, Debug)] pub struct EqualityOp { pub(crate) negated: bool, - pub(crate) value: String, + pub(crate) value: FilterValue, pub(crate) member_type: Option, } impl EqualityOp { - pub fn new(negated: bool, value: String, member_type: Option) -> Self { + pub fn new(negated: bool, value: FilterValue, member_type: Option) -> Self { Self { negated, value, diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/operators/in_list.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/operators/in_list.rs index 58cc91731fb2a..d90980608ebd7 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/operators/in_list.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/operators/in_list.rs @@ -1,14 +1,16 @@ +use crate::cube_bridge::base_query_options::FilterValue; + /// `InList` filter operation: tests whether the member belongs to, /// or — when `negated` — does not belong to, a list of values. #[derive(Clone, Debug)] pub struct InListOp { pub(crate) negated: bool, - pub(crate) values: Vec>, + pub(crate) values: Vec, pub(crate) member_type: Option, } impl InListOp { - pub fn new(negated: bool, values: Vec>, member_type: Option) -> Self { + pub fn new(negated: bool, values: Vec, member_type: Option) -> Self { Self { negated, 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 2ea52d096983b..323188184238e 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/typed_filter.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/typed_filter.rs @@ -1,3 +1,4 @@ +use crate::cube_bridge::base_query_options::FilterValue; use crate::planner::query_tools::QueryTools; use crate::planner::Compiler; use crate::planner::MemberSymbol; @@ -54,7 +55,7 @@ pub struct TypedFilter { member_evaluator: Rc, filter_type: FilterType, operator: FilterOperator, - values: Vec>, + values: Vec, use_raw_values: bool, op: FilterOp, } @@ -85,7 +86,7 @@ impl TypedFilter { &self.operator } - pub fn values(&self) -> &Vec> { + pub fn values(&self) -> &Vec { &self.values } @@ -104,7 +105,7 @@ pub struct TypedFilterBuilder { member_evaluator: Option>, filter_type: Option, operator: Option, - values: 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 @@ -147,7 +148,7 @@ impl TypedFilterBuilder { self } - pub fn values(mut self, v: Option>>) -> Self { + pub fn values(mut self, v: Option>) -> Self { self.values = v; self } @@ -161,13 +162,25 @@ impl TypedFilterBuilder { } } - fn first_non_null_value(values: &[Option]) -> Result { + fn first_non_null_value(values: &[FilterValue]) -> Result { values .iter() - .find_map(|v| v.as_ref().cloned()) + .find(|v| !v.is_null()) + .cloned() .ok_or_else(|| CubeError::user("Expected one parameter but nothing found".to_string())) } + /// First non-null value rendered as its parameter string. Used by the + /// date/interval operators, which operate on the string form. + fn first_non_null_string(values: &[FilterValue]) -> Result { + // `first_non_null_value` already rejects an all-null/empty list, and a + // non-null `FilterValue` always renders to `Some`, so the fallback here + // is unreachable. + Ok(Self::first_non_null_value(values)? + .to_param_string() + .unwrap_or_default()) + } + // 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 @@ -194,13 +207,13 @@ impl TypedFilterBuilder { match operator { FilterOperator::Equal | FilterOperator::NotEqual => { let negated = matches!(operator, FilterOperator::NotEqual); - let has_null = values.iter().any(|v| v.is_none()); + let has_null = values.iter().any(|v| v.is_null()); 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() { + } else if let Some(value) = values.into_iter().next() { FilterOp::Equality(EqualityOp::new(negated, value, member_type)) } else { return Err(CubeError::user( @@ -227,10 +240,10 @@ impl TypedFilterBuilder { 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 from = Self::first_non_null_string(&values)?; let to = values .get(1) - .and_then(|v| v.as_ref().cloned()) + .and_then(|v| v.to_param_string()) .ok_or_else(|| { CubeError::user("2 arguments expected for date range".to_string()) })?; @@ -242,36 +255,35 @@ impl TypedFilterBuilder { FilterOp::DateRange(DateRangeOp::new(kind, from, to)) } FilterOperator::BeforeDate => { - let value = Self::first_non_null_value(&values)?; + let value = Self::first_non_null_string(&values)?; FilterOp::DateSingle(DateSingleOp::new(DateSingleKind::Before, value)) } FilterOperator::BeforeOrOnDate => { - let value = Self::first_non_null_value(&values)?; + let value = Self::first_non_null_string(&values)?; FilterOp::DateSingle(DateSingleOp::new(DateSingleKind::BeforeOrOn, value)) } FilterOperator::AfterDate => { - let value = Self::first_non_null_value(&values)?; + let value = Self::first_non_null_string(&values)?; FilterOp::DateSingle(DateSingleOp::new(DateSingleKind::After, value)) } FilterOperator::AfterOrOnDate => { - let value = Self::first_non_null_value(&values)?; + let value = Self::first_non_null_string(&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()); + let trailing = values.get(2).and_then(|v| v.to_param_string()); + let leading = values.get(3).and_then(|v| v.to_param_string()); FilterOp::RegularRollingWindow(RegularRollingWindowOp::new(trailing, leading)) } FilterOperator::ToDateRollingWindowDateRange => { let granularity_name = values .get(2) - .and_then(|v| v.as_ref()) + .and_then(|v| v.to_param_string()) .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(|| { @@ -310,9 +322,9 @@ impl TypedFilterBuilder { | FilterOperator::NotStartsWith | FilterOperator::EndsWith | FilterOperator::NotEndsWith => { - let has_null = values.iter().any(|v| v.is_none()); + let has_null = values.iter().any(|v| v.is_null()); let non_null_values: Vec = - values.iter().filter_map(|v| v.clone()).collect(); + values.iter().filter_map(|v| v.to_param_string()).collect(); let (negated, start_wild, end_wild) = match operator { FilterOperator::Contains => (false, true, true), FilterOperator::NotContains => (true, true, true), diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/params_allocator.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/params_allocator.rs index 10bce7430a05f..bccd0f50439fc 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/params_allocator.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/params_allocator.rs @@ -1,3 +1,4 @@ +use crate::cube_bridge::base_query_options::FilterValue; use crate::planner::sql_templates::PlanSqlTemplates; use cubenativeutils::CubeError; use lazy_static::lazy_static; @@ -9,7 +10,7 @@ lazy_static! { static ref PARAMS_MATCH_RE: Regex = Regex::new(r"\$_(\d+)_\$").unwrap(); } pub struct ParamsAllocator { - params: Vec, + params: Vec, export_annotated_sql: bool, } @@ -26,25 +27,31 @@ impl ParamsAllocator { } pub fn allocate_param(&mut self, name: &str) -> String { - self.params.push(name.to_string()); + // Params allocated during Rust-side planning are already normalized to a + // string at the call site, so they enter the channel as `FilterValue::Str`. + // Native params (from the JS side) join later in `build_sql_and_params` + // and keep their natural type, including `Null`. + self.params.push(FilterValue::Str(name.to_string())); self.make_placeholder(self.params.len() - 1) } - pub fn get_params(&self) -> &Vec { + pub fn get_params(&self) -> &Vec { &self.params } pub fn build_sql_and_params( &self, sql: &str, - native_allocated_params: Vec, + native_allocated_params: Vec, should_reuse_params: bool, templates: &PlanSqlTemplates, - ) -> Result<(String, Vec), CubeError> { + ) -> Result<(String, Vec), CubeError> { let (sql, params) = self.add_native_allocated_params(sql, &native_allocated_params)?; + let mut params_in_sql_order = Vec::new(); let mut param_index_map: HashMap = HashMap::new(); let mut error = None; + let result_sql = if should_reuse_params { PARAMS_MATCH_RE .replace_all(&sql, |caps: &Captures| { @@ -99,8 +106,8 @@ impl ParamsAllocator { fn add_native_allocated_params( &self, sql: &str, - native_allocated_params: &Vec, - ) -> Result<(String, Vec), CubeError> { + native_allocated_params: &[FilterValue], + ) -> Result<(String, Vec), CubeError> { lazy_static! { static ref NATIVE_PARAMS_MATCH_RE: Regex = Regex::new(r"\$(\d+)\$").unwrap(); } @@ -121,3 +128,60 @@ impl ParamsAllocator { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn allocated_params_enter_channel_as_str() { + let mut allocator = ParamsAllocator::new(false); + let p0 = allocator.allocate_param("alpha"); + let p1 = allocator.allocate_param("beta"); + + assert_eq!(p0, "$_0_$"); + assert_eq!(p1, "$_1_$"); + assert_eq!( + allocator.get_params(), + &vec![ + FilterValue::Str("alpha".to_string()), + FilterValue::Str("beta".to_string()), + ] + ); + } + + #[test] + fn native_params_keep_natural_type_including_null() { + let mut allocator = ParamsAllocator::new(false); + allocator.allocate_param("internal_a"); // $_0_$ + allocator.allocate_param("internal_b"); // $_1_$ + + // Native placeholders use the single-`$` form; a `null` securityContext + // field, a number, and a boolean must survive the merge with their type + // intact — `Null` in particular must NOT collapse to an empty string. + let sql = "SELECT $_0_$, $_1_$, $0$, $1$, $2$"; + let native = vec![ + FilterValue::Null, + FilterValue::Num(42.0), + FilterValue::Bool(true), + ]; + + let (rewritten_sql, params) = allocator + .add_native_allocated_params(sql, &native) + .expect("merge should succeed"); + + // Native placeholders are rewritten into the internal `$_N_$` space, + // appended after the two internal params. + assert_eq!(rewritten_sql, "SELECT $_0_$, $_1_$, $_2_$, $_3_$, $_4_$"); + assert_eq!( + params, + vec![ + FilterValue::Str("internal_a".to_string()), + FilterValue::Str("internal_b".to_string()), + FilterValue::Null, + FilterValue::Num(42.0), + FilterValue::Bool(true), + ] + ); + } +} 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 5dfa0b0668f60..10f6792f3851b 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 @@ -3,6 +3,7 @@ use super::{ MultiStageMemberQueryPlanner, MultiStageMemberType, MultiStageQueryDescription, PlanningScope, RollingWindowDescription, TimeSeriesDescription, }; +use crate::cube_bridge::base_query_options::FilterValue; use crate::cube_bridge::measure_definition::RollingWindow; use crate::logical_plan::*; use crate::planner::apply_static_filter_to_symbol; @@ -428,7 +429,7 @@ impl MultiStageQueryPlanner { switch_member.clone(), FilterType::Dimension, FilterOperator::Equal, - Some(values.into_iter().map(Some).collect_vec()), + Some(values.into_iter().map(FilterValue::Str).collect_vec()), None, )?; state.add_dimension_filter(FilterItem::Item(filter)); diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs index 561fb74f8b734..9befe2f6b0dcd 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs @@ -8,6 +8,7 @@ use super::state::State; use super::MemberSymbol; +use crate::cube_bridge::base_query_options::FilterValue; use crate::planner::collectors::{collect_multiplied_measures, has_multi_stage_members}; use crate::planner::filter::tree_ops; use crate::planner::filter::{Filter, FilterGroup, FilterItem, FilterOperator}; @@ -960,7 +961,10 @@ impl QueryProperties { right_interval: Option, ) -> Result<(), CubeError> { let operator = FilterOperator::RegularRollingWindowDateRange; - let values = vec![left_interval.clone(), right_interval.clone()]; + let values = vec![ + FilterValue::from(left_interval), + FilterValue::from(right_interval), + ]; self.time_dimensions_filters = self.change_date_range_filter_impl( member_name, &self.time_dimensions_filters, @@ -979,7 +983,7 @@ impl QueryProperties { granularity: &String, ) -> Result<(), CubeError> { let operator = FilterOperator::ToDateRollingWindowDateRange; - let values = vec![Some(granularity.clone())]; + let values = vec![FilterValue::Str(granularity.clone())]; self.time_dimensions_filters = self.change_date_range_filter_impl( member_name, &self.time_dimensions_filters, @@ -999,7 +1003,7 @@ impl QueryProperties { new_to: String, ) -> Result<(), CubeError> { let operator = FilterOperator::InDateRange; - let replacement_values = vec![Some(new_from), Some(new_to)]; + let replacement_values = vec![FilterValue::Str(new_from), FilterValue::Str(new_to)]; self.time_dimensions_filters = self.change_date_range_filter_impl( member_name, &self.time_dimensions_filters, @@ -1021,7 +1025,7 @@ impl QueryProperties { new_to: String, ) -> Result<(), CubeError> { let operator = FilterOperator::InDateRange; - let replacement_values = vec![Some(new_from), Some(new_to)]; + let replacement_values = vec![FilterValue::Str(new_from), FilterValue::Str(new_to)]; self.time_dimensions_filters = self.change_date_range_filter_impl( member_name, &self.time_dimensions_filters, @@ -1040,8 +1044,8 @@ impl QueryProperties { filters: &[FilterItem], operator: &FilterOperator, use_raw_values: Option, - additional_values: &Vec>, - replacement_values: &Option>>, + additional_values: &Vec, + replacement_values: &Option>, ) -> Result, CubeError> { let mut result = Vec::new(); for item in filters.iter() { 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 9e41d7990fa8b..7db89bc58df7b 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties_compiler.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties_compiler.rs @@ -8,7 +8,9 @@ use std::rc::Rc; use cubenativeutils::CubeError; use itertools::Itertools; -use crate::cube_bridge::base_query_options::{BaseQueryOptions, FilterItem as NativeFilterItem}; +use crate::cube_bridge::base_query_options::{ + BaseQueryOptions, FilterItem as NativeFilterItem, FilterValue, +}; use crate::cube_bridge::member_expression::{ MemberExpressionDefinition, MemberExpressionExpressionDef, }; @@ -471,7 +473,12 @@ impl QueryPropertiesCompiler { member: Some(s.member_reference.clone()), dimension: None, operator: Some(s.operator.clone()), - values: s.values_references.clone(), + // `values_references` are SQL/member references resolved as + // plain strings; lift each into a typed `FilterValue`. + values: s + .values_references + .as_ref() + .map(|vals| vals.iter().cloned().map(FilterValue::from).collect()), }; filter_compiler.add_item(&native_filter)?; } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_tools.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_tools.rs index c746c48364815..348eaa2ecee6f 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_tools.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_tools.rs @@ -1,5 +1,5 @@ use super::ParamsAllocator; -use crate::cube_bridge::base_query_options::MaskedMemberItem; +use crate::cube_bridge::base_query_options::{FilterValue, MaskedMemberItem}; use crate::cube_bridge::base_tools::BaseTools; use crate::cube_bridge::evaluator::CubeEvaluator; use crate::cube_bridge::join_definition::JoinDefinition; @@ -182,7 +182,7 @@ impl QueryTools { pub fn allocate_param(&self, name: &str) -> String { self.params_allocator.borrow_mut().allocate_param(name) } - pub fn get_allocated_params(&self) -> Vec { + pub fn get_allocated_params(&self) -> Vec { self.params_allocator.borrow().get_params().clone() } pub fn build_sql_and_params( @@ -190,7 +190,7 @@ impl QueryTools { sql: &str, should_reuse_params: bool, templates: &PlanSqlTemplates, - ) -> Result<(String, Vec), CubeError> { + ) -> Result<(String, Vec), CubeError> { let native_allocated_params = self.base_tools.get_allocated_params()?; self.params_allocator.borrow().build_sql_and_params( sql, diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/base_query_options.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/base_query_options.rs index 4a390a5af082b..6a4940fe26ae9 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/base_query_options.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/base_query_options.rs @@ -8,8 +8,8 @@ use typed_builder::TypedBuilder; use crate::{ cube_bridge::{ base_query_options::{ - BaseQueryOptions, BaseQueryOptionsStatic, FilterItem, MaskedMemberItem, OrderByItem, - TimeDimension, + BaseQueryOptions, BaseQueryOptionsStatic, FilterItem, FilterValue, MaskedMemberItem, + OrderByItem, TimeDimension, }, base_tools::BaseTools, evaluator::CubeEvaluator, @@ -116,7 +116,12 @@ pub fn filter_item( member: Some(member.to_string()), dimension: None, operator: Some(operator.to_string()), - values: Some(values.into_iter().map(|v| Some(v.to_string())).collect()), + values: Some( + values + .into_iter() + .map(|v| FilterValue::Str(v.to_string())) + .collect(), + ), or: None, and: None, } 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 36fa441a71614..a97158f490caa 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 @@ -1,3 +1,4 @@ +use crate::cube_bridge::base_query_options::FilterValue; use crate::cube_bridge::base_tools::BaseTools; use crate::cube_bridge::driver_tools::DriverTools; use crate::cube_bridge::join_definition::JoinDefinition; @@ -66,7 +67,7 @@ impl BaseTools for MockBaseTools { Ok(self.sql_utils.clone()) } - fn get_allocated_params(&self) -> Result, CubeError> { + fn get_allocated_params(&self) -> Result, CubeError> { Ok(vec![]) } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/yaml/base_query_options.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/yaml/base_query_options.rs index f00e3e2abe179..b9bfa737531ec 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/yaml/base_query_options.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/yaml/base_query_options.rs @@ -1,5 +1,5 @@ use crate::cube_bridge::base_query_options::{ - FilterItem, MaskedMemberItem, OrderByItem, TimeDimension, + FilterItem, FilterValue, MaskedMemberItem, OrderByItem, TimeDimension, }; use serde::de; use serde::{Deserialize, Deserializer}; @@ -114,7 +114,7 @@ pub struct YamlBaseFilter { #[serde(default)] pub operator: Option, #[serde(default)] - pub values: Option>>, + pub values: Option>, } impl YamlFilterItem { diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/yaml/measure.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/yaml/measure.rs index 428dc4716d031..42c979bde16cc 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/yaml/measure.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/yaml/measure.rs @@ -1,4 +1,4 @@ -use crate::cube_bridge::base_query_options::FilterItem as NativeFilterItem; +use crate::cube_bridge::base_query_options::{FilterItem as NativeFilterItem, FilterValue}; use crate::cube_bridge::case_variant::CaseVariant; use crate::cube_bridge::measure_definition::{RollingWindow, TimeShiftReference}; use crate::test_fixtures::cube_bridge::yaml::case::YamlCaseVariant; @@ -65,7 +65,7 @@ struct YamlIncludeFilter { #[serde(default)] operator: Option, #[serde(default)] - values: Option>>, + values: Option>, #[serde(default)] and: Option>, #[serde(default)] 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 562db29f75e74..a6c1dc99eab64 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 @@ -1,4 +1,4 @@ -use crate::cube_bridge::base_query_options::{BaseQueryOptions, MaskedMemberItem}; +use crate::cube_bridge::base_query_options::{BaseQueryOptions, FilterValue, MaskedMemberItem}; use crate::cube_bridge::join_hints::JoinHintItem; use crate::cube_bridge::options_member::OptionsMember; use crate::logical_plan::PreAggregationUsage; @@ -728,17 +728,22 @@ impl TestContext { } #[cfg(feature = "integration-postgres")] - fn inline_params(sql: &str, params: &[String]) -> String { + fn inline_params(sql: &str, params: &[FilterValue]) -> String { let mut result = sql.to_string(); for (i, param) in params.iter().enumerate().rev() { let placeholder = format!("${}", i + 1); - let escaped = param.replace('\'', "''"); - result = result.replace(&placeholder, &format!("'{}'", escaped)); + // `Null` must inline as the bare SQL keyword; every other variant is + // rendered through its canonical string form and quoted. + let literal = match param.to_param_string() { + Some(value) => format!("'{}'", value.replace('\'', "''")), + None => "NULL".to_string(), + }; + result = result.replace(&placeholder, &literal); } result } - pub fn build_filter_sql(&self, yaml: &str) -> Result<(String, Vec), CubeError> { + pub fn build_filter_sql(&self, yaml: &str) -> Result<(String, Vec), CubeError> { let props = self.create_query_properties(yaml)?; let filter = Filter { @@ -769,7 +774,7 @@ impl TestContext { pub fn build_base_filter_sql( &self, base_filter: &Rc, - ) -> Result<(String, Vec), CubeError> { + ) -> Result<(String, Vec), CubeError> { let nodes_factory = SqlNodesFactory::default(); let context = Rc::new(VisitorContext::new( self.query_tools.query_tools().clone(), diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/cube_evaluator/symbol_evaluator.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/cube_evaluator/symbol_evaluator.rs index e587fbd9c91fd..a56cbccfc61cf 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/cube_evaluator/symbol_evaluator.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/cube_evaluator/symbol_evaluator.rs @@ -270,7 +270,7 @@ fn masked_sum_measure_returns_mask_literal() { #[test] fn masked_aggregate_measure_with_filter_not_in_group_by_renders_mask() { - use crate::cube_bridge::base_query_options::{FilterItem, MaskedMemberItem}; + use crate::cube_bridge::base_query_options::{FilterItem, FilterValue, MaskedMemberItem}; let schema = MockSchema::from_yaml_file("symbol_evaluator/masking_test.yaml"); // sum_revenue is masked with a row-level filter on public_dim. The aggregate @@ -287,7 +287,7 @@ fn masked_aggregate_measure_with_filter_not_in_group_by_renders_mask() { member: Some("masking_cube.public_dim".to_string()), dimension: None, operator: Some("equals".to_string()), - values: Some(vec![Some("active".to_string())]), + values: Some(vec![FilterValue::Str("active".to_string())]), }), }], ) diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/mod.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/mod.rs index 1af1f15a88691..debdf0463eebe 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/mod.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/mod.rs @@ -4,10 +4,11 @@ mod to_sql_timezone; mod tree_ops; mod use_raw_values; +use crate::cube_bridge::base_query_options::FilterValue; use crate::test_fixtures::cube_bridge::MockSchema; use crate::test_fixtures::test_utils::TestContext; -pub fn build_filter(schema_file: &str, filter_yaml: &str) -> (String, Vec) { +pub fn build_filter(schema_file: &str, filter_yaml: &str) -> (String, Vec) { let schema = MockSchema::from_yaml_file(schema_file); let ctx = TestContext::new(schema).unwrap(); @@ -16,9 +17,18 @@ pub fn build_filter(schema_file: &str, filter_yaml: &str) -> (String, Vec), expected_sql: &str, expected_params: &[&str]) { +pub fn assert_filter( + result: &(String, Vec), + expected_sql: &str, + expected_params: &[&str], +) { assert_eq!(result.0, expected_sql, "SQL mismatch"); - let params: Vec<&str> = result.1.iter().map(|s| s.as_str()).collect(); + let owned: Vec = result + .1 + .iter() + .map(|v| v.to_param_string().unwrap_or_else(|| "NULL".to_string())) + .collect(); + let params: Vec<&str> = owned.iter().map(String::as_str).collect(); assert_eq!( params, expected_params, "Params mismatch for SQL: {}", diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/partition_range.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/partition_range.rs index 25892b3e8f37c..f58add5e94a8b 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/partition_range.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/partition_range.rs @@ -1,13 +1,14 @@ use super::assert_filter; +use crate::cube_bridge::base_query_options::FilterValue; use crate::test_fixtures::cube_bridge::{MockDriverTools, MockSchema}; use crate::test_fixtures::test_utils::TestContext; use indoc::indoc; -fn build(filter_yaml: &str) -> (String, Vec) { +fn build(filter_yaml: &str) -> (String, Vec) { super::build_filter("common/visitors.yaml", filter_yaml) } -fn build_with_visible_tz(filter_yaml: &str) -> (String, Vec) { +fn build_with_visible_tz(filter_yaml: &str) -> (String, Vec) { let schema = MockSchema::from_yaml_file("common/visitors.yaml"); let driver = MockDriverTools::with_timezone("America/Los_Angeles".to_string()) .with_visible_in_db_time_zone(); diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/to_sql.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/to_sql.rs index 2960a02e66413..69a6f908bbfaf 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/to_sql.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/to_sql.rs @@ -1,7 +1,8 @@ use super::{assert_filter, build_filter}; +use crate::cube_bridge::base_query_options::FilterValue; use indoc::indoc; -fn build(filter_yaml: &str) -> (String, Vec) { +fn build(filter_yaml: &str) -> (String, Vec) { build_filter("common/visitors.yaml", filter_yaml) } @@ -47,6 +48,108 @@ fn test_equals_boolean() { ); } +#[test] +fn test_equals_boolean_native() { + let result = build(indoc! {" + filters: + - dimension: visitors.is_active + operator: equals + values: + - true + "}); + assert_filter( + &result, + r#"("visitors".is_active = $_0_$::boolean)"#, + &["true"], + ); +} + +#[test] +fn test_equals_boolean_native_false() { + let result = build(indoc! {" + filters: + - dimension: visitors.is_active + operator: equals + values: + - false + "}); + assert_filter( + &result, + r#"("visitors".is_active = $_0_$::boolean)"#, + &["false"], + ); +} + +#[test] +fn test_not_equals_boolean_native() { + let result = build(indoc! {" + filters: + - dimension: visitors.is_active + operator: notEquals + values: + - true + "}); + assert_filter( + &result, + r#"("visitors".is_active <> $_0_$::boolean OR "visitors".is_active IS NULL)"#, + &["true"], + ); +} + +#[test] +fn test_equals_number_native() { + let result = build(indoc! {" + filters: + - dimension: visitors.id + operator: equals + values: + - 42 + "}); + assert_filter(&result, r#"("visitors".id = $_0_$::numeric)"#, &["42"]); +} + +#[test] +fn test_equals_number_native_fractional() { + let result = build(indoc! {" + filters: + - dimension: visitors.id + operator: equals + values: + - 42.5 + "}); + assert_filter(&result, r#"("visitors".id = $_0_$::numeric)"#, &["42.5"]); +} + +#[test] +fn test_in_numbers_native() { + let result = build(indoc! {" + filters: + - dimension: visitors.id + operator: in + values: + - 1 + - 2 + - 3 + "}); + assert_filter( + &result, + r#"("visitors".id IN ($_0_$::numeric, $_1_$::numeric, $_2_$::numeric))"#, + &["1", "2", "3"], + ); +} + +#[test] +fn test_gt_number_native() { + let result = build(indoc! {" + filters: + - dimension: visitors.id + operator: gt + values: + - 100 + "}); + assert_filter(&result, r#"("visitors".id > $_0_$::numeric)"#, &["100"]); +} + #[test] fn test_equals_multiple_values() { let result = build(indoc! {" diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/to_sql_timezone.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/to_sql_timezone.rs index b5db2d92e07e4..eea89f83007f3 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/to_sql_timezone.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/to_sql_timezone.rs @@ -1,9 +1,10 @@ use super::assert_filter; +use crate::cube_bridge::base_query_options::FilterValue; use crate::test_fixtures::cube_bridge::{MockDriverTools, MockSchema}; use crate::test_fixtures::test_utils::TestContext; use indoc::indoc; -fn build_with_visible_tz(filter_yaml: &str) -> (String, Vec) { +fn build_with_visible_tz(filter_yaml: &str) -> (String, Vec) { let schema = MockSchema::from_yaml_file("common/visitors.yaml"); let driver = MockDriverTools::with_timezone("America/Los_Angeles".to_string()) .with_visible_in_db_time_zone(); 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 6fa881ff2fda1..d16a612572a22 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/tree_ops.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter/tree_ops.rs @@ -1,3 +1,4 @@ +use crate::cube_bridge::base_query_options::FilterValue; use crate::planner::filter::base_filter::{BaseFilter, FilterType}; use crate::planner::filter::base_segment::BaseSegment; use crate::planner::filter::filter_operator::FilterOperator; @@ -18,7 +19,7 @@ fn make_dim_filter(ctx: &TestContext, member_path: &str, value: &str) -> Rc TestContext { TestContext::new(schema).unwrap() } -fn assert_raw(result: &(String, Vec), expected_sql: &str) { +fn assert_raw(result: &(String, Vec), expected_sql: &str) { assert_eq!(result.0, expected_sql, "SQL mismatch"); assert!( result.1.is_empty(), @@ -28,8 +29,8 @@ fn test_in_date_range_use_raw_values() { FilterType::Dimension, FilterOperator::InDateRange, Some(vec![ - Some("2024-01-01".to_string()), - Some("2024-12-31".to_string()), + FilterValue::Str("2024-01-01".to_string()), + FilterValue::Str("2024-12-31".to_string()), ]), None, ) @@ -39,8 +40,8 @@ fn test_in_date_range_use_raw_values() { .change_operator( FilterOperator::InDateRange, vec![ - Some("(SELECT min(df) FROM cte)".to_string()), - Some("(SELECT max(dt) FROM cte)".to_string()), + FilterValue::Str("(SELECT min(df) FROM cte)".to_string()), + FilterValue::Str("(SELECT max(dt) FROM cte)".to_string()), ], true, ctx.query_tools().query_tools().clone(), @@ -66,8 +67,8 @@ fn test_not_in_date_range_use_raw_values() { FilterType::Dimension, FilterOperator::NotInDateRange, Some(vec![ - Some("2024-01-01".to_string()), - Some("2024-12-31".to_string()), + FilterValue::Str("2024-01-01".to_string()), + FilterValue::Str("2024-12-31".to_string()), ]), None, ) @@ -77,8 +78,8 @@ fn test_not_in_date_range_use_raw_values() { .change_operator( FilterOperator::NotInDateRange, vec![ - Some("(SELECT min(df) FROM cte)".to_string()), - Some("(SELECT max(dt) FROM cte)".to_string()), + FilterValue::Str("(SELECT min(df) FROM cte)".to_string()), + FilterValue::Str("(SELECT max(dt) FROM cte)".to_string()), ], true, ctx.query_tools().query_tools().clone(), @@ -104,8 +105,8 @@ fn test_before_or_on_date_use_raw_values() { FilterType::Dimension, FilterOperator::InDateRange, Some(vec![ - Some("2024-01-01".to_string()), - Some("2024-12-31".to_string()), + FilterValue::Str("2024-01-01".to_string()), + FilterValue::Str("2024-12-31".to_string()), ]), None, ) @@ -114,7 +115,7 @@ fn test_before_or_on_date_use_raw_values() { let raw_filter = filter .change_operator( FilterOperator::BeforeOrOnDate, - vec![Some("(SELECT max(dt) FROM cte)".to_string())], + vec![FilterValue::Str("(SELECT max(dt) FROM cte)".to_string())], true, ctx.query_tools().query_tools().clone(), None, @@ -139,8 +140,8 @@ fn test_after_or_on_date_use_raw_values() { FilterType::Dimension, FilterOperator::InDateRange, Some(vec![ - Some("2024-01-01".to_string()), - Some("2024-12-31".to_string()), + FilterValue::Str("2024-01-01".to_string()), + FilterValue::Str("2024-12-31".to_string()), ]), None, ) @@ -149,7 +150,7 @@ fn test_after_or_on_date_use_raw_values() { let raw_filter = filter .change_operator( FilterOperator::AfterOrOnDate, - vec![Some("(SELECT min(df) FROM cte)".to_string())], + vec![FilterValue::Str("(SELECT min(df) FROM cte)".to_string())], true, ctx.query_tools().query_tools().clone(), None, diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/member_expressions.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/member_expressions.rs index be41a92022834..1f48d61fe8c76 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/member_expressions.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/member_expressions.rs @@ -96,7 +96,9 @@ async fn test_expr_with_filter() { member: None, dimension: Some("orders.status".to_string()), operator: Some("equals".to_string()), - values: Some(vec![Some("completed".to_string())]), + values: Some(vec![ + crate::cube_bridge::base_query_options::FilterValue::Str("completed".to_string()), + ]), or: None, and: None, }; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/utils/debug.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/utils/debug.rs index 39fd5e2af67ca..45da06667551c 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/utils/debug.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/utils/debug.rs @@ -1,9 +1,11 @@ +use crate::cube_bridge::base_query_options::FilterValue; use crate::planner::filter::{ BaseFilter, FilterGroup, FilterGroupOperator, FilterItem, FilterOperator, }; use crate::planner::DebugSql; use crate::test_fixtures::cube_bridge::MockSchema; use crate::test_fixtures::test_utils::TestContext; +use cubenativeutils::CubeError; #[test] fn test_dimension_basic() { @@ -139,7 +141,7 @@ fn test_filter_simple_collapsed() { symbol, crate::planner::filter::base_filter::FilterType::Dimension, FilterOperator::Equal, - Some(vec![Some("google".to_string())]), + Some(vec![FilterValue::Str("google".to_string())]), None, ) .unwrap(); @@ -159,7 +161,7 @@ fn test_filter_simple_expanded() { symbol, crate::planner::filter::base_filter::FilterType::Dimension, FilterOperator::Equal, - Some(vec![Some("google".to_string())]), + Some(vec![FilterValue::Str("google".to_string())]), None, ) .unwrap(); @@ -168,6 +170,53 @@ fn test_filter_simple_expanded() { assert_eq!(sql, "source equals: ['google']"); } +#[test] +fn test_filter_bool_and_num_values_unquoted() -> Result<(), CubeError> { + let schema = MockSchema::from_yaml_file("common/visitors.yaml"); + let ctx = TestContext::new(schema)?; + let symbol = ctx.create_symbol("visitors.id")?; + + // Booleans and numbers render bare; only strings are quoted. + let filter = BaseFilter::try_new( + ctx.query_tools().query_tools().clone(), + symbol, + crate::planner::filter::base_filter::FilterType::Dimension, + FilterOperator::In, + Some(vec![ + FilterValue::Num(42.0), + FilterValue::Bool(true), + FilterValue::Str("google".to_string()), + ]), + None, + )?; + + let sql = filter.debug_sql(false); + assert_eq!(sql, "{visitors.id} in: [42, true, 'google']"); + + Ok(()) +} + +#[test] +fn test_filter_null_value_renders_null() -> Result<(), CubeError> { + let schema = MockSchema::from_yaml_file("common/visitors.yaml"); + let ctx = TestContext::new(schema)?; + let symbol = ctx.create_symbol("visitors.source")?; + + let filter = BaseFilter::try_new( + ctx.query_tools().query_tools().clone(), + symbol, + crate::planner::filter::base_filter::FilterType::Dimension, + FilterOperator::Equal, + Some(vec![FilterValue::Null]), + None, + )?; + + let sql = filter.debug_sql(false); + assert_eq!(sql, "{visitors.source} equals: [NULL]"); + + Ok(()) +} + #[test] fn test_filter_group_and_collapsed() { let schema = MockSchema::from_yaml_file("common/visitors.yaml"); @@ -180,7 +229,7 @@ fn test_filter_group_and_collapsed() { source_symbol, crate::planner::filter::base_filter::FilterType::Dimension, FilterOperator::Equal, - Some(vec![Some("google".to_string())]), + Some(vec![FilterValue::Str("google".to_string())]), None, ) .unwrap(); @@ -190,7 +239,7 @@ fn test_filter_group_and_collapsed() { id_symbol, crate::planner::filter::base_filter::FilterType::Dimension, FilterOperator::Gt, - Some(vec![Some("100".to_string())]), + Some(vec![FilterValue::Str("100".to_string())]), None, ) .unwrap();