Skip to content

Commit

Permalink
fix agg result on empty index
Browse files Browse the repository at this point in the history
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
  • Loading branch information
PSeitz committed Jan 24, 2024
1 parent cd7ef54 commit 2614c97
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 11 deletions.
33 changes: 22 additions & 11 deletions quickwit/quickwit-search/src/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -895,25 +895,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 +939,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 2614c97

Please sign in to comment.