LUCENE-7859: PackedQuadPrefixTree getTokenBytes bug

lucene/CHANGES.txt

@@ -64,6 +64,9 @@ Bug Fixes
 * LUCENE-7626: IndexWriter will no longer accept broken token offsets
   (Mike McCandless)
 
+* LUCENE-7859: Spatial-extras PackedQuadPrefixTree bug that only revealed itself
+  with the new pointsOnly optimizations in LUCENE-7845. (David Smiley)
+
 Improvements
 
 * LUCENE-7489: Better storage of sparse doc-values fields with the default

lucene/spatial-extras/src/java/org/apache/lucene/spatial/prefix/tree/PackedQuadPrefixTree.java

@@ -21,13 +21,13 @@ import java.util.Collection;
 import java.util.List;
 import java.util.NoSuchElementException;
 
+import org.apache.lucene.util.BytesRef;
 import org.locationtech.spatial4j.context.SpatialContext;
 import org.locationtech.spatial4j.shape.Point;
 import org.locationtech.spatial4j.shape.Rectangle;
 import org.locationtech.spatial4j.shape.Shape;
 import org.locationtech.spatial4j.shape.SpatialRelation;
 import org.locationtech.spatial4j.shape.impl.RectangleImpl;
-import org.apache.lucene.util.BytesRef;
 
 /**
  * Uses a compact binary representation of 8 bytes to encode a spatial quad trie.
@@ -161,7 +161,7 @@ public class PackedQuadPrefixTree extends QuadPrefixTree {
       super(null, 0, 0);
       this.term = term;
       this.b_off = 0;
-      this.bytes = longToByteArray(this.term);
+      this.bytes = longToByteArray(this.term, new byte[8]);
       this.b_len = 8;
       readLeafAdjust();
     }
@@ -229,30 +229,37 @@ public class PackedQuadPrefixTree extends QuadPrefixTree {
 
     @Override
     public BytesRef getTokenBytesWithLeaf(BytesRef result) {
-      if (isLeaf) {
+      result = getTokenBytesNoLeaf(result);
-        term |= 0x1L;
+      if (isLeaf()) {
+        result.bytes[8 - 1] |= 0x1L; // set leaf
       }
-      return getTokenBytesNoLeaf(result);
+      return result;
     }
 
     @Override
     public BytesRef getTokenBytesNoLeaf(BytesRef result) {
-      if (result == null)
+      if (result == null) {
-        return new BytesRef(bytes, b_off, b_len);
+        result = new BytesRef(8);
-      result.bytes = longToByteArray(this.term);
+      } else if (result.bytes.length < 8) {
+        result.bytes = new byte[8];
+      }
+      result.bytes = longToByteArray(this.term, result.bytes);
       result.offset = 0;
-      result.length = result.bytes.length;
+      result.length = 8;
+      // no leaf
+      result.bytes[8 - 1] &= ~1; // clear last bit (leaf bit)
       return result;
     }
 
     @Override
     public int compareToNoLeaf(Cell fromCell) {
       PackedQuadCell b = (PackedQuadCell) fromCell;
+      //TODO clear last bit without the condition
       final long thisTerm = (((0x1L)&term) == 0x1L) ? term-1 : term;
       final long fromTerm = (((0x1L)&b.term) == 0x1L) ? b.term-1 : b.term;
       final int result = Long.compareUnsigned(thisTerm, fromTerm);
       assert Math.signum(result)
-          == Math.signum(compare(longToByteArray(thisTerm), 0, 8, longToByteArray(fromTerm), 0, 8)); // TODO remove
+          == Math.signum(compare(longToByteArray(thisTerm, new byte[8]), 0, 8, longToByteArray(fromTerm, new byte[8]), 0, 8)); // TODO remove
       return result;
     }
 
@@ -343,8 +350,7 @@ public class PackedQuadPrefixTree extends QuadPrefixTree {
           | ((long)b7 & 255L) << 8 | (long)b8 & 255L;
     }
 
-    private byte[] longToByteArray(long value) {
+    private byte[] longToByteArray(long value, byte[] result) {
-      byte[] result = new byte[8];
       for(int i = 7; i >= 0; --i) {
         result[i] = (byte)((int)(value & 255L));
         value >>= 8;

lucene/spatial-extras/src/test/org/apache/lucene/spatial/prefix/RandomSpatialOpFuzzyPrefixTreeTest.java

@@ -29,14 +29,6 @@ import java.util.Map;
 import java.util.Set;
 
 import com.carrotsearch.randomizedtesting.annotations.Repeat;
-import org.locationtech.spatial4j.context.SpatialContext;
-import org.locationtech.spatial4j.context.SpatialContextFactory;
-import org.locationtech.spatial4j.shape.Point;
-import org.locationtech.spatial4j.shape.Rectangle;
-import org.locationtech.spatial4j.shape.Shape;
-import org.locationtech.spatial4j.shape.ShapeCollection;
-import org.locationtech.spatial4j.shape.SpatialRelation;
-import org.locationtech.spatial4j.shape.impl.RectangleImpl;
 import org.apache.lucene.document.Document;
 import org.apache.lucene.document.Field;
 import org.apache.lucene.document.StoredField;
@@ -52,6 +44,14 @@ import org.apache.lucene.spatial.prefix.tree.SpatialPrefixTree;
 import org.apache.lucene.spatial.query.SpatialArgs;
 import org.apache.lucene.spatial.query.SpatialOperation;
 import org.junit.Test;
+import org.locationtech.spatial4j.context.SpatialContext;
+import org.locationtech.spatial4j.context.SpatialContextFactory;
+import org.locationtech.spatial4j.shape.Point;
+import org.locationtech.spatial4j.shape.Rectangle;
+import org.locationtech.spatial4j.shape.Shape;
+import org.locationtech.spatial4j.shape.ShapeCollection;
+import org.locationtech.spatial4j.shape.SpatialRelation;
+import org.locationtech.spatial4j.shape.impl.RectangleImpl;
 
 import static com.carrotsearch.randomizedtesting.RandomizedTest.randomBoolean;
 import static com.carrotsearch.randomizedtesting.RandomizedTest.randomInt;
@@ -152,6 +152,18 @@ public class RandomSpatialOpFuzzyPrefixTreeTest extends StrategyTestCase {
     doTest(SpatialOperation.Contains);
   }
 
+  @Test
+  public void testPackedQuadPointsOnlyBug() throws IOException {
+    setupQuadGrid(1, true); // packed quad.  maxLevels doesn't matter.
+    setupCtx2D(ctx);
+    ((PrefixTreeStrategy) strategy).setPointsOnly(true);
+    Point point = ctx.makePoint(169.0, 107.0);
+    adoc("0", point);
+    commit();
+    Query query = strategy.makeQuery(new SpatialArgs(SpatialOperation.Intersects, point));
+    assertEquals(1, executeQuery(query, 1).numFound);
+  }
+
   /** See LUCENE-5062, {@link ContainsPrefixTreeQuery#multiOverlappingIndexedShapes}. */
   @Test
   public void testContainsPairOverlap() throws IOException {