jpountz commented on a change in pull request #7:
URL: https://github.com/apache/lucene/pull/7#discussion_r728708457
##########
File path:
lucene/test-framework/src/java/org/apache/lucene/index/AssertingLeafReader.java
##########
@@ -1152,6 +1139,80 @@ public int getDocCount() {
}
}
+ /** Validates that we don't call moveToChild() or clone() after having
called moveToParent() */
+ static class AssertingPointTree implements PointValues.PointTree {
+
+ final PointValues pointValues;
+ final PointValues.PointTree in;
+ private boolean moveToParent;
+
+ AssertingPointTree(PointValues pointValues, PointValues.PointTree in) {
+ this.pointValues = pointValues;
+ this.in = in;
+ }
+
+ @Override
+ public PointValues.PointTree clone() {
+ assert moveToParent == false : "calling clone() after calling
moveToParent()";
Review comment:
The more I think about it, the more I find these semantics confusing.
Can you point me to the place where we rely on the fact that this behavior is
unspecified?
##########
File path: lucene/core/src/java/org/apache/lucene/codecs/PointsWriter.java
##########
@@ -31,8 +31,9 @@
/** Sole constructor. (For invocation by subclass constructors, typically
implicit.) */
protected PointsWriter() {}
- /** Write all values contained in the provided reader */
- public abstract void writeField(FieldInfo fieldInfo, PointsReader values)
throws IOException;
+ /** Write all values contained in the provided point tree */
+ public abstract void writeField(FieldInfo fieldInfo, PointValues.PointTree
values)
Review comment:
Can we make the API still take a `PointValues` instance, and then
implementations can pull the `PointTree` if that's all they need?
##########
File path: lucene/core/src/java/org/apache/lucene/index/PointValues.java
##########
@@ -227,8 +228,56 @@ protected PointValues() {}
CELL_CROSSES_QUERY
};
+ /** Create a new {@link IndexTree} to navigate the index */
+ public abstract IndexTree getIndexTree() throws IOException;
+
+ /**
+ * Basic operations to read the KD-tree.
+ *
+ * @lucene.experimental
+ */
+ public interface IndexTree extends Cloneable {
+
+ /** Clone, the current node becomes the root of the new tree. */
+ IndexTree clone();
+
+ /**
+ * Move to the first child node and return {@code true} upon success.
Returns {@code false} for
+ * leaf nodes and {@code true} otherwise. Should not be called if the
current node has already
+ * called this method.
Review comment:
The javadoc suggests that this method should not be called twice in a
row, but actually I think it should say to not call it again after it returns
`false`?
##########
File path:
lucene/sandbox/src/java/org/apache/lucene/sandbox/search/LatLonPointPrototypeQueries.java
##########
@@ -89,20 +88,15 @@ public static TopFieldDocs nearest(
if (searcher == null) {
throw new IllegalArgumentException("searcher must not be null");
}
- List<BKDReader> readers = new ArrayList<>();
+ List<PointValues> readers = new ArrayList<>();
List<Integer> docBases = new ArrayList<>();
List<Bits> liveDocs = new ArrayList<>();
int totalHits = 0;
for (LeafReaderContext leaf : searcher.getIndexReader().leaves()) {
PointValues points = leaf.reader().getPointValues(field);
if (points != null) {
- if (points instanceof BKDReader == false) {
- throw new IllegalArgumentException(
- "can only run on Lucene60PointsReader points implementation, but
got " + points);
- }
totalHits += points.getDocCount();
- BKDReader reader = (BKDReader) points;
Review comment:
:+1: very cool that we can now remove this cast
--
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: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
