jpountz commented on code in PR #12381:
URL: https://github.com/apache/lucene/pull/12381#discussion_r1246499123
##########
lucene/core/src/java/org/apache/lucene/index/NumericDocValuesWriter.java:
##########
@@ -114,7 +121,12 @@ static DocValuesProducer getDocValuesProducer(
final NumericDVs sorted;
if (sortMap != null) {
NumericDocValues oldValues = new BufferedNumericDocValues(values,
docsWithField.iterator());
- sorted = sortDocValues(sortMap.size(), sortMap, oldValues);
+ sorted =
+ sortDocValues(
+ sortMap.size(),
+ sortMap,
+ oldValues,
+ docsWithField.dense() && sortMap.size() ==
docsWithField.cardinality());
Review Comment:
and likewise here?
```suggestion
docsWithField.dense());
```
##########
lucene/core/src/java/org/apache/lucene/index/NumericDocValuesWriter.java:
##########
@@ -234,10 +246,41 @@ public long cost() {
static class NumericDVs {
private final long[] values;
private final BitSet docsWithField;
+ private final int maxDoc;
NumericDVs(long[] values, BitSet docsWithField) {
this.values = values;
this.docsWithField = docsWithField;
+ this.maxDoc = values.length;
+ }
+
+ int maxDoc() {
+ return maxDoc;
+ }
+
+ private boolean advanceExact(int target) {
+ if (docsWithField != null) {
+ return docsWithField.get(target);
+ }
+ return true;
+ }
+
+ private int advance(int target) {
+ if (docsWithField != null) {
+ return docsWithField.nextSetBit(target);
+ }
+
+ if (target < maxDoc) {
+ return target;
+ }
+ return DocIdSetIterator.NO_MORE_DOCS;
Review Comment:
Looking at the call site, this is only called when `target` is less than
`maxDoc`, so thit could be simplified to just `return target` without the
`target < maxDoc` check?
##########
lucene/core/src/java/org/apache/lucene/index/NormValuesWriter.java:
##########
@@ -76,7 +76,8 @@ public void flush(SegmentWriteState state, Sorter.DocMap
sortMap, NormsConsumer
NumericDocValuesWriter.sortDocValues(
state.segmentInfo.maxDoc(),
sortMap,
- new BufferedNorms(values, docsWithField.iterator()));
+ new BufferedNorms(values, docsWithField.iterator()),
+ docsWithField.dense() && sortMap.size() ==
docsWithField.cardinality());
Review Comment:
Only testing `dense()` should be correct right?
```suggestion
docsWithField.dense());
```
--
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]
