jpountz commented on a change in pull request #7:
URL: https://github.com/apache/lucene/pull/7#discussion_r727845012



##########
File path: lucene/core/src/java/org/apache/lucene/index/PointValues.java
##########
@@ -331,6 +450,9 @@ public long estimateDocCount(IntersectVisitor visitor) {
   /** Returns the number of bytes per dimension */
   public abstract int getBytesPerDimension() throws IOException;
 
+  /** Returns the maximum number of points per leaf node */
+  public abstract int getMaxPointsPerLeafNode() throws IOException;

Review comment:
       Could we do without this method? It feels like an abstraction detail 
that shouldn't be exposed in the API.

##########
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:
       Would it be hard to make the behavior predictable if the method has 
already been called?

##########
File path: 
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java
##########
@@ -372,6 +372,12 @@ private void checkAndThrow() {
       }
     }
 
+    @Override
+    public IndexTree getIndexTree() throws IOException {
+      checkAndThrow();
+      return in.getIndexTree();
+    }

Review comment:
       Leave it as a TODO for now?

##########
File path: 
lucene/test-framework/src/java/org/apache/lucene/index/AssertingLeafReader.java
##########
@@ -1090,6 +1090,13 @@ private void assertStats(int maxDoc) {
       assert in.getDocCount() <= maxDoc;
     }
 
+    @Override
+    public IndexTree getIndexTree() throws IOException {
+      // TODO: assert that there are no illegal calls when navigating the tree?
+      assertThread("Points", creationThread);

Review comment:
       +1 Especially if the API says that calling moveToChild twice leads to 
unspecified behavior, we should make sure our tests never do it.




-- 
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

Reply via email to