From fe8d90a0daf6d66db801c0a6f2c55a56485ffd77 Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Fri, 6 Sep 2024 17:46:02 +0300 Subject: [PATCH 1/3] ci(cubesql): Attach cubenativeutils and cubesqlplanner to cargo clippy --- .github/workflows/rust-cubesql.yml | 4 +++ rust/cubenativeutils/Cargo.toml | 13 ++++++++ rust/cubesqlplanner/cubesqlplanner/Cargo.toml | 31 +++++++++++++++++++ rust/cubesqlplanner/nativebridge/Cargo.toml | 14 +++++++++ 4 files changed, 62 insertions(+) diff --git a/.github/workflows/rust-cubesql.yml b/.github/workflows/rust-cubesql.yml index 2966c7847cb49..87644282c5e49 100644 --- a/.github/workflows/rust-cubesql.yml +++ b/.github/workflows/rust-cubesql.yml @@ -57,6 +57,10 @@ jobs: run: cd packages/cubejs-backend-native && cargo clippy --locked --workspace --all-targets --keep-going -- -D warnings - name: Clippy Native (with Python) run: cd packages/cubejs-backend-native && cargo clippy --locked --workspace --all-targets --keep-going --features python -- -D warnings + - name: Clippy cubenativeutils + run: cd rust/cubenativeutils && cargo clippy --locked --workspace --all-targets --keep-going -- -D warnings + - name: Clippy cubesqlplanner + run: cd rust/cubesqlplanner && cargo clippy --locked --workspace --all-targets --keep-going -- -D warnings unit: # We use host instead of cross container, because it's much faster diff --git a/rust/cubenativeutils/Cargo.toml b/rust/cubenativeutils/Cargo.toml index 1e4e18574a8e2..c0ee2a18858f4 100644 --- a/rust/cubenativeutils/Cargo.toml +++ b/rust/cubenativeutils/Cargo.toml @@ -23,3 +23,16 @@ convert_case = "0.6.0" version = "=1" default-features = false features = ["napi-1", "napi-4", "napi-6", "futures"] + +# Code in cubenativeutils crate is not ready for full-blown clippy +# So we disable some rules to enable per-rule latch in CI, not for a whole clippy run +# Feel free to remove any rule from here and fix all warnings with it +# Or to write a comment why rule should stay disabled +[lints.clippy] +clone_on_copy = "allow" +len_without_is_empty = "allow" +module_inception = "allow" +multiple_bound_locations = "allow" +result_large_err = "allow" +unnecessary_cast = "allow" +useless_format = "allow" diff --git a/rust/cubesqlplanner/cubesqlplanner/Cargo.toml b/rust/cubesqlplanner/cubesqlplanner/Cargo.toml index c9774942bd4bd..c2cd838c2b149 100644 --- a/rust/cubesqlplanner/cubesqlplanner/Cargo.toml +++ b/rust/cubesqlplanner/cubesqlplanner/Cargo.toml @@ -25,3 +25,34 @@ regex = "1.3.9" version = "=1" default-features = false features = ["napi-1", "napi-4", "napi-6", "futures"] + +# Code in cubesqlplanner workspace is not ready for full-blown clippy +# So we disable some rules to enable per-rule latch in CI, not for a whole clippy run +# Feel free to remove any rule from here and fix all warnings with it +# Or to write a comment why rule should stay disabled +[lints.clippy] +clone_on_copy = "allow" +collapsible_else_if = "allow" +default_constructed_unit_structs = "allow" +enum_variant_names = "allow" +filter-map-identity = "allow" +let_and_return = "allow" +map_clone = "allow" +manual_map = "allow" +manual_unwrap_or_default = "allow" +match_like_matches_macro = "allow" +needless-bool = "allow" +needless_borrow = "allow" +needless_question_mark = "allow" +neg_multiply = "allow" +new_without_default = "allow" +only_used_in_recursion = "allow" +ptr_arg = "allow" +redundant_closure = "allow" +result_large_err = "allow" +should_implement_trait = "allow" +to_string_in_format_args = "allow" +too-many-arguments = "allow" +useless_conversion = "allow" +useless_format = "allow" +vec_init_then_push = "allow" diff --git a/rust/cubesqlplanner/nativebridge/Cargo.toml b/rust/cubesqlplanner/nativebridge/Cargo.toml index f641657712a28..aa823c3990392 100644 --- a/rust/cubesqlplanner/nativebridge/Cargo.toml +++ b/rust/cubesqlplanner/nativebridge/Cargo.toml @@ -25,3 +25,17 @@ features = ["full"] [dependencies.async-trait] version = "0.1.42" + +# Code in cubesqlplanner workspace is not ready for full-blown clippy +# So we disable some rules to enable per-rule latch in CI, not for a whole clippy run +# Feel free to remove any rule from here and fix all warnings with it +# Or to write a comment why rule should stay disabled +[lints.clippy] +cmp_owned = "allow" +collapsible_match = "allow" +len_zero = "allow" +let_and_return = "allow" +needless_borrow = "allow" +ptr_arg = "allow" +redundant_closure = "allow" +single_match = "allow" From c2e21f1662f99f94b6981a553d7079da55e9a401 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Fri, 6 Jun 2025 18:16:38 +0300 Subject: [PATCH 2/3] fix some clippy warnings and allow the rest. --- rust/cubesqlplanner/cubesqlplanner/Cargo.toml | 8 ++++++++ .../pre_aggregation/measure_matcher.rs | 6 ++---- .../optimizers/pre_aggregation/optimizer.rs | 8 +++----- .../src/physical_plan_builder/builder.rs | 16 +++------------- .../cubesqlplanner/src/plan/builder/select.rs | 7 ++----- .../cubesqlplanner/src/planner/base_dimension.rs | 4 ++-- .../cubesqlplanner/src/planner/base_measure.rs | 10 +++++----- .../src/planner/filter/base_filter.rs | 12 ++++++------ .../multi_stage/rolling_window_planner.rs | 4 ++-- .../src/planner/query_properties.rs | 14 +++++--------- .../collectors/has_cumulative_members.rs | 9 +++------ .../collectors/sub_query_dimensions.rs | 7 ++----- .../src/planner/sql_evaluator/sql_call.rs | 9 +++------ .../src/planner/sql_templates/plan.rs | 4 ++-- .../src/planner/time_dimension/date_time.rs | 2 +- .../planner/time_dimension/date_time_helper.rs | 2 +- .../planner/time_dimension/granularity_helper.rs | 10 +++++----- .../src/planner/time_dimension/sql_interval.rs | 15 +-------------- .../src/planner/visitor_context.rs | 8 +------- 19 files changed, 57 insertions(+), 98 deletions(-) diff --git a/rust/cubesqlplanner/cubesqlplanner/Cargo.toml b/rust/cubesqlplanner/cubesqlplanner/Cargo.toml index c2cd838c2b149..260ad6131a729 100644 --- a/rust/cubesqlplanner/cubesqlplanner/Cargo.toml +++ b/rust/cubesqlplanner/cubesqlplanner/Cargo.toml @@ -56,3 +56,11 @@ too-many-arguments = "allow" useless_conversion = "allow" useless_format = "allow" vec_init_then_push = "allow" +type_complexity = "allow" +if_same_then_else = "allow" +to_string_trait_impl = "allow" +field_reassign_with_default = "allow" +collapsible_match = "allow" +wrong_self_convention = "allow" +module_inception = "allow" +comparison_chain = "allow" diff --git a/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/measure_matcher.rs b/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/measure_matcher.rs index e5806db441222..32b8974f0f565 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/measure_matcher.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/measure_matcher.rs @@ -24,10 +24,8 @@ impl MeasureMatcher { pub fn try_match(&self, symbol: &Rc) -> Result { match symbol.as_ref() { MemberSymbol::Measure(measure) => { - if self.pre_aggregation_measures.contains(&measure.full_name()) { - if !self.only_addictive || measure.is_addictive() { - return Ok(true); - } + if self.pre_aggregation_measures.contains(&measure.full_name()) && (!self.only_addictive || measure.is_addictive()) { + return Ok(true); } } MemberSymbol::MemberExpression(_) => { diff --git a/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/optimizer.rs b/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/optimizer.rs index 72a44fe013309..6778e4f32b910 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/optimizer.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/optimizer.rs @@ -23,7 +23,7 @@ impl MatchState { if matches!(self, MatchState::Partial) || matches!(other, MatchState::Partial) { return MatchState::Partial; } - return MatchState::Full; + MatchState::Full } } @@ -233,9 +233,7 @@ impl PreAggregationOptimizer { } if let Some(multi_stage_item) = multi_stage_queries - .iter() - .cloned() - .find(|query| &query.name == multi_stage_name) + .iter().find(|&query| &query.name == multi_stage_name).cloned() { match &multi_stage_item.member_type { MultiStageMemberLogicalType::LeafMeasure(multi_stage_leaf_measure) => self @@ -432,7 +430,7 @@ impl PreAggregationOptimizer { .map(|(d, _)| d.clone()), ) .collect(), - measures: pre_aggregation.measures.iter().cloned().collect(), + measures: pre_aggregation.measures.to_vec(), multiplied_measures: HashSet::new(), }; let pre_aggregation = PreAggregation { diff --git a/rust/cubesqlplanner/cubesqlplanner/src/physical_plan_builder/builder.rs b/rust/cubesqlplanner/cubesqlplanner/src/physical_plan_builder/builder.rs index 8e368689f19b3..1e3cc233c49b5 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/physical_plan_builder/builder.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/physical_plan_builder/builder.rs @@ -16,10 +16,11 @@ use itertools::Itertools; use std::collections::HashMap; use std::collections::HashSet; use std::rc::Rc; -const TOTAL_COUNT: &'static str = "total_count"; -const ORIGINAL_QUERY: &'static str = "original_query"; +const TOTAL_COUNT: &str = "total_count"; +const ORIGINAL_QUERY: &str = "original_query"; #[derive(Clone, Debug)] +#[derive(Default)] struct PhysicalPlanBuilderContext { pub alias_prefix: Option, pub render_measure_as_state: bool, //Render measure as state, for example hll state for count_approx @@ -28,17 +29,6 @@ struct PhysicalPlanBuilderContext { pub original_sql_pre_aggregations: HashMap, } -impl Default for PhysicalPlanBuilderContext { - fn default() -> Self { - Self { - alias_prefix: None, - render_measure_as_state: false, - render_measure_for_ungrouped: false, - time_shifts: HashMap::new(), - original_sql_pre_aggregations: HashMap::new(), - } - } -} impl PhysicalPlanBuilderContext { pub fn make_sql_nodes_factory(&self) -> SqlNodesFactory { diff --git a/rust/cubesqlplanner/cubesqlplanner/src/plan/builder/select.rs b/rust/cubesqlplanner/cubesqlplanner/src/plan/builder/select.rs index 9e8bee36d67c5..1b2cd5c521996 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/plan/builder/select.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/plan/builder/select.rs @@ -203,11 +203,8 @@ impl SelectBuilder { source: &SingleAliasedSource, refs: &mut HashMap, ) { - match &source.source { - SingleSource::Cube(cube) => { - refs.insert(cube.name().clone(), source.alias.clone()); - } - _ => {} + if let SingleSource::Cube(cube) = &source.source { + refs.insert(cube.name().clone(), source.alias.clone()); } } diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/base_dimension.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/base_dimension.rs index 300a8798e0ab3..519916a988f50 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/base_dimension.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/base_dimension.rs @@ -178,11 +178,11 @@ impl BaseDimension { pub fn is_sub_query(&self) -> bool { self.definition .as_ref() - .map_or(false, |def| def.static_data().sub_query.unwrap_or(false)) + .is_some_and(|def| def.static_data().sub_query.unwrap_or(false)) } pub fn propagate_filters_to_sub_query(&self) -> bool { - self.definition.as_ref().map_or(false, |def| { + self.definition.as_ref().is_some_and(|def| { def.static_data() .propagate_filters_to_sub_query .unwrap_or(false) diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/base_measure.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/base_measure.rs index 4c14fb5560ba3..e7ca177a75eff 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/base_measure.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/base_measure.rs @@ -185,19 +185,19 @@ impl BaseMeasure { pub fn reduce_by(&self) -> Option> { self.definition .as_ref() - .map_or(None, |d| d.static_data().reduce_by_references.clone()) + .and_then(|d| d.static_data().reduce_by_references.clone()) } pub fn add_group_by(&self) -> Option> { self.definition .as_ref() - .map_or(None, |d| d.static_data().add_group_by_references.clone()) + .and_then(|d| d.static_data().add_group_by_references.clone()) } pub fn group_by(&self) -> Option> { self.definition .as_ref() - .map_or(None, |d| d.static_data().group_by_references.clone()) + .and_then(|d| d.static_data().group_by_references.clone()) } //FIXME dublicate with symbol @@ -218,13 +218,13 @@ impl BaseMeasure { pub fn is_multi_stage(&self) -> bool { self.definition .as_ref() - .map_or(false, |d| d.static_data().multi_stage.unwrap_or(false)) + .is_some_and(|d| d.static_data().multi_stage.unwrap_or(false)) } pub fn rolling_window(&self) -> Option { self.definition .as_ref() - .map_or(None, |d| d.static_data().rolling_window.clone()) + .and_then(|d| d.static_data().rolling_window.clone()) } pub fn is_rolling_window(&self) -> bool { diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/filter/base_filter.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/filter/base_filter.rs index aad4c13b1db62..fe3fa953d4bd8 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/filter/base_filter.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/filter/base_filter.rs @@ -8,9 +8,9 @@ use crate::planner::{evaluate_with_context, FiltersContext, VisitorContext}; use cubenativeutils::CubeError; use std::rc::Rc; -const FROM_PARTITION_RANGE: &'static str = "__FROM_PARTITION_RANGE"; +const FROM_PARTITION_RANGE: &str = "__FROM_PARTITION_RANGE"; -const TO_PARTITION_RANGE: &'static str = "__TO_PARTITION_RANGE"; +const TO_PARTITION_RANGE: &str = "__TO_PARTITION_RANGE"; #[derive(Debug, Clone, PartialEq, Eq)] pub enum FilterType { @@ -589,7 +589,7 @@ impl BaseFilter { } let from = self.format_from_date(value)?; - let res = if use_db_time_zone && &from != FROM_PARTITION_RANGE { + let res = if use_db_time_zone && from != FROM_PARTITION_RANGE { self.query_tools.base_tools().in_db_time_zone(from)? } else { from @@ -607,7 +607,7 @@ impl BaseFilter { } let from = self.format_to_date(value)?; - let res = if use_db_time_zone && &from != TO_PARTITION_RANGE { + let res = if use_db_time_zone && from != TO_PARTITION_RANGE { self.query_tools.base_tools().in_db_time_zone(from)? } else { from @@ -705,10 +705,10 @@ impl BaseFilter { as_date_time, ) } else { - return Err(CubeError::user(format!( + Err(CubeError::user(format!( "Arguments for timestamp parameter for operator {} is not valid", self.filter_operator().to_string() - ))); + ))) } } } diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/rolling_window_planner.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/rolling_window_planner.rs index bc705cce3139f..17159420954b4 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/rolling_window_planner.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/rolling_window_planner.rs @@ -63,7 +63,7 @@ impl RollingWindowPlanner { } } - if time_dimensions.len() == 0 { + if time_dimensions.is_empty() { let rolling_base = self.add_rolling_window_base( member.clone(), state.clone(), @@ -254,7 +254,7 @@ impl RollingWindowPlanner { let is_to_date = rolling_window .rolling_type .as_ref() - .map_or(false, |tp| tp == "to_date"); + .is_some_and(|tp| tp == "to_date"); if is_to_date { if let Some(granularity) = &rolling_window.granularity { diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs index 15e8ab826b39b..7418535d9acbb 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs @@ -479,7 +479,7 @@ impl QueryProperties { .into_values() .map(|measures_and_join| { ( - measures_and_join.iter().next().unwrap().1 .1.clone(), + measures_and_join.first().unwrap().1 .1.clone(), measures_and_join .into_iter() .flat_map(|m| m.0) @@ -506,7 +506,7 @@ impl QueryProperties { .join(", ") ))); } - Ok(self.multi_fact_join_groups.iter().next().unwrap().0.clone()) + Ok(self.multi_fact_join_groups.first().unwrap().0.clone()) } pub fn measures(&self) -> &Vec> { @@ -796,9 +796,7 @@ impl QueryProperties { result.multi_stage_measures.push(m.clone()) } else { let join = self - .compute_join_multi_fact_groups_with_measures(&vec![m.clone()])? - .iter() - .next() + .compute_join_multi_fact_groups_with_measures(&vec![m.clone()])?.first() .expect("No join groups returned for single measure multi-fact join group") .0 .clone(); @@ -845,10 +843,8 @@ impl QueryProperties { } FilterItem::Item(item) => { let item_member_name = item.member_name(); - if measures - .iter() - .find(|m| m.full_name() == item_member_name) - .is_none() + if !measures + .iter().any(|m| m.full_name() == item_member_name) { measures.push(BaseMeasure::try_new_required( item.member_evaluator().clone(), diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/collectors/has_cumulative_members.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/collectors/has_cumulative_members.rs index c73c578680c46..5848fc779ce17 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/collectors/has_cumulative_members.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/collectors/has_cumulative_members.rs @@ -26,13 +26,10 @@ impl TraversalVisitor for HasCumulativeMembersCollector { _path: &Vec, _: &Self::State, ) -> Result, CubeError> { - match node.as_ref() { - MemberSymbol::Measure(s) => { - if s.is_rolling_window() { - self.has_cumulative_members = true; - } + if let MemberSymbol::Measure(s) = node.as_ref() { + if s.is_rolling_window() { + self.has_cumulative_members = true; } - _ => {} }; if self.has_cumulative_members { Ok(None) diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/collectors/sub_query_dimensions.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/collectors/sub_query_dimensions.rs index c081e87c18998..e0540f4f89edd 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/collectors/sub_query_dimensions.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/collectors/sub_query_dimensions.rs @@ -27,12 +27,9 @@ impl SubQueryDimensionsCollector { fn check_dim_has_measures(&self, dim: &DimensionSymbol) -> bool { for dep in dim.get_dependencies().iter() { - match dep.as_ref() { - MemberSymbol::Measure(_) => return true, - _ => {} - } + if let MemberSymbol::Measure(_) = dep.as_ref() { return true } } - return false; + false } } diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_call.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_call.rs index 80132e0615f34..803faad127bf1 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_call.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_call.rs @@ -60,7 +60,7 @@ impl SqlCall { .collect::, _>>()?; let eval_result = self.member_sql.call(args)?; - Ok(eval_result.trim() == &reference_candidate.full_name()) + Ok(eval_result.trim() == reference_candidate.full_name()) } pub fn get_dependencies(&self) -> Vec> { @@ -180,11 +180,8 @@ impl SqlCall { result.push(cube_dep.cube_symbol.name()); for (_, v) in cube_dep.properties.iter() { - match v { - CubeDepProperty::CubeDependency(cube_dep) => { - self.extract_cube_deps_from_cube_dep(cube_dep, result) - } - _ => {} + if let CubeDepProperty::CubeDependency(cube_dep) = v { + self.extract_cube_deps_from_cube_dep(cube_dep, result) }; } } diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_templates/plan.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_templates/plan.rs index 7ca66569abd7a..564d1a57f3cac 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_templates/plan.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_templates/plan.rs @@ -15,7 +15,7 @@ pub struct PlanSqlTemplates { pub const UNDERSCORE_UPPER_BOUND: Boundary = Boundary { name: "UnderscoreUpper", condition: |s, _| { - s.get(0) == Some(&"_") + s.first() == Some(&"_") && s.get(1) .map(|c| c.to_uppercase() != c.to_lowercase() && *c == c.to_uppercase()) == Some(true) @@ -32,7 +32,7 @@ fn grapheme_is_uppercase(c: &&str) -> bool { pub const UPPER_UPPER_BOUND: Boundary = Boundary { name: "UpperUpper", condition: |s, _| { - s.get(0).map(grapheme_is_uppercase) == Some(true) + s.first().map(grapheme_is_uppercase) == Some(true) && s.get(1).map(grapheme_is_uppercase) == Some(true) }, arg: None, diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/date_time.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/date_time.rs index ae0cb5004045f..040b5ea7c52d7 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/date_time.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/date_time.rs @@ -117,7 +117,7 @@ impl QueryDateTime { pub fn add_duration(&self, duration: Duration) -> Result { let mut native = self.naive_local(); - native = native + duration; + native += duration; Self::from_local_date_time(self.date_time.timezone(), native) } diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/date_time_helper.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/date_time_helper.rs index 44011d4c9431a..5d281a0871107 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/date_time_helper.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/date_time_helper.rs @@ -63,7 +63,7 @@ impl QueryDateTimeHelper { // Ensure `high` is a valid local time (expand if needed) while let LocalResult::None = tz.from_local_datetime(&high) { - high = high + max_offset; + high += max_offset; } // Binary search for the first valid local time >= `naive` diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/granularity_helper.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/granularity_helper.rs index 8fc8be43e6355..89876ed317b54 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/granularity_helper.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/granularity_helper.rs @@ -72,15 +72,15 @@ impl GranularityHelper { pub fn granularity_from_interval(interval: &Option) -> Option { if let Some(interval) = interval { - if interval.find("day").is_some() { + if interval.contains("day") { Some("day".to_string()) - } else if interval.find("month").is_some() { + } else if interval.contains("month") { Some("month".to_string()) - } else if interval.find("year").is_some() { + } else if interval.contains("year") { Some("year".to_string()) - } else if interval.find("week").is_some() { + } else if interval.contains("week") { Some("week".to_string()) - } else if interval.find("hour").is_some() { + } else if interval.contains("hour") { Some("hour".to_string()) } else { None diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/sql_interval.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/sql_interval.rs index 1ebb228dc44b4..5aa5d8502f7a8 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/sql_interval.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/sql_interval.rs @@ -4,6 +4,7 @@ use std::ops::{Add, AddAssign, Neg, Sub}; use std::str::FromStr; #[derive(Debug, PartialEq, Clone, Hash, Eq)] +#[derive(Default)] pub struct SqlInterval { pub year: i32, pub month: i32, @@ -154,20 +155,6 @@ impl Neg for SqlInterval { } } -impl Default for SqlInterval { - fn default() -> Self { - Self { - second: 0, - minute: 0, - hour: 0, - day: 0, - week: 0, - month: 0, - year: 0, - } - } -} - impl FromStr for SqlInterval { type Err = CubeError; fn from_str(s: &str) -> Result { diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/visitor_context.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/visitor_context.rs index bbd924c5f79ff..0466d3b8f097a 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/visitor_context.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/visitor_context.rs @@ -7,17 +7,11 @@ use crate::planner::sql_templates::PlanSqlTemplates; use cubenativeutils::CubeError; use std::rc::Rc; +#[derive(Default)] pub struct FiltersContext { pub use_local_tz: bool, } -impl Default for FiltersContext { - fn default() -> Self { - Self { - use_local_tz: false, - } - } -} pub struct VisitorContext { node_processor: Rc, From e0ea17ca2f3bee772366d49ff1add4d2f15ed1d7 Mon Sep 17 00:00:00 2001 From: Konstantin Burkalev Date: Fri, 6 Jun 2025 18:19:23 +0300 Subject: [PATCH 3/3] cargo fmt --all --- .../optimizers/pre_aggregation/measure_matcher.rs | 4 +++- .../logical_plan/optimizers/pre_aggregation/optimizer.rs | 4 +++- .../cubesqlplanner/src/physical_plan_builder/builder.rs | 4 +--- .../cubesqlplanner/src/planner/query_properties.rs | 7 +++---- .../sql_evaluator/collectors/sub_query_dimensions.rs | 4 +++- .../src/planner/time_dimension/sql_interval.rs | 3 +-- .../cubesqlplanner/src/planner/visitor_context.rs | 1 - 7 files changed, 14 insertions(+), 13 deletions(-) diff --git a/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/measure_matcher.rs b/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/measure_matcher.rs index 32b8974f0f565..f09091d7582c9 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/measure_matcher.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/measure_matcher.rs @@ -24,7 +24,9 @@ impl MeasureMatcher { pub fn try_match(&self, symbol: &Rc) -> Result { match symbol.as_ref() { MemberSymbol::Measure(measure) => { - if self.pre_aggregation_measures.contains(&measure.full_name()) && (!self.only_addictive || measure.is_addictive()) { + if self.pre_aggregation_measures.contains(&measure.full_name()) + && (!self.only_addictive || measure.is_addictive()) + { return Ok(true); } } diff --git a/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/optimizer.rs b/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/optimizer.rs index 6778e4f32b910..cf1e80b170918 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/optimizer.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/optimizer.rs @@ -233,7 +233,9 @@ impl PreAggregationOptimizer { } if let Some(multi_stage_item) = multi_stage_queries - .iter().find(|&query| &query.name == multi_stage_name).cloned() + .iter() + .find(|&query| &query.name == multi_stage_name) + .cloned() { match &multi_stage_item.member_type { MultiStageMemberLogicalType::LeafMeasure(multi_stage_leaf_measure) => self diff --git a/rust/cubesqlplanner/cubesqlplanner/src/physical_plan_builder/builder.rs b/rust/cubesqlplanner/cubesqlplanner/src/physical_plan_builder/builder.rs index 1e3cc233c49b5..aa98fb7cd4b83 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/physical_plan_builder/builder.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/physical_plan_builder/builder.rs @@ -19,8 +19,7 @@ use std::rc::Rc; const TOTAL_COUNT: &str = "total_count"; const ORIGINAL_QUERY: &str = "original_query"; -#[derive(Clone, Debug)] -#[derive(Default)] +#[derive(Clone, Debug, Default)] struct PhysicalPlanBuilderContext { pub alias_prefix: Option, pub render_measure_as_state: bool, //Render measure as state, for example hll state for count_approx @@ -29,7 +28,6 @@ struct PhysicalPlanBuilderContext { pub original_sql_pre_aggregations: HashMap, } - impl PhysicalPlanBuilderContext { pub fn make_sql_nodes_factory(&self) -> SqlNodesFactory { let mut factory = SqlNodesFactory::new(); diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs index 7418535d9acbb..cdc6e055cec98 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs @@ -796,7 +796,8 @@ impl QueryProperties { result.multi_stage_measures.push(m.clone()) } else { let join = self - .compute_join_multi_fact_groups_with_measures(&vec![m.clone()])?.first() + .compute_join_multi_fact_groups_with_measures(&vec![m.clone()])? + .first() .expect("No join groups returned for single measure multi-fact join group") .0 .clone(); @@ -843,9 +844,7 @@ impl QueryProperties { } FilterItem::Item(item) => { let item_member_name = item.member_name(); - if !measures - .iter().any(|m| m.full_name() == item_member_name) - { + if !measures.iter().any(|m| m.full_name() == item_member_name) { measures.push(BaseMeasure::try_new_required( item.member_evaluator().clone(), self.query_tools.clone(), diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/collectors/sub_query_dimensions.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/collectors/sub_query_dimensions.rs index e0540f4f89edd..a3b4de986b798 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/collectors/sub_query_dimensions.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/collectors/sub_query_dimensions.rs @@ -27,7 +27,9 @@ impl SubQueryDimensionsCollector { fn check_dim_has_measures(&self, dim: &DimensionSymbol) -> bool { for dep in dim.get_dependencies().iter() { - if let MemberSymbol::Measure(_) = dep.as_ref() { return true } + if let MemberSymbol::Measure(_) = dep.as_ref() { + return true; + } } false } diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/sql_interval.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/sql_interval.rs index 5aa5d8502f7a8..f544dc27a9bda 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/sql_interval.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/sql_interval.rs @@ -3,8 +3,7 @@ use itertools::Itertools; use std::ops::{Add, AddAssign, Neg, Sub}; use std::str::FromStr; -#[derive(Debug, PartialEq, Clone, Hash, Eq)] -#[derive(Default)] +#[derive(Debug, PartialEq, Clone, Hash, Eq, Default)] pub struct SqlInterval { pub year: i32, pub month: i32, diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/visitor_context.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/visitor_context.rs index 0466d3b8f097a..2e72bebbc8a1f 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/visitor_context.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/visitor_context.rs @@ -12,7 +12,6 @@ pub struct FiltersContext { pub use_local_tz: bool, } - pub struct VisitorContext { node_processor: Rc, all_filters: Option, //To pass to FILTER_PARAMS and FILTER_GROUP