Skip to content

Commit 271e5e1

Browse files
mdashtimdashti
committed
nitz
1 parent a3712c1 commit 271e5e1

5 files changed

Lines changed: 29 additions & 36 deletions

File tree

‎src/aggregation/agg_result.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ pub enum BucketResult {
150150
///
151151
/// See [`TermsAggregation`](super::bucket::TermsAggregation)
152152
buckets: Vec<BucketEntry>,
153-
/// The number of documents that didn't make it into to TOP N due to shard_size or size
153+
/// The number of documents that didn’t make it into to TOP N due to shard_size or size
154154
sum_other_doc_count: u64,
155155
#[serde(skip_serializing_if = "Option::is_none")]
156156
/// The upper bound error for the doc count of each term.
@@ -175,6 +175,7 @@ impl BucketResult {
175175
doc_count_error_upper_bound: _,
176176
} => buckets.iter().map(|bucket| bucket.get_bucket_count()).sum(),
177177
BucketResult::Filter(filter_result) => {
178+
// add one for the filter bucket itself
178179
1 + filter_result.sub_aggregations.get_bucket_count()
179180
}
180181
}

‎src/aggregation/intermediate_agg_result.rs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ use super::metric::{
2424
};
2525
use super::segment_agg_result::AggregationLimitsGuard;
2626
use super::{format_date, AggregationError, Key, SerializedKey};
27-
use crate::aggregation::agg_result::{AggregationResults, BucketEntries, BucketEntry};
27+
use crate::aggregation::agg_result::{
28+
AggregationResults, BucketEntries, BucketEntry, FilterBucketResult,
29+
};
2830
use crate::aggregation::bucket::TermsAggregationInternal;
2931
use crate::aggregation::metric::CardinalityCollector;
3032
use crate::TantivyError;
@@ -527,9 +529,6 @@ impl IntermediateBucketResult {
527529
// Convert sub-aggregation results to final format
528530
let final_sub_aggregations = sub_aggregations
529531
.into_final_result(req.sub_aggregation().clone(), limits.clone())?;
530-
531-
// Create a filter bucket result (similar to Elasticsearch format)
532-
use crate::aggregation::agg_result::FilterBucketResult;
533532
Ok(BucketResult::Filter(FilterBucketResult {
534533
doc_count,
535534
sub_aggregations: final_sub_aggregations,

‎src/aggregation/segment_agg_result.rs‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use super::metric::{
1515
SegmentPercentilesCollector, SegmentStatsCollector, SegmentStatsType, StatsAggregation,
1616
SumAggregation,
1717
};
18-
use crate::aggregation::bucket::TermMissingAgg;
18+
use crate::aggregation::bucket::{FilterSegmentCollector, TermMissingAgg};
1919
use crate::aggregation::metric::{
2020
CardinalityAggregationReq, SegmentCardinalityCollector, SegmentExtendedStatsCollector,
2121
TopHitsSegmentCollector,
@@ -187,9 +187,6 @@ pub(crate) fn build_single_agg_segment_collector_with_reader(
187187
SegmentCardinalityCollector::from_req(req.field_type, accessor_idx, missing),
188188
)),
189189
Filter(filter_req) => {
190-
// Create FilterSegmentCollector following the same pattern as other bucket aggregations
191-
use crate::aggregation::bucket::FilterSegmentCollector;
192-
193190
if let Some(segment_reader) = segment_reader {
194191
Ok(Box::new(FilterSegmentCollector::from_req_and_validate(
195192
filter_req,
@@ -266,11 +263,6 @@ impl SegmentAggregationCollector for GenericSegmentAggregationResultsCollector {
266263
}
267264

268265
impl GenericSegmentAggregationResultsCollector {
269-
#[allow(dead_code)]
270-
pub(crate) fn from_req_and_validate(req: &mut AggregationsWithAccessor) -> crate::Result<Self> {
271-
Self::from_req_and_validate_with_reader(req, None)
272-
}
273-
274266
pub(crate) fn from_req_and_validate_with_reader(
275267
req: &mut AggregationsWithAccessor,
276268
segment_reader: Option<&crate::SegmentReader>,

‎tests/common/filter_test_helpers.rs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ pub fn aggregation_results_to_json(results: &AggregationResults) -> Value {
77
}
88

99
/// Compare two JSON values with tolerance for floating point numbers
10-
#[allow(dead_code)]
1110
pub fn json_values_match(actual: &Value, expected: &Value, tolerance: f64) -> bool {
1211
match (actual, expected) {
1312
(Value::Number(a), Value::Number(e)) => {
@@ -69,7 +68,6 @@ pub fn json_values_match(actual: &Value, expected: &Value, tolerance: f64) -> bo
6968
/// });
7069
/// assert_aggregation_results_match(&result, expected, 0.1);
7170
/// ```
72-
#[allow(dead_code)]
7371
pub fn assert_aggregation_results_match(
7472
actual_results: &AggregationResults,
7573
expected_json: Value,
@@ -90,9 +88,13 @@ pub fn assert_aggregation_results_match(
9088
#[macro_export]
9189
macro_rules! assert_agg_results {
9290
($actual:expr, $expected:expr) => {
93-
assert_aggregation_results_match($actual, $expected, 0.1)
91+
$crate::common::filter_test_helpers::assert_aggregation_results_match(
92+
$actual, $expected, 0.1,
93+
)
9494
};
9595
($actual:expr, $expected:expr, $tolerance:expr) => {
96-
assert_aggregation_results_match($actual, $expected, $tolerance)
96+
$crate::common::filter_test_helpers::assert_aggregation_results_match(
97+
$actual, $expected, $tolerance,
98+
)
9799
};
98100
}

‎tests/filter_aggregation.rs‎

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
1111
mod common;
1212

13-
use common::filter_test_helpers::*;
1413
use serde_json::json;
1514
use tantivy::aggregation::agg_req::Aggregations;
1615
use tantivy::aggregation::bucket::filter::FilterAggregation;
@@ -88,7 +87,7 @@ fn test_basic_filter_with_metric_agg() -> tantivy::Result<()> {
8887
}
8988
});
9089

91-
assert_aggregation_results_match(&result, expected, 0.1);
90+
assert_agg_results!(&result, expected);
9291
Ok(())
9392
}
9493

@@ -118,7 +117,7 @@ fn test_filter_with_no_matches() -> tantivy::Result<()> {
118117
}
119118
});
120119

121-
assert_aggregation_results_match(&result, expected, 0.1);
120+
assert_agg_results!(&result, expected);
122121
Ok(())
123122
}
124123

@@ -162,7 +161,7 @@ fn test_multiple_independent_filters() -> tantivy::Result<()> {
162161
}
163162
});
164163

165-
assert_aggregation_results_match(&result, expected, 0.1);
164+
assert_agg_results!(&result, expected);
166165
Ok(())
167166
}
168167

@@ -194,7 +193,7 @@ fn test_term_query_filter() -> tantivy::Result<()> {
194193
}
195194
});
196195

197-
assert_aggregation_results_match(&result, expected, 0.1);
196+
assert_agg_results!(&result, expected);
198197
Ok(())
199198
}
200199

@@ -222,7 +221,7 @@ fn test_range_query_filter() -> tantivy::Result<()> {
222221
}
223222
});
224223

225-
assert_aggregation_results_match(&result, expected, 0.1);
224+
assert_agg_results!(&result, expected);
226225
Ok(())
227226
}
228227

@@ -250,7 +249,7 @@ fn test_boolean_query_filter() -> tantivy::Result<()> {
250249
}
251250
});
252251

253-
assert_aggregation_results_match(&result, expected, 0.1);
252+
assert_agg_results!(&result, expected);
254253
Ok(())
255254
}
256255

@@ -286,7 +285,7 @@ fn test_bool_field_filter() -> tantivy::Result<()> {
286285
}
287286
});
288287

289-
assert_aggregation_results_match(&result, expected, 1.0);
288+
assert_agg_results!(&result, expected, 1.0);
290289
Ok(())
291290
}
292291

@@ -336,7 +335,7 @@ fn test_two_level_nested_filters() -> tantivy::Result<()> {
336335
}
337336
});
338337

339-
assert_aggregation_results_match(&result, expected, 0.1);
338+
assert_agg_results!(&result, expected);
340339
Ok(())
341340
}
342341

@@ -390,7 +389,7 @@ fn test_deeply_nested_filters() -> tantivy::Result<()> {
390389
}
391390
});
392391

393-
assert_aggregation_results_match(&result, expected, 0.1);
392+
assert_agg_results!(&result, expected);
394393
Ok(())
395394
}
396395

@@ -438,7 +437,7 @@ fn test_multiple_nested_branches() -> tantivy::Result<()> {
438437
}
439438
});
440439

441-
assert_aggregation_results_match(&result, expected, 0.1);
440+
assert_agg_results!(&result, expected);
442441
Ok(())
443442
}
444443

@@ -526,7 +525,7 @@ fn test_nested_filters_with_multiple_siblings_at_each_level() -> tantivy::Result
526525
}
527526
});
528527

529-
assert_aggregation_results_match(&result, expected, 0.1);
528+
assert_agg_results!(&result, expected);
530529
Ok(())
531530
}
532531

@@ -581,7 +580,7 @@ fn test_filter_with_terms_sub_agg() -> tantivy::Result<()> {
581580
}
582581
});
583582

584-
assert_aggregation_results_match(&result, expected, 0.1);
583+
assert_agg_results!(&result, expected);
585584
Ok(())
586585
}
587586

@@ -621,7 +620,7 @@ fn test_filter_with_multiple_metric_aggs() -> tantivy::Result<()> {
621620
}
622621
});
623622

624-
assert_aggregation_results_match(&result, expected, 0.1);
623+
assert_agg_results!(&result, expected);
625624
Ok(())
626625
}
627626

@@ -661,7 +660,7 @@ fn test_filter_on_empty_index() -> tantivy::Result<()> {
661660
}
662661
});
663662

664-
assert_aggregation_results_match(&result, expected, 0.1);
663+
assert_agg_results!(&result, expected);
665664
Ok(())
666665
}
667666

@@ -723,7 +722,7 @@ fn test_filter_with_base_query() -> tantivy::Result<()> {
723722
}
724723
});
725724

726-
assert_aggregation_results_match(&result, expected, 0.1);
725+
assert_agg_results!(&result, expected);
727726
Ok(())
728727
}
729728

@@ -865,10 +864,10 @@ fn test_filter_result_correctness_vs_separate_query() -> tantivy::Result<()> {
865864
});
866865

867866
// Verify filter aggregation result
868-
assert_aggregation_results_match(&filter_result, filter_expected, 0.1);
867+
assert_agg_results!(&filter_result, filter_expected);
869868

870869
// Verify separate query result matches
871-
assert_aggregation_results_match(&separate_result, separate_expected, 0.1);
870+
assert_agg_results!(&separate_result, separate_expected);
872871

873872
// This test demonstrates that filter aggregation produces the same results
874873
// as running a separate query with the same condition

0 commit comments

Comments
 (0)