jpountz commented on a change in pull request #486:
URL: https://github.com/apache/lucene/pull/486#discussion_r761731533
##########
File path:
lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90PointsWriter.java
##########
@@ -147,24 +145,7 @@ public void writeField(FieldInfo fieldInfo, PointsReader
reader) throws IOExcept
return;
}
- values.visitDocValues(
- new IntersectVisitor() {
- @Override
- public void visit(int docID) {
- throw new IllegalStateException();
- }
-
- @Override
- public void visit(int docID, byte[] packedValue) throws
IOException {
- writer.add(packedValue, docID);
- }
-
- @Override
- public Relation compare(byte[] minPackedValue, byte[]
maxPackedValue) {
- return Relation.CELL_CROSSES_QUERY;
- }
- });
-
+ values.visitDocValues((docID, packedValue) -> writer.add(packedValue,
docID));
Review comment:
too bad the order is reversed and we cannot use a method reference,
maybe we should change the order of parameters in BKDWriter?
##########
File path: lucene/core/src/java/org/apache/lucene/index/SortingCodecReader.java
##########
@@ -170,45 +168,20 @@ public long size() {
}
@Override
- public void visitDocIDs(PointValues.IntersectVisitor visitor) throws
IOException {
- sortingIntersectVisitor.setIntersectVisitor(visitor);
- indexTree.visitDocIDs(sortingIntersectVisitor);
+ public void visitDocIDs(PointValues.DocIdsVisitor docIdsVisitor) throws
IOException {
+ indexTree.visitDocIDs(docMap::oldToNew);
}
@Override
- public void visitDocValues(PointValues.IntersectVisitor visitor) throws
IOException {
- sortingIntersectVisitor.setIntersectVisitor(visitor);
- indexTree.visitDocValues(sortingIntersectVisitor);
- }
- }
-
- private static class SortingIntersectVisitor implements
PointValues.IntersectVisitor {
-
- private final Sorter.DocMap docMap;
-
- private PointValues.IntersectVisitor visitor;
-
- SortingIntersectVisitor(Sorter.DocMap docMap) {
- this.docMap = docMap;
- }
-
- private void setIntersectVisitor(PointValues.IntersectVisitor visitor) {
- this.visitor = visitor;
- }
-
- @Override
- public void visit(int docID) throws IOException {
- visitor.visit(docMap.oldToNew(docID));
- }
-
- @Override
- public void visit(int docID, byte[] packedValue) throws IOException {
- visitor.visit(docMap.oldToNew(docID), packedValue);
- }
-
- @Override
- public PointValues.Relation compare(byte[] minPackedValue, byte[]
maxPackedValue) {
- return visitor.compare(minPackedValue, maxPackedValue);
+ public void visitDocValues(
+ PointValues.NodeComparator nodeComparator,
+ PointValues.DocIdsVisitor docIdsVisitor,
+ PointValues.DocValuesVisitor docValuesVisitor)
+ throws IOException {
+ indexTree.visitDocValues(
+ nodeComparator,
+ docMap::oldToNew,
Review comment:
and likewise here?
##########
File path: lucene/core/src/java/org/apache/lucene/index/PointValues.java
##########
@@ -361,14 +405,29 @@ private void intersect(IntersectVisitor visitor,
PointTree pointTree) throws IOE
// TODO: we can assert that the first value here in fact matches
what the pointTree
// claimed?
// Leaf node; scan and filter all points in this block:
- pointTree.visitDocValues(visitor);
+ visitor.grow((int) pointTree.size());
Review comment:
:+1:
##########
File path: lucene/core/src/java/org/apache/lucene/index/SortingCodecReader.java
##########
@@ -170,45 +168,20 @@ public long size() {
}
@Override
- public void visitDocIDs(PointValues.IntersectVisitor visitor) throws
IOException {
- sortingIntersectVisitor.setIntersectVisitor(visitor);
- indexTree.visitDocIDs(sortingIntersectVisitor);
+ public void visitDocIDs(PointValues.DocIdsVisitor docIdsVisitor) throws
IOException {
+ indexTree.visitDocIDs(docMap::oldToNew);
Review comment:
This doesn't look right: for every doc ID we pass it to `oldToNew` but
then don't do anything with the result? We should pass the result to
docIdsVisitor?
##########
File path: lucene/core/src/java/org/apache/lucene/index/PointValues.java
##########
@@ -361,14 +405,29 @@ private void intersect(IntersectVisitor visitor,
PointTree pointTree) throws IOE
// TODO: we can assert that the first value here in fact matches
what the pointTree
// claimed?
// Leaf node; scan and filter all points in this block:
- pointTree.visitDocValues(visitor);
+ visitor.grow((int) pointTree.size());
+ pointTree.visitDocValues(visitor::compare, visitor, visitor);
}
break;
default:
throw new IllegalArgumentException("Unreachable code");
}
}
+ private void visitDocIds(IntersectVisitor visitor, PointTree pointTree)
throws IOException {
+ final long size = pointTree.size();
+ if (size <= Integer.MAX_VALUE) {
+ visitor.grow((int) size);
+ pointTree.visitDocIDs(visitor::visit);
+ } else {
+ if (pointTree.moveToChild()) {
+ do {
+ visitDocIds(visitor, pointTree);
Review comment:
ok
##########
File path: lucene/core/src/java/org/apache/lucene/index/PointValues.java
##########
@@ -269,22 +269,59 @@ protected PointValues() {}
long size();
/** Visit all the docs below the current node. */
- void visitDocIDs(IntersectVisitor visitor) throws IOException;
+ void visitDocIDs(DocIdsVisitor docIdsVisitor) throws IOException;
/** Visit all the docs and values below the current node. */
- void visitDocValues(IntersectVisitor visitor) throws IOException;
+ default void visitDocValues(DocValuesVisitor docValuesVisitor) throws
IOException {
+ visitDocValues((min, max) -> Relation.CELL_CROSSES_QUERY, docID -> {},
docValuesVisitor);
Review comment:
should the doc id visitor throw an assertion error instead?
--
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]
