From b493f0defef0a6dc07a71d428c7702be85a3182e Mon Sep 17 00:00:00 2001 From: Britta Weber Date: Fri, 20 May 2016 19:28:04 +0200 Subject: [PATCH] skip all geo point queries in plain highlighter Geo queries and plain highlighter do not seem to work well together (https://issues.apache.org/jira/browse/LUCENE-7293) so we need to skip all geo related queries when we highlight. closes #17537 --- .../search/highlight/CustomQueryScorer.java | 9 +++- .../search/highlight/HighlighterSearchIT.java | 21 ++++++-- .../highlight/PlainHighlighterTests.java | 48 +++++++++++++++++++ 3 files changed, 72 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/highlight/CustomQueryScorer.java b/core/src/main/java/org/elasticsearch/search/highlight/CustomQueryScorer.java index 7a15f67dbd6f3..03c7a535140f8 100644 --- a/core/src/main/java/org/elasticsearch/search/highlight/CustomQueryScorer.java +++ b/core/src/main/java/org/elasticsearch/search/highlight/CustomQueryScorer.java @@ -24,6 +24,7 @@ import org.apache.lucene.search.highlight.QueryScorer; import org.apache.lucene.search.highlight.WeightedSpanTerm; import org.apache.lucene.search.highlight.WeightedSpanTermExtractor; +import org.apache.lucene.spatial.geopoint.search.GeoPointInBBoxQuery; import org.elasticsearch.common.lucene.search.function.FiltersFunctionScoreQuery; import org.elasticsearch.common.lucene.search.function.FunctionScoreQuery; @@ -87,6 +88,12 @@ protected void extractUnknownQuery(Query query, } } + protected void extract(Query query, float boost, Map terms) throws IOException { + // skip all geo queries, see https://issues.apache.org/jira/browse/LUCENE-7293 and + // https://github.com/elastic/elasticsearch/issues/17537 + if (query instanceof GeoPointInBBoxQuery == false) { + super.extract(query, boost, terms); + } + } } - } diff --git a/core/src/test/java/org/elasticsearch/search/highlight/HighlighterSearchIT.java b/core/src/test/java/org/elasticsearch/search/highlight/HighlighterSearchIT.java index 869182cb51a7f..ab4d7655aa674 100644 --- a/core/src/test/java/org/elasticsearch/search/highlight/HighlighterSearchIT.java +++ b/core/src/test/java/org/elasticsearch/search/highlight/HighlighterSearchIT.java @@ -2553,9 +2553,9 @@ private void phraseBoostTestCase(String highlighterType) { assertHighlight(response, 0, "field1", 0, 1, highlightedMatcher); } - public void testGeoFieldHighlighting() throws IOException { + public void testGeoFieldHighlightingWithDifferentHighlighters() throws IOException { // check that we do not get an exception for geo_point fields in case someone tries to highlight - // it accidential with a wildcard + // it accidentially with a wildcard // see https://github.com/elastic/elasticsearch/issues/17537 XContentBuilder mappings = jsonBuilder(); mappings.startObject(); @@ -2563,6 +2563,12 @@ public void testGeoFieldHighlighting() throws IOException { .startObject("properties") .startObject("geo_point") .field("type", "geo_point") + .field("geohash", true) + .endObject() + .startObject("text") + .field("type", "text") + .field("term_vector", "with_positions_offsets_payloads") + .field("index_options", "offsets") .endObject() .endObject() .endObject(); @@ -2572,14 +2578,19 @@ public void testGeoFieldHighlighting() throws IOException { ensureYellow(); client().prepareIndex("test", "type", "1") - .setSource(jsonBuilder().startObject().field("geo_point", "60.12,100.34").endObject()) + .setSource(jsonBuilder().startObject().field("text", "Arbitrary text field which will should not cause a failure").endObject()) .get(); refresh(); + String highlighterType = randomFrom("plain", "fvh", "postings"); + QueryBuilder query = QueryBuilders.boolQuery().should(QueryBuilders.geoBoundingBoxQuery("geo_point") + .setCorners(61.10078883158897, -170.15625, -64.92354174306496, 118.47656249999999)) + .should(QueryBuilders.termQuery("text", "failure")); SearchResponse search = client().prepareSearch().setSource( - new SearchSourceBuilder().query(QueryBuilders.geoBoundingBoxQuery("geo_point").setCorners(61.10078883158897, -170.15625, - -64.92354174306496, 118.47656249999999)).highlighter(new HighlightBuilder().field("*"))).get(); + new SearchSourceBuilder().query(query) + .highlighter(new HighlightBuilder().field("*").highlighterType(highlighterType))).get(); assertNoFailures(search); assertThat(search.getHits().totalHits(), equalTo(1L)); + assertThat(search.getHits().getAt(0).highlightFields().get("text").fragments().length, equalTo(1)); } public void testKeywordFieldHighlighting() throws IOException { diff --git a/core/src/test/java/org/elasticsearch/search/highlight/PlainHighlighterTests.java b/core/src/test/java/org/elasticsearch/search/highlight/PlainHighlighterTests.java index 5156209d6f138..9c14e2116d19d 100644 --- a/core/src/test/java/org/elasticsearch/search/highlight/PlainHighlighterTests.java +++ b/core/src/test/java/org/elasticsearch/search/highlight/PlainHighlighterTests.java @@ -19,12 +19,28 @@ package org.elasticsearch.search.highlight; +import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.analysis.MockAnalyzer; +import org.apache.lucene.analysis.standard.StandardAnalyzer; import org.apache.lucene.index.Term; +import org.apache.lucene.search.BooleanClause; +import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.PhraseQuery; import org.apache.lucene.search.Query; +import org.apache.lucene.search.TermQuery; +import org.apache.lucene.search.highlight.InvalidTokenOffsetsException; import org.apache.lucene.search.highlight.QueryScorer; +import org.apache.lucene.spatial.geopoint.search.GeoPointDistanceQuery; +import org.apache.lucene.spatial.geopoint.search.GeoPointInBBoxQuery; +import org.apache.lucene.spatial.geopoint.search.GeoPointInPolygonQuery; import org.apache.lucene.util.LuceneTestCase; +import org.elasticsearch.index.analysis.FieldNameAnalyzer; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + +import static org.hamcrest.Matchers.equalTo; public class PlainHighlighterTests extends LuceneTestCase { @@ -39,4 +55,36 @@ public void testHighlightPhrase() throws Exception { assertArrayEquals(new String[] {"bar foo bar foo"}, frags); } + public void checkGeoQueryHighlighting(Query geoQuery) throws IOException, InvalidTokenOffsetsException { + Map analysers = new HashMap(); + analysers.put("text", new StandardAnalyzer()); + FieldNameAnalyzer fieldNameAnalyzer = new FieldNameAnalyzer(analysers); + Query termQuery = new TermQuery(new Term("text", "failure")); + Query boolQuery = new BooleanQuery.Builder().add(new BooleanClause(geoQuery, BooleanClause.Occur.SHOULD)) + .add(new BooleanClause(termQuery, BooleanClause.Occur.SHOULD)).build(); + org.apache.lucene.search.highlight.Highlighter highlighter = + new org.apache.lucene.search.highlight.Highlighter(new CustomQueryScorer(boolQuery)); + 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 failure")); + // 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? + } + + public void testGeoPointInBBoxQueryHighlighting() throws IOException, InvalidTokenOffsetsException { + Query geoQuery = new GeoPointDistanceQuery("geo_point", -64.92354174306496, -170.15625, 5576757); + checkGeoQueryHighlighting(geoQuery); + } + + public void testGeoPointDistanceQueryHighlighting() throws IOException, InvalidTokenOffsetsException { + Query geoQuery = new GeoPointInBBoxQuery("geo_point", -64.92354174306496, 61.10078883158897, -170.15625, 118.47656249999999); + checkGeoQueryHighlighting(geoQuery); + } + + public void testGeoPointInPolygonQueryHighlighting() throws IOException, InvalidTokenOffsetsException { + double[] polyLats = new double[]{0, 60, 0, 0}; + double[] polyLons = new double[]{0, 60, 90, 0}; + Query geoQuery = new GeoPointInPolygonQuery("geo_point", polyLats, polyLons); + checkGeoQueryHighlighting(geoQuery); + } }