diff --git a/core/src/main/java/org/elasticsearch/index/query/RangeQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/RangeQueryBuilder.java index c3953a51170..b1132c42ea9 100644 --- a/core/src/main/java/org/elasticsearch/index/query/RangeQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/RangeQueryBuilder.java @@ -22,10 +22,6 @@ package org.elasticsearch.index.query; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermRangeQuery; import org.apache.lucene.util.BytesRef; -import org.elasticsearch.action.fieldstats.FieldStats; -import org.elasticsearch.action.fieldstats.IndexConstraint; -import org.elasticsearch.action.fieldstats.IndexConstraint.Comparison; -import org.elasticsearch.action.fieldstats.IndexConstraint.Property; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -259,8 +255,8 @@ public class RangeQueryBuilder extends AbstractQueryBuilder i } @Override - protected QueryBuilder doRewrite(QueryRewriteContext queryShardContext) throws IOException { - FieldStatsProvider fieldStatsProvider = queryShardContext.getFieldStatsProvider(); + protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException { + FieldStatsProvider fieldStatsProvider = queryRewriteContext.getFieldStatsProvider(); // If the fieldStatsProvider is null we are not on the shard and cannot // rewrite so just return without rewriting if (fieldStatsProvider != null) { @@ -271,17 +267,10 @@ public class RangeQueryBuilder extends AbstractQueryBuilder i case DISJOINT: return new MatchNoneQueryBuilder(); case WITHIN: - FieldStats fieldStats = fieldStatsProvider.get(fieldName); - if (!(fieldStats.getMinValue().equals(from) && fieldStats.getMaxValue().equals(to) && includeUpper && includeLower)) { - // Rebuild the range query with the bounds for this shard. - // The includeLower/Upper values are preserved only if the - // bound has not been changed by the rewrite + if (from != null || to != null) { RangeQueryBuilder newRangeQuery = new RangeQueryBuilder(fieldName); - String dateFormatString = format == null ? null : format.format(); - newRangeQuery.from(fieldStats.getMinValue(), includeLower || fieldStats.match( - new IndexConstraint(fieldName, Property.MIN, Comparison.GT, fieldStats.stringValueOf(from, dateFormatString)))); - newRangeQuery.to(fieldStats.getMaxValue(), includeUpper || fieldStats.match( - new IndexConstraint(fieldName, Property.MAX, Comparison.LT, fieldStats.stringValueOf(to, dateFormatString)))); + newRangeQuery.from(null); + newRangeQuery.to(null); newRangeQuery.format = format; newRangeQuery.timeZone = timeZone; return newRangeQuery; diff --git a/core/src/main/java/org/elasticsearch/search/SearchService.java b/core/src/main/java/org/elasticsearch/search/SearchService.java index be2e52b5aa3..a1b7f93d0d1 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchService.java +++ b/core/src/main/java/org/elasticsearch/search/SearchService.java @@ -549,8 +549,14 @@ public class SearchService extends AbstractLifecycleComponent imp indexShard, scriptService, pageCacheRecycler, bigArrays, threadPool.estimatedTimeInMillisCounter(), parseFieldMatcher, defaultSearchTimeout, fetchPhase); context.getQueryShardContext().setFieldStatsProvider(new FieldStatsProvider(engineSearcher, indexService.mapperService())); - request.rewrite(context.getQueryShardContext()); SearchContext.setCurrent(context); + request.rewrite(context.getQueryShardContext()); + // reset that we have used nowInMillis from the context since it may + // have been rewritten so its no longer in the query and the request can + // be cached. If it is still present in the request (e.g. in a range + // aggregation) it will still be caught when the aggregation is + // evaluated. + context.resetNowInMillisUsed(); try { if (request.scroll() != null) { context.scrollContext(new ScrollContext()); diff --git a/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java b/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java index ec47c6327cf..1881109c9b6 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java @@ -149,6 +149,10 @@ public abstract class SearchContext implements Releasable { return nowInMillisUsed; } + public final void resetNowInMillisUsed() { + this.nowInMillisUsed = false; + } + protected abstract long nowInMillisImpl(); public abstract ScrollContext scrollContext(); diff --git a/core/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTests.java index 30e32c92da2..eb373539994 100644 --- a/core/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTests.java @@ -430,10 +430,8 @@ public class RangeQueryBuilderTests extends AbstractQueryTestCase> FieldStats get(String field) throws IOException { + assertThat(field, equalTo(fieldName)); + return (FieldStats) new FieldStats.Date(randomLong(), randomLong(), randomLong(), randomLong(), + shardMinValue.getMillis(), shardMaxValue.getMillis(), null); + } + }; + queryShardContext.setFieldStatsProvider(fieldStatsProvider); + QueryBuilder rewritten = query.rewrite(queryShardContext); + assertThat(rewritten, instanceOf(RangeQueryBuilder.class)); + RangeQueryBuilder rewrittenRange = (RangeQueryBuilder) rewritten; + assertThat(rewrittenRange.fieldName(), equalTo(fieldName)); + assertThat(rewrittenRange.from(), equalTo(null)); + assertThat(rewrittenRange.to(), equalTo(null)); } public void testRewriteDateToMatchNone() throws IOException { @@ -773,6 +798,27 @@ public class RangeQueryBuilderTests extends AbstractQueryTestCase rewritten = query.rewrite(queryShardContext); + assertThat(rewritten, instanceOf(MatchNoneQueryBuilder.class)); + } + public void testRewriteDateToSame() throws IOException { String fieldName = randomAsciiOfLengthBetween(1, 20); RangeQueryBuilder query = new RangeQueryBuilder(fieldName); @@ -793,4 +839,25 @@ public class RangeQueryBuilderTests extends AbstractQueryTestCase rewritten = query.rewrite(queryShardContext); assertThat(rewritten, sameInstance(query)); } + + public void testRewriteDateWithNowToSame() throws IOException { + String fieldName = randomAsciiOfLengthBetween(1, 20); + RangeQueryBuilder query = new RangeQueryBuilder(fieldName); + String queryFromValue = "now-2d"; + String queryToValue = "now"; + query.from(queryFromValue); + query.to(queryToValue); + QueryShardContext queryShardContext = queryShardContext(); + FieldStatsProvider fieldStatsProvider = new FieldStatsProvider(null, null) { + + @Override + public Relation isFieldWithinQuery(String fieldName, Object from, Object to, boolean includeLower, boolean includeUpper, + DateTimeZone timeZone, DateMathParser dateMathParser) throws IOException { + return Relation.INTERSECTS; + } + }; + queryShardContext.setFieldStatsProvider(fieldStatsProvider); + QueryBuilder rewritten = query.rewrite(queryShardContext); + assertThat(rewritten, sameInstance(query)); + } } diff --git a/core/src/test/java/org/elasticsearch/indices/IndicesRequestCacheIT.java b/core/src/test/java/org/elasticsearch/indices/IndicesRequestCacheIT.java index 94c41e5c84e..fec50cf0a27 100644 --- a/core/src/test/java/org/elasticsearch/indices/IndicesRequestCacheIT.java +++ b/core/src/test/java/org/elasticsearch/indices/IndicesRequestCacheIT.java @@ -27,7 +27,10 @@ import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInter import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram.Bucket; import org.elasticsearch.test.ESIntegTestCase; +import org.joda.time.DateTime; import org.joda.time.DateTimeZone; +import org.joda.time.chrono.ISOChronology; + import java.util.List; import static org.elasticsearch.search.aggregations.AggregationBuilders.dateHistogram; @@ -233,4 +236,122 @@ public class IndicesRequestCacheIT extends ESIntegTestCase { equalTo(1L)); } + public void testQueryRewriteDatesWithNow() throws Exception { + assertAcked(client().admin().indices().prepareCreate("index-1").addMapping("type", "d", "type=date") + .setSettings(IndicesRequestCache.INDEX_CACHE_REQUEST_ENABLED_SETTING.getKey(), true, IndexMetaData.SETTING_NUMBER_OF_SHARDS, + 1, IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0) + .get()); + assertAcked(client().admin().indices().prepareCreate("index-2").addMapping("type", "d", "type=date") + .setSettings(IndicesRequestCache.INDEX_CACHE_REQUEST_ENABLED_SETTING.getKey(), true, IndexMetaData.SETTING_NUMBER_OF_SHARDS, + 1, IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0) + .get()); + assertAcked(client().admin().indices().prepareCreate("index-3").addMapping("type", "d", "type=date") + .setSettings(IndicesRequestCache.INDEX_CACHE_REQUEST_ENABLED_SETTING.getKey(), true, IndexMetaData.SETTING_NUMBER_OF_SHARDS, + 1, IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0) + .get()); + DateTime now = new DateTime(ISOChronology.getInstanceUTC()); + indexRandom(true, client().prepareIndex("index-1", "type", "1").setSource("d", now), + client().prepareIndex("index-1", "type", "2").setSource("d", now.minusDays(1)), + client().prepareIndex("index-1", "type", "3").setSource("d", now.minusDays(2)), + client().prepareIndex("index-2", "type", "4").setSource("d", now.minusDays(3)), + client().prepareIndex("index-2", "type", "5").setSource("d", now.minusDays(4)), + client().prepareIndex("index-2", "type", "6").setSource("d", now.minusDays(5)), + client().prepareIndex("index-3", "type", "7").setSource("d", now.minusDays(6)), + client().prepareIndex("index-3", "type", "8").setSource("d", now.minusDays(7)), + client().prepareIndex("index-3", "type", "9").setSource("d", now.minusDays(8))); + ensureSearchable("index-1", "index-2", "index-3"); + + assertThat( + client().admin().indices().prepareStats("index-1").setRequestCache(true).get().getTotal().getRequestCache().getHitCount(), + equalTo(0L)); + assertThat( + client().admin().indices().prepareStats("index-1").setRequestCache(true).get().getTotal().getRequestCache().getMissCount(), + equalTo(0L)); + + assertThat( + client().admin().indices().prepareStats("index-2").setRequestCache(true).get().getTotal().getRequestCache().getHitCount(), + equalTo(0L)); + assertThat( + client().admin().indices().prepareStats("index-2").setRequestCache(true).get().getTotal().getRequestCache().getMissCount(), + equalTo(0L)); + assertThat( + client().admin().indices().prepareStats("index-3").setRequestCache(true).get().getTotal().getRequestCache().getHitCount(), + equalTo(0L)); + assertThat( + client().admin().indices().prepareStats("index-3").setRequestCache(true).get().getTotal().getRequestCache().getMissCount(), + equalTo(0L)); + + final SearchResponse r1 = client().prepareSearch("index-*").setSearchType(SearchType.QUERY_THEN_FETCH).setSize(0) + .setQuery(QueryBuilders.rangeQuery("d").gte("now-7d/d").lte("now")).get(); + assertSearchResponse(r1); + assertThat(r1.getHits().getTotalHits(), equalTo(8L)); + assertThat( + client().admin().indices().prepareStats("index-1").setRequestCache(true).get().getTotal().getRequestCache().getHitCount(), + equalTo(0L)); + assertThat( + client().admin().indices().prepareStats("index-1").setRequestCache(true).get().getTotal().getRequestCache().getMissCount(), + equalTo(1L)); + assertThat( + client().admin().indices().prepareStats("index-2").setRequestCache(true).get().getTotal().getRequestCache().getHitCount(), + equalTo(0L)); + assertThat( + client().admin().indices().prepareStats("index-2").setRequestCache(true).get().getTotal().getRequestCache().getMissCount(), + equalTo(1L)); + // Because the query will INTERSECT with the 3rd index it will not be + // rewritten and will still contain `now` so won't be recorded as a + // cache miss or cache hit since queries containing now can't be cached + assertThat( + client().admin().indices().prepareStats("index-3").setRequestCache(true).get().getTotal().getRequestCache().getHitCount(), + equalTo(0L)); + assertThat( + client().admin().indices().prepareStats("index-3").setRequestCache(true).get().getTotal().getRequestCache().getMissCount(), + equalTo(0L)); + + final SearchResponse r2 = client().prepareSearch("index-*").setSearchType(SearchType.QUERY_THEN_FETCH).setSize(0) + .setQuery(QueryBuilders.rangeQuery("d").gte("now-7d/d").lte("now")).get(); + assertSearchResponse(r2); + assertThat(r2.getHits().getTotalHits(), equalTo(8L)); + assertThat( + client().admin().indices().prepareStats("index-1").setRequestCache(true).get().getTotal().getRequestCache().getHitCount(), + equalTo(1L)); + assertThat( + client().admin().indices().prepareStats("index-1").setRequestCache(true).get().getTotal().getRequestCache().getMissCount(), + equalTo(1L)); + assertThat( + client().admin().indices().prepareStats("index-2").setRequestCache(true).get().getTotal().getRequestCache().getHitCount(), + equalTo(1L)); + assertThat( + client().admin().indices().prepareStats("index-2").setRequestCache(true).get().getTotal().getRequestCache().getMissCount(), + equalTo(1L)); + assertThat( + client().admin().indices().prepareStats("index-3").setRequestCache(true).get().getTotal().getRequestCache().getHitCount(), + equalTo(0L)); + assertThat( + client().admin().indices().prepareStats("index-3").setRequestCache(true).get().getTotal().getRequestCache().getMissCount(), + equalTo(0L)); + + final SearchResponse r3 = client().prepareSearch("index-*").setSearchType(SearchType.QUERY_THEN_FETCH).setSize(0) + .setQuery(QueryBuilders.rangeQuery("d").gte("now-7d/d").lte("now")).get(); + assertSearchResponse(r3); + assertThat(r3.getHits().getTotalHits(), equalTo(8L)); + assertThat( + client().admin().indices().prepareStats("index-1").setRequestCache(true).get().getTotal().getRequestCache().getHitCount(), + equalTo(2L)); + assertThat( + client().admin().indices().prepareStats("index-1").setRequestCache(true).get().getTotal().getRequestCache().getMissCount(), + equalTo(1L)); + assertThat( + client().admin().indices().prepareStats("index-2").setRequestCache(true).get().getTotal().getRequestCache().getHitCount(), + equalTo(2L)); + assertThat( + client().admin().indices().prepareStats("index-2").setRequestCache(true).get().getTotal().getRequestCache().getMissCount(), + equalTo(1L)); + assertThat( + client().admin().indices().prepareStats("index-3").setRequestCache(true).get().getTotal().getRequestCache().getHitCount(), + equalTo(0L)); + assertThat( + client().admin().indices().prepareStats("index-3").setRequestCache(true).get().getTotal().getRequestCache().getMissCount(), + equalTo(0L)); + } + }