Skip to content

Commit

Permalink
skip all geo point queries in plain highlighter
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@brwe
brwe committed May 25, 2016
1 parent 1b29fc4 commit 13c3dd8
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 7 deletions.
9 changes: 8 additions & 1 deletion core/src/main/java/org/elasticsearch/search/highlight/CustomQueryScorer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -87,6 +88,12 @@ protected void extractUnknownQuery(Query query,
}
}

protected void extract(Query query, float boost, Map<String, WeightedSpanTerm> 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);
}
}
}

}
28 changes: 22 additions & 6 deletions core/src/test/java/org/elasticsearch/search/highlight/HighlighterSearchIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@
import com.carrotsearch.randomizedtesting.generators.RandomPicks;
import com.google.common.base.Joiner;
import com.google.common.collect.Iterables;
import org.elasticsearch.Version;
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.action.search.SearchRequestBuilder;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.settings.Settings.Builder;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
Expand Down Expand Up @@ -2692,16 +2695,22 @@ 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();
mappings.startObject("type")
.startObject("properties")
.startObject("geo_point")
.field("type", "geo_point")
.field("geohash", true)
.endObject()
.startObject("text")
.field("type", "string")
.field("term_vector", "with_positions_offsets_payloads")
.field("index_options", "offsets")
.endObject()
.endObject()
.endObject();
Expand All @@ -2711,13 +2720,20 @@ 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();
SearchResponse search = client().prepareSearch().setQuery(
QueryBuilders.geoBoundingBoxQuery("geo_point").topLeft(61.10078883158897, -170.15625)
.bottomRight(-64.92354174306496, 118.47656249999999)).addHighlightedField("*").get();

String highlighterType = randomFrom("plain", "fvh", "postings");
QueryBuilder query = QueryBuilders.boolQuery().should(QueryBuilders.geoBoundingBoxQuery("geo_point")
.bottomLeft(-64.92354174306496, -170.15625)
.topRight(61.10078883158897, 118.47656249999999))
.should(QueryBuilders.termQuery("text", "failure"));
SearchResponse search = client().prepareSearch().setSource(
new SearchSourceBuilder().query(query)
.highlight(new HighlightBuilder().field("*").highlighterType(highlighterType)).buildAsBytes()).get();
assertNoFailures(search);
assertThat(search.getHits().totalHits(), equalTo(1L));
assertThat(search.getHits().getAt(0).highlightFields().get("text").fragments().length, equalTo(1));
}
}
48 changes: 48 additions & 0 deletions core/src/test/java/org/elasticsearch/search/highlight/PlainHighlighterTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -39,4 +55,36 @@ public void testHighlightPhrase() throws Exception {
assertArrayEquals(new String[] {"bar <B>foo</B> <B>bar</B> foo"}, frags);
}

public void checkGeoQueryHighlighting(Query geoQuery) throws IOException, InvalidTokenOffsetsException {
Map analysers = new HashMap<String, Analyzer>();
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 <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?
}

public void testGeoPointInBBoxQueryHighlighting() throws IOException, InvalidTokenOffsetsException {
Query geoQuery = new GeoPointDistanceQuery("geo_point", -170.15625, -64.92354174306496, 5576757);
checkGeoQueryHighlighting(geoQuery);
}

public void testGeoPointDistanceQueryHighlighting() throws IOException, InvalidTokenOffsetsException {
Query geoQuery = new GeoPointInBBoxQuery("geo_point", -170.15625, -64.92354174306496, 118.47656249999999, 61.10078883158897);
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", polyLons, polyLats);
checkGeoQueryHighlighting(geoQuery);
}
}

0 comments on commit 13c3dd8

Please sign in to comment.