Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix agg result on empty index #4449

Merged
merged 3 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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