LUCENE-7085: PointRangeQuery.equals sometimes returns false even if queries were in fact equal

This commit is contained in:
Mike McCandless 2016-03-09 10:07:15 -05:00
parent 004e83bb6c
commit 770e508fd3
9 changed files with 202 additions and 23 deletions

View File

@ -103,7 +103,7 @@ public abstract class PointInSetQuery extends Query {
}
@Override
public Weight createWeight(IndexSearcher searcher, boolean needsScores) throws IOException {
public final Weight createWeight(IndexSearcher searcher, boolean needsScores) throws IOException {
// We don't use RandomAccessWeight here: it's no good to approximate with "match all docs".
// This is an inverted structure and should be used in the first pass:
@ -161,14 +161,12 @@ public abstract class PointInSetQuery extends Query {
private final DocIdSetBuilder result;
private TermIterator iterator;
private BytesRef nextQueryPoint;
private final byte[] lastMaxPackedValue;
private final BytesRef scratch = new BytesRef();
private final PrefixCodedTerms sortedPackedPoints;
public MergePointVisitor(PrefixCodedTerms sortedPackedPoints, DocIdSetBuilder result) throws IOException {
this.result = result;
this.sortedPackedPoints = sortedPackedPoints;
lastMaxPackedValue = new byte[bytesPerDim];
scratch.length = bytesPerDim;
this.iterator = sortedPackedPoints.iterator();
nextQueryPoint = iterator.next();
@ -304,7 +302,7 @@ public abstract class PointInSetQuery extends Query {
}
@Override
public int hashCode() {
public final int hashCode() {
int hash = super.hashCode();
hash = 31 * hash + sortedPackedPointsHashCode;
hash = 31 * hash + numDims;
@ -313,7 +311,7 @@ public abstract class PointInSetQuery extends Query {
}
@Override
public boolean equals(Object other) {
public final boolean equals(Object other) {
if (super.equals(other)) {
final PointInSetQuery q = (PointInSetQuery) other;
return q.numDims == numDims &&
@ -326,7 +324,7 @@ public abstract class PointInSetQuery extends Query {
}
@Override
public String toString(String field) {
public final String toString(String field) {
final StringBuilder sb = new StringBuilder();
if (this.field.equals(field) == false) {
sb.append(this.field);

View File

@ -109,7 +109,7 @@ public abstract class PointRangeQuery extends Query {
}
@Override
public Weight createWeight(IndexSearcher searcher, boolean needsScores) throws IOException {
public final Weight createWeight(IndexSearcher searcher, boolean needsScores) throws IOException {
// We don't use RandomAccessWeight here: it's no good to approximate with "match all docs".
// This is an inverted structure and should be used in the first pass:
@ -239,7 +239,7 @@ public abstract class PointRangeQuery extends Query {
}
@Override
public int hashCode() {
public final int hashCode() {
int hash = super.hashCode();
hash = 31 * hash + Arrays.hashCode(lowerPoint);
hash = 31 * hash + Arrays.hashCode(upperPoint);
@ -249,20 +249,36 @@ public abstract class PointRangeQuery extends Query {
}
@Override
public boolean equals(Object other) {
if (super.equals(other)) {
final PointRangeQuery q = (PointRangeQuery) other;
return q.numDims == numDims &&
q.bytesPerDim == bytesPerDim &&
Arrays.equals(lowerPoint, q.lowerPoint) &&
Arrays.equals(upperPoint, q.upperPoint);
public final boolean equals(Object other) {
if (super.equals(other) == false) {
return false;
}
return false;
final PointRangeQuery q = (PointRangeQuery) other;
if (q.numDims != numDims) {
return false;
}
if (q.bytesPerDim != bytesPerDim) {
return false;
}
// Cannot use Arrays.equals here, because it in turn uses byte[].equals
// to compare each value, which only uses "=="
for(int dim=0;dim<numDims;dim++) {
if (Arrays.equals(lowerPoint[dim], q.lowerPoint[dim]) == false) {
return false;
}
if (Arrays.equals(upperPoint[dim], q.upperPoint[dim]) == false) {
return false;
}
}
return true;
}
@Override
public String toString(String field) {
public final String toString(String field) {
final StringBuilder sb = new StringBuilder();
if (this.field.equals(field) == false) {
sb.append(this.field);

View File

@ -1886,4 +1886,84 @@ public class TestPointQueries extends LuceneTestCase {
w.close();
dir.close();
}
public void testPointRangeEquals() {
Query q = IntPoint.newRangeQuery("a", 0, 1000);
assertEquals(q, IntPoint.newRangeQuery("a", 0, 1000));
assertFalse(q.equals(IntPoint.newRangeQuery("a", 1, 1000)));
q = LongPoint.newRangeQuery("a", 0, 1000);
assertEquals(q, LongPoint.newRangeQuery("a", 0, 1000));
assertFalse(q.equals(LongPoint.newRangeQuery("a", 1, 1000)));
q = FloatPoint.newRangeQuery("a", 0, 1000);
assertEquals(q, FloatPoint.newRangeQuery("a", 0, 1000));
assertFalse(q.equals(FloatPoint.newRangeQuery("a", 1, 1000)));
q = DoublePoint.newRangeQuery("a", 0, 1000);
assertEquals(q, DoublePoint.newRangeQuery("a", 0, 1000));
assertFalse(q.equals(DoublePoint.newRangeQuery("a", 1, 1000)));
byte[] zeros = new byte[5];
byte[] ones = new byte[5];
Arrays.fill(ones, (byte) 0xff);
q = BinaryPoint.newRangeQuery("a", new byte[][] {zeros}, new byte[][] {ones});
assertEquals(q, BinaryPoint.newRangeQuery("a", new byte[][] {zeros}, new byte[][] {ones}));
byte[] other = ones.clone();
other[2] = (byte) 5;
assertFalse(q.equals(BinaryPoint.newRangeQuery("a", new byte[][] {zeros}, new byte[][] {other})));
}
public void testPointExactEquals() {
Query q = IntPoint.newExactQuery("a", 1000);
assertEquals(q, IntPoint.newExactQuery("a", 1000));
assertFalse(q.equals(IntPoint.newExactQuery("a", 1)));
q = LongPoint.newExactQuery("a", 1000);
assertEquals(q, LongPoint.newExactQuery("a", 1000));
assertFalse(q.equals(LongPoint.newExactQuery("a", 1)));
q = FloatPoint.newExactQuery("a", 1000);
assertEquals(q, FloatPoint.newExactQuery("a", 1000));
assertFalse(q.equals(FloatPoint.newExactQuery("a", 1)));
q = DoublePoint.newExactQuery("a", 1000);
assertEquals(q, DoublePoint.newExactQuery("a", 1000));
assertFalse(q.equals(DoublePoint.newExactQuery("a", 1)));
byte[] ones = new byte[5];
Arrays.fill(ones, (byte) 0xff);
q = BinaryPoint.newExactQuery("a", ones);
assertEquals(q, BinaryPoint.newExactQuery("a", ones));
byte[] other = ones.clone();
other[2] = (byte) 5;
assertFalse(q.equals(BinaryPoint.newExactQuery("a", other)));
}
public void testPointInSetEquals() {
Query q = IntPoint.newSetQuery("a", 0, 1000, 17);
assertEquals(q, IntPoint.newSetQuery("a", 17, 0, 1000));
assertFalse(q.equals(IntPoint.newSetQuery("a", 1, 17, 1000)));
q = LongPoint.newSetQuery("a", 0, 1000, 17);
assertEquals(q, LongPoint.newSetQuery("a", 17, 0, 1000));
assertFalse(q.equals(LongPoint.newSetQuery("a", 1, 17, 1000)));
q = FloatPoint.newSetQuery("a", 0, 1000, 17);
assertEquals(q, FloatPoint.newSetQuery("a", 17, 0, 1000));
assertFalse(q.equals(FloatPoint.newSetQuery("a", 1, 17, 1000)));
q = DoublePoint.newSetQuery("a", 0, 1000, 17);
assertEquals(q, DoublePoint.newSetQuery("a", 17, 0, 1000));
assertFalse(q.equals(DoublePoint.newSetQuery("a", 1, 17, 1000)));
byte[] zeros = new byte[5];
byte[] ones = new byte[5];
Arrays.fill(ones, (byte) 0xff);
q = BinaryPoint.newSetQuery("a", new byte[][] {zeros, ones});
assertEquals(q, BinaryPoint.newSetQuery("a", new byte[][] {zeros, ones}));
byte[] other = ones.clone();
other[2] = (byte) 5;
assertFalse(q.equals(BinaryPoint.newSetQuery("a", new byte[][] {zeros, other})));
}
}

View File

@ -19,12 +19,14 @@ package org.apache.lucene.document;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.Arrays;
import java.util.Comparator;
import org.apache.lucene.index.PointValues;
import org.apache.lucene.search.PointInSetQuery;
import org.apache.lucene.search.PointRangeQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.util.BytesRef;
import org.apache.lucene.util.StringHelper;
/**
* An indexed 128-bit {@code InetAddress} field.
@ -214,9 +216,22 @@ public class InetAddressPoint extends Field {
*/
public static Query newSetQuery(String field, InetAddress... values) {
// Don't unexpectedly change the user's incoming values array:
InetAddress[] sortedValues = values.clone();
Arrays.sort(sortedValues);
// We must compare the encoded form (InetAddress doesn't implement Comparable, and even if it
// did, we do our own thing with ipv4 addresses):
// NOTE: we could instead convert-per-comparison and save this extra array, at cost of slower sort:
byte[][] sortedValues = new byte[values.length][];
for(int i=0;i<values.length;i++) {
sortedValues[i] = encode(values[i]);
}
Arrays.sort(sortedValues,
new Comparator<byte[]>() {
@Override
public int compare(byte[] a, byte[] b) {
return StringHelper.compare(BYTES, a, 0, b, 0);
}
});
final BytesRef encoded = new BytesRef(new byte[BYTES]);
@ -230,7 +245,7 @@ public class InetAddressPoint extends Field {
if (upto == sortedValues.length) {
return null;
} else {
encoded.bytes = encode(sortedValues[upto]);
encoded.bytes = sortedValues[upto];
assert encoded.bytes.length == encoded.length;
upto++;
return encoded;

View File

@ -21,6 +21,7 @@ import java.math.BigInteger;
import org.apache.lucene.index.IndexReader;
import org.apache.lucene.index.RandomIndexWriter;
import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.search.Query;
import org.apache.lucene.store.Directory;
import org.apache.lucene.util.LuceneTestCase;
@ -93,4 +94,18 @@ public class TestBigIntegerPoint extends LuceneTestCase {
new BigInteger[] {BigInteger.valueOf(17), BigInteger.valueOf(42)}).toString());
assertEquals("field:{1}", BigIntegerPoint.newSetQuery("field", BigInteger.ONE).toString());
}
public void testQueryEquals() throws Exception {
Query q = BigIntegerPoint.newRangeQuery("a", BigInteger.valueOf(0), BigInteger.valueOf(1000));
assertEquals(q, BigIntegerPoint.newRangeQuery("a", BigInteger.valueOf(0), BigInteger.valueOf(1000)));
assertFalse(q.equals(BigIntegerPoint.newRangeQuery("a", BigInteger.valueOf(1), BigInteger.valueOf(1000))));
q = BigIntegerPoint.newExactQuery("a", BigInteger.valueOf(1000));
assertEquals(q, BigIntegerPoint.newExactQuery("a", BigInteger.valueOf(1000)));
assertFalse(q.equals(BigIntegerPoint.newExactQuery("a", BigInteger.valueOf(1))));
q = BigIntegerPoint.newSetQuery("a", BigInteger.valueOf(0), BigInteger.valueOf(1000), BigInteger.valueOf(17));
assertEquals(q, BigIntegerPoint.newSetQuery("a", BigInteger.valueOf(17), BigInteger.valueOf(0), BigInteger.valueOf(1000)));
assertFalse(q.equals(BigIntegerPoint.newSetQuery("a", BigInteger.valueOf(1), BigInteger.valueOf(17), BigInteger.valueOf(1000))));
}
}

View File

@ -21,6 +21,7 @@ import java.net.InetAddress;
import org.apache.lucene.index.IndexReader;
import org.apache.lucene.index.RandomIndexWriter;
import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.search.Query;
import org.apache.lucene.store.Directory;
import org.apache.lucene.util.LuceneTestCase;
@ -45,6 +46,7 @@ public class TestInetAddressPoint extends LuceneTestCase {
assertEquals(1, searcher.count(InetAddressPoint.newPrefixQuery("field", address, 24)));
assertEquals(1, searcher.count(InetAddressPoint.newRangeQuery("field", InetAddress.getByName("1.2.3.3"), InetAddress.getByName("1.2.3.5"))));
assertEquals(1, searcher.count(InetAddressPoint.newSetQuery("field", InetAddress.getByName("1.2.3.4"))));
assertEquals(1, searcher.count(InetAddressPoint.newSetQuery("field", InetAddress.getByName("1.2.3.4"), InetAddress.getByName("1.2.3.5"))));
assertEquals(0, searcher.count(InetAddressPoint.newSetQuery("field", InetAddress.getByName("1.2.3.3"))));
assertEquals(0, searcher.count(InetAddressPoint.newSetQuery("field")));
@ -88,4 +90,23 @@ public class TestInetAddressPoint extends LuceneTestCase {
assertEquals("field:[fdc8:57ed:f042:ad1:0:0:0:0 TO fdc8:57ed:f042:ad1:ffff:ffff:ffff:ffff]", InetAddressPoint.newPrefixQuery("field", InetAddress.getByName("fdc8:57ed:f042:0ad1:f66d:4ff:fe90:ce0c"), 64).toString());
assertEquals("field:{fdc8:57ed:f042:ad1:f66d:4ff:fe90:ce0c}", InetAddressPoint.newSetQuery("field", InetAddress.getByName("fdc8:57ed:f042:0ad1:f66d:4ff:fe90:ce0c")).toString());
}
public void testQueryEquals() throws Exception {
Query q = InetAddressPoint.newRangeQuery("a", InetAddress.getByName("1.2.3.3"), InetAddress.getByName("1.2.3.5"));
assertEquals(q, InetAddressPoint.newRangeQuery("a", InetAddress.getByName("1.2.3.3"), InetAddress.getByName("1.2.3.5")));
assertFalse(q.equals(InetAddressPoint.newRangeQuery("a", InetAddress.getByName("1.2.3.3"), InetAddress.getByName("1.2.3.7"))));
q = InetAddressPoint.newPrefixQuery("a", InetAddress.getByName("1.2.3.3"), 16);
assertEquals(q, InetAddressPoint.newPrefixQuery("a", InetAddress.getByName("1.2.3.3"), 16));
assertFalse(q.equals(InetAddressPoint.newPrefixQuery("a", InetAddress.getByName("1.1.3.5"), 16)));
assertFalse(q.equals(InetAddressPoint.newPrefixQuery("a", InetAddress.getByName("1.2.3.5"), 24)));
q = InetAddressPoint.newExactQuery("a", InetAddress.getByName("1.2.3.3"));
assertEquals(q, InetAddressPoint.newExactQuery("a", InetAddress.getByName("1.2.3.3")));
assertFalse(q.equals(InetAddressPoint.newExactQuery("a", InetAddress.getByName("1.2.3.5"))));
q = InetAddressPoint.newSetQuery("a", InetAddress.getByName("1.2.3.3"), InetAddress.getByName("1.2.3.5"));
assertEquals(q, InetAddressPoint.newSetQuery("a", InetAddress.getByName("1.2.3.3"), InetAddress.getByName("1.2.3.5")));
assertFalse(q.equals(InetAddressPoint.newSetQuery("a", InetAddress.getByName("1.2.3.3"), InetAddress.getByName("1.2.3.7"))));
}
}

View File

@ -19,6 +19,7 @@ package org.apache.lucene.document;
import org.apache.lucene.index.IndexReader;
import org.apache.lucene.index.RandomIndexWriter;
import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.search.Query;
import org.apache.lucene.store.Directory;
import org.apache.lucene.util.LuceneTestCase;
@ -170,4 +171,23 @@ public class TestLatLonPoint extends LuceneTestCase {
assertEquals(lonEnc, lonEnc2, 0.0);
}
}
public void testQueryEquals() throws Exception {
Query q = LatLonPoint.newBoxQuery("field", 50, 70, -40, 20);
assertEquals(q, LatLonPoint.newBoxQuery("field", 50, 70, -40, 20));
assertFalse(q.equals(LatLonPoint.newBoxQuery("field", 50, 70, -40, 10)));
q = LatLonPoint.newDistanceQuery("field", 50, 70, 10000);
assertEquals(q, LatLonPoint.newDistanceQuery("field", 50, 70, 10000));
assertFalse(q.equals(LatLonPoint.newDistanceQuery("field", 50, 70, 11000)));
assertFalse(q.equals(LatLonPoint.newDistanceQuery("field", 50, 60, 10000)));
double[] polyLats1 = new double[] {30, 40, 40, 30, 30};
double[] polyLons1 = new double[] {90, 90, -40, -40, 90};
double[] polyLats2 = new double[] {20, 40, 40, 20, 20};
q = LatLonPoint.newPolygonQuery("field", polyLats1, polyLons1);
assertEquals(q, LatLonPoint.newPolygonQuery("field", polyLats1, polyLons1));
assertFalse(q.equals(LatLonPoint.newPolygonQuery("field", polyLats2, polyLons1)));
}
}

View File

@ -43,7 +43,7 @@ import org.apache.lucene.util.NumericUtils;
*
* @lucene.experimental */
class PointInGeo3DShapeQuery extends Query {
final class PointInGeo3DShapeQuery extends Query {
final String field;
final GeoShape shape;
@ -192,7 +192,7 @@ class PointInGeo3DShapeQuery extends Query {
}
@Override
public final int hashCode() {
public int hashCode() {
int result = super.hashCode();
result = 31 * result + shape.hashCode();
return result;

View File

@ -807,4 +807,18 @@ public class TestGeo3DPoint extends LuceneTestCase {
private static Directory getDirectory() {
return newDirectory();
}
public void testEquals() {
GeoShape shape = randomShape(PlanetModel.WGS84);
Query q = Geo3DPoint.newShapeQuery("point", shape);
assertEquals(q, Geo3DPoint.newShapeQuery("point", shape));
// make a different random shape:
GeoShape shape2;
do {
shape2 = randomShape(PlanetModel.WGS84);
} while (shape.equals(shape2));
assertFalse(q.equals(Geo3DPoint.newShapeQuery("point", shape2)));
}
}