epotyom commented on code in PR #15711:
URL: https://github.com/apache/lucene/pull/15711#discussion_r2816452920
##########
lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyReader.java:
##########
@@ -540,55 +546,52 @@ protected void swap(int i, int j) {
public int compare(int i, int j) {
return Integer.compare(ordinals[i], ordinals[j]);
}
- }.sort(0, ordinalsLength);
+ }.sort(0, uncachedLength);
int readerIndex;
int leafReaderMaxDoc = 0;
int leafReaderDocBase = 0;
LeafReader leafReader;
LeafReaderContext leafReaderContext;
BinaryDocValues values = null;
- IntArrayList uncachedOrdinalPositions = new IntArrayList();
- for (int i = 0; i < ordinalsLength; i++) {
- if (bulkPath[originalPosition[i]] == null) {
+ for (int i = 0; i < uncachedLength; i++) {
+ int ord = ordinals[i];
+ int idx = originalPosition[i];
+ /*
+ If ord >= leafReaderDocBase + leafReaderMaxDoc then we find the next
leaf that contains our ordinal.
+ Remember: ord operates in the global ordinal space and hence we add
leafReaderDocBase to the leafReaderMaxDoc
+ (which is the maxDoc of the specific leaf)
+ */
+ if (values == null || ord >= leafReaderDocBase + leafReaderMaxDoc) {
+ readerIndex = ReaderUtil.subIndex(ord, indexReader.leaves());
+ leafReaderContext = indexReader.leaves().get(readerIndex);
+ leafReader = leafReaderContext.reader();
+ leafReaderMaxDoc = leafReader.maxDoc();
+ leafReaderDocBase = leafReaderContext.docBase;
+ values = leafReader.getBinaryDocValues(Consts.FULL);
+
/*
- If ordinals[i] >= leafReaderDocBase + leafReaderMaxDoc then we find
the next leaf that contains our ordinal.
- Remember: ordinals[i] operates in the global ordinal space and hence
we add leafReaderDocBase to the leafReaderMaxDoc
- (which is the maxDoc of the specific leaf)
+ If the index is constructed with the older StoredFields it will not
have any BinaryDocValues field and will return null
Review Comment:
It looks like we may not need to support this backwards compatibility
anymore, at least the getPath method above throws an IllegalStateException in
this case. And AFAIU, there are no unit tests that cover this branch?
##########
lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyReader.java:
##########
@@ -540,55 +546,52 @@ protected void swap(int i, int j) {
public int compare(int i, int j) {
return Integer.compare(ordinals[i], ordinals[j]);
}
- }.sort(0, ordinalsLength);
+ }.sort(0, uncachedLength);
int readerIndex;
int leafReaderMaxDoc = 0;
int leafReaderDocBase = 0;
LeafReader leafReader;
LeafReaderContext leafReaderContext;
BinaryDocValues values = null;
- IntArrayList uncachedOrdinalPositions = new IntArrayList();
- for (int i = 0; i < ordinalsLength; i++) {
- if (bulkPath[originalPosition[i]] == null) {
+ for (int i = 0; i < uncachedLength; i++) {
+ int ord = ordinals[i];
+ int idx = originalPosition[i];
Review Comment:
[nit] idx is only used once, could inline ordinals[i] into the call to
reduce noise, unless you prefer the local for readability/debugging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
