remove workaround for highlighter bug with geo queries

This has been fixed in Lucene https://issues.apache.org/jira/browse/LUCENE-7293 This commit also adds the tests from elastic#20412
brwe · Sep 12, 2016 · b177f10 · b177f10
1 parent b1a2768
commit b177f10
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 7 deletions.
diff --git a/core/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/CustomQueryScorer.java b/core/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/CustomQueryScorer.java
@@ -90,11 +90,7 @@ protected void extractUnknownQuery(Query query,
         }
 
         protected void extract(Query query, float boost, Map<String, WeightedSpanTerm> terms) throws IOException {
-            if (query instanceof GeoPointInBBoxQuery) {
-                // skip all geo queries, see https://issues.apache.org/jira/browse/LUCENE-7293 and
-                // https://github.com/elastic/elasticsearch/issues/17537
-                return;
-            } else if (query instanceof HasChildQueryBuilder.LateParsingQuery) {
+            if (query instanceof HasChildQueryBuilder.LateParsingQuery) {
                 // skip has_child or has_parent queries, see: https://github.com/elastic/elasticsearch/issues/14999
                 return;
             }

diff --git a/.../src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlighterSearchIT.java b/.../src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlighterSearchIT.java
@@ -27,6 +27,7 @@
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.action.support.WriteRequest;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
+import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.Settings.Builder;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -2756,6 +2757,45 @@ public void testGeoFieldHighlightingWithDifferentHighlighters() throws IOExcepti
         assertThat(search.getHits().getAt(0).highlightFields().get("text").fragments().length, equalTo(1));
     }
 
+    public void testGeoFieldHighlightingWhenQueryGetsRewritten() throws IOException {
+        // same as above but in this example the query gets rewritten during highlighting
+        // see https://github.com/elastic/elasticsearch/issues/17537#issuecomment-244939633
+        XContentBuilder mappings = jsonBuilder();
+        mappings.startObject();
+        mappings.startObject("jobs")
+            .startObject("_all")
+            .field("enabled", false)
+            .endObject()
+            .startObject("properties")
+            .startObject("loc")
+            .field("type", "geo_point")
+            .endObject()
+            .startObject("jd")
+            .field("type", "string")
+            .endObject()
+            .endObject()
+            .endObject();
+        mappings.endObject();
+        assertAcked(prepareCreate("test")
+            .addMapping("jobs", mappings));
+        ensureYellow();
+
+        client().prepareIndex("test", "jobs", "1")
+            .setSource(jsonBuilder().startObject().field("jd", "some आवश्यकता है- आर्य समाज अनाथालय, 68 सिविल लाइन्स, बरेली को एक पुरूष" +
+                " रस text")
+                .field("loc", "12.934059,77.610741").endObject())
+            .get();
+        refresh();
+
+        QueryBuilder query = QueryBuilders.functionScoreQuery(QueryBuilders.boolQuery().filter(QueryBuilders.geoBoundingBoxQuery("loc")
+            .setCorners(new GeoPoint(48.934059, 41.610741), new GeoPoint(-23.065941, 113.610741))));
+        SearchResponse search = client().prepareSearch().setSource(
+            new SearchSourceBuilder().query(query).highlighter(new HighlightBuilder().highlighterType("plain").field("jd"))).get();
+        assertNoFailures(search);
+        assertThat(search.getHits().totalHits(), equalTo(1L));
+    }
+
+
     public void testKeywordFieldHighlighting() throws IOException {
         // check that keyword highlighting works
         XContentBuilder mappings = jsonBuilder();

diff --git a/...rc/test/java/org/elasticsearch/search/fetch/subphase/highlight/PlainHighlighterTests.java b/...rc/test/java/org/elasticsearch/search/fetch/subphase/highlight/PlainHighlighterTests.java
@@ -68,8 +68,11 @@ public void checkGeoQueryHighlighting(Query geoQuery) throws IOException, Invali
         String fragment = highlighter.getBestFragment(fieldNameAnalyzer.tokenStream("text", "Arbitrary text field which should not cause " +
             "a failure"), "Arbitrary text field which should not cause a failure");
         assertThat(fragment, equalTo("Arbitrary text field which should not cause a <B>failure</B>"));
-        // TODO: This test will fail if we pass in an instance of GeoPointInBBoxQueryImpl too. Should we also find a way to work around that
-        // or can the query not be rewritten before it is passed into the highlighter?
+        Query rewritten = boolQuery.rewrite(null);
+        highlighter = new org.apache.lucene.search.highlight.Highlighter(new CustomQueryScorer(rewritten));
+        fragment = highlighter.getBestFragment(fieldNameAnalyzer.tokenStream("text", "Arbitrary text field which should not cause " +
+            "a failure"), "Arbitrary text field which should not cause a failure");
+        assertThat(fragment, equalTo("Arbitrary text field which should not cause a <B>failure</B>"));
     }
 
     public void testGeoPointInBBoxQueryHighlighting() throws IOException, InvalidTokenOffsetsException {