Skip to content

Commit 6616e36

Browse files
mcheshkovKSDaemon
mcheshkov
and
KSDaemon
authored
ci(cubesql): Attach cubenativeutils and cubesqlplanner to cargo fmt and cargo check (#8677)
* ci(cubesql): Attach cubenativeutils and cubesqlplanner to cargo clippy * fix some clippy warnings and allow the rest. * cargo fmt --all --------- Co-authored-by: Konstantin Burkalev <KSDaemon@gmail.com>
1 parent fe810c4 commit 6616e36

File tree

22 files changed

+121
-99
lines changed

22 files changed

+121
-99
lines changed

‎.github/workflows/rust-cubesql.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ jobs:
5757
run: cd packages/cubejs-backend-native && cargo clippy --locked --workspace --all-targets --keep-going -- -D warnings
5858
- name: Clippy Native (with Python)
5959
run: cd packages/cubejs-backend-native && cargo clippy --locked --workspace --all-targets --keep-going --features python -- -D warnings
60+
- name: Clippy cubenativeutils
61+
run: cd rust/cubenativeutils && cargo clippy --locked --workspace --all-targets --keep-going -- -D warnings
62+
- name: Clippy cubesqlplanner
63+
run: cd rust/cubesqlplanner && cargo clippy --locked --workspace --all-targets --keep-going -- -D warnings
6064

6165
unit:
6266
# We use host instead of cross container, because it's much faster

‎rust/cubenativeutils/Cargo.toml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,16 @@ convert_case = "0.6.0"
2323
version = "=1"
2424
default-features = false
2525
features = ["napi-1", "napi-4", "napi-6", "futures"]
26+
27+
# Code in cubenativeutils crate is not ready for full-blown clippy
28+
# So we disable some rules to enable per-rule latch in CI, not for a whole clippy run
29+
# Feel free to remove any rule from here and fix all warnings with it
30+
# Or to write a comment why rule should stay disabled
31+
[lints.clippy]
32+
clone_on_copy = "allow"
33+
len_without_is_empty = "allow"
34+
module_inception = "allow"
35+
multiple_bound_locations = "allow"
36+
result_large_err = "allow"
37+
unnecessary_cast = "allow"
38+
useless_format = "allow"

‎rust/cubesqlplanner/cubesqlplanner/Cargo.toml

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,42 @@ regex = "1.3.9"
2525
version = "=1"
2626
default-features = false
2727
features = ["napi-1", "napi-4", "napi-6", "futures"]
28+
29+
# Code in cubesqlplanner workspace is not ready for full-blown clippy
30+
# So we disable some rules to enable per-rule latch in CI, not for a whole clippy run
31+
# Feel free to remove any rule from here and fix all warnings with it
32+
# Or to write a comment why rule should stay disabled
33+
[lints.clippy]
34+
clone_on_copy = "allow"
35+
collapsible_else_if = "allow"
36+
default_constructed_unit_structs = "allow"
37+
enum_variant_names = "allow"
38+
filter-map-identity = "allow"
39+
let_and_return = "allow"
40+
map_clone = "allow"
41+
manual_map = "allow"
42+
manual_unwrap_or_default = "allow"
43+
match_like_matches_macro = "allow"
44+
needless-bool = "allow"
45+
needless_borrow = "allow"
46+
needless_question_mark = "allow"
47+
neg_multiply = "allow"
48+
new_without_default = "allow"
49+
only_used_in_recursion = "allow"
50+
ptr_arg = "allow"
51+
redundant_closure = "allow"
52+
result_large_err = "allow"
53+
should_implement_trait = "allow"
54+
to_string_in_format_args = "allow"
55+
too-many-arguments = "allow"
56+
useless_conversion = "allow"
57+
useless_format = "allow"
58+
vec_init_then_push = "allow"
59+
type_complexity = "allow"
60+
if_same_then_else = "allow"
61+
to_string_trait_impl = "allow"
62+
field_reassign_with_default = "allow"
63+
collapsible_match = "allow"
64+
wrong_self_convention = "allow"
65+
module_inception = "allow"
66+
comparison_chain = "allow"

‎rust/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/measure_matcher.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ impl MeasureMatcher {
2424
pub fn try_match(&self, symbol: &Rc<MemberSymbol>) -> Result<bool, CubeError> {
2525
match symbol.as_ref() {
2626
MemberSymbol::Measure(measure) => {
27-
if self.pre_aggregation_measures.contains(&measure.full_name()) {
28-
if !self.only_addictive || measure.is_addictive() {
29-
return Ok(true);
30-
}
27+
if self.pre_aggregation_measures.contains(&measure.full_name())
28+
&& (!self.only_addictive || measure.is_addictive())
29+
{
30+
return Ok(true);
3131
}
3232
}
3333
MemberSymbol::MemberExpression(_) => {

‎rust/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/optimizer.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ impl MatchState {
2323
if matches!(self, MatchState::Partial) || matches!(other, MatchState::Partial) {
2424
return MatchState::Partial;
2525
}
26-
return MatchState::Full;
26+
MatchState::Full
2727
}
2828
}
2929

@@ -234,8 +234,8 @@ impl PreAggregationOptimizer {
234234

235235
if let Some(multi_stage_item) = multi_stage_queries
236236
.iter()
237+
.find(|&query| &query.name == multi_stage_name)
237238
.cloned()
238-
.find(|query| &query.name == multi_stage_name)
239239
{
240240
match &multi_stage_item.member_type {
241241
MultiStageMemberLogicalType::LeafMeasure(multi_stage_leaf_measure) => self
@@ -432,7 +432,7 @@ impl PreAggregationOptimizer {
432432
.map(|(d, _)| d.clone()),
433433
)
434434
.collect(),
435-
measures: pre_aggregation.measures.iter().cloned().collect(),
435+
measures: pre_aggregation.measures.to_vec(),
436436
multiplied_measures: HashSet::new(),
437437
};
438438
let pre_aggregation = PreAggregation {

‎rust/cubesqlplanner/cubesqlplanner/src/physical_plan_builder/builder.rs

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@ use itertools::Itertools;
1616
use std::collections::HashMap;
1717
use std::collections::HashSet;
1818
use std::rc::Rc;
19-
const TOTAL_COUNT: &'static str = "total_count";
20-
const ORIGINAL_QUERY: &'static str = "original_query";
19+
const TOTAL_COUNT: &str = "total_count";
20+
const ORIGINAL_QUERY: &str = "original_query";
2121

22-
#[derive(Clone, Debug)]
22+
#[derive(Clone, Debug, Default)]
2323
struct PhysicalPlanBuilderContext {
2424
pub alias_prefix: Option<String>,
2525
pub render_measure_as_state: bool, //Render measure as state, for example hll state for count_approx
@@ -28,18 +28,6 @@ struct PhysicalPlanBuilderContext {
2828
pub original_sql_pre_aggregations: HashMap<String, String>,
2929
}
3030

31-
impl Default for PhysicalPlanBuilderContext {
32-
fn default() -> Self {
33-
Self {
34-
alias_prefix: None,
35-
render_measure_as_state: false,
36-
render_measure_for_ungrouped: false,
37-
time_shifts: HashMap::new(),
38-
original_sql_pre_aggregations: HashMap::new(),
39-
}
40-
}
41-
}
42-
4331
impl PhysicalPlanBuilderContext {
4432
pub fn make_sql_nodes_factory(&self) -> SqlNodesFactory {
4533
let mut factory = SqlNodesFactory::new();

‎rust/cubesqlplanner/cubesqlplanner/src/plan/builder/select.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -203,11 +203,8 @@ impl SelectBuilder {
203203
source: &SingleAliasedSource,
204204
refs: &mut HashMap<String, String>,
205205
) {
206-
match &source.source {
207-
SingleSource::Cube(cube) => {
208-
refs.insert(cube.name().clone(), source.alias.clone());
209-
}
210-
_ => {}
206+
if let SingleSource::Cube(cube) = &source.source {
207+
refs.insert(cube.name().clone(), source.alias.clone());
211208
}
212209
}
213210

‎rust/cubesqlplanner/cubesqlplanner/src/planner/base_dimension.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,11 +178,11 @@ impl BaseDimension {
178178
pub fn is_sub_query(&self) -> bool {
179179
self.definition
180180
.as_ref()
181-
.map_or(false, |def| def.static_data().sub_query.unwrap_or(false))
181+
.is_some_and(|def| def.static_data().sub_query.unwrap_or(false))
182182
}
183183

184184
pub fn propagate_filters_to_sub_query(&self) -> bool {
185-
self.definition.as_ref().map_or(false, |def| {
185+
self.definition.as_ref().is_some_and(|def| {
186186
def.static_data()
187187
.propagate_filters_to_sub_query
188188
.unwrap_or(false)

‎rust/cubesqlplanner/cubesqlplanner/src/planner/base_measure.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -185,19 +185,19 @@ impl BaseMeasure {
185185
pub fn reduce_by(&self) -> Option<Vec<String>> {
186186
self.definition
187187
.as_ref()
188-
.map_or(None, |d| d.static_data().reduce_by_references.clone())
188+
.and_then(|d| d.static_data().reduce_by_references.clone())
189189
}
190190

191191
pub fn add_group_by(&self) -> Option<Vec<String>> {
192192
self.definition
193193
.as_ref()
194-
.map_or(None, |d| d.static_data().add_group_by_references.clone())
194+
.and_then(|d| d.static_data().add_group_by_references.clone())
195195
}
196196

197197
pub fn group_by(&self) -> Option<Vec<String>> {
198198
self.definition
199199
.as_ref()
200-
.map_or(None, |d| d.static_data().group_by_references.clone())
200+
.and_then(|d| d.static_data().group_by_references.clone())
201201
}
202202

203203
//FIXME dublicate with symbol
@@ -218,13 +218,13 @@ impl BaseMeasure {
218218
pub fn is_multi_stage(&self) -> bool {
219219
self.definition
220220
.as_ref()
221-
.map_or(false, |d| d.static_data().multi_stage.unwrap_or(false))
221+
.is_some_and(|d| d.static_data().multi_stage.unwrap_or(false))
222222
}
223223

224224
pub fn rolling_window(&self) -> Option<RollingWindow> {
225225
self.definition
226226
.as_ref()
227-
.map_or(None, |d| d.static_data().rolling_window.clone())
227+
.and_then(|d| d.static_data().rolling_window.clone())
228228
}
229229

230230
pub fn is_rolling_window(&self) -> bool {

‎rust/cubesqlplanner/cubesqlplanner/src/planner/filter/base_filter.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ use crate::planner::{evaluate_with_context, FiltersContext, VisitorContext};
88
use cubenativeutils::CubeError;
99
use std::rc::Rc;
1010

11-
const FROM_PARTITION_RANGE: &'static str = "__FROM_PARTITION_RANGE";
11+
const FROM_PARTITION_RANGE: &str = "__FROM_PARTITION_RANGE";
1212

13-
const TO_PARTITION_RANGE: &'static str = "__TO_PARTITION_RANGE";
13+
const TO_PARTITION_RANGE: &str = "__TO_PARTITION_RANGE";
1414

1515
#[derive(Debug, Clone, PartialEq, Eq)]
1616
pub enum FilterType {
@@ -589,7 +589,7 @@ impl BaseFilter {
589589
}
590590
let from = self.format_from_date(value)?;
591591

592-
let res = if use_db_time_zone && &from != FROM_PARTITION_RANGE {
592+
let res = if use_db_time_zone && from != FROM_PARTITION_RANGE {
593593
self.query_tools.base_tools().in_db_time_zone(from)?
594594
} else {
595595
from
@@ -607,7 +607,7 @@ impl BaseFilter {
607607
}
608608
let from = self.format_to_date(value)?;
609609

610-
let res = if use_db_time_zone && &from != TO_PARTITION_RANGE {
610+
let res = if use_db_time_zone && from != TO_PARTITION_RANGE {
611611
self.query_tools.base_tools().in_db_time_zone(from)?
612612
} else {
613613
from
@@ -705,10 +705,10 @@ impl BaseFilter {
705705
as_date_time,
706706
)
707707
} else {
708-
return Err(CubeError::user(format!(
708+
Err(CubeError::user(format!(
709709
"Arguments for timestamp parameter for operator {} is not valid",
710710
self.filter_operator().to_string()
711-
)));
711+
)))
712712
}
713713
}
714714
}

0 commit comments

Comments
 (0)