Skip to content

Commit 0ac96ac

Browse files
CodeBlanchCodeBlanch
authored
[query-engine] Clippy cleanup and pair down for data & primitives (open-telemetry#586)
## Changes * Resolve clippy warnings for data & primitives * Remove stuff not needed for initial prototyping
1 parent be8580e commit 0ac96ac

File tree

12 files changed

+85
-562
lines changed

12 files changed

+85
-562
lines changed

‎rust/experimental/query_engine/engine-recordset/src/data/data_record.rs‎

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,20 @@ pub trait AttachedDataRecords: Debug {
2525
fn get_attached_data_record(&self, name: &str) -> Option<&dyn DataRecord>;
2626
}
2727

28-
pub fn create_string_value_resolver<T: DataRecord, R, M, S>(
28+
pub fn create_string_value_resolver<T: DataRecord, R, S>(
2929
path: &ValuePath,
3030
read_action: &'static R,
31-
read_mut_action: &'static M,
3231
set_action: &'static S,
3332
) -> DataRecordAnyValueResolver<T>
3433
where
3534
R: Fn(&T) -> Option<&AnyValue>,
36-
M: Fn(&mut T) -> Option<&mut AnyValue>,
3735
S: Fn(&mut T, Option<AnyValue>) -> Option<AnyValue>,
3836
{
3937
if !path.is_value_selector() {
4038
return DataRecordAnyValueResolver::new_no_op();
4139
}
4240

43-
return DataRecordAnyValueResolver::new(
41+
DataRecordAnyValueResolver::new(
4442
path.clone(),
4543
|_, data_record| {
4644
let root = read_action(data_record);
@@ -49,13 +47,6 @@ where
4947
None => DataRecordReadAnyValueResult::NotFound,
5048
}
5149
},
52-
|_, data_record| {
53-
let root = read_mut_action(data_record);
54-
match root {
55-
Some(v) => DataRecordReadMutAnyValueResult::Found(v),
56-
None => DataRecordReadMutAnyValueResult::NotFound,
57-
}
58-
},
5950
move |_, data_record, v| {
6051
if let AnyValue::StringValue(_) = &v {
6152
let old_value = set_action(data_record, Some(v));
@@ -75,7 +66,7 @@ where
7566
return DataRecordSetAnyValueResult::Updated(old_value.unwrap());
7667
}
7768

78-
return DataRecordSetAnyValueResult::NotSupported("Value was not a String");
69+
DataRecordSetAnyValueResult::NotSupported("Value was not a String")
7970
},
8071
move |_, data_record| {
8172
let old_value = set_action(data_record, None);
@@ -84,9 +75,9 @@ where
8475
return DataRecordRemoveAnyValueResult::NotFound;
8576
}
8677

87-
return DataRecordRemoveAnyValueResult::Removed(old_value.unwrap());
78+
DataRecordRemoveAnyValueResult::Removed(old_value.unwrap())
8879
},
89-
);
80+
)
9081
}
9182

9283
pub fn create_map_value_resolver<T: DataRecord, R, M, S>(
@@ -100,7 +91,7 @@ where
10091
M: Fn(&mut T) -> Option<&mut AnyValue>,
10192
S: Fn(&mut T, Option<AnyValue>) -> Option<AnyValue>,
10293
{
103-
return DataRecordAnyValueResolver::new(
94+
DataRecordAnyValueResolver::new(
10495
path.clone(),
10596
|path, data_record| {
10697
let root = read_action(data_record);
@@ -109,13 +100,6 @@ where
109100
None => DataRecordReadAnyValueResult::NotFound,
110101
}
111102
},
112-
|path, data_record| {
113-
let root = read_mut_action(data_record);
114-
match root {
115-
Some(v) => path.read_mut(v),
116-
None => DataRecordReadMutAnyValueResult::NotFound,
117-
}
118-
},
119103
move |path, data_record, v| {
120104
if path.is_value_selector() {
121105
if let AnyValue::MapValue(_) = &v {
@@ -136,7 +120,7 @@ where
136120
return DataRecordSetAnyValueResult::Updated(old_value.unwrap());
137121
}
138122

139-
return DataRecordSetAnyValueResult::NotSupported("Value was not a Map");
123+
DataRecordSetAnyValueResult::NotSupported("Value was not a Map")
140124
} else {
141125
let root = read_mut_action(data_record);
142126
match root {
@@ -153,7 +137,7 @@ where
153137
return DataRecordRemoveAnyValueResult::NotFound;
154138
}
155139

156-
return DataRecordRemoveAnyValueResult::Removed(old_value.unwrap());
140+
DataRecordRemoveAnyValueResult::Removed(old_value.unwrap())
157141
} else {
158142
let root = read_mut_action(data_record);
159143
match root {
@@ -162,5 +146,5 @@ where
162146
}
163147
}
164148
},
165-
);
149+
)
166150
}

‎rust/experimental/query_engine/engine-recordset/src/data/data_record_resolver.rs‎

Lines changed: 16 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::{any::Any, cell::RefCell, mem::replace};
1+
use std::{any::Any, cell::RefCell};
22

33
use crate::{Error, ValuePath, execution_context::ExecutionContext, primitives::AnyValue};
44

@@ -15,33 +15,31 @@ pub(crate) trait DynamicDataRecordAnyValueResolver: Any {
1515
) -> Result<(), Error>;
1616
}
1717

18+
type DataRecordAnyValueResolverReadValueCallback<T> =
19+
dyn for<'a, 'b> Fn(&'a ValuePath, &'b T) -> DataRecordReadAnyValueResult<'b>;
20+
type DataRecordAnyValueResolverSetValueCallback<T> =
21+
dyn Fn(&ValuePath, &mut T, AnyValue) -> DataRecordSetAnyValueResult;
22+
type DataRecordAnyValueResolverRemoveValueCallback<T> =
23+
dyn Fn(&ValuePath, &mut T) -> DataRecordRemoveAnyValueResult;
24+
1825
pub struct DataRecordAnyValueResolver<T: DataRecord> {
1926
path: ValuePath,
20-
read_value_fn:
21-
Box<dyn for<'a, 'b> Fn(&'a ValuePath, &'b T) -> DataRecordReadAnyValueResult<'b>>,
22-
read_value_mut_fn:
23-
Box<dyn for<'a, 'b> Fn(&'a ValuePath, &'b mut T) -> DataRecordReadMutAnyValueResult<'b>>,
24-
set_value_fn: Box<dyn Fn(&ValuePath, &mut T, AnyValue) -> DataRecordSetAnyValueResult>,
25-
remove_value_fn: Box<dyn Fn(&ValuePath, &mut T) -> DataRecordRemoveAnyValueResult>,
27+
read_value_fn: Box<DataRecordAnyValueResolverReadValueCallback<T>>,
28+
set_value_fn: Box<DataRecordAnyValueResolverSetValueCallback<T>>,
29+
remove_value_fn: Box<DataRecordAnyValueResolverRemoveValueCallback<T>>,
2630
}
2731

2832
impl<T: DataRecord> DataRecordAnyValueResolver<T> {
2933
pub fn new(
3034
path: ValuePath,
3135
read_value: impl for<'a, 'b> Fn(&'a ValuePath, &'b T) -> DataRecordReadAnyValueResult<'b>
3236
+ 'static,
33-
read_value_mut: impl for<'a, 'b> Fn(
34-
&'a ValuePath,
35-
&'b mut T,
36-
) -> DataRecordReadMutAnyValueResult<'b>
37-
+ 'static,
3837
set_value: impl Fn(&ValuePath, &mut T, AnyValue) -> DataRecordSetAnyValueResult + 'static,
3938
remove_value: impl Fn(&ValuePath, &mut T) -> DataRecordRemoveAnyValueResult + 'static,
4039
) -> DataRecordAnyValueResolver<T> {
4140
Self {
4241
path,
4342
read_value_fn: Box::new(read_value),
44-
read_value_mut_fn: Box::new(read_value_mut),
4543
set_value_fn: Box::new(set_value),
4644
remove_value_fn: Box::new(remove_value),
4745
}
@@ -51,7 +49,6 @@ impl<T: DataRecord> DataRecordAnyValueResolver<T> {
5149
DataRecordAnyValueResolver::new(
5250
ValuePath::new("").unwrap(),
5351
|_, _| DataRecordReadAnyValueResult::NotFound,
54-
|_, _| DataRecordReadMutAnyValueResult::NotFound,
5552
|_, _, _| DataRecordSetAnyValueResult::NotFound,
5653
|_, _| DataRecordRemoveAnyValueResult::NotFound,
5754
)
@@ -77,31 +74,20 @@ impl<T: DataRecord> DataRecordAnyValueResolver<T> {
7774
action(result);
7875
}
7976

80-
pub(crate) fn read_value_mut<F>(&self, data_record: &RefCell<T>, action: F)
81-
where
82-
F: FnOnce(DataRecordReadMutAnyValueResult),
83-
{
84-
let mut borrow = data_record.borrow_mut();
85-
86-
let result = (self.read_value_mut_fn)(&self.path, &mut borrow);
87-
88-
action(result);
89-
}
90-
9177
pub(crate) fn set_value(
9278
&self,
9379
data_record: &RefCell<T>,
9480
value: AnyValue,
9581
) -> DataRecordSetAnyValueResult {
9682
let mut borrow = data_record.borrow_mut();
9783

98-
return (self.set_value_fn)(&self.path, &mut borrow, value);
84+
(self.set_value_fn)(&self.path, &mut borrow, value)
9985
}
10086

10187
pub(crate) fn remove_value(&self, data_record: &RefCell<T>) -> DataRecordRemoveAnyValueResult {
10288
let mut borrow = data_record.borrow_mut();
10389

104-
return (self.remove_value_fn)(&self.path, &mut borrow);
90+
(self.remove_value_fn)(&self.path, &mut borrow)
10591
}
10692
}
10793

@@ -160,43 +146,9 @@ where
160146
F: FnOnce(DataRecordReadAnyValueResult),
161147
{
162148
fn invoke_once(&mut self, result: DataRecordReadAnyValueResult) {
163-
let callback = replace(&mut self.callback, None);
164-
if !callback.is_none() {
165-
(callback.unwrap())(result);
166-
}
167-
}
168-
}
169-
170-
pub(crate) trait DataRecordAnyValueReadMutCallback {
171-
fn invoke_once(&mut self, result: DataRecordReadMutAnyValueResult);
172-
}
173-
174-
pub(crate) struct DataRecordAnyValueReadMutClosureCallback<F>
175-
where
176-
F: FnOnce(DataRecordReadMutAnyValueResult),
177-
{
178-
callback: Option<F>,
179-
}
180-
181-
impl<F> DataRecordAnyValueReadMutClosureCallback<F>
182-
where
183-
F: FnOnce(DataRecordReadMutAnyValueResult),
184-
{
185-
pub fn new(callback: F) -> DataRecordAnyValueReadMutClosureCallback<F> {
186-
Self {
187-
callback: Some(callback),
188-
}
189-
}
190-
}
191-
192-
impl<F> DataRecordAnyValueReadMutCallback for DataRecordAnyValueReadMutClosureCallback<F>
193-
where
194-
F: FnOnce(DataRecordReadMutAnyValueResult),
195-
{
196-
fn invoke_once(&mut self, result: DataRecordReadMutAnyValueResult) {
197-
let callback = replace(&mut self.callback, None);
198-
if !callback.is_none() {
199-
(callback.unwrap())(result);
149+
let callback = self.callback.take();
150+
if let Some(c) = callback {
151+
(c)(result);
200152
}
201153
}
202154
}

‎rust/experimental/query_engine/engine-recordset/src/data/data_record_resolver_cache.rs‎

Lines changed: 11 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,38 +6,25 @@ use std::{
66

77
use crate::{Error, ValuePath, execution_context::*, expression::ExpressionMessage};
88

9-
use super::{
10-
DataRecord, array_data_record::ArrayDataRecord, data_record_resolver::*,
11-
key_value_data_record::KeyValueDataRecord,
12-
};
9+
use super::{DataRecord, data_record_resolver::*};
1310

1411
pub(crate) struct DataRecordAnyValueResolverCache {
1512
cache: HashMap<TypeId, Box<dyn DynamicDataRecordAnyValueResolver>>,
1613
}
1714

1815
impl DataRecordAnyValueResolverCache {
1916
pub fn new() -> DataRecordAnyValueResolverCache {
20-
let mut cache = Self {
17+
Self {
2118
cache: HashMap::new(),
22-
};
23-
24-
if cache.register::<ArrayDataRecord>().is_err()
25-
|| cache.register::<KeyValueDataRecord>().is_err()
26-
{
27-
panic!("DataRecordAnyValueResolverCache default registration failure")
2819
}
29-
30-
return cache;
3120
}
3221

3322
pub fn register<T: DataRecord>(&mut self) -> Result<(), Error> {
3423
match self.cache.entry(TypeId::of::<T>()) {
35-
Entry::Occupied(_) => {
36-
return Err(Error::RegistrationError("DataRecord already registered"));
37-
}
24+
Entry::Occupied(_) => Err(Error::RegistrationError("DataRecord already registered")),
3825
Entry::Vacant(vacant_entry) => {
3926
vacant_entry.insert(Box::new(GenericDataRecordAnyValueResolverCache::<T>::new()));
40-
return Ok(());
27+
Ok(())
4128
}
4229
}
4330
}
@@ -127,7 +114,7 @@ impl<T: DataRecord> GenericDataRecordAnyValueResolverCache<T> {
127114
{
128115
let cache_read_borrow = self.cache.borrow();
129116
let resolver = cache_read_borrow.get(path);
130-
if !resolver.is_none() {
117+
if resolver.is_some() {
131118
execution_context.add_message_for_expression_id(
132119
expression_id,
133120
ExpressionMessage::info(format!(
@@ -145,7 +132,7 @@ impl<T: DataRecord> GenericDataRecordAnyValueResolverCache<T> {
145132
.entry(path.clone())
146133
.or_insert(T::get_any_value_resolver_for_path(path));
147134

148-
return action(resolver, data_record);
135+
action(resolver, data_record)
149136
}
150137
}
151138

@@ -163,7 +150,7 @@ impl<T: DataRecord> DynamicDataRecordAnyValueResolver
163150
{
164151
let cache_read_borrow = self.cache.borrow();
165152
let resolver = cache_read_borrow.get(path);
166-
if !resolver.is_none() {
153+
if resolver.is_some() {
167154
execution_context.add_message_for_expression_id(
168155
expression_id,
169156
ExpressionMessage::info(format!(
@@ -197,13 +184,11 @@ impl<T: DataRecord> DynamicDataRecordAnyValueResolver
197184
match (data_record as &dyn Any).downcast_ref::<T>() {
198185
Some(typed_data_record) => {
199186
resolver.read_value_direct(typed_data_record, |r| action.invoke_once(r));
200-
return Ok(());
201-
}
202-
None => {
203-
return Err(Error::RegistrationError(
204-
"DataRecord registration type mismatch",
205-
));
187+
Ok(())
206188
}
189+
None => Err(Error::RegistrationError(
190+
"DataRecord registration type mismatch",
191+
)),
207192
}
208193
}
209194
}

0 commit comments

Comments
 (0)