Skip to content

Commit 4dc92c5

Browse files
committed
refactor(engine): replace long argument lists with params structs
Vector and spatial executor handlers took upwards of eight positional arguments. Bundle them into per-handler params structs and update the dispatch call sites to construct them, matching the pattern already applied elsewhere in the engine.
1 parent 43a239e commit 4dc92c5

7 files changed

Lines changed: 247 additions & 135 deletions

File tree

nodedb/src/data/executor/dispatch/spatial.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,20 +58,20 @@ impl CoreLoop {
5858
projection,
5959
rls_filters,
6060
prefilter,
61-
} => self.execute_spatial_scan(
61+
} => self.execute_spatial_scan(super::super::handlers::spatial::SpatialScanParams {
6262
task,
6363
tid,
6464
collection,
6565
field,
6666
predicate,
6767
query_geometry,
68-
*distance_meters,
68+
distance_meters: *distance_meters,
6969
attribute_filters,
70-
*limit,
70+
limit: *limit,
7171
projection,
7272
rls_filters,
73-
prefilter.as_ref(),
74-
),
73+
prefilter: prefilter.as_ref(),
74+
}),
7575
}
7676
}
7777
}

nodedb/src/data/executor/dispatch/vector.rs

Lines changed: 41 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -157,13 +157,15 @@ impl CoreLoop {
157157
m0,
158158
ef_construction,
159159
} => self.execute_vector_rebuild(
160-
task,
161-
tid,
162-
collection,
163-
field_name,
164-
*m,
165-
*m0,
166-
*ef_construction,
160+
super::super::handlers::vector_lifecycle::VectorRebuildParams {
161+
task,
162+
tid,
163+
collection,
164+
field_name,
165+
m: *m,
166+
m0: *m0,
167+
ef_construction: *ef_construction,
168+
},
167169
),
168170

169171
VectorOp::SparseInsert {
@@ -196,14 +198,16 @@ impl CoreLoop {
196198
count,
197199
dim,
198200
} => self.execute_multi_vector_insert(
199-
task,
200-
tid,
201-
collection,
202-
field_name,
203-
*document_surrogate,
204-
vectors,
205-
*count,
206-
*dim,
201+
super::super::handlers::vector_multi::MultiVectorInsertParams {
202+
task,
203+
tid,
204+
collection,
205+
field_name,
206+
document_surrogate: *document_surrogate,
207+
vectors_flat: vectors,
208+
count: *count,
209+
dim: *dim,
210+
},
207211
),
208212

209213
VectorOp::MultiVectorDelete {
@@ -226,14 +230,16 @@ impl CoreLoop {
226230
ef_search,
227231
mode,
228232
} => self.execute_multi_vector_score_search(
229-
task,
230-
tid,
231-
collection,
232-
field_name,
233-
query_vector,
234-
*top_k,
235-
*ef_search,
236-
mode,
233+
super::super::handlers::vector_multi::MultiVectorScoreSearchParams {
234+
task,
235+
tid,
236+
collection,
237+
field_name,
238+
query_vector,
239+
top_k: *top_k,
240+
ef_search: *ef_search,
241+
mode_str: mode,
242+
},
237243
),
238244

239245
VectorOp::DirectUpsert {
@@ -246,16 +252,18 @@ impl CoreLoop {
246252
storage_dtype,
247253
payload_indexes,
248254
} => self.execute_vector_direct_upsert(
249-
task,
250-
tid,
251-
collection,
252-
field,
253-
*surrogate,
254-
vector,
255-
payload,
256-
*quantization,
257-
*storage_dtype,
258-
payload_indexes,
255+
super::super::handlers::vector_upsert::VectorDirectUpsertParams {
256+
task,
257+
tid,
258+
collection,
259+
field,
260+
surrogate: *surrogate,
261+
vector,
262+
payload,
263+
quantization: *quantization,
264+
storage_dtype: *storage_dtype,
265+
payload_indexes,
266+
},
259267
),
260268

261269
VectorOp::DeleteBySurrogate {

nodedb/src/data/executor/handlers/spatial.rs

Lines changed: 78 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -18,29 +18,63 @@ use crate::data::executor::task::ExecutionTask;
1818
use nodedb_physical::physical_plan::SpatialPredicate;
1919
use nodedb_types::{Surrogate, SurrogateBitmap, Value};
2020

21+
/// Parameters for [`CoreLoop::execute_spatial_scan`].
22+
pub(in crate::data::executor) struct SpatialScanParams<'a> {
23+
pub task: &'a ExecutionTask,
24+
pub tid: u64,
25+
pub collection: &'a str,
26+
pub field: &'a str,
27+
pub predicate: &'a SpatialPredicate,
28+
pub query_geometry: &'a nodedb_types::geometry::Geometry,
29+
pub distance_meters: f64,
30+
pub attribute_filters: &'a [u8],
31+
pub limit: usize,
32+
pub projection: &'a [String],
33+
pub rls_filters: &'a [u8],
34+
pub prefilter: Option<&'a SurrogateBitmap>,
35+
}
36+
37+
/// Parameters for [`CoreLoop::spatial_full_scan`].
38+
struct SpatialFullScanParams<'a> {
39+
task: &'a ExecutionTask,
40+
tid: u64,
41+
collection: &'a str,
42+
field: &'a str,
43+
predicate: &'a SpatialPredicate,
44+
query_geom: &'a nodedb_types::geometry::Geometry,
45+
distance_meters: f64,
46+
limit: usize,
47+
projection: &'a [String],
48+
attr_filters: &'a [ScanFilter],
49+
rls_filters: &'a [ScanFilter],
50+
prefilter: Option<&'a SurrogateBitmap>,
51+
}
52+
2153
impl CoreLoop {
2254
/// Execute a spatial scan using the R-tree index.
2355
///
2456
/// 1. Parse query geometry from GeoJSON bytes
2557
/// 2. R-tree range search for bbox candidates
2658
/// 3. Exact predicate refinement (extract geometry, apply ST_*)
2759
/// 4. Return matching documents up to limit
28-
#[allow(clippy::too_many_arguments)]
2960
pub(in crate::data::executor) fn execute_spatial_scan(
3061
&mut self,
31-
task: &ExecutionTask,
32-
tid: u64,
33-
collection: &str,
34-
field: &str,
35-
predicate: &SpatialPredicate,
36-
query_geometry: &nodedb_types::geometry::Geometry,
37-
distance_meters: f64,
38-
attribute_filters: &[u8],
39-
limit: usize,
40-
projection: &[String],
41-
rls_filters: &[u8],
42-
prefilter: Option<&SurrogateBitmap>,
62+
params: SpatialScanParams<'_>,
4363
) -> Response {
64+
let SpatialScanParams {
65+
task,
66+
tid,
67+
collection,
68+
field,
69+
predicate,
70+
query_geometry,
71+
distance_meters,
72+
attribute_filters,
73+
limit,
74+
projection,
75+
rls_filters,
76+
prefilter,
77+
} = params;
4478
debug!(
4579
core = self.core_id,
4680
%collection,
@@ -86,7 +120,7 @@ impl CoreLoop {
86120

87121
// No R-tree: full scan with predicate post-filter.
88122
if !has_index {
89-
return self.spatial_full_scan(
123+
return self.spatial_full_scan(SpatialFullScanParams {
90124
task,
91125
tid,
92126
collection,
@@ -96,10 +130,10 @@ impl CoreLoop {
96130
distance_meters,
97131
limit,
98132
projection,
99-
&attr_filters,
100-
&row_level_filters,
133+
attr_filters: &attr_filters,
134+
rls_filters: &row_level_filters,
101135
prefilter,
102-
);
136+
});
103137
}
104138

105139
let coll_key = (db_id, tid_id, collection.to_string());
@@ -224,22 +258,21 @@ impl CoreLoop {
224258
}
225259

226260
/// Full scan when no R-tree exists for the field.
227-
#[allow(clippy::too_many_arguments)]
228-
fn spatial_full_scan(
229-
&self,
230-
task: &ExecutionTask,
231-
tid: u64,
232-
collection: &str,
233-
field: &str,
234-
predicate: &SpatialPredicate,
235-
query_geom: &nodedb_types::geometry::Geometry,
236-
distance_meters: f64,
237-
limit: usize,
238-
projection: &[String],
239-
attr_filters: &[ScanFilter],
240-
rls_filters: &[ScanFilter],
241-
prefilter: Option<&SurrogateBitmap>,
242-
) -> Response {
261+
fn spatial_full_scan(&self, params: SpatialFullScanParams<'_>) -> Response {
262+
let SpatialFullScanParams {
263+
task,
264+
tid,
265+
collection,
266+
field,
267+
predicate,
268+
query_geom,
269+
distance_meters,
270+
limit,
271+
projection,
272+
attr_filters,
273+
rls_filters,
274+
prefilter,
275+
} = params;
243276
debug!(core = self.core_id, %collection, "spatial full scan (no R-tree index yet)");
244277

245278
let scan_limit = limit * 10;
@@ -419,6 +452,7 @@ fn expand_bbox(bbox: &nodedb_types::BoundingBox, meters: f64) -> nodedb_types::B
419452

420453
#[cfg(test)]
421454
mod tests {
455+
use super::SpatialScanParams;
422456
use crate::bridge::envelope::{PhysicalPlan, Priority, Request, Status};
423457
use crate::data::executor::task::ExecutionTask;
424458
use crate::engine::spatial::RTreeEntry;
@@ -579,20 +613,20 @@ mod tests {
579613
let task = make_task(dummy_spatial_plan());
580614
let empty_bitmap = SurrogateBitmap::new();
581615

582-
let resp = core.execute_spatial_scan(
583-
&task,
616+
let resp = core.execute_spatial_scan(SpatialScanParams {
617+
task: &task,
584618
tid,
585619
collection,
586620
field,
587-
&nodedb_physical::physical_plan::SpatialPredicate::DWithin,
588-
&origin_point(),
589-
1_000_000.0,
590-
&[],
591-
100,
592-
&[],
593-
&[],
594-
Some(&empty_bitmap),
595-
);
621+
predicate: &nodedb_physical::physical_plan::SpatialPredicate::DWithin,
622+
query_geometry: &origin_point(),
623+
distance_meters: 1_000_000.0,
624+
attribute_filters: &[],
625+
limit: 100,
626+
projection: &[],
627+
rls_filters: &[],
628+
prefilter: Some(&empty_bitmap),
629+
});
596630
assert_eq!(resp.status, Status::Ok);
597631
let decoded: Vec<nodedb_types::Value> =
598632
zerompk::from_msgpack(resp.payload.as_bytes()).unwrap_or_default();

nodedb/src/data/executor/handlers/vector_lifecycle.rs

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,17 @@ use crate::bridge::envelope::{ErrorCode, Response};
1111
use crate::data::executor::core_loop::CoreLoop;
1212
use crate::data::executor::task::ExecutionTask;
1313

14+
/// Parameters for [`CoreLoop::execute_vector_rebuild`].
15+
pub(in crate::data::executor) struct VectorRebuildParams<'a> {
16+
pub task: &'a ExecutionTask,
17+
pub tid: u64,
18+
pub collection: &'a str,
19+
pub field_name: &'a str,
20+
pub m: usize,
21+
pub m0: usize,
22+
pub ef_construction: usize,
23+
}
24+
1425
impl CoreLoop {
1526
/// Return live `VectorIndexStats` for a collection/field.
1627
///
@@ -177,19 +188,22 @@ impl CoreLoop {
177188
/// Updates the params so future builds use them. Then re-seals and
178189
/// re-builds all sealed segments with the new params by extracting their
179190
/// vectors, creating new HNSW indexes, and swapping atomically.
180-
#[allow(clippy::too_many_arguments)]
181191
pub(in crate::data::executor) fn execute_vector_rebuild(
182192
&mut self,
183-
task: &ExecutionTask,
184-
tid: u64,
185-
collection: &str,
186-
field_name: &str,
187-
m: usize,
188-
m0: usize,
189-
ef_construction: usize,
193+
params: VectorRebuildParams<'_>,
190194
) -> Response {
191195
use crate::engine::vector::hnsw::{HnswIndex, HnswParams};
192196

197+
let VectorRebuildParams {
198+
task,
199+
tid,
200+
collection,
201+
field_name,
202+
m,
203+
m0,
204+
ef_construction,
205+
} = params;
206+
193207
let database_id = task.request.database_id.as_u64();
194208
let index_key = CoreLoop::vector_index_key(database_id, tid, collection, field_name);
195209
debug!(

0 commit comments

Comments
 (0)