Skip to content

Commit

Permalink
fix agg result on empty index (#4449)
Browse files Browse the repository at this point in the history
* fix agg result on empty index

empty indices return empty intermediate aggregation results. To get the
correct result structure we need to call `into_final` instead of early exit.

fixes #4437

* clear aggregation if no index is hit
  • Loading branch information
PSeitz authored Jan 25, 2024
1 parent 04fc073 commit 376df71
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 12 deletions.
39 changes: 27 additions & 12 deletions quickwit/quickwit-search/src/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -876,11 +876,15 @@ async fn root_search_aux(
)
.await?;

let aggregation_result_json_opt = finalize_aggregation_if_any(
let mut aggregation_result_json_opt = finalize_aggregation_if_any(
&search_request,
first_phase_result.intermediate_aggregation_result,
searcher_context,
)?;
// In case there is no index, we don't want the response to contain any aggregation structure
if indexes_metas_for_leaf_search.is_empty() {
aggregation_result_json_opt = None;
}

Ok(SearchResponse {
aggregation: aggregation_result_json_opt,
Expand All @@ -895,25 +899,39 @@ async fn root_search_aux(
}

fn finalize_aggregation(
intermediate_aggregation_result_bytes: &[u8],
intermediate_aggregation_result_bytes_opt: Option<Vec<u8>>,
aggregations: QuickwitAggregations,
searcher_context: &SearcherContext,
) -> crate::Result<String> {
) -> crate::Result<Option<String>> {
let merge_aggregation_result = match aggregations {
QuickwitAggregations::FindTraceIdsAggregation(_) => {
let Some(intermediate_aggregation_result_bytes) =
intermediate_aggregation_result_bytes_opt
else {
return Ok(None);
};
// The merge collector has already merged the intermediate results.
let aggs: Vec<Span> = postcard::from_bytes(intermediate_aggregation_result_bytes)?;
let aggs: Vec<Span> = postcard::from_bytes(&intermediate_aggregation_result_bytes)?;
serde_json::to_string(&aggs)?
}
QuickwitAggregations::TantivyAggregations(aggregations) => {
let intermediate_aggregation_results: IntermediateAggregationResults =
postcard::from_bytes(intermediate_aggregation_result_bytes)?;
let intermediate_aggregation_results =
if let Some(intermediate_aggregation_result_bytes) =
intermediate_aggregation_result_bytes_opt
{
let intermediate_aggregation_results: IntermediateAggregationResults =
postcard::from_bytes(&intermediate_aggregation_result_bytes)?;
intermediate_aggregation_results
} else {
// Default, to return correct structure
Default::default()
};
let final_aggregation_results: AggregationResults = intermediate_aggregation_results
.into_final_result(aggregations, &searcher_context.get_aggregation_limits())?;
serde_json::to_string(&final_aggregation_results)?
}
};
Ok(merge_aggregation_result)
Ok(Some(merge_aggregation_result))
}

fn finalize_aggregation_if_any(
Expand All @@ -925,15 +943,12 @@ fn finalize_aggregation_if_any(
return Ok(None);
};
let aggregations: QuickwitAggregations = serde_json::from_str(aggregations_json)?;
let Some(intermediate_result_bytes) = intermediate_aggregation_result_bytes_opt else {
return Ok(None);
};
let aggregation_result_json = finalize_aggregation(
&intermediate_result_bytes[..],
intermediate_aggregation_result_bytes_opt,
aggregations,
searcher_context,
)?;
Ok(Some(aggregation_result_json))
Ok(aggregation_result_json)
}

/// Checks that all of the index researched as found.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,3 +192,21 @@ expected:
- doc_count: 3
key: 100.0

---
# Test histogram empty result on empty index
method: [GET]
engines:
- quickwit
endpoint: _elastic/empty_aggregations/_search
json:
query: { match_all: {} }
aggs:
metrics:
histogram:
field: response
interval: 50
expected:
aggregations:
metrics:
buckets: []

24 changes: 24 additions & 0 deletions quickwit/rest-api-tests/scenarii/aggregations/_setup.quickwit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ method: DELETE
endpoint: indexes/aggregations
status_code: null
---
method: DELETE
endpoint: indexes/empty_aggregations
status_code: null
---
# Create index
method: POST
endpoint: indexes/
Expand All @@ -23,6 +27,26 @@ json:
fast: true
sleep_after: 3
---
# Create empty index
method: POST
endpoint: indexes/
json:
version: "0.7"
index_id: empty_aggregations
doc_mapping:
mode: dynamic
dynamic_mapping:
tokenizer: default
fast: true
field_mappings:
- name: date
type: datetime
input_formats:
- rfc3339
fast_precision: seconds
fast: true
sleep_after: 3
---
# Ingest documents
method: POST
endpoint: aggregations/ingest
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# # Delete index
method: DELETE
endpoint: indexes/aggregations
---
method: DELETE
endpoint: indexes/empty_aggregations

0 comments on commit 376df71

Please sign in to comment.