Add bitmap64 query support#20606
Conversation
divyaruhil
requested review from
a team,
Bukhtawar,
CEHENKLE,
Rishikesh1159,
anasalkouz,
andrross,
ashking94,
cwperks,
dbwiddis,
gbbafna,
jed326,
kotwanikunal,
mch2,
msfroh,
owaiskazi19,
reta,
sachinpkale,
saratvemulapalli,
shwetathareja and
sohami
as code owners
📝 WalkthroughWalkthroughThis pull request introduces 64-bit bitmap query support to OpenSearch by adding two new Lucene Query implementations (for index and doc values), integrating them into NumberFieldMapper for LONG numeric fields, and providing comprehensive test coverage for both query types. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Mapper as NumberFieldMapper
participant Deserializer as Bitmap Deserializer
participant QueryFactory as Query Type Selector
participant IndexQuery as Bitmap64IndexQuery
participant DocValuesQuery as Bitmap64DocValuesQuery
participant Searcher as IndexSearcher
Client->>Mapper: bitmapQuery(field, BytesArray, isSearchable, hasDocValues)
Mapper->>Deserializer: deserialize 64-bit Roaring bitmap
Deserializer-->>Mapper: Roaring64NavigableMap
alt isSearchable && hasDocValues
Mapper->>QueryFactory: Create IndexOrDocValuesQuery
QueryFactory->>IndexQuery: new Bitmap64IndexQuery(field, bitmap)
QueryFactory->>DocValuesQuery: new Bitmap64DocValuesQuery(field, bitmap)
QueryFactory-->>Mapper: IndexOrDocValuesQuery(indexQuery, docValuesQuery)
else isSearchable
Mapper->>IndexQuery: new Bitmap64IndexQuery(field, bitmap)
IndexQuery-->>Mapper: IndexQuery
else hasDocValues
Mapper->>DocValuesQuery: new Bitmap64DocValuesQuery(field, bitmap)
DocValuesQuery-->>Mapper: DocValuesQuery
end
Mapper-->>Client: Query
Client->>Searcher: search(query)
alt IndexQuery Path
Searcher->>IndexQuery: createWeight()
IndexQuery->>Searcher: PointValues ∩ Bitmap
Searcher-->>Client: DocIdSet(matching docs)
else DocValuesQuery Path
Searcher->>DocValuesQuery: createWeight()
DocValuesQuery->>Searcher: SortedNumericDocValues ∩ Bitmap
Searcher-->>Client: DocIdSet(matching docs)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested labels
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
divyaruhil
force-pushed
the
bitmap_64
branch
from
9f76e75 to
0dcc129
Compare
|
❌ Gradle check result for 0dcc129: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@server/src/main/java/org/opensearch/index/mapper/NumberFieldMapper.java`:
- Around line 1376-1399: The catch in NumberFieldMapper.bitmapQuery currently
catches Exception; change it to catch IOException (the declared exception from
deserializePortable) to avoid masking other errors—update the catch block in the
bitmapQuery method that deserializes into Roaring64NavigableMap to "catch
(IOException e)"; make the identical change in the INTEGER.bitmapQuery
deserialization catch as well; and add a short inline comment near both
bitmap64/bitmap32 usages (and the constructors for
Bitmap64IndexQuery/Bitmap64DocValuesQuery or where they are instantiated) noting
that the Roaring* bitmap is read-only after deserialization and must not be
modified by the query classes.
In
`@server/src/main/java/org/opensearch/search/query/Bitmap64DocValuesQuery.java`:
- Around line 1-3: Replace the short 3-line SPDX headers (or missing header) in
the new source and test files with the project's full 7-line header including
the OpenSearch contributor notice; specifically update the header comment at the
top of Bitmap64DocValuesQuery.java (class Bitmap64DocValuesQuery),
Bitmap64IndexQuery.java (class Bitmap64IndexQuery), and
Bitmap64IndexQueryTests.java (class Bitmap64IndexQueryTests) so they all start
with: "/*\n * SPDX-License-Identifier: Apache-2.0\n *\n * The OpenSearch
Contributors require contributions made to\n * this file be licensed under the
Apache-2.0 license or a\n * compatible open source license.\n */" ensuring exact
text and spacing to match existing project files.
- Around line 104-105: The code currently constructs a DefaultScorerSupplier
directly (e.g., in Bitmap64DocValuesQuery where you return new
DefaultScorerSupplier(scorer)); change these to use the fully-qualified nested
type Weight.DefaultScorerSupplier instead (return new
Weight.DefaultScorerSupplier(scorer)), and update all similar usages (e.g., in
BitmapDocValuesQuery, ShardSplittingQuery, etc.) so they reference
Weight.DefaultScorerSupplier rather than the unqualified DefaultScorerSupplier.
In
`@server/src/test/java/org/opensearch/search/query/Bitmap64IndexQueryTests.java`:
- Line 1: Add the missing SPDX license header to the top of the file for
Bitmap64IndexQueryTests: insert the same Apache-2.0 SPDX header block used in
other new files (for example Bitmap64DocValuesQueryTests) as the very first
lines of
server/src/test/java/org/opensearch/search/query/Bitmap64IndexQueryTests.java so
the file begins with the standard SPDX/Apache-2.0 header before the package
declaration and class definition.
🧹 Nitpick comments (7)
server/src/main/java/org/opensearch/search/query/Bitmap64IndexQuery.java (3)
29-29: Unused import:PeekableLongIteratoris imported but never used.Line 64 uses
org.roaringbitmap.longlong.LongIteratordirectly.PeekableLongIterator(which extendsLongIteratorand addsadvanceIfNeeded(long)) would actually be useful here — see next comment aboutadvance()performance.
66-66: Inconsistent indentation: tab character mixed with spaces.Line 66 uses a tab character for indentation while the rest of the file uses spaces.
141-208:MergePointVisitoris an inner class but could be static with the bitmap passed as a parameter.
MergePointVisitoris a non-static inner class that captures the enclosingBitmap64IndexQueryinstance solely to accessbitmap. Making itstaticand passingbitmapto the constructor would make the dependency explicit and avoid retaining a reference to the entire query object.That said, this mirrors the pattern used in the 32-bit
BitmapIndexQuery, so it's consistent with the existing codebase.server/src/test/java/org/opensearch/search/query/Bitmap64DocValuesQueryTests.java (2)
130-136:query.rewrite(null)relies on short-circuit beforesuper.rewrite(null)is reached.This works today because the empty-bitmap check returns early. If the implementation ever changes to invoke super before the check, this test will NPE. Consider creating a minimal
IndexSearcher(e.g. from an empty directory) instead.
163-193: Consider adding a test with negative long values.The current
testLargeValuestests nearLong.MAX_VALUE, which is great. However, there's no test for negative long values (e.g.,Long.MIN_VALUE, negative ranges). SinceLongPoint.encodeDimensionuses an offset encoding that mapsLong.MIN_VALUE → 0x0000...andLong.MAX_VALUE → 0xFFFF..., ensuring correct behavior across the signed boundary would strengthen coverage—especially for the doc-values path wheremin/maxrange checks (lines 71 and 86-88 inBitmap64DocValuesQuery) must handle negative values correctly.server/src/test/java/org/opensearch/search/query/Bitmap64IndexQueryTests.java (1)
76-99: Consider adding edge case tests for the index query path.The random test is a good fuzz-style check, but explicit tests for edge cases would improve confidence:
- Empty bitmap (verify rewrite to
MatchNoDocsQuery)- Negative long values and values spanning the signed boundary (e.g.,
-1L,Long.MIN_VALUE)equals/hashCodecontract verificationThe doc-values test file covers some of these for its query type, but the index path has a very different code path (BKD tree intersection) that warrants its own edge case coverage.
server/src/main/java/org/opensearch/search/query/Bitmap64DocValuesQuery.java (1)
30-30: Cross-class dependency forcheckArgsvalidation.
Bitmap64DocValuesQueryimportscheckArgsfromBitmap64IndexQuery. This creates a tight coupling where the doc-values query depends on the index query class for basic validation. This is minor but could be improved by either:
- Extracting
checkArgsinto a shared utility (e.g., aBitmap64QueryUtilsclass), or- Duplicating the trivial 2-line validation in each class.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
server/src/main/java/org/opensearch/index/mapper/NumberFieldMapper.javaserver/src/main/java/org/opensearch/search/query/Bitmap64DocValuesQuery.javaserver/src/main/java/org/opensearch/search/query/Bitmap64IndexQuery.javaserver/src/test/java/org/opensearch/search/query/Bitmap64DocValuesQueryTests.javaserver/src/test/java/org/opensearch/search/query/Bitmap64IndexQueryTests.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-13T17:40:27.167Z
Learnt from: reta
Repo: opensearch-project/OpenSearch PR: 20411
File: server/src/main/java/org/opensearch/index/codec/CodecService.java:112-133
Timestamp: 2026-01-13T17:40:27.167Z
Learning: Avoid capturing or evaluating a supplier (e.g., this::defaultCodec) upfront when passing it to a registry during object construction. If registries may replace defaults during iteration (as in EnginePlugin.getAdditionalCodecs), pass the supplier itself and only resolve it at use time. This ensures dynamic behavior is preserved during initialization and prevents premature binding of defaults in codecs/registry setup. This pattern should apply to similar initialization paths in Java server code where registries may mutate defaults during construction.
Applied to files:
server/src/main/java/org/opensearch/index/mapper/NumberFieldMapper.javaserver/src/main/java/org/opensearch/search/query/Bitmap64DocValuesQuery.javaserver/src/main/java/org/opensearch/search/query/Bitmap64IndexQuery.java
🧬 Code graph analysis (2)
server/src/main/java/org/opensearch/index/mapper/NumberFieldMapper.java (2)
server/src/main/java/org/opensearch/search/query/Bitmap64IndexQuery.java (1)
Bitmap64IndexQuery(41-244)server/src/main/java/org/opensearch/search/query/Bitmap64DocValuesQuery.java (1)
Bitmap64DocValuesQuery(36-155)
server/src/main/java/org/opensearch/search/query/Bitmap64DocValuesQuery.java (1)
server/src/main/java/org/opensearch/search/query/Bitmap64IndexQuery.java (1)
Bitmap64IndexQuery(41-244)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: gradle-check
🔇 Additional comments (7)
server/src/main/java/org/opensearch/search/query/Bitmap64IndexQuery.java (3)
99-139: LGTM —createWeightimplementation looks correct.The weight creates a scorer supplier that uses BKD tree intersection with the merge-sort style
MergePointVisitor. The cost heuristic (cardinality × 20) matches the 32-bit version. Returningnullwhenvalues == nullcorrectly signals no matching docs for the segment. Caching is set totruesince the query depends only on the immutable segment point values and the bitmap.
210-243: LGTM —rewrite,equals,hashCode,ramBytesUsedlook correct.Empty bitmap rewrites to
MatchNoDocsQuery. Equality and hashing delegate to the field and bitmap. RAM usage accounts for the query instance, the field string, and the bitmap's internal memory.
62-97: The suggested optimization cannot be applied toRoaring64NavigableMap.
Roaring64NavigableMap#getLongIterator()returnsLongIterator, notPeekableLongIterator. ThePeekableLongIteratorinterface with theadvanceIfNeeded()method is only available inRoaring64Bitmap, notRoaring64NavigableMap. Applying this suggestion would fail at compile or runtime because the types are incompatible.To use this optimization, the code would need to either change the input type from
Roaring64NavigableMaptoRoaring64Bitmap, or useRoaring64Bitmap#getLongIteratorFrom(long minval)which directly accepts a target value. Neither is feasible here without changing the public API ofBitmap64IndexQuery.Likely an incorrect or invalid review comment.
server/src/test/java/org/opensearch/search/query/Bitmap64DocValuesQueryTests.java (1)
32-219: Good test coverage for the doc-values query path.The tests cover single-value, multi-value, empty bitmap, rewrite, range, large values, and null-argument validation. This provides solid baseline coverage.
server/src/test/java/org/opensearch/search/query/Bitmap64IndexQueryTests.java (1)
18-138: Overall test structure looks good.The
@Before/@Afterlifecycle management, therefresh()helper, and thegetMatchingValues()extraction utility are well-structured. The tests correctly validate both single-value and multi-value document scenarios.server/src/main/java/org/opensearch/search/query/Bitmap64DocValuesQuery.java (2)
56-113:createWeightimplementation is well-structured.The singleton vs. multi-value path distinction, the
[min, max]range guard before thebitmap.contains()call, and the early termination in the multi-value loop (leveragingSortedNumericDocValuesascending order guarantee) are all correct and efficient. ThematchCost()of 5 is a reasonable constant consistent with the 32-bit counterpart.
120-126: LGTM — empty bitmap rewrite.Correctly short-circuits to
MatchNoDocsQueryfor empty bitmaps, avoiding unnecessary scorer creation.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| @Override | ||
| public Query bitmapQuery(String field, BytesArray bitmapArray, boolean isSearchable, boolean hasDocValues) { | ||
| // Extract bytes safely | ||
| BytesRef ref = bitmapArray.toBytesRef(); | ||
| byte[] bytes = Arrays.copyOfRange(ref.bytes, ref.offset, ref.offset + ref.length); | ||
|
|
||
| Roaring64NavigableMap bitmap64 = new Roaring64NavigableMap(); | ||
| try { | ||
| bitmap64.deserializePortable(new DataInputStream(new ByteArrayInputStream(bytes))); | ||
| } catch (Exception e) { | ||
| throw new IllegalArgumentException("Failed to deserialize the 64-bit bitmap.", e); | ||
| } | ||
|
|
||
| if (isSearchable && hasDocValues) { | ||
| return new IndexOrDocValuesQuery( | ||
| new Bitmap64IndexQuery(field, bitmap64), | ||
| new Bitmap64DocValuesQuery(field, bitmap64) | ||
| ); | ||
| } | ||
| if (isSearchable) { | ||
| return new Bitmap64IndexQuery(field, bitmap64); | ||
| } | ||
| return new Bitmap64DocValuesQuery(field, bitmap64); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the INTEGER.bitmapQuery implementation to compare patterns
grep -A 30 "INTEGER.bitmapQuery" /dev/stdin << 'EOF'
Looking at line 1168 and surrounding context in NumberFieldMapper.java
EOF
# Let's search for the INTEGER case in the file
rg -A 30 "case INTEGER:" server/src/main/java/org/opensearch/index/mapper/NumberFieldMapper.java | head -50Repository: opensearch-project/OpenSearch
Length of output: 116
🌐 Web query:
Roaring64NavigableMap mutability thread safety RoaringBitmap
💡 Result:
-
Mutability:
Roaring64NavigableMapis mutable. It has in-place mutators likeadd…,addRange…,clear(),and(...),andNot(...), etc. (so it is not an immutable/persistent data structure). [1] -
Thread safety (general RoaringBitmap Java guidance): Roaring bitmaps (including
Roaring64NavigableMap) are not synchronized; it is unsafe to access the same bitmap concurrently from multiple threads if any thread may mutate it. If you need multi-thread access, you must provide external synchronization, or use per-thread copies. [2] -
Read-only sharing: You can safely share a bitmap across threads only if it is not being modified (i.e., effectively immutable for the duration of sharing). This same “safe as long as unchanged” rule is also stated for several static operations in the library Javadocs. [3]
Practical rule: treat Roaring64NavigableMap as mutable + not thread-safe; share only for concurrent reads, and otherwise synchronize or copy per thread. [1][2]
References:
[1] Roaring64NavigableMap Javadoc (methods like add, clear, and, andNot) https://www.javadoc.io/static/org.roaringbitmap/RoaringBitmap/1.2.0/roaringbitmap/org/roaringbitmap/longlong/Roaring64NavigableMap.html
[2] RoaringBitmap GitHub README — “Thread safety” section https://github.com/RoaringBitmap/RoaringBitmap
[3] RoaringBitmap Javadoc (examples noting operations are thread-safe if inputs remain unchanged) https://javadoc.io/static/org.roaringbitmap/RoaringBitmap/0.8.1/org/roaringbitmap/RoaringBitmap.html
🌐 Web query:
RoaringBitmap deserializePortable IOException API exceptions
💡 Result:
In the Java RoaringBitmap API, deserializePortable(...) is declared to throw only java.io.IOException (a checked exception). For example, Roaring64NavigableMap.deserializePortable(DataInput in) has the signature throws IOException. [1]
What that means in practice:
- You only need to catch/declare
IOExceptionfor the portable deserialization call site. [1] - The concrete
IOExceptionyou actually see depends on the underlyingDataInputimplementation:- If the serialized data is truncated,
DataInputmethods can throwEOFException(a subclass ofIOException). [2] - Other read failures will surface as other
IOExceptionsubclasses from the underlying stream. [2]
- If the serialized data is truncated,
Important: IOException is about I/O/read failures, not necessarily “bad bitmap contents”. If you are deserializing untrusted or potentially corrupted bytes, RoaringBitmap’s own docs recommend validating after deserialization (e.g., validate() for RoaringBitmap) to ensure the internal structure is sane. [3]
Sources: [1] RoaringBitmap Javadoc (deserializePortable signature/throws) (javadoc.io), [2] Oracle DataInput Javadoc (EOFException/IOException behavior) (docs.oracle.com), [3] RoaringBitmap repo README (recommend validate() after deserialize for untrusted data) (github.com)
Citations:
- 1: https://www.javadoc.io/static/org.roaringbitmap/RoaringBitmap/1.2.0/roaringbitmap/org/roaringbitmap/longlong/Roaring64NavigableMap.html?utm_source=openai
- 2: https://docs.oracle.com/javase/9/docs/api/java/io/DataInput.html?utm_source=openai
- 3: https://github.com/RoaringBitmap/RoaringBitmap?utm_source=openai
🏁 Script executed:
rg "case INTEGER" server/src/main/java/org/opensearch/index/mapper/NumberFieldMapper.java -A 40 | head -60Repository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Let's also verify what happens in Bitmap64IndexQuery and Bitmap64DocValuesQuery
# to understand if the shared bitmap could be modified
rg -l "class Bitmap64IndexQuery|class Bitmap64DocValuesQuery" server/src/main/java/org/opensearch/index/mapper/Repository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Search for Bitmap64IndexQuery and Bitmap64DocValuesQuery definitions
rg "class Bitmap64IndexQuery|class Bitmap64DocValuesQuery" --type javaRepository: opensearch-project/OpenSearch
Length of output: 664
🏁 Script executed:
# Also search for the INTEGER case more carefully
rg "INTEGER\s*\.\s*bitmapQuery" server/src/main/java/org/opensearch/index/mapper/NumberFieldMapper.java -B 5 -A 20Repository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Let's read the full context around lines 1376-1399 and compare with INTEGER implementation
cat -n server/src/main/java/org/opensearch/index/mapper/NumberFieldMapper.java | sed -n '1150,1200p'Repository: opensearch-project/OpenSearch
Length of output: 2642
🏁 Script executed:
# Read the Bitmap64IndexQuery implementation
cat -n server/src/main/java/org/opensearch/search/query/Bitmap64IndexQuery.javaRepository: opensearch-project/OpenSearch
Length of output: 10416
🏁 Script executed:
# Read the Bitmap64DocValuesQuery implementation
cat -n server/src/main/java/org/opensearch/search/query/Bitmap64DocValuesQuery.javaRepository: opensearch-project/OpenSearch
Length of output: 6467
🏁 Script executed:
# Also check BitmapIndexQuery and BitmapDocValuesQuery (INTEGER versions) for comparison
fd "BitmapIndexQuery.java|BitmapDocValuesQuery.java" --type f | grep -v "Bitmap64" | head -5Repository: opensearch-project/OpenSearch
Length of output: 218
🏁 Script executed:
# Let's verify that the INTEGER.bitmapQuery exception handling and check BitmapIndexQuery/BitmapDocValuesQuery
cat -n server/src/main/java/org/opensearch/search/query/BitmapIndexQuery.java | head -100Repository: opensearch-project/OpenSearch
Length of output: 4288
🏁 Script executed:
# Check if IOException is declared in method signatures for deserialize
rg "deserialize" server/src/main/java/org/opensearch/search/query/Bitmap64IndexQuery.javaRepository: opensearch-project/OpenSearch
Length of output: 55
Catch only IOException for deserialization, not generic Exception.
The deserializePortable() method declares throws IOException. Catching the broader Exception is imprecise and masks the true exception type. Change line 1385 to catch IOException (which includes EOFException for truncated data).
Note: The same overly broad exception handling exists in INTEGER.bitmapQuery() at line 1169 and should be corrected there as well.
Regarding the shared bitmap64 instance between Bitmap64IndexQuery and Bitmap64DocValuesQuery: this is safe in practice because both query classes perform only read-only operations (contains(), first(), last(), isEmpty()) on the bitmap after deserialization. However, this pattern (also present in the 32-bit INTEGER.bitmapQuery()) could benefit from a code comment clarifying that the bitmap must not be modified post-deserialization.
🤖 Prompt for AI Agents
In `@server/src/main/java/org/opensearch/index/mapper/NumberFieldMapper.java`
around lines 1376 - 1399, The catch in NumberFieldMapper.bitmapQuery currently
catches Exception; change it to catch IOException (the declared exception from
deserializePortable) to avoid masking other errors—update the catch block in the
bitmapQuery method that deserializes into Roaring64NavigableMap to "catch
(IOException e)"; make the identical change in the INTEGER.bitmapQuery
deserialization catch as well; and add a short inline comment near both
bitmap64/bitmap32 usages (and the constructors for
Bitmap64IndexQuery/Bitmap64DocValuesQuery or where they are instantiated) noting
that the Roaring* bitmap is read-only after deserialization and must not be
modified by the query classes.
| /* | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check license headers across the new bitmap64 files
echo "=== Bitmap64DocValuesQuery.java ==="
head -10 server/src/main/java/org/opensearch/search/query/Bitmap64DocValuesQuery.java
echo ""
echo "=== Bitmap64IndexQuery.java ==="
head -10 server/src/main/java/org/opensearch/search/query/Bitmap64IndexQuery.java
echo ""
echo "=== Bitmap64DocValuesQueryTests.java ==="
head -10 server/src/test/java/org/opensearch/search/query/Bitmap64DocValuesQueryTests.java
echo ""
echo "=== Bitmap64IndexQueryTests.java ==="
head -10 server/src/test/java/org/opensearch/search/query/Bitmap64IndexQueryTests.java
echo ""
echo "=== Reference: existing BitmapDocValuesQuery.java header ==="
head -10 server/src/main/java/org/opensearch/search/query/BitmapDocValuesQuery.javaRepository: opensearch-project/OpenSearch
Length of output: 1706
Inconsistent and missing SPDX license headers across new files.
The new source files use a short 3-line SPDX header, while test files and the existing BitmapDocValuesQuery.java reference file use the full 7-line header with the OpenSearch contributor requirement notice. Additionally, Bitmap64IndexQueryTests.java is missing a license header entirely. Align all new files to use the full header format for consistency with the project standard:
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/
Apply this to Bitmap64DocValuesQuery.java, Bitmap64IndexQuery.java, and Bitmap64IndexQueryTests.java.
🤖 Prompt for AI Agents
In `@server/src/main/java/org/opensearch/search/query/Bitmap64DocValuesQuery.java`
around lines 1 - 3, Replace the short 3-line SPDX headers (or missing header) in
the new source and test files with the project's full 7-line header including
the OpenSearch contributor notice; specifically update the header comment at the
top of Bitmap64DocValuesQuery.java (class Bitmap64DocValuesQuery),
Bitmap64IndexQuery.java (class Bitmap64IndexQuery), and
Bitmap64IndexQueryTests.java (class Bitmap64IndexQueryTests) so they all start
with: "/*\n * SPDX-License-Identifier: Apache-2.0\n *\n * The OpenSearch
Contributors require contributions made to\n * this file be licensed under the
Apache-2.0 license or a\n * compatible open source license.\n */" ensuring exact
text and spacing to match existing project files.
| final Scorer scorer = new ConstantScoreScorer(score(), scoreMode, iterator); | ||
| return new DefaultScorerSupplier(scorer); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check for DefaultScorerSupplier usage and imports in the codebase
rg "DefaultScorerSupplier" --type java -C 2Repository: opensearch-project/OpenSearch
Length of output: 11146
🏁 Script executed:
# Find the Lucene version being used
fd "pom.xml" | head -5 | xargs grep -l "lucene" 2>/dev/null | head -3Repository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Check the specific file for imports and context
cat -n server/src/main/java/org/opensearch/search/query/Bitmap64DocValuesQuery.java | sed -n '1,120p'Repository: opensearch-project/OpenSearch
Length of output: 5236
🏁 Script executed:
# Check for wildcard imports and all imports in the file
cat server/src/main/java/org/opensearch/search/query/Bitmap64DocValuesQuery.java | head -30Repository: opensearch-project/OpenSearch
Length of output: 1140
🏁 Script executed:
# Search for where DefaultScorerSupplier is imported from
rg "import.*DefaultScorerSupplier" --type javaRepository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Check if it's in a wildcard import - look at a file that uses it
cat server/src/main/java/org/opensearch/search/query/BitmapDocValuesQuery.java | head -40Repository: opensearch-project/OpenSearch
Length of output: 1369
🏁 Script executed:
# Find the Lucene version in pom.xml
find . -name "pom.xml" -type f | xargs grep -l "lucene" | head -1 | xargs cat | grep -A 5 -B 5 "lucene.version\|<lucene>" | head -30Repository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Search for the class definition of DefaultScorerSupplier
rg "class DefaultScorerSupplier" --type javaRepository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Look for any file that might define DefaultScorerSupplier in OpenSearch
fd "DefaultScorerSupplier.java"Repository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Check the main pom.xml for Lucene version
find . -maxdepth 2 -name "pom.xml" -exec grep -H "lucene" {} \; | head -20Repository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Also check build.gradle files
fd "build.gradle" -x grep -l "lucene" {} \;Repository: opensearch-project/OpenSearch
Length of output: 561
🏁 Script executed:
# Check server build.gradle for Lucene version
cat ./server/build.gradle | grep -A 5 -B 5 "lucene\|org.apache.lucene" | head -50Repository: opensearch-project/OpenSearch
Length of output: 1635
🏁 Script executed:
# Try a broader search for where DefaultScorerSupplier could come from
rg "DefaultScorerSupplier" --type java -B 5 | grep -E "import|class|package" | head -30Repository: opensearch-project/OpenSearch
Length of output: 55
🌐 Web query:
Lucene DefaultScorerSupplier class availability
💡 Result:
DefaultScorerSupplier is not a standalone public class in Lucene. In current Lucene it exists as a nested, protected class:
- Name / location:
org.apache.lucene.search.Weight.DefaultScorerSupplier(nested insideWeight) and marked internal-only (“might change in incompatible ways”). [1] - Availability: Present in Lucene 10.x (it appears in the
Weightnested class summary starting with Lucene 10.0.0). [2] - Not present in Lucene 9.x / 8.x: The
Weightnested classes in those versions listDefaultBulkScorer(and other helpers) but do not includeDefaultScorerSupplier. [3]
Implication: if you’re on Lucene 9.x or earlier, you won’t find it; on 10.x you can only reference it as Weight.DefaultScorerSupplier, and only where protected access allows. [1][2]
Sources:
[1] Lucene 10.2.1 Javadoc: Weight.DefaultScorerSupplier
[2] Lucene 10.0.0 Javadoc: Weight nested class summary includes Weight.DefaultScorerSupplier
[3] Lucene 8.9.0 Javadoc: Weight nested class summary does not include DefaultScorerSupplier
🏁 Script executed:
# Find Lucene version in gradle files
fd "gradle.properties" -o "settings.gradle" | xargs cat | grep -i lucene | head -20Repository: opensearch-project/OpenSearch
Length of output: 222
🏁 Script executed:
# Check if OpenSearch defines its own DefaultScorerSupplier
rg "class DefaultScorerSupplier" --type java -A 3Repository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Look for Weight.DefaultScorerSupplier usage (proper Lucene reference)
rg "Weight\.DefaultScorerSupplier" --type javaRepository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Check if there's a local/private DefaultScorerSupplier in opensearch.search.query package or common locations
fd DefaultScorerSupplier --type fRepository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Find gradle.properties correctly
find . -name "gradle.properties" -type f | head -5 | xargs cat | grep -i luceneRepository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Look in the root for version info
cat gradle.properties 2>/dev/null | head -50Repository: opensearch-project/OpenSearch
Length of output: 1435
🏁 Script executed:
# Check the actual imports and source more carefully - maybe it's a wildcard or re-export
ast-grep --pattern 'import org.apache.lucene.search.*' | head -20Repository: opensearch-project/OpenSearch
Length of output: 187
🏁 Script executed:
# Look at the compiled code structure - check if there's any DefaultScorerSupplier anywhere
find . -type f -name "*.java" | xargs grep -l "DefaultScorerSupplier" | xargs grep -B 10 "class.*Weight\|interface.*ScorerSupplier" | head -40Repository: opensearch-project/OpenSearch
Length of output: 1534
🏁 Script executed:
# Check for lucene version more broadly in build files
find . -name "build.gradle*" -o -name "pom.xml" | head -10 | xargs grep -h "lucene" | grep -i "version\|=" | head -20Repository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Look at the libs configuration in gradle
cat libs.versions.toml 2>/dev/null | grep -i luceneRepository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Check OpenSearch's approach - maybe they provide DefaultScorerSupplier as a utility
find . -path "*/lucene/*" -name "*.java" | xargs grep -l "DefaultScorerSupplier" 2>/dev/null | head -5Repository: opensearch-project/OpenSearch
Length of output: 402
🏁 Script executed:
# Double-check the exact file again - does it compile without the import?
cat -n server/src/main/java/org/opensearch/search/query/Bitmap64DocValuesQuery.java | grep -A 50 "^[ ]*1"Repository: opensearch-project/OpenSearch
Length of output: 4939
🏁 Script executed:
# Check one of the other files that uses DefaultScorerSupplier
cat -n server/src/main/java/org/opensearch/lucene/queries/MinDocQuery.java | head -50Repository: opensearch-project/OpenSearch
Length of output: 2161
🏁 Script executed:
# Search for any local DefaultScorerSupplier definition or wrapper in opensearch codebase
rg "DefaultScorerSupplier" --type java -B 3 -A 3 | grep -E "^[^-].*package|^[^-].*class|^[^-].*interface" | head -20Repository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Check what version of Lucene is actually being used by looking at gradle dependencies more carefully
find . -name "gradle" -o -name "build.gradle" | xargs grep -l "lucene\|elasticsearch" 2>/dev/null | head -3 | xargs cat | grep -E "dependencies|lucene|version" | head -30Repository: opensearch-project/OpenSearch
Length of output: 680
🌐 Web query:
OpenSearch Lucene version 2024 2025
💡 Result:
OpenSearch’s embedded Apache Lucene versions around 2024–2025 (by OpenSearch release line):
- OpenSearch 2.13 / 2.14 / 2.15 (2024) → Lucene 9.10.0. [1]
- OpenSearch 2.16 / 2.17 (2024) → Lucene 9.11.1. [2]
- OpenSearch 2.18 (2024) → Lucene 9.12.0. [3]
- OpenSearch 2.19.x (released Feb 11, 2025) → Lucene 9.12.1. [4]
- OpenSearch 3.0.0 (released May 6, 2025) → Lucene 10.1.0 (Lucene 10 major upgrade). [5][6]
Sources:
[1] OpenSearch 2.13.0 GitHub release notes (dependency: “Bump Apache Lucene from 9.9.2 to 9.10.0”)
[2] OpenSearch 2.16.0 GitHub release notes (dependency: “Update to Apache Lucene 9.11.1”)
[3] OpenSearch 2.18.0 GitHub release notes (dependency: “Bump Apache lucene from 9.11.1 to 9.12.0”)
[4] Maven artifact metadata for org.opensearch:opensearch-core:2.19.0 (depends on lucene-core:9.12.1)
[5] OpenSearch 3.0.0 GitHub release notes (breaking change: “Upgrade to Lucene 10.1.0”)
[6] OpenSearch documentation “Version history” (3.0.0: “Upgrades to Lucene 10…”, release date May 6, 2025)
🏁 Script executed:
# Check the OpenSearch version in the repo
cat gradle.properties | grep -i "version\|opensearch" | head -10Repository: opensearch-project/OpenSearch
Length of output: 314
🏁 Script executed:
# Also check the build.gradle in libs/core which defines lucene version
cat libs/core/build.gradle | grep -E "lucene|version" | head -20Repository: opensearch-project/OpenSearch
Length of output: 543
🏁 Script executed:
# Check the versions.properties or similar for lucene version
find . -maxdepth 3 -name "versions.properties" -o -name "versions.gradle" | xargs cat 2>/dev/null | grep -i luceneRepository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Most direct approach - check what version of lucene-core the build uses
find . -name "build.gradle" -type f | xargs grep -h "lucene.core\|lucene-core" | head -10Repository: opensearch-project/OpenSearch
Length of output: 153
🏁 Script executed:
# Find the versions catalog or versions file
find . -maxdepth 3 -name "*.toml" -o -name "*.gradle" -o -name "*.properties" | xargs grep -l "lucene.*=" 2>/dev/null | head -5Repository: opensearch-project/OpenSearch
Length of output: 100
🏁 Script executed:
# Check gradle/libs.versions.toml which is the standard location
cat gradle/libs.versions.toml 2>/dev/null | grep -A 2 -B 2 luceneRepository: opensearch-project/OpenSearch
Length of output: 2328
🏁 Script executed:
# Or check if it's in build.gradle
find . -maxdepth 1 -name "build.gradle" | xargs cat | grep -A 5 "versions\s*{" | head -30Repository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Check settings.gradle for version catalog
cat settings.gradle 2>/dev/null | grep -A 50 "versionCatalogs\|dependencyResolutionManagement"Repository: opensearch-project/OpenSearch
Length of output: 55
🌐 Web query:
Lucene 10.3.2 DefaultScorerSupplier public class
💡 Result:
In Lucene 10.3.2, there is no public top-level class named DefaultScorerSupplier.
What you’re looking for is the nested class:
org.apache.lucene.search.Weight.DefaultScorerSupplier
and it is protected static final (ie, not public and not instantiable/accessible from arbitrary application code). It’s explicitly marked as internal API. [1], [2]
If your goal is to create/return a scorer supplier in your own query/weight, use the public API surface (Weight#scorerSupplier(...) returning a ScorerSupplier implementation) rather than trying to reference Weight.DefaultScorerSupplier. [2]
Sources
[1] Lucene 10.3.2 org.apache.lucene.search package summary (lists Weight.DefaultScorerSupplier)
[2] Lucene Weight / Weight.DefaultScorerSupplier javadocs showing it as protected static final and internal API
🏁 Script executed:
# Check if maybe there's a custom wrapper or if it's re-exported
rg "DefaultScorerSupplier" --type java | grep -E "extends|implements|class.*DefaultScorerSupplier" | head -10Repository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Verify the file actually compiles by checking if it's in test or actual source
git log --oneline server/src/main/java/org/opensearch/search/query/Bitmap64DocValuesQuery.java 2>/dev/null | head -5Repository: opensearch-project/OpenSearch
Length of output: 108
🏁 Script executed:
# Check if there are any compilation errors or build issues reported
find . -name "build.gradle" -type f -path "*/server/*" | head -1 | xargs grep -A 10 "compileJava\|tasks.named.*compile"Repository: opensearch-project/OpenSearch
Length of output: 497
🏁 Script executed:
# Check if maybe OpenSearch provides a shim or utility for DefaultScorerSupplier
rg "class.*DefaultScorerSupplier\|object.*DefaultScorerSupplier" --type java -A 5Repository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
# Verify the exact import behavior - maybe there's a package-private access or factory
cat server/src/main/java/org/opensearch/search/query/Bitmap64DocValuesQuery.java | grep -E "^import|^package|DefaultScorerSupplier"Repository: opensearch-project/OpenSearch
Length of output: 1147
🌐 Web query:
Lucene 10 Weight DefaultScorerSupplier how to use create ScorerSupplier
💡 Result:
In Lucene 10, Weight.scorerSupplier(LeafReaderContext) is abstract, so custom Weight implementations must return a ScorerSupplier (or null if no matches). The simplest way is to build a Scorer immediately and wrap it in Weight.DefaultScorerSupplier. [1]
Minimal pattern (use Weight.DefaultScorerSupplier)
`@Override`
public ScorerSupplier scorerSupplier(LeafReaderContext ctx) throws IOException {
Scorer scorer = buildScorer(ctx); // your scorer creation
if (scorer == null) return null; // required contract
return new Weight.DefaultScorerSupplier(scorer);
}Weight.DefaultScorerSupplier is just a small wrapper around an already-built Scorer and provides cost() and get(long leadCost). [2]
Lazy pattern (implement ScorerSupplier yourself)
If building the scorer is expensive and you want to:
- expose a cheap
cost()up front, and/or - use
leadCostto choose an implementation,
then implement ScorerSupplier:
`@Override`
public ScorerSupplier scorerSupplier(LeafReaderContext ctx) throws IOException {
long cost = estimatedCost(ctx);
return new ScorerSupplier() {
private Scorer scorer; // build at most once
`@Override`
public long cost() {
return cost;
}
`@Override`
public Scorer get(long leadCost) throws IOException {
if (scorer == null) {
scorer = buildScorerConsideringLeadCost(ctx, leadCost);
}
return scorer; // must be non-null and get() must be called at most once
}
};
}Notes:
ScorerSupplier.get(long leadCost)must be called at most once and must not returnnull. [2]- If unsure what to do with
leadCost, treat it as “how iteration-heavy the caller expects things to be” and ignore it (or behave as if it wereLong.MAX_VALUE). [2][3]
Sources: Weight.scorerSupplier(...) contract and bulkScorer() delegation [1]; Weight.DefaultScorerSupplier API and get/cost behavior [2]; ScorerSupplier role/shape [3].
[1] (lucene.apache.org)
[2] (lucene.apache.org)
[3] (lucene.apache.org)
Citations:
- 1: https://lucene.apache.org/core/10_1_0/core/org/apache/lucene/search/Weight.html
- 2: https://lucene.apache.org/core/10_2_1/core/org/apache/lucene/search/Weight.DefaultScorerSupplier.html
- 3: https://lucene.apache.org/core/10_1_0/core/org/apache/lucene/search/ScorerSupplier.html?utm_source=openai
Use fully qualified class name Weight.DefaultScorerSupplier.
DefaultScorerSupplier is a protected nested class in Lucene 10.3.2 (Weight.DefaultScorerSupplier), not a standalone public class. It must be referenced with its qualifying class name. Change line 105 from:
return new DefaultScorerSupplier(scorer);to:
return new Weight.DefaultScorerSupplier(scorer);This applies to all similar usages throughout the codebase (BitmapDocValuesQuery, ShardSplittingQuery, and others).
🤖 Prompt for AI Agents
In `@server/src/main/java/org/opensearch/search/query/Bitmap64DocValuesQuery.java`
around lines 104 - 105, The code currently constructs a DefaultScorerSupplier
directly (e.g., in Bitmap64DocValuesQuery where you return new
DefaultScorerSupplier(scorer)); change these to use the fully-qualified nested
type Weight.DefaultScorerSupplier instead (return new
Weight.DefaultScorerSupplier(scorer)), and update all similar usages (e.g., in
BitmapDocValuesQuery, ShardSplittingQuery, etc.) so they reference
Weight.DefaultScorerSupplier rather than the unqualified DefaultScorerSupplier.
| @@ -0,0 +1,139 @@ | |||
| package org.opensearch.search.query; | |||
There was a problem hiding this comment.
Missing SPDX license header.
This file is missing the SPDX license header that other new files in this PR include (e.g., Bitmap64DocValuesQueryTests.java has the full Apache-2.0 header block).
🤖 Prompt for AI Agents
In
`@server/src/test/java/org/opensearch/search/query/Bitmap64IndexQueryTests.java`
at line 1, Add the missing SPDX license header to the top of the file for
Bitmap64IndexQueryTests: insert the same Apache-2.0 SPDX header block used in
other new files (for example Bitmap64DocValuesQueryTests) as the very first
lines of
server/src/test/java/org/opensearch/search/query/Bitmap64IndexQueryTests.java so
the file begins with the standard SPDX/Apache-2.0 header before the package
declaration and class definition.
d90957b to
a0e6ef7
Compare
|
❌ Gradle check result for a0e6ef7: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
The Gradle Check failure is in , which appears to be a known Flaky Test 14559 This test passes consistently locally, including with --rerun-tasks. |
Signed-off-by: Divya <DIVYA2@ibm.com> Signed-off-by: Divya <divyaruhil999@gmail.com>
Signed-off-by: Divya <divyaruhil999@gmail.com>
Signed-off-by: Divya <divyaruhil999@gmail.com>
divyaruhil
force-pushed
the
bitmap_64
branch
from
59b4e7f to
c598688
Compare
|
Hi @andrross @anasalkouz @saratvemulapalli @epugh @kyleconroy I was excited to work on this and have raised the PR. Whenever you get a chance, I’d appreciate a review. I’m happy to update anything based on your suggestions. Thank you! |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #20606 +/- ##
============================================
- Coverage 73.37% 73.37% -0.01%
- Complexity 72197 72219 +22
============================================
Files 5798 5800 +2
Lines 329816 329967 +151
Branches 47538 47565 +27
============================================
+ Hits 242001 242097 +96
- Misses 68458 68476 +18
- Partials 19357 19394 +37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
github-actions
bot
added
enhancement
1 participant
Description
Feature Request :- #20597
Related Issues
Resolves #20597
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.