Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added caching support to geohash_filter #6478

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/reference/query-dsl/filters/geohash-cell-filter.asciidoc
Expand Up @@ -60,3 +60,11 @@ next to the given cell.
}
}
--------------------------------------------------

[float]
==== Caching

The result of the filter is not cached by default. The
`_cache` parameter can be set to `true` to turn caching on.
By default the filter uses the resulting geohash cells as a cache key.
This can be changed by using the `_cache_key` option.
45 changes: 43 additions & 2 deletions src/main/java/org/elasticsearch/index/query/GeohashCellFilter.java
Expand Up @@ -32,6 +32,7 @@
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentParser.Token;
import org.elasticsearch.index.cache.filter.support.CacheKeyFilter;
import org.elasticsearch.index.mapper.FieldMapper;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.core.StringFieldMapper;
Expand Down Expand Up @@ -61,6 +62,8 @@ public class GeohashCellFilter {
public static final String NAME = "geohash_cell";
public static final String NEIGHBORS = "neighbors";
public static final String PRECISION = "precision";
public static final String CACHE = "_cache";
public static final String CACHE_KEY = "_cache_key";

/**
* Create a new geohash filter for a given set of geohashes. In general this method
Expand Down Expand Up @@ -100,6 +103,9 @@ public static class Builder extends BaseFilterBuilder {
private String geohash;
private int levels = -1;
private boolean neighbors;
private Boolean cache;
private String cacheKey;


public Builder(String field) {
this(field, null, false);
Expand Down Expand Up @@ -155,6 +161,19 @@ public Builder field(String field) {
return this;
}

/**
* Should the filter be cached or not. Defaults to <tt>false</tt>.
*/
public Builder cache(boolean cache) {
this.cache = cache;
return this;
}

public Builder cacheKey(String cacheKey) {
this.cacheKey = cacheKey;
return this;
}

@Override
protected void doXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(NAME);
Expand All @@ -164,6 +183,12 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
if(levels > 0) {
builder.field(PRECISION, levels);
}
if (cache != null) {
builder.field(CACHE, cache);
}
if (cacheKey != null) {
builder.field(CACHE_KEY, cacheKey);
}
builder.field(field, geohash);

builder.endObject();
Expand All @@ -189,6 +214,9 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
String geohash = null;
int levels = -1;
boolean neighbors = false;
boolean cache = false;
CacheKeyFilter.Key cacheKey = null;


XContentParser.Token token;
if ((token = parser.currentToken()) != Token.START_OBJECT) {
Expand All @@ -210,6 +238,12 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
} else if (NEIGHBORS.equals(field)) {
parser.nextToken();
neighbors = parser.booleanValue();
} else if (CACHE.equals(field)) {
parser.nextToken();
cache = parser.booleanValue();
} else if (CACHE_KEY.equals(field)) {
parser.nextToken();
cacheKey = new CacheKeyFilter.Key(parser.text());
} else {
fieldName = field;
token = parser.nextToken();
Expand Down Expand Up @@ -254,11 +288,18 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
geohash = geohash.substring(0, len);
}

Filter filter;
if (neighbors) {
return create(parseContext, geoMapper, geohash, GeoHashUtils.addNeighbors(geohash, new ArrayList<CharSequence>(8)));
filter = create(parseContext, geoMapper, geohash, GeoHashUtils.addNeighbors(geohash, new ArrayList<CharSequence>(8)));
} else {
return create(parseContext, geoMapper, geohash, null);
filter = create(parseContext, geoMapper, geohash, null);
}

if (cache) {
filter = parseContext.cacheFilter(filter, cacheKey);
}

return filter;
}
}
}
66 changes: 53 additions & 13 deletions src/test/java/org/elasticsearch/search/geo/GeoFilterTests.java
Expand Up @@ -34,6 +34,7 @@
import org.elasticsearch.action.bulk.BulkResponse;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.common.Priority;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.geo.GeoHashUtils;
import org.elasticsearch.common.geo.GeoPoint;
Expand All @@ -44,6 +45,7 @@
import org.elasticsearch.common.io.Streams;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.index.query.FilterBuilders;
import org.elasticsearch.index.query.GeohashCellFilter;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.test.ElasticsearchIntegrationTest;
Expand All @@ -54,10 +56,7 @@
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
import java.util.Random;
import java.util.*;
import java.util.zip.GZIPInputStream;

import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
Expand Down Expand Up @@ -500,27 +499,68 @@ public void testGeohashCellFilter() throws IOException {

client().admin().indices().prepareRefresh("locations").execute().actionGet();

// Result of this geohash search should contain the geohash only
SearchResponse results1 = client().prepareSearch("locations").setQuery(QueryBuilders.matchAllQuery()).setPostFilter("{\"geohash_cell\": {\"pin\": \"" + geohash + "\", \"neighbors\": false}}").execute().actionGet();
assertHitCount(results1, 1);
Map<GeohashCellFilter.Builder, Long> expectedCounts = new HashMap<>();
Map<GeohashCellFilter.Builder, String[]> expectedResults = new HashMap<>();
Map<GeohashCellFilter.Builder, String> cacheKeys = new HashMap<>();

// test the same, just with the builder
results1 = client().prepareSearch("locations").setQuery(QueryBuilders.matchAllQuery()).setPostFilter(geoHashCellFilter("pin", geohash, false)).execute().actionGet();
assertHitCount(results1, 1);
expectedCounts.put(geoHashCellFilter("pin", geohash, false), 1L);

// Result of the parent query should contain the parent it self, its neighbors, the child and all its neighbors
SearchResponse results2 = client().prepareSearch("locations").setQuery(QueryBuilders.matchAllQuery()).setPostFilter("{\"geohash_cell\": {\"pin\": \"" + geohash.substring(0, geohash.length() - 1) + "\", \"neighbors\": true}}").execute().actionGet();
assertHitCount(results2, 2 + neighbors.size() + parentNeighbors.size());
expectedCounts.put(geoHashCellFilter("pin", geohash.substring(0, geohash.length() - 1), true), 2L + neighbors.size() + parentNeighbors.size());

// Testing point formats and precision
GeoPoint point = GeoHashUtils.decode(geohash);
int precision = geohash.length();

expectedCounts.put(geoHashCellFilter("pin", point).neighbors(true).precision(precision), 1L + neighbors.size());

logger.info("random testing of setting");

List<GeohashCellFilter.Builder> filterBuilders = new ArrayList<>(expectedCounts.keySet());
for (int j = filterBuilders.size() * 2 * randomIntBetween(1, 5); j > 0; j--) {
Collections.shuffle(filterBuilders, getRandom());
for (GeohashCellFilter.Builder builder : filterBuilders) {
if (randomBoolean()) {
builder.cache(randomBoolean());
}
if (randomBoolean()) {
String cacheKey = cacheKeys.get(builder);
if (cacheKey == null) {
cacheKey = randomUnicodeOfLength(6);
cacheKeys.put(builder, cacheKey);
}
builder.cacheKey(cacheKey);
} else {
builder.cacheKey(null);
}
try {
SearchResponse response = client().prepareSearch("locations").setQuery(QueryBuilders.matchAllQuery()).setPostFilter(builder).get();
assertHitCount(response, expectedCounts.get(builder));
String[] expectedIds = expectedResults.get(builder);
if (expectedIds == null) {
ArrayList<String> ids = new ArrayList<>();
for (SearchHit hit : response.getHits()) {
ids.add(hit.id());
}
expectedResults.put(builder, ids.toArray(Strings.EMPTY_ARRAY));
continue;
}

assertSearchHits(response, expectedIds);

} catch (AssertionError error) {
throw new AssertionError(error.getMessage() + "\n geohash_cell filter:" + builder, error);
}


}
}

logger.info("Testing lat/lon format");
String pointTest1 = "{\"geohash_cell\": {\"pin\": {\"lat\": " + point.lat() + ",\"lon\": " + point.lon() + "},\"precision\": " + precision + ",\"neighbors\": true}}";
SearchResponse results3 = client().prepareSearch("locations").setQuery(QueryBuilders.matchAllQuery()).setPostFilter(pointTest1).execute().actionGet();
assertHitCount(results3, neighbors.size() + 1);


logger.info("Testing String format");
String pointTest2 = "{\"geohash_cell\": {\"pin\": \"" + point.lat() + "," + point.lon() + "\",\"precision\": " + precision + ",\"neighbors\": true}}";
SearchResponse results4 = client().prepareSearch("locations").setQuery(QueryBuilders.matchAllQuery()).setPostFilter(pointTest2).execute().actionGet();
Expand Down