You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by cp...@apache.org on 2017/06/02 10:56:46 UTC
[23/38] lucene-solr:jira/solr-8668: LUCENE-7859: PackedQuadPrefixTree
getTokenBytes bug
LUCENE-7859: PackedQuadPrefixTree getTokenBytes bug
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/38741ece
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/38741ece
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/38741ece
Branch: refs/heads/jira/solr-8668
Commit: 38741ece587f2bfb4a2f6393ea84684f44a52dd5
Parents: 4a378eb
Author: David Smiley <ds...@apache.org>
Authored: Thu Jun 1 01:13:26 2017 -0400
Committer: David Smiley <ds...@apache.org>
Committed: Thu Jun 1 01:13:26 2017 -0400
----------------------------------------------------------------------
lucene/CHANGES.txt | 3 ++
.../prefix/tree/PackedQuadPrefixTree.java | 30 ++++++++++++--------
.../RandomSpatialOpFuzzyPrefixTreeTest.java | 28 ++++++++++++------
3 files changed, 41 insertions(+), 20 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/38741ece/lucene/CHANGES.txt
----------------------------------------------------------------------
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 0c1d351..79a30fa 100644
--- a/lucene/CHANGES.txt
+++ b/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
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/38741ece/lucene/spatial-extras/src/java/org/apache/lucene/spatial/prefix/tree/PackedQuadPrefixTree.java
----------------------------------------------------------------------
diff --git a/lucene/spatial-extras/src/java/org/apache/lucene/spatial/prefix/tree/PackedQuadPrefixTree.java b/lucene/spatial-extras/src/java/org/apache/lucene/spatial/prefix/tree/PackedQuadPrefixTree.java
index b86a6d1..6046ed2 100644
--- a/lucene/spatial-extras/src/java/org/apache/lucene/spatial/prefix/tree/PackedQuadPrefixTree.java
+++ b/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) {
- term |= 0x1L;
+ result = getTokenBytesNoLeaf(result);
+ if (isLeaf()) {
+ result.bytes[8 - 1] |= 0x1L; // set leaf
}
- return getTokenBytesNoLeaf(result);
+ return result;
}
@Override
public BytesRef getTokenBytesNoLeaf(BytesRef result) {
- if (result == null)
- return new BytesRef(bytes, b_off, b_len);
- result.bytes = longToByteArray(this.term);
+ if (result == null) {
+ result = new BytesRef(8);
+ } 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) {
- byte[] result = new byte[8];
+ private byte[] longToByteArray(long value, byte[] result) {
for(int i = 7; i >= 0; --i) {
result[i] = (byte)((int)(value & 255L));
value >>= 8;
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/38741ece/lucene/spatial-extras/src/test/org/apache/lucene/spatial/prefix/RandomSpatialOpFuzzyPrefixTreeTest.java
----------------------------------------------------------------------
diff --git a/lucene/spatial-extras/src/test/org/apache/lucene/spatial/prefix/RandomSpatialOpFuzzyPrefixTreeTest.java b/lucene/spatial-extras/src/test/org/apache/lucene/spatial/prefix/RandomSpatialOpFuzzyPrefixTreeTest.java
index c7e107c..cfc9980 100644
--- a/lucene/spatial-extras/src/test/org/apache/lucene/spatial/prefix/RandomSpatialOpFuzzyPrefixTreeTest.java
+++ b/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 {