You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by rm...@apache.org on 2016/03/28 14:53:47 UTC
lucene-solr:branch_6_0: LUCENE-7126: GeoPointDistanceRangeQuery not
valid for multi-valued docs
Repository: lucene-solr
Updated Branches:
refs/heads/branch_6_0 d61097626 -> 8b9ce1cfd
LUCENE-7126: GeoPointDistanceRangeQuery not valid for multi-valued docs
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/8b9ce1cf
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/8b9ce1cf
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/8b9ce1cf
Branch: refs/heads/branch_6_0
Commit: 8b9ce1cfd02f099826cff5250c90b045219044ff
Parents: d610976
Author: Robert Muir <rm...@apache.org>
Authored: Mon Mar 28 07:46:08 2016 -0400
Committer: Robert Muir <rm...@apache.org>
Committed: Mon Mar 28 08:45:44 2016 -0400
----------------------------------------------------------------------
lucene/CHANGES.txt | 3 +
.../lucene/search/TestLatLonPointQueries.java | 11 --
.../geopoint/search/GeoPointDistanceQuery.java | 9 +-
.../search/GeoPointDistanceRangeQuery.java | 122 -------------------
.../geopoint/search/TestGeoPointQuery.java | 31 +----
.../spatial/util/BaseGeoPointTestCase.java | 34 +-----
6 files changed, 11 insertions(+), 199 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/8b9ce1cf/lucene/CHANGES.txt
----------------------------------------------------------------------
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 5462b89..f7f18b9 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -187,6 +187,9 @@ Bug Fixes
* LUCENE-7112: WeightedSpanTermExtractor.extractUnknownQuery is only called
on queries that could not be extracted. (Adrien Grand)
+* LUCENE-7126: Remove GeoPointDistanceRangeQuery. This query was implemented
+ with boolean NOT, and incorrect for multi-valued documents. (Robert Muir)
+
Other
* LUCENE-7035: Upgrade icu4j to 56.1/unicode 8. (Robert Muir)
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/8b9ce1cf/lucene/sandbox/src/test/org/apache/lucene/search/TestLatLonPointQueries.java
----------------------------------------------------------------------
diff --git a/lucene/sandbox/src/test/org/apache/lucene/search/TestLatLonPointQueries.java b/lucene/sandbox/src/test/org/apache/lucene/search/TestLatLonPointQueries.java
index da866be..71f8fbf 100644
--- a/lucene/sandbox/src/test/org/apache/lucene/search/TestLatLonPointQueries.java
+++ b/lucene/sandbox/src/test/org/apache/lucene/search/TestLatLonPointQueries.java
@@ -43,11 +43,6 @@ public class TestLatLonPointQueries extends BaseGeoPointTestCase {
}
@Override
- protected Query newDistanceRangeQuery(String field, double centerLat, double centerLon, double minRadiusMeters, double radiusMeters) {
- return null;
- }
-
- @Override
protected Query newPolygonQuery(String field, double[] lats, double[] lons) {
return LatLonPoint.newPolygonQuery(field, lats, lons);
}
@@ -142,12 +137,6 @@ public class TestLatLonPointQueries extends BaseGeoPointTestCase {
return result;
}
- @Override
- protected Boolean distanceRangeContainsPoint(double centerLat, double centerLon, double minRadiusMeters, double radiusMeters, double pointLat, double pointLon) {
- final double d = SloppyMath.haversinMeters(centerLat, centerLon, pointLat, pointLon);
- return d >= minRadiusMeters && d <= radiusMeters;
- }
-
/** Returns random double min to max or up to 1% outside of that range */
private double randomRangeMaybeSlightlyOutside(double min, double max) {
return min + (random().nextDouble() + (0.5 - random().nextDouble()) * .02) * (max - min);
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/8b9ce1cf/lucene/spatial/src/java/org/apache/lucene/spatial/geopoint/search/GeoPointDistanceQuery.java
----------------------------------------------------------------------
diff --git a/lucene/spatial/src/java/org/apache/lucene/spatial/geopoint/search/GeoPointDistanceQuery.java b/lucene/spatial/src/java/org/apache/lucene/spatial/geopoint/search/GeoPointDistanceQuery.java
index 2486852..762d303 100644
--- a/lucene/spatial/src/java/org/apache/lucene/spatial/geopoint/search/GeoPointDistanceQuery.java
+++ b/lucene/spatial/src/java/org/apache/lucene/spatial/geopoint/search/GeoPointDistanceQuery.java
@@ -80,13 +80,8 @@ public class GeoPointDistanceQuery extends GeoPointInBBoxQuery {
}
}
- if (GeoUtils.isValidLat(centerLat) == false) {
- throw new IllegalArgumentException("invalid centerLat " + centerLat);
- }
-
- if (GeoUtils.isValidLon(centerLon) == false) {
- throw new IllegalArgumentException("invalid centerLon " + centerLon);
- }
+ GeoUtils.checkLatitude(centerLat);
+ GeoUtils.checkLongitude(centerLon);
if (radiusMeters <= 0.0) {
throw new IllegalArgumentException("invalid radiusMeters " + radiusMeters);
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/8b9ce1cf/lucene/spatial/src/java/org/apache/lucene/spatial/geopoint/search/GeoPointDistanceRangeQuery.java
----------------------------------------------------------------------
diff --git a/lucene/spatial/src/java/org/apache/lucene/spatial/geopoint/search/GeoPointDistanceRangeQuery.java b/lucene/spatial/src/java/org/apache/lucene/spatial/geopoint/search/GeoPointDistanceRangeQuery.java
deleted file mode 100644
index 5cc778a..0000000
--- a/lucene/spatial/src/java/org/apache/lucene/spatial/geopoint/search/GeoPointDistanceRangeQuery.java
+++ /dev/null
@@ -1,122 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.lucene.spatial.geopoint.search;
-
-import org.apache.lucene.index.IndexReader;
-import org.apache.lucene.search.BooleanClause;
-import org.apache.lucene.search.BooleanQuery;
-import org.apache.lucene.search.Query;
-import org.apache.lucene.spatial.geopoint.document.GeoPointField.TermEncoding;
-
-/** Implements a point distance range query on a GeoPoint field. This is based on
- * {@code org.apache.lucene.spatial.geopoint.search.GeoPointDistanceQuery} and is implemented using a
- * {@code org.apache.lucene.search.BooleanClause.MUST_NOT} clause to exclude any points that fall within
- * minRadiusMeters from the provided point.
- *
- * @lucene.experimental
- */
-public final class GeoPointDistanceRangeQuery extends GeoPointDistanceQuery {
- /** minimum distance range (in meters) from lat, lon center location, maximum is inherited */
- protected final double minRadiusMeters;
-
- /**
- * Constructs a query for all {@link org.apache.lucene.spatial.geopoint.document.GeoPointField} types within a minimum / maximum
- * distance (in meters) range from a given point
- */
- public GeoPointDistanceRangeQuery(final String field, final double centerLat, final double centerLon,
- final double minRadiusMeters, final double maxRadiusMeters) {
- this(field, TermEncoding.PREFIX, centerLat, centerLon, minRadiusMeters, maxRadiusMeters);
- }
-
- /**
- * Constructs a query for all {@link org.apache.lucene.spatial.geopoint.document.GeoPointField} types within a minimum / maximum
- * distance (in meters) range from a given point. Accepts an optional
- * {@link org.apache.lucene.spatial.geopoint.document.GeoPointField.TermEncoding}
- */
- public GeoPointDistanceRangeQuery(final String field, final TermEncoding termEncoding, final double centerLat, final double centerLon,
- final double minRadiusMeters, final double maxRadius) {
- super(field, termEncoding, centerLat, centerLon, maxRadius);
- this.minRadiusMeters = minRadiusMeters;
- }
-
- @Override
- public Query rewrite(IndexReader reader) {
- Query q = super.rewrite(reader);
- if (minRadiusMeters == 0.0) {
- return q;
- }
-
- // add an exclusion query
- BooleanQuery.Builder bqb = new BooleanQuery.Builder();
-
- // create a new exclusion query
- GeoPointDistanceQuery exclude = new GeoPointDistanceQuery(field, termEncoding, centerLat, centerLon, minRadiusMeters);
- // full map search
-// if (radiusMeters >= GeoProjectionUtils.SEMIMINOR_AXIS) {
-// bqb.add(new BooleanClause(new GeoPointInBBoxQuery(this.field, -180.0, -90.0, 180.0, 90.0), BooleanClause.Occur.MUST));
-// } else {
- bqb.add(new BooleanClause(q, BooleanClause.Occur.MUST));
-// }
- bqb.add(new BooleanClause(exclude, BooleanClause.Occur.MUST_NOT));
-
- return bqb.build();
- }
-
- @Override
- public String toString(String field) {
- final StringBuilder sb = new StringBuilder();
- sb.append(getClass().getSimpleName());
- sb.append(':');
- if (!this.field.equals(field)) {
- sb.append(" field=");
- sb.append(this.field);
- sb.append(':');
- }
- return sb.append( " Center: [")
- .append(centerLat)
- .append(',')
- .append(centerLon)
- .append(']')
- .append(" From Distance: ")
- .append(minRadiusMeters)
- .append(" m")
- .append(" To Distance: ")
- .append(radiusMeters)
- .append(" m")
- .append(" Lower Left: [")
- .append(minLat)
- .append(',')
- .append(minLon)
- .append(']')
- .append(" Upper Right: [")
- .append(maxLat)
- .append(',')
- .append(maxLon)
- .append("]")
- .toString();
- }
-
- /** getter method for minimum distance */
- public double getMinRadiusMeters() {
- return this.minRadiusMeters;
- }
-
- /** getter method for maximum distance */
- public double getMaxRadiusMeters() {
- return this.radiusMeters;
- }
-}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/8b9ce1cf/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/TestGeoPointQuery.java
----------------------------------------------------------------------
diff --git a/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/TestGeoPointQuery.java b/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/TestGeoPointQuery.java
index dedda26..7acf13f 100644
--- a/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/TestGeoPointQuery.java
+++ b/lucene/spatial/src/test/org/apache/lucene/spatial/geopoint/search/TestGeoPointQuery.java
@@ -78,11 +78,6 @@ public class TestGeoPointQuery extends BaseGeoPointTestCase {
}
@Override
- protected Query newDistanceRangeQuery(String field, double centerLat, double centerLon, double minRadiusMeters, double radiusMeters) {
- return new GeoPointDistanceRangeQuery(field, termEncoding, centerLat, centerLon, minRadiusMeters, radiusMeters);
- }
-
- @Override
protected Query newPolygonQuery(String field, double[] lats, double[] lons) {
return new GeoPointInPolygonQuery(field, termEncoding, lats, lons);
}
@@ -211,17 +206,6 @@ public class TestGeoPointQuery extends BaseGeoPointTestCase {
}
}
- @Override
- protected Boolean distanceRangeContainsPoint(double centerLat, double centerLon, double minRadiusMeters, double radiusMeters, double pointLat, double pointLon) {
- if (radiusQueryCanBeWrong(centerLat, centerLon, pointLon, pointLat, minRadiusMeters)
- || radiusQueryCanBeWrong(centerLat, centerLon, pointLon, pointLat, radiusMeters)) {
- return null;
- } else {
- final double d = SloppyMath.haversinMeters(centerLat, centerLon, pointLat, pointLon);
- return d >= minRadiusMeters && d <= radiusMeters;
- }
- }
-
private static boolean radiusQueryCanBeWrong(double centerLat, double centerLon, double ptLon, double ptLat,
final double radius) {
final long hashedCntr = GeoEncodingUtils.mortonHash(centerLat, centerLon);
@@ -242,12 +226,6 @@ public class TestGeoPointQuery extends BaseGeoPointTestCase {
assertTrue(GeoRelationUtils.rectCrossesCircle(-90, 0.0, -180, 180, 0.0, 0.667, 88000.0, false));
}
- private TopDocs geoDistanceRangeQuery(double lat, double lon, double minRadius, double maxRadius, int limit)
- throws Exception {
- GeoPointDistanceRangeQuery q = new GeoPointDistanceRangeQuery(FIELD_NAME, termEncoding, lat, lon, minRadius, maxRadius);
- return searcher.search(q, limit);
- }
-
public void testBBoxQuery() throws Exception {
TopDocs td = bboxQuery(32.778650, 32.778950, -96.7772, -96.77690000, 5);
assertEquals("GeoBoundingBoxQuery failed", 4, td.totalHits);
@@ -356,11 +334,6 @@ public class TestGeoPointQuery extends BaseGeoPointTestCase {
});
}
- public void testMaxDistanceRangeQuery() throws Exception {
- TopDocs td = geoDistanceRangeQuery(0.0, 0.0, 10, 20000000, 20);
- assertEquals("GeoDistanceRangeQuery failed", 24, td.totalHits);
- }
-
public void testMortonEncoding() throws Exception {
long hash = GeoEncodingUtils.mortonHash(90, 180);
assertEquals(180.0, GeoEncodingUtils.mortonUnhashLon(hash), 0);
@@ -408,12 +381,12 @@ public class TestGeoPointQuery extends BaseGeoPointTestCase {
() -> {
new GeoPointField("field", 180.0, 0.0, Field.Store.NO);
});
- assertEquals("invalid lat=180.0 for field \"field\"", e.getMessage());
+ assertEquals("invalid latitude 180.0; must be between -90.0 and 90.0", e.getMessage());
e = expectThrows(IllegalArgumentException.class,
() -> {
new GeoPointField("field", 0.0, 190.0, Field.Store.NO);
});
- assertEquals("invalid lon=190.0 for field \"field\"", e.getMessage());
+ assertEquals("invalid longitude 190.0; must be between -180.0 and 180.0", e.getMessage());
}
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/8b9ce1cf/lucene/spatial/src/test/org/apache/lucene/spatial/util/BaseGeoPointTestCase.java
----------------------------------------------------------------------
diff --git a/lucene/spatial/src/test/org/apache/lucene/spatial/util/BaseGeoPointTestCase.java b/lucene/spatial/src/test/org/apache/lucene/spatial/util/BaseGeoPointTestCase.java
index fc3286f..ede3603 100644
--- a/lucene/spatial/src/test/org/apache/lucene/spatial/util/BaseGeoPointTestCase.java
+++ b/lucene/spatial/src/test/org/apache/lucene/spatial/util/BaseGeoPointTestCase.java
@@ -502,8 +502,6 @@ public abstract class BaseGeoPointTestCase extends LuceneTestCase {
protected abstract Query newDistanceQuery(String field, double centerLat, double centerLon, double radiusMeters);
- protected abstract Query newDistanceRangeQuery(String field, double centerLat, double centerLon, double minRadiusMeters, double radiusMeters);
-
protected abstract Query newPolygonQuery(String field, double[] lats, double[] lons);
/** Returns null if it's borderline case */
@@ -515,8 +513,6 @@ public abstract class BaseGeoPointTestCase extends LuceneTestCase {
/** Returns null if it's borderline case */
protected abstract Boolean circleContainsPoint(double centerLat, double centerLon, double radiusMeters, double pointLat, double pointLon);
- protected abstract Boolean distanceRangeContainsPoint(double centerLat, double centerLon, double minRadiusMeters, double radiusMeters, double pointLat, double pointLon);
-
private static abstract class VerifyHits {
public void test(AtomicBoolean failed, boolean small, IndexSearcher s, NumericDocValues docIDToID, Set<Integer> deleted, Query query, double[] lats, double[] lons) throws Exception {
@@ -707,12 +703,10 @@ public abstract class BaseGeoPointTestCase extends LuceneTestCase {
} else if (random().nextBoolean()) {
// Distance
- final boolean rangeQuery = random().nextBoolean();
final double centerLat = randomLat(small);
final double centerLon = randomLon(small);
double radiusMeters;
- double minRadiusMeters;
if (small) {
// Approx 3 degrees lon at the equator:
@@ -722,21 +716,13 @@ public abstract class BaseGeoPointTestCase extends LuceneTestCase {
radiusMeters = random().nextDouble() * GeoProjectionUtils.SEMIMAJOR_AXIS * Math.PI / 2.0 + 1.0;
}
- // generate a random minimum radius between 1% and 95% the max radius
- minRadiusMeters = (0.01 + 0.94 * random().nextDouble()) * radiusMeters;
-
if (VERBOSE) {
final DecimalFormat df = new DecimalFormat("#,###.00", DecimalFormatSymbols.getInstance(Locale.ENGLISH));
- System.out.println(" radiusMeters = " + df.format(radiusMeters)
- + ((rangeQuery == true) ? " minRadiusMeters = " + df.format(minRadiusMeters) : ""));
+ System.out.println(" radiusMeters = " + df.format(radiusMeters));
}
try {
- if (rangeQuery == true) {
- query = newDistanceRangeQuery(FIELD_NAME, centerLat, centerLon, minRadiusMeters, radiusMeters);
- } else {
- query = newDistanceQuery(FIELD_NAME, centerLat, centerLon, radiusMeters);
- }
+ query = newDistanceQuery(FIELD_NAME, centerLat, centerLon, radiusMeters);
} catch (IllegalArgumentException e) {
if (e.getMessage().contains("exceeds maxRadius")) {
continue;
@@ -747,11 +733,7 @@ public abstract class BaseGeoPointTestCase extends LuceneTestCase {
verifyHits = new VerifyHits() {
@Override
protected Boolean shouldMatch(double pointLat, double pointLon) {
- if (rangeQuery == false) {
- return circleContainsPoint(centerLat, centerLon, radiusMeters, pointLat, pointLon);
- } else {
- return distanceRangeContainsPoint(centerLat, centerLon, minRadiusMeters, radiusMeters, pointLat, pointLon);
- }
+ return circleContainsPoint(centerLat, centerLon, radiusMeters, pointLat, pointLon);
}
@Override
@@ -759,7 +741,7 @@ public abstract class BaseGeoPointTestCase extends LuceneTestCase {
double distanceMeters = SloppyMath.haversinMeters(centerLat, centerLon, pointLat, pointLon);
System.out.println(" docID=" + docID + " centerLat=" + centerLat + " centerLon=" + centerLon
+ " pointLat=" + pointLat + " pointLon=" + pointLon + " distanceMeters=" + distanceMeters
- + " vs" + ((rangeQuery == true) ? " minRadiusMeters=" + minRadiusMeters : "") + " radiusMeters=" + radiusMeters);
+ + " vs radiusMeters=" + radiusMeters);
}
};
@@ -883,14 +865,6 @@ public abstract class BaseGeoPointTestCase extends LuceneTestCase {
assertEquals(q1, q2);
assertFalse(q1.equals(newDistanceQuery("field2", lat, lon, 10000.0)));
- q1 = newDistanceRangeQuery("field", lat, lon, 10000.0, 100000.0);
- if (q1 != null) {
- // Not all subclasses can make distance range query!
- q2 = newDistanceRangeQuery("field", lat, lon, 10000.0, 100000.0);
- assertEquals(q1, q2);
- assertFalse(q1.equals(newDistanceRangeQuery("field2", lat, lon, 10000.0, 100000.0)));
- }
-
double[] lats = new double[5];
double[] lons = new double[5];
lats[0] = rect.minLat;