Skip to content

Commit d79eccf

Browse files
committed
refactor(aggregate): bundle cache key inputs into a params struct
Replace aggregate_cache_key's nine positional arguments (and its too_many_arguments allow) with an AggregateCacheKeyInputs struct, naming the complete query shape that constitutes the cache identity.
1 parent 8f0c9a8 commit d79eccf

3 files changed

Lines changed: 39 additions & 21 deletions

File tree

nodedb/src/data/executor/handlers/aggregate/cache_key.rs

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,19 +46,37 @@ fn aggregate_specs_key(aggregates: &[AggregateSpec]) -> String {
4646
.join(",")
4747
}
4848

49-
#[allow(clippy::too_many_arguments)] // complete query shape is the cache identity
49+
/// Complete query shape that constitutes an aggregate result's cache identity.
50+
/// Every field participates in the key — two aggregate queries that differ in
51+
/// any one of them (output aliases, `LIMIT`, `ORDER BY`, sub-aggregation) must
52+
/// not collide on a shared cache entry.
53+
pub(super) struct AggregateCacheKeyInputs<'a> {
54+
pub database_id: u64,
55+
pub tid: u64,
56+
pub collection: &'a str,
57+
pub group_by: &'a [GroupKeySpec],
58+
pub aggregates: &'a [AggregateSpec],
59+
pub sub_group_by: &'a [String],
60+
pub sub_aggregates: &'a [AggregateSpec],
61+
pub limit: usize,
62+
pub sort_keys: &'a [(String, bool)],
63+
}
64+
5065
pub(super) fn aggregate_cache_key(
51-
database_id: u64,
52-
tid: u64,
53-
collection: &str,
54-
group_by: &[GroupKeySpec],
55-
aggregates: &[AggregateSpec],
56-
sub_group_by: &[String],
57-
sub_aggregates: &[AggregateSpec],
58-
limit: usize,
59-
sort_keys: &[(String, bool)],
66+
inputs: AggregateCacheKeyInputs<'_>,
6067
) -> (crate::types::DatabaseId, crate::types::TenantId, String) {
6168
use std::fmt::Write;
69+
let AggregateCacheKeyInputs {
70+
database_id,
71+
tid,
72+
collection,
73+
group_by,
74+
aggregates,
75+
sub_group_by,
76+
sub_aggregates,
77+
limit,
78+
sort_keys,
79+
} = inputs;
6280
let mut rest = format!(
6381
"{collection}\0{}\0{}",
6482
group_specs_key(group_by),

nodedb/src/data/executor/handlers/aggregate/exec.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
77
use tracing::debug;
88

9-
use super::cache_key::{aggregate_cache_key, legacy_aggregate_pairs};
9+
use super::cache_key::{AggregateCacheKeyInputs, aggregate_cache_key, legacy_aggregate_pairs};
1010
use super::rows::{apply_user_aliases_to_rows, sort_aggregated_rows};
1111
use crate::bridge::envelope::{ErrorCode, Response};
1212
use crate::bridge::scan_filter::ScanFilter;
@@ -119,8 +119,8 @@ impl CoreLoop {
119119

120120
// Fast path: incremental aggregate cache.
121121
if filters.is_empty() && having.is_empty() {
122-
let cache_key = aggregate_cache_key(
123-
task.request.database_id.as_u64(),
122+
let cache_key = aggregate_cache_key(AggregateCacheKeyInputs {
123+
database_id: task.request.database_id.as_u64(),
124124
tid,
125125
collection,
126126
group_by,
@@ -129,7 +129,7 @@ impl CoreLoop {
129129
sub_aggregates,
130130
limit,
131131
sort_keys,
132-
);
132+
});
133133
if let Some(cached) = self.aggregate_cache.get(&cache_key) {
134134
debug!(core = self.core_id, %collection, "aggregate cache hit");
135135
return self.response_with_payload(task, cached.clone());
@@ -273,8 +273,8 @@ impl CoreLoop {
273273
) {
274274
Ok(payload) => {
275275
if filters.is_empty() && having.is_empty() {
276-
let cache_key = aggregate_cache_key(
277-
task.request.database_id.as_u64(),
276+
let cache_key = aggregate_cache_key(AggregateCacheKeyInputs {
277+
database_id: task.request.database_id.as_u64(),
278278
tid,
279279
collection,
280280
group_by,
@@ -283,7 +283,7 @@ impl CoreLoop {
283283
sub_aggregates,
284284
limit,
285285
sort_keys,
286-
);
286+
});
287287
if self.aggregate_cache.len() < 256 {
288288
self.aggregate_cache.insert(cache_key, payload.clone());
289289
}

nodedb/src/data/executor/handlers/aggregate/streaming/over_docs.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//! `aggregate_over_docs`: orchestrate accumulate + finalize over an
44
//! already-materialized doc set, layering the per-shard result cache on top.
55
6-
use super::super::cache_key::aggregate_cache_key;
6+
use super::super::cache_key::{AggregateCacheKeyInputs, aggregate_cache_key};
77
use crate::bridge::envelope::{ErrorCode, Response};
88
use crate::data::executor::core_loop::CoreLoop;
99
use crate::data::executor::task::ExecutionTask;
@@ -104,8 +104,8 @@ impl CoreLoop {
104104
&& filters.is_empty()
105105
&& having.is_empty()
106106
{
107-
let cache_key = aggregate_cache_key(
108-
task.request.database_id.as_u64(),
107+
let cache_key = aggregate_cache_key(AggregateCacheKeyInputs {
108+
database_id: task.request.database_id.as_u64(),
109109
tid,
110110
collection,
111111
group_by,
@@ -114,7 +114,7 @@ impl CoreLoop {
114114
sub_aggregates,
115115
limit,
116116
sort_keys,
117-
);
117+
});
118118
if self.aggregate_cache.len() < 256 {
119119
self.aggregate_cache.insert(cache_key, payload.clone());
120120
}

0 commit comments

Comments
 (0)