Skip to content

Commit fc82925

Browse files
genezhangclaude
genezhang
and
claude
authored
fix: resolve 111 clippy warnings across 45 files (#200)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 7eae1b4 commit fc82925

46 files changed

Lines changed: 206 additions & 233 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎Cargo.lock‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/clickhouse_query_generator/function_translator.rs‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -295,12 +295,10 @@ pub fn translate_scalar_function(
295295
// Special handling for datetime({epochMillis: x}) -> identity pass-through
296296
// The epochMillis value is already an Int64 epoch timestamp, so just return it directly.
297297
// Temporal accessors like .month/.day will wrap it via fromUnixTimestamp64Milli().
298-
if fn_name_lower == "datetime" {
299-
if fn_call.args.len() == 1 {
300-
if let LogicalExpr::MapLiteral(entries) = &fn_call.args[0] {
301-
if entries.len() == 1 && entries[0].0.to_lowercase() == "epochmillis" {
302-
return entries[0].1.to_sql();
303-
}
298+
if fn_name_lower == "datetime" && fn_call.args.len() == 1 {
299+
if let LogicalExpr::MapLiteral(entries) = &fn_call.args[0] {
300+
if entries.len() == 1 && entries[0].0.to_lowercase() == "epochmillis" {
301+
return entries[0].1.to_sql();
304302
}
305303
}
306304
// Fall through to normal function_registry handling

‎src/clickhouse_query_generator/to_sql_query.rs‎

Lines changed: 31 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -445,13 +445,13 @@ fn rewrite_vlp_select_aliases(mut plan: RenderPlan) -> RenderPlan {
445445
// FROM is None — likely a Union shell where branches have their own FROM.
446446
// Check if any Union branch FROM uses the VLP CTE. If not, the VLP CTE
447447
// is consumed by a WITH CTE (not by the main query) — skip rewriting.
448-
let any_branch_uses_vlp = plan.union.0.as_ref().map_or(false, |union| {
448+
let any_branch_uses_vlp = plan.union.0.as_ref().is_some_and(|union| {
449449
union.input.iter().any(|branch| {
450450
branch
451451
.from
452452
.0
453453
.as_ref()
454-
.map_or(false, |f| f.name.starts_with("vlp_"))
454+
.is_some_and(|f| f.name.starts_with("vlp_"))
455455
})
456456
});
457457
if !any_branch_uses_vlp {
@@ -954,6 +954,32 @@ pub(crate) fn rewrite_expr_for_vlp(
954954
)));
955955
}
956956
}
957+
// Handle bare path variable: p -> tuple(t.path_nodes, t.path_relationships, t.hop_count)
958+
// When RETURN p is used for a path variable, expand it to a tuple of path components
959+
if path_variable.as_ref() == Some(&alias.0) {
960+
log::info!(
961+
"VLP path variable expansion: {} -> tuple({}.path_nodes, ...)",
962+
alias.0,
963+
VLP_CTE_FROM_ALIAS,
964+
);
965+
return RenderExpr::ScalarFnCall(ScalarFnCall {
966+
name: "tuple".to_string(),
967+
args: vec![
968+
RenderExpr::Column(Column(PropertyValue::Column(format!(
969+
"{}.path_nodes",
970+
VLP_CTE_FROM_ALIAS
971+
)))),
972+
RenderExpr::Column(Column(PropertyValue::Column(format!(
973+
"{}.path_relationships",
974+
VLP_CTE_FROM_ALIAS
975+
)))),
976+
RenderExpr::Column(Column(PropertyValue::Column(format!(
977+
"{}.hop_count",
978+
VLP_CTE_FROM_ALIAS
979+
)))),
980+
],
981+
});
982+
}
957983
expr.clone()
958984
}
959985

@@ -1138,34 +1164,6 @@ pub(crate) fn rewrite_expr_for_vlp(
11381164
.collect(),
11391165
}),
11401166

1141-
// Handle bare path variable: p → tuple(t.path_nodes, t.path_relationships, t.hop_count)
1142-
// When RETURN p is used for a path variable, expand it to a tuple of path components
1143-
RenderExpr::TableAlias(alias) if path_variable.as_ref() == Some(&alias.0) => {
1144-
log::info!(
1145-
"🔧 VLP path variable expansion: {} → tuple({}.path_nodes, ...)",
1146-
alias.0,
1147-
VLP_CTE_FROM_ALIAS,
1148-
);
1149-
// Expand to tuple of path components using VLP_CTE_FROM_ALIAS constant
1150-
RenderExpr::ScalarFnCall(ScalarFnCall {
1151-
name: "tuple".to_string(),
1152-
args: vec![
1153-
RenderExpr::Column(Column(PropertyValue::Column(format!(
1154-
"{}.path_nodes",
1155-
VLP_CTE_FROM_ALIAS
1156-
)))),
1157-
RenderExpr::Column(Column(PropertyValue::Column(format!(
1158-
"{}.path_relationships",
1159-
VLP_CTE_FROM_ALIAS
1160-
)))),
1161-
RenderExpr::Column(Column(PropertyValue::Column(format!(
1162-
"{}.hop_count",
1163-
VLP_CTE_FROM_ALIAS
1164-
)))),
1165-
],
1166-
})
1167-
}
1168-
11691167
RenderExpr::ColumnAlias(ColumnAlias(alias_str))
11701168
if path_variable.as_ref() == Some(alias_str) =>
11711169
{
@@ -2336,7 +2334,7 @@ fn flatten_all_ctes(plan: &mut RenderPlan) {
23362334
plan.ctes.0 = collected;
23372335
}
23382336

2339-
pub fn render_plan_to_sql(mut plan: RenderPlan, max_cte_depth: u32) -> String {
2337+
pub fn render_plan_to_sql(mut plan: RenderPlan, _max_cte_depth: u32) -> String {
23402338
// STEP 0: Flatten ALL CTEs to top level in dependency order.
23412339
// CTEs are always a flat, linear chain — never nested inside other CTEs or union branches.
23422340
flatten_all_ctes(&mut plan);
@@ -2547,7 +2545,7 @@ pub fn render_plan_to_sql(mut plan: RenderPlan, max_cte_depth: u32) -> String {
25472545
if let Some(_union) = &plan.union.0 {
25482546
if has_aggregation {
25492547
// Collect aggregate aliases to detect dependent order columns
2550-
let agg_aliases: std::collections::HashSet<String> = plan
2548+
let _agg_aliases: std::collections::HashSet<String> = plan
25512549
.select
25522550
.items
25532551
.iter()
@@ -3536,8 +3534,7 @@ impl RenderExpr {
35363534
// CTE column names use p{N}_ prefix (e.g., p6_friend_lastName).
35373535
// These are output aliases after GROUP BY/UNION and should NOT get
35383536
// a heuristic table prefix.
3539-
if raw_value.starts_with('p') {
3540-
let rest = &raw_value[1..];
3537+
if let Some(rest) = raw_value.strip_prefix('p') {
35413538
if let Some(pos) = rest.find('_') {
35423539
if pos > 0 && rest[..pos].chars().all(|c| c.is_ascii_digit()) {
35433540
return raw_value.to_string();

‎src/graph_catalog/config.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3114,12 +3114,12 @@ mod zeek_tests {
31143114
"GraphSchema relationships: {}",
31153115
schema.get_relationships_schemas().len()
31163116
);
3117-
for (name, _rel) in schema.get_relationships_schemas() {
3117+
for name in schema.get_relationships_schemas().keys() {
31183118
println!(" - Relationship: {}", name);
31193119
}
31203120

31213121
assert!(
3122-
schema.get_relationships_schemas().len() > 0,
3122+
!schema.get_relationships_schemas().is_empty(),
31233123
"Should have relationships!"
31243124
);
31253125
}

‎src/graph_catalog/constraint_compiler.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
use crate::graph_catalog::errors::GraphSchemaError;
3131
use crate::graph_catalog::expression_parser::PropertyValue;
3232
use crate::graph_catalog::graph_schema::NodeSchema;
33-
use crate::graph_catalog::schema_types::SchemaType;
3433

3534
/// Compile an edge constraint expression into SQL predicate
3635
///

‎src/graph_catalog/node_classification.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
//! ```
3232
3333
use crate::graph_catalog::graph_schema::{NodeSchema, RelationshipSchema};
34-
use crate::graph_catalog::schema_types::SchemaType;
3534

3635
/// Check if a node is denormalized (has properties defined in relationships)
3736
///

‎src/graph_catalog/pattern_schema.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ use super::config::Identifier;
4242
use super::graph_schema::{
4343
classify_edge_table_pattern, EdgeTablePattern, GraphSchema, NodeSchema, RelationshipSchema,
4444
};
45-
use super::schema_types::SchemaType;
4645
use std::collections::HashMap;
4746

4847
// ============================================================================
@@ -1350,6 +1349,7 @@ mod tests {
13501349
use crate::graph_catalog::config::Identifier;
13511350
use crate::graph_catalog::expression_parser::PropertyValue;
13521351
use crate::graph_catalog::graph_schema::NodeIdSchema;
1352+
use crate::graph_catalog::schema_types::SchemaType;
13531353

13541354
fn make_test_node(_label: &str, table: &str, id_col: &str) -> NodeSchema {
13551355
NodeSchema {

‎src/open_cypher_parser/delete_clause.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ mod tests {
6363
match res {
6464
Ok((remaining, delete_clause)) => {
6565
assert_eq!(remaining, "");
66-
assert_eq!(delete_clause.is_detach, false);
66+
assert!(!delete_clause.is_detach);
6767
assert_eq!(delete_clause.delete_items.len(), 1);
6868
assert_eq!(&delete_clause.delete_items[0], &Expression::Variable("a"));
6969
}
@@ -80,7 +80,7 @@ mod tests {
8080
match res {
8181
Ok((remaining, delete_clause)) => {
8282
assert_eq!(remaining, "");
83-
assert_eq!(delete_clause.is_detach, true);
83+
assert!(delete_clause.is_detach);
8484
assert_eq!(delete_clause.delete_items.len(), 2);
8585
assert_eq!(&delete_clause.delete_items[0], &Expression::Variable("a"));
8686
assert_eq!(&delete_clause.delete_items[1], &Expression::Variable("b"));
@@ -98,7 +98,7 @@ mod tests {
9898
match res {
9999
Ok((remaining, delete_clause)) => {
100100
assert_eq!(remaining, "");
101-
assert_eq!(delete_clause.is_detach, true);
101+
assert!(delete_clause.is_detach);
102102
assert_eq!(delete_clause.delete_items.len(), 3);
103103
assert_eq!(&delete_clause.delete_items[0], &Expression::Variable("a"));
104104
assert_eq!(&delete_clause.delete_items[1], &Expression::Variable("b"));

‎src/open_cypher_parser/expression.rs‎

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ fn parse_postfix_expression(input: &'_ str) -> IResult<&'_ str, Expression<'_>>
177177
let is_complete_token = new_input
178178
.chars()
179179
.next()
180-
.map_or(true, |c| !c.is_alphanumeric() && c != '_');
180+
.is_none_or(|c| !c.is_alphanumeric() && c != '_');
181181
if is_complete_token {
182182
expr = Expression::FunctionCallExp(crate::open_cypher_parser::ast::FunctionCall {
183183
name: accessor.to_string(),
@@ -1171,27 +1171,24 @@ pub fn parse_property_access(input: &'_ str) -> IResult<&'_ str, Expression<'_>>
11711171
let mut segments: Vec<&str> = vec![first_key];
11721172
let mut compound_input = input;
11731173

1174-
loop {
1175-
match preceded(char('.'), parse_property_name).parse(compound_input) {
1176-
Ok((new_input, next_key)) => {
1177-
if temporal_accessors.contains(&next_key) {
1178-
// Temporal accessor ends the chain — build expression from segments so far
1179-
let inner_expr =
1180-
build_property_access_from_segments(base_expr, &segments, original_input)?;
1181-
return Ok((
1182-
new_input,
1183-
Expression::FunctionCallExp(crate::open_cypher_parser::ast::FunctionCall {
1184-
name: next_key.to_string(),
1185-
args: vec![inner_expr],
1186-
}),
1187-
));
1188-
}
1189-
// Not temporal — add segment and continue
1190-
segments.push(next_key);
1191-
compound_input = new_input;
1192-
}
1193-
Err(_) => break,
1174+
while let Ok((new_input, next_key)) =
1175+
preceded(char('.'), parse_property_name).parse(compound_input)
1176+
{
1177+
if temporal_accessors.contains(&next_key) {
1178+
// Temporal accessor ends the chain — build expression from segments so far
1179+
let inner_expr =
1180+
build_property_access_from_segments(base_expr, &segments, original_input)?;
1181+
return Ok((
1182+
new_input,
1183+
Expression::FunctionCallExp(crate::open_cypher_parser::ast::FunctionCall {
1184+
name: next_key.to_string(),
1185+
args: vec![inner_expr],
1186+
}),
1187+
));
11941188
}
1189+
// Not temporal — add segment and continue
1190+
segments.push(next_key);
1191+
compound_input = new_input;
11951192
}
11961193

11971194
let expr = build_property_access_from_segments(base_expr, &segments, original_input)?;
@@ -1223,7 +1220,7 @@ fn build_property_access_from_segments<'a>(
12231220
&original_input[first_start..last_end]
12241221
};
12251222
match parse_literal_or_variable_expression(key) {
1226-
Ok((remainder, Expression::Variable(key))) if remainder.is_empty() => {
1223+
Ok(("", Expression::Variable(key))) => {
12271224
Ok(Expression::PropertyAccessExp(PropertyAccess { base, key }))
12281225
}
12291226
_ => Err(nom::Err::Error(Error::new(

‎src/open_cypher_parser/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ mod tests {
386386
assert_eq!(remove_item.key, "temp");
387387

388388
let delete_clause = ast.delete_clause.unwrap();
389-
assert_eq!(delete_clause.is_detach, false);
389+
assert!(!delete_clause.is_detach);
390390
assert_eq!(delete_clause.delete_items.len(), 1);
391391
assert_eq!(delete_clause.delete_items[0], Expression::Variable("a"));
392392

0 commit comments

Comments
 (0)