Open
Conversation
fulmicoton-dd
force-pushed
the
composite-aggregations-rebase
branch
from
a9b38fb to
32a8e64
Compare
Collaborator
|
@rdettai-sk Merging your PR in this. Claude made it in a different PR for the rebase. (I hardocded you as the author the author so you get the github stats and street cred. ( |
congx4
approved these changes
Mar 23, 2026
Rebase of #5957 by @rdettai-sk onto main, updated to use the latest tantivy revision (545169c0d8) which includes composite aggregation support upstream. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fulmicoton-dd
force-pushed
the
composite-aggregations-rebase
branch
from
32a8e64 to
2914a35
Compare
The updated tantivy revision (545169c0d8) produces a slightly different floating-point result for the p85 percentile aggregation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ation The updated tantivy revision changes HLL cardinality estimates: - unique_names: 8.0 -> 7.0 (actual: 8 unique values) - unique_dates: 6.0 -> 5.0 (actual: 6 unique values) - unique_response: 5.0 unchanged Verified locally by running quickwit and querying the aggregation endpoint. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cardinality (HLL) and percentile values vary across runs because data lands in different splits, producing different approximation results. Use $expect with range checks instead of exact value matching. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
rdettai-sk
approved these changes
Mar 25, 2026
Collaborator
rdettai-sk
left a comment
rdettai-sk
left a comment
There was a problem hiding this comment.
thanks @fulmicoton!
The tests are failing because I have updated tantivy in between to have a safer pagination: just relying on json types is not enough to fully represent all possible page positions inside quickwit fastfield column types. Ideally we should not assert on the pagination key value as it is supposed to be opaque according to the ES spec, but I found it convenient to have a clear test that showcases the behavior here.
Comment on lines
+441
to
+443
| host: "192.168.0.10" | ||
| name: "Albert" | ||
| response: 100.0 |
Collaborator
There was a problem hiding this comment.
Suggested change
| host: "192.168.0.10" | |
| name: "Albert" | |
| response: 100.0 | |
| host: "str:192.168.0.10" | |
| name: "str:Albert" | |
| response: "f64:100" |
Comment on lines
+470
to
+472
| host: "192.168.0.10" | ||
| name: "Albert" | ||
| response: 100.0 |
Collaborator
There was a problem hiding this comment.
Suggested change
| host: "192.168.0.10" | |
| name: "Albert" | |
| response: 100.0 | |
| host: "str:192.168.0.10" | |
| name: "str:Albert" | |
| response: "f64:100" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
545169c0d8) which includes composite aggregation support upstreamquickwit-queryaggregation mappingsSend+Syncconstraint incontrolled_directory.rsfor the updated tantivy APITest plan
cargo checkpassescargo nextest run -p quickwit-query— 139 tests passcargo nextest run -p quickwit-indexing— 165 tests passcargo nextest run -p quickwit-search— 159 tests pass🤖 Generated with Claude Code