Skip to content

Commit 8fd4444

Browse files
WenyXuWenyXu
committed
refactor: remove too_many_arguments
1 parent 590adb0 commit 8fd4444

File tree

9 files changed

+65
-116
lines changed

9 files changed

+65
-116
lines changed

‎src/mito2/src/read/scan_region.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -422,8 +422,6 @@ impl ScanRegion {
422422
InvertedIndexApplierBuilder::new(
423423
self.access_layer.region_dir().to_string(),
424424
self.access_layer.object_store().clone(),
425-
file_cache,
426-
index_cache,
427425
self.version.metadata.as_ref(),
428426
self.version.metadata.inverted_indexed_column_ids(
429427
self.version
@@ -434,8 +432,10 @@ impl ScanRegion {
434432
.iter(),
435433
),
436434
self.access_layer.puffin_manager_factory().clone(),
437-
puffin_metadata_cache,
438435
)
436+
.with_file_cache(file_cache)
437+
.with_index_cache(index_cache)
438+
.with_puffin_metadata_cache(puffin_metadata_cache)
439439
.build(&self.request.filters)
440440
.inspect_err(|err| warn!(err; "Failed to build invereted index applier"))
441441
.ok()

‎src/mito2/src/sst/index/inverted_index/applier.rs

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -69,16 +69,11 @@ pub(crate) struct InvertedIndexApplier {
6969
pub(crate) type InvertedIndexApplierRef = Arc<InvertedIndexApplier>;
7070

7171
impl InvertedIndexApplier {
72-
// TODO(weny): remove this after refactoring.
73-
#[allow(clippy::too_many_arguments)]
7472
/// Creates a new `InvertedIndexApplier`.
7573
pub fn new(
7674
region_dir: String,
7775
region_id: RegionId,
7876
store: ObjectStore,
79-
file_cache: Option<FileCacheRef>,
80-
index_cache: Option<InvertedIndexCacheRef>,
81-
puffin_metadata_cache: Option<PuffinMetadataCacheRef>,
8277
index_applier: Box<dyn IndexApplier>,
8378
puffin_manager_factory: PuffinManagerFactory,
8479
) -> Self {
@@ -88,14 +83,35 @@ impl InvertedIndexApplier {
8883
region_dir,
8984
region_id,
9085
store,
91-
file_cache,
86+
file_cache: None,
9287
index_applier,
9388
puffin_manager_factory,
94-
inverted_index_cache: index_cache,
95-
puffin_metadata_cache,
89+
inverted_index_cache: None,
90+
puffin_metadata_cache: None,
9691
}
9792
}
9893

94+
/// Sets the file cache.
95+
pub fn with_file_cache(mut self, file_cache: Option<FileCacheRef>) -> Self {
96+
self.file_cache = file_cache;
97+
self
98+
}
99+
100+
/// Sets the index cache.
101+
pub fn with_index_cache(mut self, index_cache: Option<InvertedIndexCacheRef>) -> Self {
102+
self.inverted_index_cache = index_cache;
103+
self
104+
}
105+
106+
/// Sets the puffin metadata cache.
107+
pub fn with_puffin_metadata_cache(
108+
mut self,
109+
puffin_metadata_cache: Option<PuffinMetadataCacheRef>,
110+
) -> Self {
111+
self.puffin_metadata_cache = puffin_metadata_cache;
112+
self
113+
}
114+
99115
/// Applies predicates to the provided SST file id and returns the relevant row group ids
100116
pub async fn apply(&self, file_id: FileId) -> Result<ApplyOutput> {
101117
let _timer = INDEX_APPLY_ELAPSED
@@ -231,9 +247,6 @@ mod tests {
231247
region_dir.clone(),
232248
RegionId::new(0, 0),
233249
object_store,
234-
None,
235-
None,
236-
None,
237250
Box::new(mock_index_applier),
238251
puffin_manager_factory,
239252
);
@@ -274,9 +287,6 @@ mod tests {
274287
region_dir.clone(),
275288
RegionId::new(0, 0),
276289
object_store,
277-
None,
278-
None,
279-
None,
280290
Box::new(mock_index_applier),
281291
puffin_manager_factory,
282292
);

‎src/mito2/src/sst/index/inverted_index/applier/builder.rs

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -72,32 +72,48 @@ pub(crate) struct InvertedIndexApplierBuilder<'a> {
7272
}
7373

7474
impl<'a> InvertedIndexApplierBuilder<'a> {
75-
// TODO(weny): remove this after refactoring.
76-
#[allow(clippy::too_many_arguments)]
7775
/// Creates a new [`InvertedIndexApplierBuilder`].
7876
pub fn new(
7977
region_dir: String,
8078
object_store: ObjectStore,
81-
file_cache: Option<FileCacheRef>,
82-
index_cache: Option<InvertedIndexCacheRef>,
8379
metadata: &'a RegionMetadata,
8480
indexed_column_ids: HashSet<ColumnId>,
8581
puffin_manager_factory: PuffinManagerFactory,
86-
puffin_metadata_cache: Option<PuffinMetadataCacheRef>,
8782
) -> Self {
8883
Self {
8984
region_dir,
9085
object_store,
91-
file_cache,
9286
metadata,
9387
indexed_column_ids,
9488
output: HashMap::default(),
95-
index_cache,
9689
puffin_manager_factory,
97-
puffin_metadata_cache,
90+
file_cache: None,
91+
index_cache: None,
92+
puffin_metadata_cache: None,
9893
}
9994
}
10095

96+
/// Sets the file cache.
97+
pub fn with_file_cache(mut self, file_cache: Option<FileCacheRef>) -> Self {
98+
self.file_cache = file_cache;
99+
self
100+
}
101+
102+
/// Sets the puffin metadata cache.
103+
pub fn with_puffin_metadata_cache(
104+
mut self,
105+
puffin_metadata_cache: Option<PuffinMetadataCacheRef>,
106+
) -> Self {
107+
self.puffin_metadata_cache = puffin_metadata_cache;
108+
self
109+
}
110+
111+
/// Sets the index cache.
112+
pub fn with_index_cache(mut self, index_cache: Option<InvertedIndexCacheRef>) -> Self {
113+
self.index_cache = index_cache;
114+
self
115+
}
116+
101117
/// Consumes the builder to construct an [`InvertedIndexApplier`], optionally returned based on
102118
/// the expressions provided. If no predicates match, returns `None`.
103119
pub fn build(mut self, exprs: &[Expr]) -> Result<Option<InvertedIndexApplier>> {
@@ -116,16 +132,18 @@ impl<'a> InvertedIndexApplierBuilder<'a> {
116132
.collect();
117133
let applier = PredicatesIndexApplier::try_from(predicates);
118134

119-
Ok(Some(InvertedIndexApplier::new(
120-
self.region_dir,
121-
self.metadata.region_id,
122-
self.object_store,
123-
self.file_cache,
124-
self.index_cache,
125-
self.puffin_metadata_cache,
126-
Box::new(applier.context(BuildIndexApplierSnafu)?),
127-
self.puffin_manager_factory,
128-
)))
135+
Ok(Some(
136+
InvertedIndexApplier::new(
137+
self.region_dir,
138+
self.metadata.region_id,
139+
self.object_store,
140+
Box::new(applier.context(BuildIndexApplierSnafu)?),
141+
self.puffin_manager_factory,
142+
)
143+
.with_file_cache(self.file_cache)
144+
.with_puffin_metadata_cache(self.puffin_metadata_cache)
145+
.with_index_cache(self.index_cache),
146+
))
129147
}
130148

131149
/// Recursively traverses expressions to collect predicates.
@@ -331,12 +349,9 @@ mod tests {
331349
let mut builder = InvertedIndexApplierBuilder::new(
332350
"test".to_string(),
333351
test_object_store(),
334-
None,
335-
None,
336352
&metadata,
337353
HashSet::from_iter([1, 2, 3]),
338354
facotry,
339-
None,
340355
);
341356

342357
let expr = Expr::BinaryExpr(BinaryExpr {

‎src/mito2/src/sst/index/inverted_index/applier/builder/between.rs

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -75,12 +75,9 @@ mod tests {
7575
let mut builder = InvertedIndexApplierBuilder::new(
7676
"test".to_string(),
7777
test_object_store(),
78-
None,
79-
None,
8078
&metadata,
8179
HashSet::from_iter([1, 2, 3]),
8280
facotry,
83-
None,
8481
);
8582

8683
let between = Between {
@@ -119,12 +116,9 @@ mod tests {
119116
let mut builder = InvertedIndexApplierBuilder::new(
120117
"test".to_string(),
121118
test_object_store(),
122-
None,
123-
None,
124119
&metadata,
125120
HashSet::from_iter([1, 2, 3]),
126121
facotry,
127-
None,
128122
);
129123

130124
let between = Between {
@@ -146,12 +140,9 @@ mod tests {
146140
let mut builder = InvertedIndexApplierBuilder::new(
147141
"test".to_string(),
148142
test_object_store(),
149-
None,
150-
None,
151143
&metadata,
152144
HashSet::from_iter([1, 2, 3]),
153145
facotry,
154-
None,
155146
);
156147

157148
let between = Between {
@@ -190,12 +181,9 @@ mod tests {
190181
let mut builder = InvertedIndexApplierBuilder::new(
191182
"test".to_string(),
192183
test_object_store(),
193-
None,
194-
None,
195184
&metadata,
196185
HashSet::from_iter([1, 2, 3]),
197186
facotry,
198-
None,
199187
);
200188

201189
let between = Between {
@@ -218,12 +206,9 @@ mod tests {
218206
let mut builder = InvertedIndexApplierBuilder::new(
219207
"test".to_string(),
220208
test_object_store(),
221-
None,
222-
None,
223209
&metadata,
224210
HashSet::from_iter([1, 2, 3]),
225211
facotry,
226-
None,
227212
);
228213

229214
let between = Between {

‎src/mito2/src/sst/index/inverted_index/applier/builder/comparison.rs

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -231,12 +231,9 @@ mod tests {
231231
let mut builder = InvertedIndexApplierBuilder::new(
232232
"test".to_string(),
233233
test_object_store(),
234-
None,
235-
None,
236234
&metadata,
237235
HashSet::from_iter([1, 2, 3]),
238236
facotry,
239-
None,
240237
);
241238

242239
for ((left, op, right), _) in &cases {
@@ -261,12 +258,9 @@ mod tests {
261258
let mut builder = InvertedIndexApplierBuilder::new(
262259
"test".to_string(),
263260
test_object_store(),
264-
None,
265-
None,
266261
&metadata,
267262
HashSet::from_iter([1, 2, 3]),
268263
facotry,
269-
None,
270264
);
271265

272266
let res = builder.collect_comparison_expr(&tag_column(), &Operator::Lt, &int64_lit(10));
@@ -282,12 +276,9 @@ mod tests {
282276
let mut builder = InvertedIndexApplierBuilder::new(
283277
"test".to_string(),
284278
test_object_store(),
285-
None,
286-
None,
287279
&metadata,
288280
HashSet::from_iter([1, 2, 3]),
289281
facotry,
290-
None,
291282
);
292283

293284
builder
@@ -318,12 +309,9 @@ mod tests {
318309
let mut builder = InvertedIndexApplierBuilder::new(
319310
"test".to_string(),
320311
test_object_store(),
321-
None,
322-
None,
323312
&metadata,
324313
HashSet::from_iter([1, 2, 3]),
325314
facotry,
326-
None,
327315
);
328316

329317
let res = builder.collect_comparison_expr(

0 commit comments

Comments
 (0)