Lucene - Core
  1. Lucene - Core
  2. LUCENE-6778

Add GeoPointDistanceRangeQuery support for GeoPointField types

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.4, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      GeoPointDistanceQuery currently handles a single point distance. This improvement adds a GeoPointDistanceRangeQuery for supporting use cases such as: find all points between 10km and 20km of a known location.

      1. LUCENE-6778.patch
        13 kB
        Nicholas Knize
      2. LUCENE-6778.patch
        11 kB
        Nicholas Knize
      3. LUCENE-6778.patch
        11 kB
        Nicholas Knize

        Activity

        Hide
        Nicholas Knize added a comment -

        Attached patch adds GeoPointDistanceRangeQuery support to GeoPointField. This patch includes both explicit and randomized testing.

        Show
        Nicholas Knize added a comment - Attached patch adds GeoPointDistanceRangeQuery support to GeoPointField. This patch includes both explicit and randomized testing.
        Hide
        Michael McCandless added a comment -

        This looks nice Nicholas Knize! I like how simple the query is, just rewriting to a BQ that excludes the minRadius distance points.

        When the radius is too big, instead of doing the "whole world bbox query", couldn't we just do a MatchAllDocsQuery? Should be the same thing but faster?

        I know BQ effectively rewrites correctly, but instead of BooleanClause.Occur.SHOULD can you use MUST, for the outer radius query? It just makes it clear that we have a MUST and a MUST_NOT clause.

        In the randomized test, instead of using the bbox to derive a radius, why not just make a random radius to begin with (this is pre-existing)?

        Can you please make this a an if statement instead?

        +                query = (rangeQuery) ? new GeoPointDistanceRangeQuery(FIELD_NAME, centerLon, centerLat, radius, radiusMax) :
        +                    new GeoPointDistanceQuery(FIELD_NAME, centerLon, centerLat, radius);
        

        Thanks.

        Show
        Michael McCandless added a comment - This looks nice Nicholas Knize ! I like how simple the query is, just rewriting to a BQ that excludes the minRadius distance points. When the radius is too big, instead of doing the "whole world bbox query", couldn't we just do a MatchAllDocsQuery ? Should be the same thing but faster? I know BQ effectively rewrites correctly, but instead of BooleanClause.Occur.SHOULD can you use MUST , for the outer radius query? It just makes it clear that we have a MUST and a MUST_NOT clause. In the randomized test, instead of using the bbox to derive a radius, why not just make a random radius to begin with (this is pre-existing)? Can you please make this a an if statement instead? + query = (rangeQuery) ? new GeoPointDistanceRangeQuery(FIELD_NAME, centerLon, centerLat, radius, radiusMax) : + new GeoPointDistanceQuery(FIELD_NAME, centerLon, centerLat, radius); Thanks.
        Hide
        Nicholas Knize added a comment -

        Updated patch based on feedback.

        Show
        Nicholas Knize added a comment - Updated patch based on feedback.
        Hide
        Nicholas Knize added a comment -

        added incorrect patch... correct one now attached.

        Show
        Nicholas Knize added a comment - added incorrect patch... correct one now attached.
        Hide
        Michael McCandless added a comment -

        Thanks Nicholas Knize, patch looks great, but I'm seeing test failures, e.g:

           [junit4] Suite: org.apache.lucene.search.TestGeoPointQuery
           [junit4]   1> T1: id=0 docID=0 lat=0.0 lon=0.0 deleted?=false expected=true but got false query=GeoPointDistanceRangeQuery: field=geoField: Center: [0.0,0.0] Distance: 290419.347234561 m Lower Left: [-2.6088813842482064,-2.626445466260717] Upper Right: [2.6088813842482064,2.626445466260717]
           [junit4]   1> T0: id=0 docID=0 lat=0.0 lon=0.0 deleted?=false expected=true but got false query=GeoPointDistanceRangeQuery: field=geoField: Center: [0.0,0.0] Distance: 290419.347234561 m Lower Left: [-2.6088813842482064,-2.626445466260717] Upper Right: [2.6088813842482064,2.626445466260717]
           [junit4]   2> Spa 16, 2015 1:08:38 PM com.carrotsearch.randomizedtesting.RandomizedRunner$QueueUncaughtExceptionsHandler uncaughtException
           [junit4]   2> WARNING: Uncaught exception in thread: Thread[T0,5,TGRP-TestGeoPointQuery]
           [junit4]   2> java.lang.AssertionError: wrong hit
           [junit4]   2> 	at __randomizedtesting.SeedInfo.seed([7080C492088EC9F3]:0)
           [junit4]   2> 	at org.junit.Assert.fail(Assert.java:93)
           [junit4]   2> 	at org.apache.lucene.search.TestGeoPointQuery$VerifyHits.test(TestGeoPointQuery.java:582)
           [junit4]   2> 	at org.apache.lucene.search.TestGeoPointQuery$1._run(TestGeoPointQuery.java:523)
           [junit4]   2> 	at org.apache.lucene.search.TestGeoPointQuery$1.run(TestGeoPointQuery.java:408)
           [junit4]   2> 
           [junit4]   2> Spa 16, 2015 1:08:38 PM com.carrotsearch.randomizedtesting.RandomizedRunner$QueueUncaughtExceptionsHandler uncaughtException
           [junit4]   2> WARNING: Uncaught exception in thread: Thread[T1,5,TGRP-TestGeoPointQuery]
           [junit4]   2> java.lang.AssertionError: wrong hit
           [junit4]   2> 	at __randomizedtesting.SeedInfo.seed([7080C492088EC9F3]:0)
           [junit4]   2> 	at org.junit.Assert.fail(Assert.java:93)
           [junit4]   2> 	at org.apache.lucene.search.TestGeoPointQuery$VerifyHits.test(TestGeoPointQuery.java:582)
           [junit4]   2> 	at org.apache.lucene.search.TestGeoPointQuery$1._run(TestGeoPointQuery.java:523)
           [junit4]   2> 	at org.apache.lucene.search.TestGeoPointQuery$1.run(TestGeoPointQuery.java:408)
           [junit4]   2> 
           [junit4]   2> NOTE: reproduce with: ant test  -Dtestcase=TestGeoPointQuery -Dtests.method=testRandom -Dtests.seed=7080C492088EC9F3 -Dtests.locale=lt_LT -Dtests.timezone=AST -Dtests.asserts=true -Dtests.file.encoding=UTF-8
        

        Also, I realized (after struggling with it on LUCENE-6780) I was wrong about rewriting to MatchAllDocsQuery: this is not safe, because in general some docs won't have the geo field and we will then incorrectly match them. So I think you should go back to the whole world bbox query? (Maybe this explains the test failures?).

        Show
        Michael McCandless added a comment - Thanks Nicholas Knize , patch looks great, but I'm seeing test failures, e.g: [junit4] Suite: org.apache.lucene.search.TestGeoPointQuery [junit4] 1> T1: id=0 docID=0 lat=0.0 lon=0.0 deleted?=false expected=true but got false query=GeoPointDistanceRangeQuery: field=geoField: Center: [0.0,0.0] Distance: 290419.347234561 m Lower Left: [-2.6088813842482064,-2.626445466260717] Upper Right: [2.6088813842482064,2.626445466260717] [junit4] 1> T0: id=0 docID=0 lat=0.0 lon=0.0 deleted?=false expected=true but got false query=GeoPointDistanceRangeQuery: field=geoField: Center: [0.0,0.0] Distance: 290419.347234561 m Lower Left: [-2.6088813842482064,-2.626445466260717] Upper Right: [2.6088813842482064,2.626445466260717] [junit4] 2> Spa 16, 2015 1:08:38 PM com.carrotsearch.randomizedtesting.RandomizedRunner$QueueUncaughtExceptionsHandler uncaughtException [junit4] 2> WARNING: Uncaught exception in thread: Thread[T0,5,TGRP-TestGeoPointQuery] [junit4] 2> java.lang.AssertionError: wrong hit [junit4] 2> at __randomizedtesting.SeedInfo.seed([7080C492088EC9F3]:0) [junit4] 2> at org.junit.Assert.fail(Assert.java:93) [junit4] 2> at org.apache.lucene.search.TestGeoPointQuery$VerifyHits.test(TestGeoPointQuery.java:582) [junit4] 2> at org.apache.lucene.search.TestGeoPointQuery$1._run(TestGeoPointQuery.java:523) [junit4] 2> at org.apache.lucene.search.TestGeoPointQuery$1.run(TestGeoPointQuery.java:408) [junit4] 2> [junit4] 2> Spa 16, 2015 1:08:38 PM com.carrotsearch.randomizedtesting.RandomizedRunner$QueueUncaughtExceptionsHandler uncaughtException [junit4] 2> WARNING: Uncaught exception in thread: Thread[T1,5,TGRP-TestGeoPointQuery] [junit4] 2> java.lang.AssertionError: wrong hit [junit4] 2> at __randomizedtesting.SeedInfo.seed([7080C492088EC9F3]:0) [junit4] 2> at org.junit.Assert.fail(Assert.java:93) [junit4] 2> at org.apache.lucene.search.TestGeoPointQuery$VerifyHits.test(TestGeoPointQuery.java:582) [junit4] 2> at org.apache.lucene.search.TestGeoPointQuery$1._run(TestGeoPointQuery.java:523) [junit4] 2> at org.apache.lucene.search.TestGeoPointQuery$1.run(TestGeoPointQuery.java:408) [junit4] 2> [junit4] 2> NOTE: reproduce with: ant test -Dtestcase=TestGeoPointQuery -Dtests.method=testRandom -Dtests.seed=7080C492088EC9F3 -Dtests.locale=lt_LT -Dtests.timezone=AST -Dtests.asserts=true -Dtests.file.encoding=UTF-8 Also, I realized (after struggling with it on LUCENE-6780 ) I was wrong about rewriting to MatchAllDocsQuery : this is not safe, because in general some docs won't have the geo field and we will then incorrectly match them. So I think you should go back to the whole world bbox query? (Maybe this explains the test failures?).
        Hide
        Nicholas Knize added a comment -

        Updated patch

        • Reverts matchAllDocs change
        • Fixes PointDistanceRangeQuery test bug
        Show
        Nicholas Knize added a comment - Updated patch Reverts matchAllDocs change Fixes PointDistanceRangeQuery test bug
        Hide
        Michael McCandless added a comment -

        Thanks Nicholas Knize, looks great, I'll commit soon!

        Show
        Michael McCandless added a comment - Thanks Nicholas Knize , looks great, I'll commit soon!
        Hide
        ASF subversion and git services added a comment -

        Commit 1709476 from Michael McCandless in branch 'dev/trunk'
        [ https://svn.apache.org/r1709476 ]

        LUCENE-6778: add GeoPointDistanceRangeQuery

        Show
        ASF subversion and git services added a comment - Commit 1709476 from Michael McCandless in branch 'dev/trunk' [ https://svn.apache.org/r1709476 ] LUCENE-6778 : add GeoPointDistanceRangeQuery
        Hide
        ASF subversion and git services added a comment -

        Commit 1709477 from Michael McCandless in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1709477 ]

        LUCENE-6778: add GeoPointDistanceRangeQuery

        Show
        ASF subversion and git services added a comment - Commit 1709477 from Michael McCandless in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1709477 ] LUCENE-6778 : add GeoPointDistanceRangeQuery
        Hide
        ASF subversion and git services added a comment -

        Commit 1709478 from Michael McCandless in branch 'dev/trunk'
        [ https://svn.apache.org/r1709478 ]

        LUCENE-6778: just use this.radius

        Show
        ASF subversion and git services added a comment - Commit 1709478 from Michael McCandless in branch 'dev/trunk' [ https://svn.apache.org/r1709478 ] LUCENE-6778 : just use this.radius
        Hide
        ASF subversion and git services added a comment -

        Commit 1709479 from Michael McCandless in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1709479 ]

        LUCENE-6778: just use this.radius

        Show
        ASF subversion and git services added a comment - Commit 1709479 from Michael McCandless in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1709479 ] LUCENE-6778 : just use this.radius

          People

          • Assignee:
            Unassigned
            Reporter:
            Nicholas Knize
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development