Repository navigation
Push contains binary doc values query down to es819 codec - #143898
Merged
parkertimmins merged 9 commits intoMar 20, 2026
Merged
parkertimmins merged 9 commits into
parkertimmins merged 9 commits into
Conversation
For single-valued binary doc values fields, push the contains term check into the ES819 codec's BinaryDecoder via a containsIterator. This avoids per-document BinaryDocValues lookups by scanning the already-decompressed block data directly. Extract multiValuedIterator into AbstractBinaryDocValuesQuery for reuse across BinaryDocValuesContainsTermQuery and BinaryDocValuesLengthQuery. Made-with: Cursor
Test the BinaryDecoder.containsTermIterator directly via the ES819BinaryDocValues.containsIterator method, verifying nextDoc, advance, and no-match exhaustion. Also fix SlowCustomBinaryDocValuesWildcardQueryTests to use indexed numeric doc values fields so the DocValuesSkipper is available. Made-with: Cursor
Collaborator
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
Collaborator
|
Hi @parkertimmins, I've created a changelog YAML for you. |
Contributor
Author
|
Buildkite benchmark this with clickbench-columnar-mode please |
martijnvg
reviewed
Mar 10, 2026
martijnvg
left a comment
Member
There was a problem hiding this comment.
Looks good! I left a few comments and let's run benchmark again to see what q22 does.
Contributor
Author
|
Buildkite benchmark this with clickbench-columnar-mode please |
Contributor
Author
|
Buildkite benchmark this with clickbench-columnar-mode please |
Rename containsIterator/lengthIterator to tryContainsIterator/ tryLengthIterator to follow the try* naming convention. Remove the redundant countsSkipper.maxValue() == 1 guard in BinaryDocValuesContainsTermQuery since the codec already only returns an iterator for the dense single-valued case. Randomize test values using randomUnicodeOfCodepointLengthBetween. Made-with: Cursor
Contributor
Author
|
Buildkite benchmark this with clickbench-columnar-mode please |
Contributor
Author
|
Buildkite benchmark this with clickbench-columnar-mode please |
1 similar comment
Contributor
Author
|
Buildkite benchmark this with clickbench-columnar-mode please |
Collaborator
💚 Build Succeeded
This build ran two clickbench-columnar-mode benchmarks to evaluate performance impact of this PR. History
|
salvatore-campagna
added a commit
to salvatore-campagna/elasticsearch
that referenced
this pull request
Mar 23, 2026
Resolve conflict with elastic#143898: move tryContainsIterator and containsTermIterator into AbstractTSDBDocValuesProducer, rename lengthIterator to tryLengthIterator.
salvatore-campagna
added a commit
to salvatore-campagna/elasticsearch
that referenced
this pull request
Mar 23, 2026
Resolve conflict with elastic#143898: keep thin ES819 producer, use TSDBBinaryDocValues/getTSDBBinaryValues in tests.
salvatore-campagna
added a commit
to salvatore-campagna/elasticsearch
that referenced
this pull request
Mar 23, 2026
Rename lengthIterator to tryLengthIterator, add tryContainsIterator and containsTermIterator in AbstractTSDBDocValuesProducer.
michalborek
pushed a commit
to michalborek/elasticsearch
that referenced
this pull request
Mar 23, 2026
…3898) This PR pushes contains-term matching down into the ES819 codec's BinaryDecoder for single-valued binary doc values fields. Instead of iterating doc-by-doc through BinaryDocValues and checking each value, the new containsIterator scans already-decompressed block data directly, avoiding per-document doc values lookups. Also includes some cleanup: the shared TwoPhaseIterator logic used by AbstractBinaryDocValuesQuery, BinaryDocValuesContainsTermQuery, and BinaryDocValuesLengthQuery is extracted into AbstractBinaryDocValuesQuery.multiValuedIterator.
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.
This PR pushes contains-term matching down into the ES819 codec's BinaryDecoder for single-valued binary doc values fields. Instead of iterating doc-by-doc through BinaryDocValues and checking each value, the new containsIterator scans already-decompressed block data directly, avoiding per-document doc values lookups.
Also includes some cleanup: the shared TwoPhaseIterator logic used by AbstractBinaryDocValuesQuery, BinaryDocValuesContainsTermQuery, and BinaryDocValuesLengthQuery is extracted into AbstractBinaryDocValuesQuery.multiValuedIterator.
Based on #142503