Skip to content

Commit 3cf099a

Browse files
CodeBlanchCodeBlanch
authored
[query-engine] Clippy cleanup and pair down for recordset engine root files (open-telemetry#587)
## Changes * Resolve clippy warnings for recordset engine root files * Remove stuff not needed for initial prototyping
1 parent 0ac96ac commit 3cf099a

File tree

6 files changed

+149
-214
lines changed

6 files changed

+149
-214
lines changed

‎rust/experimental/query_engine/engine-recordset/src/execution_context.rs‎

Lines changed: 11 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,6 @@ pub(crate) trait ExecutionContext<'a> {
4545
) where
4646
'b: 'a;
4747

48-
fn read_any_value_mut<'b>(
49-
&self,
50-
expression_id: usize,
51-
resolve_value_expr: &'b ResolveValueExpression,
52-
action: &mut dyn DataRecordAnyValueReadMutCallback,
53-
) where
54-
'b: 'a;
55-
5648
fn set_any_value<'b>(
5749
&self,
5850
expression_id: usize,
@@ -78,8 +70,6 @@ pub(crate) trait ExecutionContext<'a> {
7870

7971
fn get_observed_timestamp(&self) -> Option<SystemTime>;
8072

81-
fn get_messages(&self) -> &RefCell<HashMap<usize, Vec<ExpressionMessage>>>;
82-
8373
fn get_summaries(&self) -> &Summaries;
8474

8575
fn get_external_summary_index(&self) -> Result<Option<usize>, Error>;
@@ -88,8 +78,6 @@ pub(crate) trait ExecutionContext<'a> {
8878

8979
fn set_summary_index(&self, index: usize);
9080

91-
fn get_resolver_cache(&self) -> &DataRecordAnyValueResolverCache;
92-
9381
fn get_variables(&self) -> &RefCell<HashMap<String, AnyValue>>;
9482

9583
fn get_resolved_values(&self) -> &RefCell<Vec<DataRecordResolvedValue<'a>>>;
@@ -146,7 +134,7 @@ impl<'a> DataRecordResolvedValue<'a> {
146134
}
147135
}
148136

149-
impl<'a> DataRecordResolvedValue<'a> {
137+
impl DataRecordResolvedValue<'_> {
150138
pub fn get_name(&self) -> Option<&str> {
151139
self.name
152140
}
@@ -178,7 +166,7 @@ impl<'a, T: DataRecord> ExecutionContext<'a> for DataRecordExecutionContext<'a,
178166
resolved_values.truncate(resolved_values_index);
179167
}
180168

181-
return result;
169+
result
182170
}
183171

184172
fn clear(&self) {
@@ -279,37 +267,6 @@ impl<'a, T: DataRecord> ExecutionContext<'a> for DataRecordExecutionContext<'a,
279267
}
280268
}
281269

282-
fn read_any_value_mut<'b>(
283-
&self,
284-
expression_id: usize,
285-
resolve_value_expr: &'b ResolveValueExpression,
286-
action: &mut dyn DataRecordAnyValueReadMutCallback,
287-
) where
288-
'b: 'a,
289-
{
290-
let r = self.resolver_cache.invoke_resolver(
291-
expression_id,
292-
self,
293-
resolve_value_expr.get_path(),
294-
self.data_record,
295-
|resolver, data_record| {
296-
resolver.read_value_mut(data_record, |r| action.invoke_once(r));
297-
},
298-
);
299-
300-
if r.is_err() {
301-
self.add_message_for_expression_id(
302-
expression_id,
303-
ExpressionMessage::err(format!(
304-
"ExecutionContext read_mut operation returned an error: {}",
305-
r.unwrap_err()
306-
)),
307-
);
308-
309-
action.invoke_once(DataRecordReadMutAnyValueResult::NotFound);
310-
}
311-
}
312-
313270
fn set_any_value<'b>(
314271
&self,
315272
expression_id: usize,
@@ -324,9 +281,7 @@ impl<'a, T: DataRecord> ExecutionContext<'a> for DataRecordExecutionContext<'a,
324281
self,
325282
resolve_value_expr.get_path(),
326283
self.data_record,
327-
|resolver, data_record| {
328-
return resolver.set_value(data_record, value);
329-
},
284+
|resolver, data_record| resolver.set_value(data_record, value),
330285
);
331286

332287
if r.is_err() {
@@ -341,7 +296,7 @@ impl<'a, T: DataRecord> ExecutionContext<'a> for DataRecordExecutionContext<'a,
341296
return DataRecordSetAnyValueResult::NotFound;
342297
}
343298

344-
return r.unwrap();
299+
r.unwrap()
345300
}
346301

347302
fn remove_any_value<'b>(
@@ -357,9 +312,7 @@ impl<'a, T: DataRecord> ExecutionContext<'a> for DataRecordExecutionContext<'a,
357312
self,
358313
resolve_value_expr.get_path(),
359314
self.data_record,
360-
|resolver, data_record| {
361-
return resolver.remove_value(data_record);
362-
},
315+
|resolver, data_record| resolver.remove_value(data_record),
363316
);
364317

365318
if r.is_err() {
@@ -374,7 +327,7 @@ impl<'a, T: DataRecord> ExecutionContext<'a> for DataRecordExecutionContext<'a,
374327
return DataRecordRemoveAnyValueResult::NotFound;
375328
}
376329

377-
return r.unwrap();
330+
r.unwrap()
378331
}
379332

380333
fn get_attached_data_records(&self) -> &dyn AttachedDataRecords {
@@ -393,10 +346,6 @@ impl<'a, T: DataRecord> ExecutionContext<'a> for DataRecordExecutionContext<'a,
393346
self.data_record.borrow().get_observed_timestamp()
394347
}
395348

396-
fn get_messages(&self) -> &RefCell<HashMap<usize, Vec<ExpressionMessage>>> {
397-
self.messages
398-
}
399-
400349
fn get_summaries(&self) -> &Summaries {
401350
self.summaries
402351
}
@@ -416,7 +365,7 @@ impl<'a, T: DataRecord> ExecutionContext<'a> for DataRecordExecutionContext<'a,
416365
return Err(Error::ExternalSummaryNotFound(summary_id.into()));
417366
}
418367

419-
return Ok(Some(summary_index.unwrap()));
368+
Ok(Some(summary_index.unwrap()))
420369
}
421370

422371
fn get_summary_index(&self) -> Option<usize> {
@@ -432,10 +381,6 @@ impl<'a, T: DataRecord> ExecutionContext<'a> for DataRecordExecutionContext<'a,
432381
*self.summary_index.borrow_mut() = Some(index);
433382
}
434383

435-
fn get_resolver_cache(&self) -> &DataRecordAnyValueResolverCache {
436-
self.resolver_cache
437-
}
438-
439384
fn get_variables(&self) -> &RefCell<HashMap<String, AnyValue>> {
440385
&self.variables
441386
}
@@ -449,7 +394,7 @@ impl<'a, T: DataRecord> ExecutionContext<'a> for DataRecordExecutionContext<'a,
449394
}
450395

451396
fn add_message_for_expression_id(&self, expression_id: usize, mut message: ExpressionMessage) {
452-
if !self.message_scope.is_none() {
397+
if self.message_scope.is_some() {
453398
message.add_scope(self.message_scope.as_ref().unwrap());
454399
}
455400

@@ -478,9 +423,9 @@ impl<'a, T: DataRecord> ExecutionContext<'a> for DataRecordExecutionContext<'a,
478423
) {
479424
let messages = self.messages.borrow();
480425
let messages_for_expression = messages.get(&expression_id);
481-
if !messages_for_expression.is_none() {
426+
if messages_for_expression.is_some() {
482427
for message in messages_for_expression.unwrap() {
483-
output.push_str(&padding);
428+
output.push_str(padding);
484429
message.write_debug_comment(output);
485430
}
486431
}
@@ -517,6 +462,6 @@ impl<'a, T: DataRecord> DataRecordExecutionContext<'a, T> {
517462
) -> &'b mut Vec<ExpressionMessage> {
518463
let entry = messages.entry(expression_id);
519464

520-
return entry.or_insert_with(|| Vec::new());
465+
entry.or_default()
521466
}
522467
}

‎rust/experimental/query_engine/engine-recordset/src/expression.rs‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::execution_context::ExecutionContext;
99
pub(crate) fn get_next_id() -> usize {
1010
static COUNTER: AtomicUsize = AtomicUsize::new(1);
1111

12-
return COUNTER.fetch_add(1, Ordering::Relaxed);
12+
COUNTER.fetch_add(1, Ordering::Relaxed)
1313
}
1414

1515
pub(crate) struct Hasher {
@@ -28,26 +28,26 @@ impl Hasher {
2828
}
2929
}
3030

31-
impl Into<ExpressionHash> for Hasher {
32-
fn into(self) -> ExpressionHash {
33-
let hash = self.hasher.finalize();
31+
impl From<Hasher> for ExpressionHash {
32+
fn from(val: Hasher) -> Self {
33+
let hash = val.hasher.finalize();
3434

3535
let bytes = &hash[..];
3636

37-
return ExpressionHash {
37+
ExpressionHash {
3838
bytes: bytes.into(),
3939
hex: hex::encode(bytes).into(),
40-
};
40+
}
4141
}
4242
}
4343

44-
impl Into<Box<str>> for Hasher {
45-
fn into(self) -> Box<str> {
46-
let hash = self.hasher.finalize();
44+
impl From<Hasher> for Box<str> {
45+
fn from(val: Hasher) -> Self {
46+
let hash = val.hasher.finalize();
4747

4848
let bytes = &hash[..];
4949

50-
return hex::encode(bytes).into();
50+
hex::encode(bytes).into()
5151
}
5252
}
5353

@@ -149,15 +149,17 @@ impl ExpressionMessage {
149149
};
150150

151151
if !result.1.is_none() {
152-
write!(
152+
writeln!(
153153
output,
154-
"// [{}] {}: {}\n",
154+
"// [{}] {}: {}",
155155
result.1.as_ref().unwrap(),
156156
result.0,
157157
result.2
158-
);
158+
)
159+
.expect("debug comments could not be written");
159160
} else {
160-
write!(output, "// {}: {}\n", result.0, result.2);
161+
writeln!(output, "// {}: {}", result.0, result.2)
162+
.expect("debug comments could not be written");
161163
}
162164
}
163165
}

‎rust/experimental/query_engine/engine-recordset/src/macros.rs‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
1-
#[macro_use]
2-
mod macros {
3-
//#[macro_export]
4-
macro_rules! either {
5-
($test:expr => $true_expr:expr; $false_expr:expr) => {
6-
if $test { $true_expr } else { $false_expr }
7-
};
8-
}
1+
macro_rules! either {
2+
($test:expr => $true_expr:expr; $false_expr:expr) => {
3+
if $test { $true_expr } else { $false_expr }
4+
};
95
}

‎rust/experimental/query_engine/engine-recordset/src/pipeline_expression.rs‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,15 @@ impl Expression for PipelineExpression {
1919

2020
fn get_hash(&self) -> &ExpressionHash {
2121
self.hash.get_or_init(|| {
22-
return ExpressionHash::new(|h| {
22+
ExpressionHash::new(|h| {
2323
h.add_bytes(b"pipeline");
24-
if self.expressions.len() > 0 {
24+
if !self.expressions.is_empty() {
2525
h.add_bytes(b"expressions:");
2626
for expression in self.expressions.iter() {
2727
h.add_bytes(expression.get_hash().get_bytes());
2828
}
2929
}
30-
});
30+
})
3131
})
3232
}
3333

@@ -62,6 +62,12 @@ impl Expression for PipelineExpression {
6262
}
6363
}
6464

65+
impl Default for PipelineExpression {
66+
fn default() -> Self {
67+
Self::new()
68+
}
69+
}
70+
6571
impl PipelineExpression {
6672
pub fn new() -> PipelineExpression {
6773
Self {
@@ -93,6 +99,6 @@ impl PipelineExpression {
9399
return Ok(result);
94100
}
95101

96-
return Ok(DataExpressionResult::Include(self.get_id()));
102+
Ok(DataExpressionResult::Include(self.get_id()))
97103
}
98104
}

0 commit comments

Comments
 (0)