From 94b7873b49a83ac769b6b83ddd715b88e0697dbf Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Tue, 4 Oct 2016 18:05:00 +0200 Subject: [PATCH 01/20] Add a #markAsNotCachable() method to context to mark requests as not cachable --- .../index/query/InnerHitBuilder.java | 3 ++- .../index/query/QueryRewriteContext.java | 9 ++++++++ .../index/query/QueryShardContext.java | 9 ++++++++ .../index/query/ScriptQueryBuilder.java | 4 +++- .../ScriptScoreFunctionBuilder.java | 1 + .../elasticsearch/indices/IndicesService.java | 2 +- .../elasticsearch/search/SearchService.java | 4 ++-- .../SignificantTermsAggregationBuilder.java | 3 +++ .../heuristics/ScriptHeuristic.java | 6 ++++++ .../heuristics/SignificanceHeuristic.java | 4 ++++ .../ScriptedMetricAggregationBuilder.java | 1 + .../scripted/ScriptedMetricAggregator.java | 2 ++ .../tophits/TopHitsAggregationBuilder.java | 3 +++ .../tophits/TopHitsAggregatorFactory.java | 1 + .../ValuesSourceAggregationBuilder.java | 8 +++++-- .../search/internal/SearchContext.java | 16 ++++++++------ .../search/sort/ScriptSortBuilder.java | 3 +-- .../phrase/PhraseSuggestionBuilder.java | 1 + .../index/query/ScriptQueryBuilderTests.java | 21 ++++++++++++------- .../FunctionScoreQueryBuilderTests.java | 11 ++++++++++ .../script/mustache/TemplateQueryBuilder.java | 1 + .../test/AbstractQueryTestCase.java | 12 +++++++++++ 22 files changed, 102 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java b/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java index 454c808a5d5..0f4a1f3c429 100644 --- a/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java @@ -574,8 +574,9 @@ public final class InnerHitBuilder extends ToXContentToBytes implements Writeabl innerHitsContext.docValueFieldsContext(new DocValueFieldsContext(docValueFields)); } if (scriptFields != null) { + context.markAsNotCachable(); for (ScriptField field : scriptFields) { - SearchScript searchScript = innerHitsContext.scriptService().search(innerHitsContext.lookup(), field.script(), + SearchScript searchScript = innerHitsContext.getQueryShardContext().getScriptService().search(innerHitsContext.lookup(), field.script(), ScriptContext.Standard.SEARCH, Collections.emptyMap()); innerHitsContext.scriptFields().add(new org.elasticsearch.search.fetch.subphase.ScriptFieldsContext.ScriptField( field.fieldName(), searchScript, field.ignoreFailure())); diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java index f12605088e6..64329c314dd 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java @@ -41,6 +41,7 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { protected final Client client; protected final IndexReader reader; protected final ClusterState clusterState; + protected boolean cachable; public QueryRewriteContext(IndexSettings indexSettings, MapperService mapperService, ScriptService scriptService, IndicesQueriesRegistry indicesQueriesRegistry, Client client, IndexReader reader, @@ -116,4 +117,12 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { String defaultScriptLanguage = ScriptSettings.getLegacyDefaultLang(indexSettings.getNodeSettings()); return new QueryParseContext(defaultScriptLanguage, indicesQueriesRegistry, parser, indexSettings.getParseFieldMatcher()); } + + public void markAsNotCachable() { + this.cachable = false; + } + + public boolean isCachable() { + return cachable; + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index 377afca2f68..25b7ef35961 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -324,6 +324,15 @@ public class QueryShardContext extends QueryRewriteContext { } } + @Override + public void markAsNotCachable() { + super.markAsNotCachable(); + SearchContext current = SearchContext.current(); + if (current != null) { + current.markAsNotCachable(); + } + } + public final Index index() { return indexSettings.getIndex(); } diff --git a/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java index 3ff924b28db..5a65b4438f0 100644 --- a/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java @@ -41,7 +41,6 @@ import org.elasticsearch.search.lookup.SearchLookup; import java.io.IOException; import java.util.Collections; -import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -134,6 +133,7 @@ public class ScriptQueryBuilder extends AbstractQueryBuilder @Override protected Query doToQuery(QueryShardContext context) throws IOException { + context.markAsNotCachable(); return new ScriptQuery(script, context.getScriptService(), context.lookup()); } @@ -216,4 +216,6 @@ public class ScriptQueryBuilder extends AbstractQueryBuilder protected boolean doEquals(ScriptQueryBuilder other) { return Objects.equals(script, other.script); } + + } diff --git a/core/src/main/java/org/elasticsearch/index/query/functionscore/ScriptScoreFunctionBuilder.java b/core/src/main/java/org/elasticsearch/index/query/functionscore/ScriptScoreFunctionBuilder.java index e2fbc5955d7..d7bff2bdd44 100644 --- a/core/src/main/java/org/elasticsearch/index/query/functionscore/ScriptScoreFunctionBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/functionscore/ScriptScoreFunctionBuilder.java @@ -95,6 +95,7 @@ public class ScriptScoreFunctionBuilder extends ScoreFunctionBuilder innerBuild(AggregationContext context, ValuesSourceConfig config, AggregatorFactory parent, Builder subFactoriesBuilder) throws IOException { + if (significanceHeuristic.canCache()) { + context.searchContext().markAsNotCachable(); + } return new SignificantTermsAggregatorFactory(name, type, config, includeExclude, executionHint, filterBuilder, bucketCountThresholds, significanceHeuristic, context, parent, subFactoriesBuilder, metaData); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java index c933f9ef596..1c3f5b7ed56 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java @@ -78,6 +78,7 @@ public class ScriptHeuristic extends SignificanceHeuristic { @Override public void initialize(SearchContext context) { + context.markAsNotCachable(); initialize(context.scriptService()); } @@ -217,5 +218,10 @@ public class ScriptHeuristic extends SignificanceHeuristic { return Long.toString(value); } } + + @Override + public boolean canCache() { + return false; + } } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristic.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristic.java index db9711c1a8d..094f529e92c 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristic.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristic.java @@ -57,4 +57,8 @@ public abstract class SignificanceHeuristic implements NamedWriteable, ToXConten public void initialize(SearchContext context) { } + + public boolean canCache() { + return true; + } } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java index 244881a5155..b13b6fe3019 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java @@ -182,6 +182,7 @@ public class ScriptedMetricAggregationBuilder extends AbstractAggregationBuilder @Override protected ScriptedMetricAggregatorFactory doBuild(AggregationContext context, AggregatorFactory parent, Builder subfactoriesBuilder) throws IOException { + context.searchContext().markAsNotCachable(); return new ScriptedMetricAggregatorFactory(name, type, initScript, mapScript, combineScript, reduceScript, params, context, parent, subfactoriesBuilder, metaData); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java index e2d3034fa11..89ec5183c32 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java @@ -53,10 +53,12 @@ public class ScriptedMetricAggregator extends MetricsAggregator { this.params = params; ScriptService scriptService = context.searchContext().scriptService(); if (initScript != null) { + context.searchContext().markAsNotCachable(); scriptService.executable(initScript, ScriptContext.Standard.AGGS, Collections.emptyMap()).run(); } this.mapScript = scriptService.search(context.searchContext().lookup(), mapScript, ScriptContext.Standard.AGGS, Collections.emptyMap()); if (combineScript != null) { + context.searchContext().markAsNotCachable(); this.combineScript = scriptService.executable(combineScript, ScriptContext.Standard.AGGS, Collections.emptyMap()); } else { this.combineScript = null; diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregationBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregationBuilder.java index 3547db7140c..6c26743d4dc 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregationBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregationBuilder.java @@ -527,6 +527,9 @@ public class TopHitsAggregationBuilder extends AbstractAggregationBuilder parent, Builder subfactoriesBuilder) throws IOException { + if (scriptFields != null && scriptFields.isEmpty() == false) { + context.searchContext().markAsNotCachable(); + } return new TopHitsAggregatorFactory(name, type, from, size, explain, version, trackScores, sorts, highlightBuilder, storedFieldsContext, fieldDataFields, scriptFields, fetchSourceContext, context, parent, subfactoriesBuilder, metaData); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java index 7c6a743a20b..5dff86c5eed 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java @@ -99,6 +99,7 @@ public class TopHitsAggregatorFactory extends AggregatorFactory, Collector> queryCollectors(); + public final void markAsNotCachable() { + this.canCache = false; + } + /** * The life time of an object that is used during search execution. */ diff --git a/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java index 0a7cb5e1b36..d41e8c66d6f 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java @@ -55,9 +55,7 @@ import org.elasticsearch.search.MultiValueMode; import java.io.IOException; import java.util.Collections; -import java.util.HashMap; import java.util.Locale; -import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -286,6 +284,7 @@ public class ScriptSortBuilder extends SortBuilder { @Override public SortFieldAndFormat build(QueryShardContext context) throws IOException { + context.markAsNotCachable(); final SearchScript searchScript = context.getScriptService().search( context.lookup(), script, ScriptContext.Standard.SEARCH, Collections.emptyMap()); diff --git a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionBuilder.java b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionBuilder.java index b0cd6a20499..a17a4f5de80 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionBuilder.java @@ -633,6 +633,7 @@ public class PhraseSuggestionBuilder extends SuggestionBuilder> } private QueryBuilder rewriteQuery(QB queryBuilder, QueryRewriteContext rewriteContext) throws IOException { + final boolean wasCachable = rewriteContext.isCachable(); QueryBuilder rewritten = QueryBuilder.rewriteQuery(queryBuilder, rewriteContext); + if (wasCachable == true) { // it's not resettable so we can only check it once + if (isCachable(queryBuilder)) { + assert rewriteContext.isCachable() : queryBuilder.toString(); + } else { + assert rewriteContext.isCachable() == false; + } + } // extra safety to fail fast - serialize the rewritten version to ensure it's serializable. assertSerialization(rewritten); return rewritten; } + protected boolean isCachable(QB queryBuilder) { + return true; + } + /** * Few queries allow you to set the boost and queryName on the java api, although the corresponding parser * doesn't parse them as they are not supported. This method allows to disable boost and queryName related tests for those queries. From c3622271bf1f4196030a4697fef08c609bdfa95b Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Tue, 4 Oct 2016 19:31:51 +0100 Subject: [PATCH 02/20] Tests to make sure markAsNotCacheable() works when scripts are used --- .../SignificantTermsAggregationBuilder.java | 2 +- .../aggregations/bucket/DateHistogramIT.java | 42 ++++++++++++++++ .../aggregations/bucket/DateRangeIT.java | 48 ++++++++++++++++++ .../aggregations/bucket/DoubleTermsIT.java | 42 +++++++++++++++- .../aggregations/bucket/HistogramIT.java | 40 +++++++++++++++ .../aggregations/bucket/LongTermsIT.java | 42 +++++++++++++++- .../search/aggregations/bucket/RangeIT.java | 50 ++++++++++++++++++- .../SignificantTermsSignificanceScoreIT.java | 40 +++++++++++++++ .../aggregations/bucket/StringTermsIT.java | 43 +++++++++++++++- .../search/aggregations/metrics/AvgIT.java | 40 +++++++++++++++ .../aggregations/metrics/CardinalityIT.java | 43 +++++++++++++++- .../aggregations/metrics/ExtendedStatsIT.java | 43 ++++++++++++++++ .../metrics/HDRPercentileRanksIT.java | 44 ++++++++++++++++ .../metrics/HDRPercentilesIT.java | 44 ++++++++++++++++ .../search/aggregations/metrics/MaxIT.java | 42 ++++++++++++++++ .../search/aggregations/metrics/MinIT.java | 42 ++++++++++++++++ .../metrics/ScriptedMetricIT.java | 29 +++++++++++ .../search/aggregations/metrics/StatsIT.java | 41 +++++++++++++++ .../search/aggregations/metrics/SumIT.java | 40 +++++++++++++++ .../metrics/TDigestPercentileRanksIT.java | 45 +++++++++++++++-- .../metrics/TDigestPercentilesIT.java | 42 ++++++++++++++++ .../aggregations/metrics/TopHitsIT.java | 40 +++++++++++++++ .../aggregations/metrics/ValueCountIT.java | 44 ++++++++++++++-- 23 files changed, 915 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregationBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregationBuilder.java index bc210c7f897..4c1629ff39d 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregationBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregationBuilder.java @@ -220,7 +220,7 @@ public class SignificantTermsAggregationBuilder extends ValuesSourceAggregationB @Override protected ValuesSourceAggregatorFactory innerBuild(AggregationContext context, ValuesSourceConfig config, AggregatorFactory parent, Builder subFactoriesBuilder) throws IOException { - if (significanceHeuristic.canCache()) { + if (significanceHeuristic.canCache() == false) { context.searchContext().markAsNotCachable(); } return new SignificantTermsAggregatorFactory(name, type, config, includeExclude, executionHint, filterBuilder, diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DateHistogramIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DateHistogramIT.java index 66dd387623a..47e6bdbd71b 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DateHistogramIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DateHistogramIT.java @@ -1203,4 +1203,46 @@ public class DateHistogramIT extends ESIntegTestCase { assertThat(((DateTime) buckets.get(2).getKey()).getMillis() - ((DateTime) buckets.get(1).getKey()).getMillis(), equalTo(3600000L)); assertThat(((DateTime) buckets.get(3).getKey()).getMillis() - ((DateTime) buckets.get(2).getKey()).getMillis(), equalTo(3600000L)); } + + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=date") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("d", date(1, 1)), + client().prepareIndex("cache_test_idx", "type", "2").setSource("d", date(2, 1))); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + Map params = new HashMap<>(); + params.put("fieldname", "d"); + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(dateHistogram("histo").field("d") + .script(new Script(DateScriptMocks.PlusOneMonthScript.NAME, ScriptType.INLINE, "native", params)) + .dateHistogramInterval(DateHistogramInterval.MONTH)).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(dateHistogram("histo").field("d").dateHistogramInterval(DateHistogramInterval.MONTH)).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DateRangeIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DateRangeIT.java index 1e250681004..a6e0273beff 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DateRangeIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DateRangeIT.java @@ -20,6 +20,7 @@ package org.elasticsearch.search.aggregations.bucket; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptService.ScriptType; @@ -866,4 +867,51 @@ public class DateRangeIT extends ESIntegTestCase { assertThat(buckets.get(0).getDocCount(), equalTo(0L)); assertThat(buckets.get(0).getAggregations().asList().isEmpty(), is(true)); } + + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "date", "type=date") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, + client().prepareIndex("cache_test_idx", "type", "1") + .setSource(jsonBuilder().startObject().field("date", date(1, 1)).endObject()), + client().prepareIndex("cache_test_idx", "type", "2") + .setSource(jsonBuilder().startObject().field("date", date(2, 1)).endObject())); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + Map params = new HashMap<>(); + params.put("fieldname", "date"); + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(dateRange("foo").field("date") + .script(new Script(DateScriptMocks.PlusOneMonthScript.NAME, ScriptType.INLINE, "native", params)) + .addRange(new DateTime(2012, 1, 1, 0, 0, 0, 0, DateTimeZone.UTC), new DateTime(2013, 1, 1, 0, 0, 0, 0, DateTimeZone.UTC))) + .get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(dateRange("foo").field("date") + .addRange(new DateTime(2012, 1, 1, 0, 0, 0, 0, DateTimeZone.UTC), new DateTime(2013, 1, 1, 0, 0, 0, 0, DateTimeZone.UTC))) + .get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsIT.java index ed43ba8bd63..2393512c1a3 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsIT.java @@ -21,13 +21,16 @@ package org.elasticsearch.search.aggregations.bucket; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.script.ScoreAccessor; import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptService.ScriptType; import org.elasticsearch.search.aggregations.AggregationTestScriptsPlugin; import org.elasticsearch.search.aggregations.Aggregator.SubAggCollectionMode; +import org.elasticsearch.search.aggregations.bucket.LongTermsIT.CustomScriptPlugin; import org.elasticsearch.search.aggregations.bucket.filter.Filter; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; import org.elasticsearch.search.aggregations.bucket.terms.Terms; @@ -56,7 +59,6 @@ import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.index.query.QueryBuilders.functionScoreQuery; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; import static org.elasticsearch.index.query.functionscore.ScoreFunctionBuilders.scriptFunction; -import static org.elasticsearch.script.ScriptService.ScriptType; import static org.elasticsearch.search.aggregations.AggregationBuilders.avg; import static org.elasticsearch.search.aggregations.AggregationBuilders.extendedStats; import static org.elasticsearch.search.aggregations.AggregationBuilders.filter; @@ -1179,4 +1181,42 @@ public class DoubleTermsIT extends AbstractTermsTestCase { public void testOtherDocCount() { testOtherDocCount(SINGLE_VALUED_FIELD_NAME, MULTI_VALUED_FIELD_NAME); } + + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=float") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("s", 1.5), + client().prepareIndex("cache_test_idx", "type", "2").setSource("s", 2.5)); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation( + terms("terms").field("d").script(new Script("_value + 1", ScriptType.INLINE, CustomScriptPlugin.NAME, null))).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(terms("terms").field("d")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/HistogramIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/HistogramIT.java index e24f0d39d4b..0c4de629c6e 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/HistogramIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/HistogramIT.java @@ -21,6 +21,7 @@ package org.elasticsearch.search.aggregations.bucket; import com.carrotsearch.hppc.LongHashSet; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.script.MockScriptPlugin; @@ -995,4 +996,43 @@ public class HistogramIT extends ESIntegTestCase { assertEquals(0.05, (double) buckets.get(1).getKey(), 0.01d); assertEquals(1, buckets.get(1).getDocCount()); } + + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=float") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("d", -0.6), + client().prepareIndex("cache_test_idx", "type", "2").setSource("d", 0.1)); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(histogram("histo").field("d") + .script(new Script("_value + 1", ScriptType.INLINE, CustomScriptPlugin.NAME, emptyMap())).interval(0.7).offset(0.05)).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(histogram("histo").field("d").interval(0.7).offset(0.05)) + .get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsIT.java index 6d5e190b542..635b1635cc2 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsIT.java @@ -21,12 +21,15 @@ package org.elasticsearch.search.aggregations.bucket; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptService.ScriptType; import org.elasticsearch.search.aggregations.AggregationTestScriptsPlugin; import org.elasticsearch.search.aggregations.Aggregator.SubAggCollectionMode; +import org.elasticsearch.search.aggregations.bucket.StringTermsIT.CustomScriptPlugin; import org.elasticsearch.search.aggregations.bucket.filter.Filter; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; import org.elasticsearch.search.aggregations.bucket.terms.Terms; @@ -53,7 +56,6 @@ import java.util.function.Function; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; -import static org.elasticsearch.script.ScriptService.ScriptType; import static org.elasticsearch.search.aggregations.AggregationBuilders.avg; import static org.elasticsearch.search.aggregations.AggregationBuilders.extendedStats; import static org.elasticsearch.search.aggregations.AggregationBuilders.filter; @@ -1136,4 +1138,42 @@ public class LongTermsIT extends AbstractTermsTestCase { public void testOtherDocCount() { testOtherDocCount(SINGLE_VALUED_FIELD_NAME, MULTI_VALUED_FIELD_NAME); } + + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("s", 1), + client().prepareIndex("cache_test_idx", "type", "2").setSource("s", 2)); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation( + terms("terms").field("d").script(new Script("_value + 1", ScriptType.INLINE, CustomScriptPlugin.NAME, null))).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(terms("terms").field("d")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/RangeIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/RangeIT.java index d3e934d875f..c64fc213096 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/RangeIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/RangeIT.java @@ -20,9 +20,11 @@ package org.elasticsearch.search.aggregations.bucket; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptService.ScriptType; import org.elasticsearch.search.aggregations.AggregationTestScriptsPlugin; import org.elasticsearch.search.aggregations.Aggregator.SubAggCollectionMode; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; @@ -32,21 +34,25 @@ import org.elasticsearch.search.aggregations.bucket.terms.Terms; import org.elasticsearch.search.aggregations.metrics.sum.Sum; import org.elasticsearch.test.ESIntegTestCase; import org.hamcrest.Matchers; +import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.function.Function; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; -import static org.elasticsearch.script.ScriptService.ScriptType; +import static org.elasticsearch.search.aggregations.AggregationBuilders.dateRange; import static org.elasticsearch.search.aggregations.AggregationBuilders.histogram; import static org.elasticsearch.search.aggregations.AggregationBuilders.range; import static org.elasticsearch.search.aggregations.AggregationBuilders.sum; import static org.elasticsearch.search.aggregations.AggregationBuilders.terms; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; @@ -907,4 +913,46 @@ public class RangeIT extends ESIntegTestCase { assertThat(buckets.get(0).getDocCount(), equalTo(0L)); } + + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "i", "type=integer") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, + client().prepareIndex("cache_test_idx", "type", "1").setSource(jsonBuilder().startObject().field("i", 1).endObject()), + client().prepareIndex("cache_test_idx", "type", "2").setSource(jsonBuilder().startObject().field("i", 2).endObject())); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + Map params = new HashMap<>(); + params.put("fieldname", "date"); + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation( + range("foo").field("i").script(new Script("_value + 1", ScriptType.INLINE, CustomScriptPlugin.NAME, null)).addRange(0, 10)) + .get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(range("foo").field("i").addRange(0, 10)).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java index fab1f8b7d3e..150f4a74a96 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java @@ -22,6 +22,7 @@ import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.query.QueryBuilders; @@ -547,4 +548,43 @@ public class SignificantTermsSignificanceScoreIT extends ESIntegTestCase { SharedSignificantTermsTestMethods.aggregateAndCheckFromSeveralShards(this); } + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("s", 1), + client().prepareIndex("cache_test_idx", "type", "2").setSource("s", 2)); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + ScriptHeuristic scriptHeuristic = getScriptSignificanceHeuristic(); + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(significantTerms("foo").field("s").significanceHeuristic(scriptHeuristic)).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(significantTerms("foo").field("s")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } + } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsIT.java index 8ccccf3f33a..df1741dc8b0 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsIT.java @@ -24,11 +24,13 @@ import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchPhaseExecutionException; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.index.mapper.IndexFieldMapper; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptService.ScriptType; import org.elasticsearch.search.aggregations.AggregationExecutionException; import org.elasticsearch.search.aggregations.AggregationTestScriptsPlugin; import org.elasticsearch.search.aggregations.Aggregator.SubAggCollectionMode; @@ -61,7 +63,6 @@ import java.util.function.Function; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; import static org.elasticsearch.index.query.QueryBuilders.termQuery; -import static org.elasticsearch.script.ScriptService.ScriptType; import static org.elasticsearch.search.aggregations.AggregationBuilders.avg; import static org.elasticsearch.search.aggregations.AggregationBuilders.count; import static org.elasticsearch.search.aggregations.AggregationBuilders.extendedStats; @@ -1511,4 +1512,44 @@ public class StringTermsIT extends AbstractTermsTestCase { public void testOtherDocCount() { testOtherDocCount(SINGLE_VALUED_FIELD_NAME, MULTI_VALUED_FIELD_NAME); } + + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=keyword") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("s", "foo"), + client().prepareIndex("cache_test_idx", "type", "2").setSource("s", "bar")); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation( + terms("terms").field("d").script(new Script("'foo_' + _value", ScriptType.INLINE, CustomScriptPlugin.NAME, null))) + .get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(terms("terms").field("d")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgIT.java index ccf03df37fc..37a3d126e3f 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgIT.java @@ -56,7 +56,9 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.filter; import static org.elasticsearch.search.aggregations.AggregationBuilders.global; import static org.elasticsearch.search.aggregations.AggregationBuilders.histogram; import static org.elasticsearch.search.aggregations.AggregationBuilders.terms; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; @@ -353,6 +355,44 @@ public class AvgIT extends AbstractNumericTestCase { } } + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("s", 1), + client().prepareIndex("cache_test_idx", "type", "2").setSource("s", 2)); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(avg("foo").field("d").script(new Script("", ScriptType.INLINE, FieldValueScriptEngine.NAME, null))).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(avg("foo").field("d")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } + /** * Mock plugin for the {@link ExtractFieldScriptEngine} */ diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java index cff1fa746dc..35e46b50785 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java @@ -26,6 +26,7 @@ import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.script.MockScriptPlugin; import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptService.ScriptType; import org.elasticsearch.search.aggregations.Aggregator.SubAggCollectionMode; import org.elasticsearch.search.aggregations.bucket.global.Global; import org.elasticsearch.search.aggregations.bucket.terms.Terms; @@ -41,10 +42,10 @@ import java.util.function.Function; import static java.util.Collections.emptyMap; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; -import static org.elasticsearch.script.ScriptService.ScriptType; import static org.elasticsearch.search.aggregations.AggregationBuilders.cardinality; import static org.elasticsearch.search.aggregations.AggregationBuilders.global; import static org.elasticsearch.search.aggregations.AggregationBuilders.terms; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; @@ -445,4 +446,44 @@ public class CardinalityIT extends ESIntegTestCase { assertCount(count, 2); } } + + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("s", 1), + client().prepareIndex("cache_test_idx", "type", "2").setSource("s", 2)); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation( + cardinality("foo").field("d").script(new Script("_value", ScriptType.INLINE, CustomScriptPlugin.NAME, emptyMap()))) + .get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(cardinality("foo").field("d")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java index 6f4f891ea65..319a7d64230 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java @@ -19,6 +19,7 @@ package org.elasticsearch.search.aggregations.metrics; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptService.ScriptType; @@ -46,7 +47,9 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.global; import static org.elasticsearch.search.aggregations.AggregationBuilders.histogram; import static org.elasticsearch.search.aggregations.AggregationBuilders.missing; import static org.elasticsearch.search.aggregations.AggregationBuilders.terms; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; @@ -630,4 +633,44 @@ public class ExtendedStatsIT extends AbstractNumericTestCase { assertThat(stats.getStdDeviationBound(ExtendedStats.Bounds.LOWER), equalTo(stats.getAvg() - (stats.getStdDeviation() * sigma))); } + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("s", 1), + client().prepareIndex("cache_test_idx", "type", "2").setSource("s", 2)); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(extendedStats("foo").field("d") + .script(new Script("_value + 1", ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, null))) + .get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(extendedStats("foo").field("d")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } + } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksIT.java index 57bdc7d5dfc..46130fca21f 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksIT.java @@ -20,6 +20,7 @@ package org.elasticsearch.search.aggregations.metrics; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.logging.Loggers; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptService.ScriptType; @@ -49,7 +50,9 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.global; import static org.elasticsearch.search.aggregations.AggregationBuilders.histogram; import static org.elasticsearch.search.aggregations.AggregationBuilders.percentileRanks; import static org.elasticsearch.search.aggregations.AggregationBuilders.terms; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.lessThanOrEqualTo; @@ -530,4 +533,45 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { } } + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("s", 1), + client().prepareIndex("cache_test_idx", "type", "2").setSource("s", 2)); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + SearchResponse r = client() + .prepareSearch("cache_test_idx").setSize(0).addAggregation(percentileRanks("foo").method(PercentilesMethod.HDR).field("d") + .values(50.0).script(new Script("_value - 1", ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, emptyMap()))) + .get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(percentileRanks("foo").method(PercentilesMethod.HDR).field("d").values(50.0)).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } + } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java index 8112551f53c..784b4816393 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java @@ -20,6 +20,7 @@ package org.elasticsearch.search.aggregations.metrics; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.logging.Loggers; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.CollectionUtils; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.script.Script; @@ -49,7 +50,9 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.global; import static org.elasticsearch.search.aggregations.AggregationBuilders.histogram; import static org.elasticsearch.search.aggregations.AggregationBuilders.percentiles; import static org.elasticsearch.search.aggregations.AggregationBuilders.terms; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.hamcrest.Matchers.closeTo; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; @@ -521,4 +524,45 @@ public class HDRPercentilesIT extends AbstractNumericTestCase { } } + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("s", 1), + client().prepareIndex("cache_test_idx", "type", "2").setSource("s", 2)); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(percentiles("foo").method(PercentilesMethod.HDR).field("d").percentiles(50.0) + .script(new Script("_value - 1", ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, emptyMap()))) + .get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(percentiles("foo").method(PercentilesMethod.HDR).field("d").percentiles(50.0)).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } + } \ No newline at end of file diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxIT.java index d4ac8835ee0..c3e693b5307 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxIT.java @@ -19,6 +19,7 @@ package org.elasticsearch.search.aggregations.metrics; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptService.ScriptType; @@ -44,7 +45,9 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.global; import static org.elasticsearch.search.aggregations.AggregationBuilders.histogram; import static org.elasticsearch.search.aggregations.AggregationBuilders.max; import static org.elasticsearch.search.aggregations.AggregationBuilders.terms; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.notNullValue; @@ -352,4 +355,43 @@ public class MaxIT extends AbstractNumericTestCase { } } + + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("s", 1), + client().prepareIndex("cache_test_idx", "type", "2").setSource("s", 2)); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation( + max("foo").field("d").script(new Script("_value + 1", ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, emptyMap()))) + .get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(max("foo").field("d")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } } \ No newline at end of file diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/MinIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/MinIT.java index 56c12fbc77f..93402534619 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/MinIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/MinIT.java @@ -19,6 +19,7 @@ package org.elasticsearch.search.aggregations.metrics; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptService.ScriptType; @@ -44,7 +45,9 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.global; import static org.elasticsearch.search.aggregations.AggregationBuilders.histogram; import static org.elasticsearch.search.aggregations.AggregationBuilders.min; import static org.elasticsearch.search.aggregations.AggregationBuilders.terms; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.notNullValue; @@ -365,4 +368,43 @@ public class MinIT extends AbstractNumericTestCase { } } + + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("s", 1), + client().prepareIndex("cache_test_idx", "type", "2").setSource("s", 2)); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation( + min("foo").field("d").script(new Script("_value - 1", ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, emptyMap()))) + .get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(min("foo").field("d")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } } \ No newline at end of file diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java index e1800b2f9f1..c1a5aad556f 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java @@ -933,4 +933,33 @@ public class ScriptedMetricIT extends ESIntegTestCase { assertThat(aggregationResult.size(), equalTo(1)); assertThat(aggregationResult.get(0), equalTo(0)); } + + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + Script mapScript = new Script("_agg['count'] = 1", ScriptType.INLINE, CustomScriptPlugin.NAME, null); + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("s", 1), + client().prepareIndex("cache_test_idx", "type", "2").setSource("s", 2)); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(scriptedMetric("foo").mapScript(mapScript)).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsIT.java index 510df4c572b..7ab44c49447 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsIT.java @@ -22,6 +22,7 @@ import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.util.Supplier; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.ShardSearchFailure; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptService.ScriptType; @@ -47,7 +48,9 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.global; import static org.elasticsearch.search.aggregations.AggregationBuilders.histogram; import static org.elasticsearch.search.aggregations.AggregationBuilders.stats; import static org.elasticsearch.search.aggregations.AggregationBuilders.terms; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; @@ -485,4 +488,42 @@ public class StatsIT extends AbstractNumericTestCase { } assertThat("Not all shards are initialized", response.getSuccessfulShards(), equalTo(response.getTotalShards())); } + + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("s", 1), + client().prepareIndex("cache_test_idx", "type", "2").setSource("s", 2)); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation( + stats("foo").field("d").script(new Script("_value + 1", ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, null))).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(stats("foo").field("d")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/SumIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/SumIT.java index 186bc8fb27b..3ff00529655 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/SumIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/SumIT.java @@ -55,7 +55,9 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.global; import static org.elasticsearch.search.aggregations.AggregationBuilders.histogram; import static org.elasticsearch.search.aggregations.AggregationBuilders.sum; import static org.elasticsearch.search.aggregations.AggregationBuilders.terms; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.notNullValue; @@ -347,6 +349,44 @@ public class SumIT extends AbstractNumericTestCase { } } + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("s", 1), + client().prepareIndex("cache_test_idx", "type", "2").setSource("s", 2)); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(sum("foo").field("d").script(new Script("", ScriptType.INLINE, FieldValueScriptEngine.NAME, null))).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(sum("foo").field("d")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } + /** * Mock plugin for the {@link ExtractFieldScriptEngine} */ diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksIT.java index e50b89d8b96..3a9ab247f5a 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksIT.java @@ -20,10 +20,9 @@ package org.elasticsearch.search.aggregations.metrics; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.logging.Loggers; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.CollectionUtils; -import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.plugins.Plugin; -import org.elasticsearch.script.MockScriptPlugin; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptService.ScriptType; import org.elasticsearch.search.aggregations.AggregationTestScriptsPlugin; @@ -43,8 +42,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.function.Function; - import static java.util.Collections.emptyMap; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; import static org.elasticsearch.index.query.QueryBuilders.termQuery; @@ -53,7 +50,9 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.global; import static org.elasticsearch.search.aggregations.AggregationBuilders.histogram; import static org.elasticsearch.search.aggregations.AggregationBuilders.percentileRanks; import static org.elasticsearch.search.aggregations.AggregationBuilders.terms; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.lessThanOrEqualTo; @@ -484,4 +483,42 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase { } } + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("s", 1), + client().prepareIndex("cache_test_idx", "type", "2").setSource("s", 2)); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(percentileRanks("foo").field("d").values(50.0) + .script(new Script("_value - 1", ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, emptyMap()))).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(percentileRanks("foo").field("d").values(50.0)).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } + } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesIT.java index b0268351954..5f3fbb7e869 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesIT.java @@ -20,6 +20,7 @@ package org.elasticsearch.search.aggregations.metrics; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.logging.Loggers; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.CollectionUtils; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.script.Script; @@ -50,7 +51,9 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.global; import static org.elasticsearch.search.aggregations.AggregationBuilders.histogram; import static org.elasticsearch.search.aggregations.AggregationBuilders.percentiles; import static org.elasticsearch.search.aggregations.AggregationBuilders.terms; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.lessThanOrEqualTo; @@ -465,4 +468,43 @@ public class TDigestPercentilesIT extends AbstractNumericTestCase { } } + + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("s", 1), + client().prepareIndex("cache_test_idx", "type", "2").setSource("s", 2)); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(percentiles("foo").field("d") + .percentiles(50.0).script(new Script("_value - 1", ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, emptyMap()))) + .get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(percentiles("foo").field("d").percentiles(50.0)).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java index 45d44c863a4..383d25c4011 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java @@ -25,6 +25,7 @@ import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchPhaseExecutionException; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.SearchType; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.elasticsearch.index.query.QueryBuilders; @@ -33,6 +34,7 @@ import org.elasticsearch.script.MockScriptEngine; import org.elasticsearch.script.MockScriptPlugin; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptService; +import org.elasticsearch.script.ScriptService.ScriptType; import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.SearchHitField; import org.elasticsearch.search.SearchHits; @@ -993,4 +995,42 @@ public class TopHitsIT extends ESIntegTestCase { } } } + + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("s", 1), + client().prepareIndex("cache_test_idx", "type", "2").setSource("s", 2)); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(topHits("foo").scriptField("bar", new Script("5", ScriptType.INLINE, CustomScriptPlugin.NAME, null))).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(topHits("foo")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountIT.java index 7de37fdb6a8..90e56249ff3 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountIT.java @@ -28,9 +28,7 @@ import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ExecutableScript; import org.elasticsearch.script.LeafSearchScript; import org.elasticsearch.script.Script; -import org.elasticsearch.script.ScriptEngineRegistry; import org.elasticsearch.script.ScriptEngineService; -import org.elasticsearch.script.ScriptModule; import org.elasticsearch.script.ScriptService.ScriptType; import org.elasticsearch.script.SearchScript; import org.elasticsearch.search.aggregations.bucket.global.Global; @@ -43,14 +41,15 @@ import java.io.IOException; import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.List; import java.util.Map; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; import static org.elasticsearch.search.aggregations.AggregationBuilders.count; import static org.elasticsearch.search.aggregations.AggregationBuilders.global; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.notNullValue; @@ -209,6 +208,45 @@ public class ValueCountIT extends ESIntegTestCase { assertThat(valueCount.getValue(), equalTo(20L)); } + /** + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. + */ + public void testDontCacheScripts() throws Exception { + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") + .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) + .get()); + indexRandom(true, client().prepareIndex("cache_test_idx", "type", "1").setSource("s", 1), + client().prepareIndex("cache_test_idx", "type", "2").setSource("s", 2)); + + // Make sure we are starting with a clear cache + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a script does not get cached + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(count("foo").field("d").script(new Script("value", ScriptType.INLINE, FieldValueScriptEngine.NAME, null))) + .get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(count("foo").field("d")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + } + /** * Mock plugin for the {@link FieldValueScriptEngine} */ From 587bdcef38d804904f8eef18ab459fb927cedc6e Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 5 Oct 2016 09:41:48 +0200 Subject: [PATCH 03/20] add extra safety when accessing scripts or now and reqeusts are cached --- .../org/elasticsearch/index/IndexService.java | 9 +-- .../index/mapper/DateFieldMapper.java | 2 +- .../index/mapper/LegacyDateFieldMapper.java | 2 +- .../index/mapper/TTLFieldMapper.java | 2 +- .../index/query/InnerHitBuilder.java | 5 +- .../index/query/QueryRewriteContext.java | 38 ++++++++++--- .../index/query/QueryShardContext.java | 56 +++++++++++++------ .../index/query/ScriptQueryBuilder.java | 7 +-- .../ScriptScoreFunctionBuilder.java | 4 +- .../java/org/elasticsearch/script/Script.java | 1 - .../elasticsearch/script/ScriptService.java | 6 +- .../search/DefaultSearchContext.java | 21 ++----- .../elasticsearch/search/SearchService.java | 3 +- .../bucket/histogram/ExtendedBounds.java | 4 +- .../bucket/range/RangeAggregator.java | 4 +- .../SignificantTermsAggregationBuilder.java | 4 +- .../SignificantTermsAggregatorFactory.java | 5 +- .../heuristics/ScriptHeuristic.java | 49 ++++------------ .../heuristics/SignificanceHeuristic.java | 4 -- .../ScriptedMetricAggregationBuilder.java | 34 ++++++++++- .../scripted/ScriptedMetricAggregator.java | 18 ++---- .../ScriptedMetricAggregatorFactory.java | 46 ++++++++------- .../tophits/TopHitsAggregationBuilder.java | 15 ++++- .../tophits/TopHitsAggregatorFactory.java | 19 +++---- .../support/AggregationContext.java | 2 +- .../ValuesSourceAggregationBuilder.java | 3 +- .../internal/FilteredSearchContext.java | 10 ---- .../search/internal/SearchContext.java | 19 +------ .../search/sort/ScriptSortBuilder.java | 4 +- .../suggest/phrase/PhraseSuggester.java | 6 +- .../phrase/PhraseSuggestionBuilder.java | 7 ++- .../phrase/PhraseSuggestionContext.java | 8 ++- .../index/SearchSlowLogTests.java | 3 +- .../index/query/QueryShardContextTests.java | 4 +- .../bucket/histogram/ExtendedBoundsTests.java | 5 +- .../highlight/HighlightBuilderTests.java | 5 +- .../rescore/QueryRescoreBuilderTests.java | 2 +- .../search/sort/AbstractSortTestCase.java | 3 +- .../script/mustache/TemplateQueryBuilder.java | 5 +- .../test/AbstractQueryTestCase.java | 2 +- .../test/ESSingleNodeTestCase.java | 2 +- .../elasticsearch/test/TestSearchContext.java | 16 +----- .../search/MockSearchServiceTests.java | 2 +- 43 files changed, 229 insertions(+), 237 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/IndexService.java b/core/src/main/java/org/elasticsearch/index/IndexService.java index 58457417fa7..8fea01e6fab 100644 --- a/core/src/main/java/org/elasticsearch/index/IndexService.java +++ b/core/src/main/java/org/elasticsearch/index/IndexService.java @@ -89,6 +89,7 @@ import java.util.Set; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.LongSupplier; import static java.util.Collections.emptyMap; import static java.util.Collections.unmodifiableMap; @@ -448,13 +449,13 @@ public class IndexService extends AbstractIndexComponent implements IndicesClust * Creates a new QueryShardContext. The context has not types set yet, if types are required set them via * {@link QueryShardContext#setTypes(String...)} */ - public QueryShardContext newQueryShardContext(IndexReader indexReader) { + public QueryShardContext newQueryShardContext(IndexReader indexReader, LongSupplier nowInMillis) { return new QueryShardContext( indexSettings, indexCache.bitsetFilterCache(), indexFieldData, mapperService(), similarityService(), nodeServicesProvider.getScriptService(), nodeServicesProvider.getIndicesQueriesRegistry(), nodeServicesProvider.getClient(), indexReader, - nodeServicesProvider.getClusterService().state() - ); + nodeServicesProvider.getClusterService().state(), + nowInMillis); } /** @@ -463,7 +464,7 @@ public class IndexService extends AbstractIndexComponent implements IndicesClust * used for rewriting since it does not know about the current {@link IndexReader}. */ public QueryShardContext newQueryShardContext() { - return newQueryShardContext(null); + return newQueryShardContext(null, threadPool::estimatedTimeInMillis); } public ThreadPool getThreadPool() { diff --git a/core/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java index 717f0361552..62b86c98a8c 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java @@ -366,7 +366,7 @@ public class DateFieldMapper extends FieldMapper { return () -> { final SearchContext context = SearchContext.current(); return context != null - ? context.nowInMillis() + ? context.getQueryShardContext().nowInMillis() : System.currentTimeMillis(); }; } diff --git a/core/src/main/java/org/elasticsearch/index/mapper/LegacyDateFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/LegacyDateFieldMapper.java index 29689d06dff..5428dbe74f5 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/LegacyDateFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/LegacyDateFieldMapper.java @@ -480,7 +480,7 @@ public class LegacyDateFieldMapper extends LegacyNumberFieldMapper { public Long call() { final SearchContext context = SearchContext.current(); return context != null - ? context.nowInMillis() + ? context.getQueryShardContext().nowInMillis() : System.currentTimeMillis(); } }; diff --git a/core/src/main/java/org/elasticsearch/index/mapper/TTLFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/TTLFieldMapper.java index bb2da75dad0..ea11dc04ee4 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/TTLFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/TTLFieldMapper.java @@ -143,7 +143,7 @@ public class TTLFieldMapper extends MetadataFieldMapper { long now; SearchContext searchContext = SearchContext.current(); if (searchContext != null) { - now = searchContext.nowInMillis(); + now = searchContext.getQueryShardContext().nowInMillis(); } else { now = System.currentTimeMillis(); } diff --git a/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java b/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java index 0f4a1f3c429..f2c83ce2a73 100644 --- a/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java @@ -574,10 +574,9 @@ public final class InnerHitBuilder extends ToXContentToBytes implements Writeabl innerHitsContext.docValueFieldsContext(new DocValueFieldsContext(docValueFields)); } if (scriptFields != null) { - context.markAsNotCachable(); for (ScriptField field : scriptFields) { - SearchScript searchScript = innerHitsContext.getQueryShardContext().getScriptService().search(innerHitsContext.lookup(), field.script(), - ScriptContext.Standard.SEARCH, Collections.emptyMap()); + SearchScript searchScript = innerHitsContext.getQueryShardContext().getSearchScript(field.script(), + ScriptContext.Standard.SEARCH, Collections.emptyMap()); innerHitsContext.scriptFields().add(new org.elasticsearch.search.fetch.subphase.ScriptFieldsContext.ScriptField( field.fieldName(), searchScript, field.ignoreFailure())); } diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java index 64329c314dd..bc3c6ab52ab 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java @@ -19,17 +19,24 @@ package org.elasticsearch.index.query; import org.apache.lucene.index.IndexReader; +import org.apache.lucene.util.SetOnce; import org.elasticsearch.client.Client; import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParseFieldMatcherSupplier; import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.indices.query.IndicesQueriesRegistry; +import org.elasticsearch.script.ExecutableScript; +import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.ScriptSettings; +import java.util.Collections; + /** * Context object used to rewrite {@link QueryBuilder} instances into simplified version. */ @@ -42,6 +49,7 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { protected final IndexReader reader; protected final ClusterState clusterState; protected boolean cachable; + private final SetOnce executionMode = new SetOnce<>(); public QueryRewriteContext(IndexSettings indexSettings, MapperService mapperService, ScriptService scriptService, IndicesQueriesRegistry indicesQueriesRegistry, Client client, IndexReader reader, @@ -70,13 +78,6 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { return indexSettings; } - /** - * Returns a script service to fetch scripts. - */ - public final ScriptService getScriptService() { - return scriptService; - } - /** * Return the MapperService. */ @@ -118,11 +119,32 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { return new QueryParseContext(defaultScriptLanguage, indicesQueriesRegistry, parser, indexSettings.getParseFieldMatcher()); } - public void markAsNotCachable() { + protected final void markAsNotCachable() { this.cachable = false; } public boolean isCachable() { return cachable; } + + public void setCachabe(boolean cachabe) { this.cachable = cachabe; } + + public BytesReference getTemplateBytes(Script template) { + failIfExecutionMode(); + ExecutableScript executable = scriptService.executable(template, + ScriptContext.Standard.SEARCH, Collections.emptyMap()); + return (BytesReference) executable.run(); + } + + public void setExecutionMode() { + this.executionMode.set(Boolean.TRUE); + } + + protected void failIfExecutionMode() { + if (executionMode.get() == Boolean.TRUE) { + throw new IllegalArgumentException("features that prevent cachability are disabled on this context"); + } else { + assert executionMode.get() == null : executionMode.get(); + } + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index 25b7ef35961..671a89a3884 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -26,6 +26,9 @@ import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.Map; +import java.util.function.Function; +import java.util.function.LongSupplier; + import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.index.IndexReader; import org.apache.lucene.queryparser.classic.MapperQueryParser; @@ -54,7 +57,12 @@ import org.elasticsearch.index.mapper.TextFieldMapper; import org.elasticsearch.index.query.support.NestedScope; import org.elasticsearch.index.similarity.SimilarityService; import org.elasticsearch.indices.query.IndicesQueriesRegistry; +import org.elasticsearch.script.CompiledScript; +import org.elasticsearch.script.ExecutableScript; +import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptService; +import org.elasticsearch.script.SearchScript; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.search.lookup.SearchLookup; @@ -85,11 +93,12 @@ public class QueryShardContext extends QueryRewriteContext { private boolean mapUnmappedFieldAsString; private NestedScope nestedScope; private boolean isFilter; + private final LongSupplier nowInMillis; public QueryShardContext(IndexSettings indexSettings, BitsetFilterCache bitsetFilterCache, IndexFieldDataService indexFieldDataService, MapperService mapperService, SimilarityService similarityService, ScriptService scriptService, final IndicesQueriesRegistry indicesQueriesRegistry, Client client, - IndexReader reader, ClusterState clusterState) { + IndexReader reader, ClusterState clusterState, LongSupplier nowInMillis) { super(indexSettings, mapperService, scriptService, indicesQueriesRegistry, client, reader, clusterState); this.indexSettings = indexSettings; this.similarityService = similarityService; @@ -99,12 +108,13 @@ public class QueryShardContext extends QueryRewriteContext { this.allowUnmappedFields = indexSettings.isDefaultAllowUnmappedFields(); this.indicesQueriesRegistry = indicesQueriesRegistry; this.nestedScope = new NestedScope(); + this.nowInMillis = nowInMillis; } public QueryShardContext(QueryShardContext source) { this(source.indexSettings, source.bitsetFilterCache, source.indexFieldDataService, source.mapperService, source.similarityService, source.scriptService, source.indicesQueriesRegistry, source.client, - source.reader, source.clusterState); + source.reader, source.clusterState, source.nowInMillis); this.types = source.getTypes(); } @@ -261,11 +271,8 @@ public class QueryShardContext extends QueryRewriteContext { } public long nowInMillis() { - SearchContext current = SearchContext.current(); - if (current != null) { - return current.nowInMillis(); - } - return System.currentTimeMillis(); + failIfExecutionMode(); + return nowInMillis.getAsLong(); } public NestedScope nestedScope() { @@ -324,16 +331,33 @@ public class QueryShardContext extends QueryRewriteContext { } } - @Override - public void markAsNotCachable() { - super.markAsNotCachable(); - SearchContext current = SearchContext.current(); - if (current != null) { - current.markAsNotCachable(); - } - } - public final Index index() { return indexSettings.getIndex(); } + + public SearchScript getSearchScript(Script script, ScriptContext context, Map params) { + failIfExecutionMode(); + markAsNotCachable(); + return scriptService.search(lookup(), script, context, params); + } + + public Function, SearchScript> getLazySearchScript(Script script, ScriptContext context, Map params) { + failIfExecutionMode(); + markAsNotCachable(); + CompiledScript compile = scriptService.compile(script, context, params); + return (p) -> scriptService.search(lookup(), compile, p); + } + + public ExecutableScript getExecutableScript(Script script, ScriptContext context, Map params) { + failIfExecutionMode(); + markAsNotCachable(); + return scriptService.executable(script, context, params); + } + + public Function, ExecutableScript> getLazyExecutableScript(Script script, ScriptContext context, Map params) { + failIfExecutionMode(); + markAsNotCachable(); + CompiledScript executable = scriptService.compile(script, context, params); + return (p) -> scriptService.executable(executable, p); + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java index 5a65b4438f0..4501d12b91f 100644 --- a/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java @@ -133,8 +133,7 @@ public class ScriptQueryBuilder extends AbstractQueryBuilder @Override protected Query doToQuery(QueryShardContext context) throws IOException { - context.markAsNotCachable(); - return new ScriptQuery(script, context.getScriptService(), context.lookup()); + return new ScriptQuery(script, context.getSearchScript(script, ScriptContext.Standard.SEARCH, Collections.emptyMap())); } static class ScriptQuery extends Query { @@ -143,9 +142,9 @@ public class ScriptQueryBuilder extends AbstractQueryBuilder private final SearchScript searchScript; - public ScriptQuery(Script script, ScriptService scriptService, SearchLookup searchLookup) { + public ScriptQuery(Script script, SearchScript searchScript) { this.script = script; - this.searchScript = scriptService.search(searchLookup, script, ScriptContext.Standard.SEARCH, Collections.emptyMap()); + this.searchScript = searchScript; } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/functionscore/ScriptScoreFunctionBuilder.java b/core/src/main/java/org/elasticsearch/index/query/functionscore/ScriptScoreFunctionBuilder.java index d7bff2bdd44..77923df09b4 100644 --- a/core/src/main/java/org/elasticsearch/index/query/functionscore/ScriptScoreFunctionBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/functionscore/ScriptScoreFunctionBuilder.java @@ -95,10 +95,8 @@ public class ScriptScoreFunctionBuilder extends ScoreFunctionBuilder params) { CompiledScript compiledScript = compile(script, scriptContext, params); - return getScriptEngineServiceForLang(compiledScript.lang()).search(compiledScript, lookup, script.getParams()); + return search(lookup, compiledScript, script.getParams()); + } + + public SearchScript search(SearchLookup lookup, CompiledScript compiledScript, Map params) { + return getScriptEngineServiceForLang(compiledScript.lang()).search(compiledScript, lookup, params); } private boolean isAnyScriptContextEnabled(String lang, ScriptType scriptType) { diff --git a/core/src/main/java/org/elasticsearch/search/DefaultSearchContext.java b/core/src/main/java/org/elasticsearch/search/DefaultSearchContext.java index 9aab786aa34..26af6d85d06 100644 --- a/core/src/main/java/org/elasticsearch/search/DefaultSearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/DefaultSearchContext.java @@ -51,7 +51,6 @@ import org.elasticsearch.index.query.ParsedQuery; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.similarity.SimilarityService; -import org.elasticsearch.script.ScriptService; import org.elasticsearch.search.aggregations.SearchContextAggregations; import org.elasticsearch.search.dfs.DfsSearchResult; import org.elasticsearch.search.fetch.FetchPhase; @@ -89,7 +88,6 @@ final class DefaultSearchContext extends SearchContext { private final Counter timeEstimateCounter; private SearchType searchType; private final Engine.Searcher engineSearcher; - private final ScriptService scriptService; private final BigArrays bigArrays; private final IndexShard indexShard; private final IndexService indexService; @@ -150,9 +148,9 @@ final class DefaultSearchContext extends SearchContext { private FetchPhase fetchPhase; DefaultSearchContext(long id, ShardSearchRequest request, SearchShardTarget shardTarget, Engine.Searcher engineSearcher, - IndexService indexService, IndexShard indexShard, ScriptService scriptService, - BigArrays bigArrays, Counter timeEstimateCounter, ParseFieldMatcher parseFieldMatcher, TimeValue timeout, - FetchPhase fetchPhase) { + IndexService indexService, IndexShard indexShard, + BigArrays bigArrays, Counter timeEstimateCounter, ParseFieldMatcher parseFieldMatcher, TimeValue timeout, + FetchPhase fetchPhase) { super(parseFieldMatcher); this.id = id; this.request = request; @@ -160,7 +158,6 @@ final class DefaultSearchContext extends SearchContext { this.searchType = request.searchType(); this.shardTarget = shardTarget; this.engineSearcher = engineSearcher; - this.scriptService = scriptService; // SearchContexts use a BigArrays that can circuit break this.bigArrays = bigArrays.withCircuitBreaking(); this.dfsResult = new DfsSearchResult(id, shardTarget); @@ -171,7 +168,7 @@ final class DefaultSearchContext extends SearchContext { this.searcher = new ContextIndexSearcher(engineSearcher, indexService.cache().query(), indexShard.getQueryCachingPolicy()); this.timeEstimateCounter = timeEstimateCounter; this.timeout = timeout; - queryShardContext = indexService.newQueryShardContext(searcher.getIndexReader()); + queryShardContext = indexService.newQueryShardContext(searcher.getIndexReader(), request::nowInMillis); queryShardContext.setTypes(request.types()); } @@ -358,11 +355,6 @@ final class DefaultSearchContext extends SearchContext { return originNanoTime; } - @Override - protected long nowInMillisImpl() { - return request.nowInMillis(); - } - @Override public ScrollContext scrollContext() { return this.scrollContext; @@ -501,11 +493,6 @@ final class DefaultSearchContext extends SearchContext { return indexService.similarityService(); } - @Override - public ScriptService scriptService() { - return scriptService; - } - @Override public BigArrays bigArrays() { return bigArrays; diff --git a/core/src/main/java/org/elasticsearch/search/SearchService.java b/core/src/main/java/org/elasticsearch/search/SearchService.java index da3cd4729db..3a9d0d58dc5 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchService.java +++ b/core/src/main/java/org/elasticsearch/search/SearchService.java @@ -231,6 +231,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv */ private void loadOrExecuteQueryPhase(final ShardSearchRequest request, final SearchContext context) throws Exception { final boolean canCache = indicesService.canCache(request, context); + context.getQueryShardContext().setExecutionMode(); if (canCache) { indicesService.loadIntoContext(request, context, queryPhase); } else { @@ -568,7 +569,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv return new DefaultSearchContext(idGenerator.incrementAndGet(), request, shardTarget, engineSearcher, indexService, - indexShard, scriptService, bigArrays, threadPool.estimatedTimeInMillisCounter(), parseFieldMatcher, + indexShard, bigArrays, threadPool.estimatedTimeInMillisCounter(), parseFieldMatcher, timeout, fetchPhase); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/ExtendedBounds.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/ExtendedBounds.java index 46fae19e49f..85160347612 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/ExtendedBounds.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/ExtendedBounds.java @@ -153,11 +153,11 @@ public class ExtendedBounds implements ToXContent, Writeable { Long max = this.max; assert format != null; if (minAsStr != null) { - min = format.parseLong(minAsStr, false, context::nowInMillis); + min = format.parseLong(minAsStr, false, context.getQueryShardContext()::nowInMillis); } if (maxAsStr != null) { // TODO: Should we rather pass roundUp=true? - max = format.parseLong(maxAsStr, false, context::nowInMillis); + max = format.parseLong(maxAsStr, false, context.getQueryShardContext()::nowInMillis); } if (min != null && max != null && min.compareTo(max) > 0) { throw new SearchParseException(context, "[extended_bounds.min][" + min + "] cannot be greater than " + diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregator.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregator.java index c83e2d2c721..2eeb8db02b9 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregator.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregator.java @@ -119,10 +119,10 @@ public class RangeAggregator extends BucketsAggregator { Double from = this.from; Double to = this.to; if (fromAsStr != null) { - from = parser.parseDouble(fromAsStr, false, context::nowInMillis); + from = parser.parseDouble(fromAsStr, false, context.getQueryShardContext()::nowInMillis); } if (toAsStr != null) { - to = parser.parseDouble(toAsStr, false, context::nowInMillis); + to = parser.parseDouble(toAsStr, false, context.getQueryShardContext()::nowInMillis); } return new Range(key, from, fromAsStr, to, toAsStr); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregationBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregationBuilder.java index 4c1629ff39d..46f5d881fa1 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregationBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregationBuilder.java @@ -220,9 +220,7 @@ public class SignificantTermsAggregationBuilder extends ValuesSourceAggregationB @Override protected ValuesSourceAggregatorFactory innerBuild(AggregationContext context, ValuesSourceConfig config, AggregatorFactory parent, Builder subFactoriesBuilder) throws IOException { - if (significanceHeuristic.canCache() == false) { - context.searchContext().markAsNotCachable(); - } + this.significanceHeuristic.initialize(context.searchContext()); return new SignificantTermsAggregatorFactory(name, type, config, includeExclude, executionHint, filterBuilder, bucketCountThresholds, significanceHeuristic, context, parent, subFactoriesBuilder, metaData); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorFactory.java index ab30e1b2d4a..1a5338f7458 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorFactory.java @@ -91,7 +91,6 @@ public class SignificantTermsAggregatorFactory extends ValuesSourceAggregatorFac : searcher.count(filter); this.bucketCountThresholds = bucketCountThresholds; this.significanceHeuristic = significanceHeuristic; - this.significanceHeuristic.initialize(context.searchContext()); setFieldInfo(); } @@ -211,13 +210,13 @@ public class SignificantTermsAggregatorFactory extends ValuesSourceAggregatorFac } } assert execution != null; - + DocValueFormat format = config.format(); if ((includeExclude != null) && (includeExclude.isRegexBased()) && format != DocValueFormat.RAW) { throw new AggregationExecutionException("Aggregation [" + name + "] cannot support regular expression style include/exclude " + "settings as they can only be applied to string fields. Use an array of values for include/exclude clauses"); } - + return execution.create(name, factories, valuesSource, format, bucketCountThresholds, includeExclude, context, parent, significanceHeuristic, this, pipelineAggregators, metaData); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java index 1c3f5b7ed56..ee829943ae1 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java @@ -32,7 +32,6 @@ import org.elasticsearch.script.ExecutableScript; import org.elasticsearch.script.Script; import org.elasticsearch.script.Script.ScriptField; import org.elasticsearch.script.ScriptContext; -import org.elasticsearch.script.ScriptService; import org.elasticsearch.search.aggregations.InternalAggregation; import org.elasticsearch.search.aggregations.support.XContentParseContext; import org.elasticsearch.search.internal.SearchContext; @@ -49,7 +48,7 @@ public class ScriptHeuristic extends SignificanceHeuristic { private final LongAccessor subsetDfHolder; private final LongAccessor supersetDfHolder; private final Script script; - ExecutableScript searchScript = null; + ExecutableScript executableScript = null; public ScriptHeuristic(Script script) { subsetSizeHolder = new LongAccessor(); @@ -73,21 +72,20 @@ public class ScriptHeuristic extends SignificanceHeuristic { @Override public void initialize(InternalAggregation.ReduceContext context) { - initialize(context.scriptService()); + initialize(context.scriptService().executable(script, ScriptContext.Standard.AGGS, Collections.emptyMap())); } @Override public void initialize(SearchContext context) { - context.markAsNotCachable(); - initialize(context.scriptService()); + initialize(context.getQueryShardContext().getExecutableScript(script, ScriptContext.Standard.AGGS, Collections.emptyMap())); } - public void initialize(ScriptService scriptService) { - searchScript = scriptService.executable(script, ScriptContext.Standard.AGGS, Collections.emptyMap()); - searchScript.setNextVar("_subset_freq", subsetDfHolder); - searchScript.setNextVar("_subset_size", subsetSizeHolder); - searchScript.setNextVar("_superset_freq", supersetDfHolder); - searchScript.setNextVar("_superset_size", supersetSizeHolder); + public void initialize(ExecutableScript executableScript) { + this.executableScript = executableScript; + this.executableScript.setNextVar("_subset_freq", subsetDfHolder); + this.executableScript.setNextVar("_subset_size", subsetSizeHolder); + this.executableScript.setNextVar("_superset_freq", supersetDfHolder); + this.executableScript.setNextVar("_superset_size", supersetSizeHolder); } /** @@ -101,7 +99,7 @@ public class ScriptHeuristic extends SignificanceHeuristic { */ @Override public double getScore(long subsetFreq, long subsetSize, long supersetFreq, long supersetSize) { - if (searchScript == null) { + if (executableScript == null) { //In tests, wehn calling assertSearchResponse(..) the response is streamed one additional time with an arbitrary version, see assertVersionSerializable(..). // Now, for version before 1.5.0 the score is computed after streaming the response but for scripts the script does not exists yet. // assertSearchResponse() might therefore fail although there is no problem. @@ -113,7 +111,7 @@ public class ScriptHeuristic extends SignificanceHeuristic { supersetSizeHolder.value = supersetSize; subsetDfHolder.value = subsetFreq; supersetDfHolder.value = supersetFreq; - return ((Number) searchScript.run()).doubleValue(); + return ((Number) executableScript.run()).doubleValue(); } @Override @@ -172,26 +170,6 @@ public class ScriptHeuristic extends SignificanceHeuristic { return new ScriptHeuristic(script); } - public static class ScriptHeuristicBuilder implements SignificanceHeuristicBuilder { - - private Script script = null; - - public ScriptHeuristicBuilder setScript(Script script) { - this.script = script; - return this; - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params builderParams) throws IOException { - builder.startObject(NAME); - builder.field(ScriptField.SCRIPT.getPreferredName()); - script.toXContent(builder, builderParams); - builder.endObject(); - return builder; - } - - } - public final class LongAccessor extends Number { public long value; @Override @@ -218,10 +196,5 @@ public class ScriptHeuristic extends SignificanceHeuristic { return Long.toString(value); } } - - @Override - public boolean canCache() { - return false; - } } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristic.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristic.java index 094f529e92c..db9711c1a8d 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristic.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristic.java @@ -57,8 +57,4 @@ public abstract class SignificanceHeuristic implements NamedWriteable, ToXConten public void initialize(SearchContext context) { } - - public boolean canCache() { - return true; - } } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java index b13b6fe3019..208fae15701 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java @@ -26,18 +26,30 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.query.QueryParseContext; +import org.elasticsearch.index.query.QueryShardContext; +import org.elasticsearch.script.CompiledScript; +import org.elasticsearch.script.ExecutableScript; import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptContext; +import org.elasticsearch.script.SearchScript; +import org.elasticsearch.search.SearchParseException; import org.elasticsearch.search.aggregations.AbstractAggregationBuilder; import org.elasticsearch.search.aggregations.AggregatorFactories.Builder; import org.elasticsearch.search.aggregations.AggregatorFactory; import org.elasticsearch.search.aggregations.InternalAggregation.Type; import org.elasticsearch.search.aggregations.support.AggregationContext; +import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.function.Function; public class ScriptedMetricAggregationBuilder extends AbstractAggregationBuilder { @@ -182,11 +194,29 @@ public class ScriptedMetricAggregationBuilder extends AbstractAggregationBuilder @Override protected ScriptedMetricAggregatorFactory doBuild(AggregationContext context, AggregatorFactory parent, Builder subfactoriesBuilder) throws IOException { - context.searchContext().markAsNotCachable(); - return new ScriptedMetricAggregatorFactory(name, type, initScript, mapScript, combineScript, reduceScript, params, context, + + QueryShardContext queryShardContext = context.searchContext().getQueryShardContext(); + Function, ExecutableScript> executableInitScript; + if (initScript != null) { + executableInitScript = queryShardContext.getLazyExecutableScript(initScript, ScriptContext.Standard.AGGS, + Collections.emptyMap()); + } else { + executableInitScript = (p) -> null;; + } + Function, SearchScript> searchMapScript = queryShardContext.getLazySearchScript(mapScript, + ScriptContext.Standard.AGGS, Collections.emptyMap()); + Function, ExecutableScript> executableCombineScript; + if (combineScript != null) { + executableCombineScript = queryShardContext.getLazyExecutableScript(combineScript, ScriptContext.Standard.AGGS, + Collections.emptyMap()); + } else { + executableCombineScript = (p) -> null; + } + return new ScriptedMetricAggregatorFactory(name, type, searchMapScript, executableInitScript, executableCombineScript, reduceScript, params, context, parent, subfactoriesBuilder, metaData); } + @Override protected XContentBuilder internalXContent(XContentBuilder builder, Params builderParams) throws IOException { builder.startObject(); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java index 89ec5183c32..57fd49779fc 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java @@ -20,6 +20,7 @@ package org.elasticsearch.search.aggregations.metrics.scripted; import org.apache.lucene.index.LeafReaderContext; +import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.script.ExecutableScript; import org.elasticsearch.script.LeafSearchScript; import org.elasticsearch.script.Script; @@ -46,23 +47,14 @@ public class ScriptedMetricAggregator extends MetricsAggregator { private final Script reduceScript; private Map params; - protected ScriptedMetricAggregator(String name, Script initScript, Script mapScript, Script combineScript, Script reduceScript, + protected ScriptedMetricAggregator(String name, SearchScript mapScript, ExecutableScript combineScript, + Script reduceScript, Map params, AggregationContext context, Aggregator parent, List pipelineAggregators, Map metaData) throws IOException { super(name, context, parent, pipelineAggregators, metaData); this.params = params; - ScriptService scriptService = context.searchContext().scriptService(); - if (initScript != null) { - context.searchContext().markAsNotCachable(); - scriptService.executable(initScript, ScriptContext.Standard.AGGS, Collections.emptyMap()).run(); - } - this.mapScript = scriptService.search(context.searchContext().lookup(), mapScript, ScriptContext.Standard.AGGS, Collections.emptyMap()); - if (combineScript != null) { - context.searchContext().markAsNotCachable(); - this.combineScript = scriptService.executable(combineScript, ScriptContext.Standard.AGGS, Collections.emptyMap()); - } else { - this.combineScript = null; - } + this.mapScript = mapScript; + this.combineScript = combineScript; this.reduceScript = reduceScript; } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorFactory.java index f89e99f44b3..1dc02ea42dc 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorFactory.java @@ -19,7 +19,11 @@ package org.elasticsearch.search.aggregations.metrics.scripted; +import org.elasticsearch.index.query.QueryShardContext; +import org.elasticsearch.script.CompiledScript; +import org.elasticsearch.script.ExecutableScript; import org.elasticsearch.script.Script; +import org.elasticsearch.script.SearchScript; import org.elasticsearch.search.SearchParseException; import org.elasticsearch.search.aggregations.Aggregator; import org.elasticsearch.search.aggregations.AggregatorFactories; @@ -35,21 +39,22 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.function.Function; public class ScriptedMetricAggregatorFactory extends AggregatorFactory { - private final Script initScript; - private final Script mapScript; - private final Script combineScript; + private final Function, SearchScript> mapScript; + private final Function, ExecutableScript> combineScript; private final Script reduceScript; private final Map params; + private final Function, ExecutableScript> initScript; - public ScriptedMetricAggregatorFactory(String name, Type type, Script initScript, Script mapScript, Script combineScript, - Script reduceScript, Map params, AggregationContext context, AggregatorFactory parent, - AggregatorFactories.Builder subFactories, Map metaData) throws IOException { + public ScriptedMetricAggregatorFactory(String name, Type type, Function, SearchScript> mapScript, Function, ExecutableScript> initScript, Function, ExecutableScript> combineScript, + Script reduceScript, Map params, AggregationContext context, AggregatorFactory parent, + AggregatorFactories.Builder subFactories, Map metaData) throws IOException { super(name, type, context, parent, subFactories, metaData); - this.initScript = initScript; this.mapScript = mapScript; + this.initScript = initScript; this.combineScript = combineScript; this.reduceScript = reduceScript; this.params = params; @@ -68,16 +73,18 @@ public class ScriptedMetricAggregatorFactory extends AggregatorFactory(); params.put("_agg", new HashMap()); } - return new ScriptedMetricAggregator(name, insertParams(initScript, params), insertParams(mapScript, params), - insertParams(combineScript, params), deepCopyScript(reduceScript, context.searchContext()), params, context, parent, - pipelineAggregators, metaData); - } - private static Script insertParams(Script script, Map params) { - if (script == null) { - return null; + final ExecutableScript initScript = this.initScript.apply(params); + final SearchScript mapScript = this.mapScript.apply(params); + final ExecutableScript combineScript = this.combineScript.apply(params); + + final Script reduceScript = deepCopyScript(this.reduceScript, context.searchContext()); + if (initScript != null) { + initScript.run(); } - return new Script(script.getScript(), script.getType(), script.getLang(), params); + return new ScriptedMetricAggregator(name, mapScript, + combineScript, reduceScript, params, context, parent, + pipelineAggregators, metaData); } private static Script deepCopyScript(Script script, SearchContext context) { @@ -98,7 +105,7 @@ public class ScriptedMetricAggregatorFactory extends AggregatorFactory originalMap = (Map) original; Map clonedMap = new HashMap<>(); - for (Entry e : originalMap.entrySet()) { + for (Map.Entry e : originalMap.entrySet()) { clonedMap.put(deepCopyParams(e.getKey(), context), deepCopyParams(e.getValue(), context)); } clone = (T) clonedMap; @@ -110,14 +117,15 @@ public class ScriptedMetricAggregatorFactory extends AggregatorFactory parent, Builder subfactoriesBuilder) throws IOException { - if (scriptFields != null && scriptFields.isEmpty() == false) { - context.searchContext().markAsNotCachable(); + List fields = new ArrayList<>(); + if (scriptFields != null) { + for (ScriptField field : scriptFields) { + SearchScript searchScript = context.searchContext().getQueryShardContext().getSearchScript(field.script(), + ScriptContext.Standard.SEARCH, Collections.emptyMap()); + fields.add(new org.elasticsearch.search.fetch.subphase.ScriptFieldsContext.ScriptField( + field.fieldName(), searchScript, field.ignoreFailure())); + } } return new TopHitsAggregatorFactory(name, type, from, size, explain, version, trackScores, sorts, highlightBuilder, - storedFieldsContext, fieldDataFields, scriptFields, fetchSourceContext, context, + storedFieldsContext, fieldDataFields, fields, fetchSourceContext, context, parent, subfactoriesBuilder, metaData); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java index 5dff86c5eed..42477c3212f 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java @@ -31,6 +31,7 @@ import org.elasticsearch.search.builder.SearchSourceBuilder.ScriptField; import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; +import org.elasticsearch.search.fetch.subphase.ScriptFieldsContext; import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilder; import org.elasticsearch.search.internal.SubSearchContext; import org.elasticsearch.search.sort.SortAndFormats; @@ -54,14 +55,14 @@ public class TopHitsAggregatorFactory extends AggregatorFactory docValueFields; - private final Set scriptFields; + private final List scriptFields; private final FetchSourceContext fetchSourceContext; public TopHitsAggregatorFactory(String name, Type type, int from, int size, boolean explain, boolean version, boolean trackScores, - List> sorts, HighlightBuilder highlightBuilder, StoredFieldsContext storedFieldsContext, - List docValueFields, Set scriptFields, FetchSourceContext fetchSourceContext, - AggregationContext context, AggregatorFactory parent, AggregatorFactories.Builder subFactories, - Map metaData) throws IOException { + List> sorts, HighlightBuilder highlightBuilder, StoredFieldsContext storedFieldsContext, + List docValueFields, List scriptFields, FetchSourceContext fetchSourceContext, + AggregationContext context, AggregatorFactory parent, AggregatorFactories.Builder subFactories, + Map metaData) throws IOException { super(name, type, context, parent, subFactories, metaData); this.from = from; this.size = size; @@ -99,12 +100,8 @@ public class TopHitsAggregatorFactory extends AggregatorFactory, Collector> queryCollectors(); - public final void markAsNotCachable() { - this.canCache = false; - } - /** * The life time of an object that is used during search execution. */ diff --git a/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java index d41e8c66d6f..2a1ca78f76c 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java @@ -284,9 +284,7 @@ public class ScriptSortBuilder extends SortBuilder { @Override public SortFieldAndFormat build(QueryShardContext context) throws IOException { - context.markAsNotCachable(); - final SearchScript searchScript = context.getScriptService().search( - context.lookup(), script, ScriptContext.Standard.SEARCH, Collections.emptyMap()); + final SearchScript searchScript = context.getSearchScript(script, ScriptContext.Standard.SEARCH, Collections.emptyMap()); MultiValueMode valueMode = null; if (sortMode != null) { diff --git a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java index a9f5accb918..41168852d2c 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java @@ -55,6 +55,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.function.Function; public final class PhraseSuggester extends Suggester { private final BytesRef SEPARATOR = new BytesRef(" "); @@ -109,7 +110,7 @@ public final class PhraseSuggester extends Suggester { response.addTerm(resultEntry); final BytesRefBuilder byteSpare = new BytesRefBuilder(); - final CompiledScript collateScript = suggestion.getCollateQueryScript(); + final Function, ExecutableScript> collateScript = suggestion.getCollateQueryScript(); final boolean collatePrune = (collateScript != null) && suggestion.collatePrune(); for (int i = 0; i < checkerResult.corrections.length; i++) { Correction correction = checkerResult.corrections[i]; @@ -121,8 +122,7 @@ public final class PhraseSuggester extends Suggester { final Map vars = suggestion.getCollateScriptParams(); vars.put(SUGGESTION_TEMPLATE_VAR_NAME, spare.toString()); QueryShardContext shardContext = suggestion.getShardContext(); - ScriptService scriptService = shardContext.getScriptService(); - final ExecutableScript executable = scriptService.executable(collateScript, vars); + final ExecutableScript executable = collateScript.apply(vars); final BytesReference querySource = (BytesReference) executable.run(); try (XContentParser parser = XContentFactory.xContent(querySource).createParser(querySource)) { Optional innerQueryBuilder = shardContext.newParseContext(parser).parseInnerQueryBuilder(); diff --git a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionBuilder.java b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionBuilder.java index a17a4f5de80..7ec7fdda069 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionBuilder.java @@ -40,6 +40,7 @@ import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.script.CompiledScript; +import org.elasticsearch.script.ExecutableScript; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptService; @@ -56,6 +57,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Objects; import java.util.Set; +import java.util.function.Function; /** * Defines the actual suggest command for phrase suggestions ( phrase). @@ -633,9 +635,8 @@ public class PhraseSuggestionBuilder extends SuggestionBuilder, ExecutableScript> compiledScript = context.getLazyExecutableScript(this.collateQuery, + ScriptContext.Standard.SEARCH, Collections.emptyMap()); suggestionContext.setCollateQueryScript(compiledScript); if (this.collateParams != null) { suggestionContext.setCollateScriptParams(this.collateParams); diff --git a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionContext.java b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionContext.java index e2d7ff1c41d..960dca419f7 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionContext.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionContext.java @@ -24,6 +24,7 @@ import org.apache.lucene.index.Terms; import org.apache.lucene.util.BytesRef; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.script.CompiledScript; +import org.elasticsearch.script.ExecutableScript; import org.elasticsearch.search.suggest.DirectSpellcheckerSettings; import org.elasticsearch.search.suggest.SuggestionSearchContext.SuggestionContext; @@ -31,6 +32,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.Function; class PhraseSuggestionContext extends SuggestionContext { static final boolean DEFAULT_COLLATE_PRUNE = false; @@ -52,7 +54,7 @@ class PhraseSuggestionContext extends SuggestionContext { private boolean requireUnigram = DEFAULT_REQUIRE_UNIGRAM; private BytesRef preTag; private BytesRef postTag; - private CompiledScript collateQueryScript; + private Function, ExecutableScript> collateQueryScript; private boolean prune = DEFAULT_COLLATE_PRUNE; private List generators = new ArrayList<>(); private Map collateScriptParams = new HashMap<>(1); @@ -192,11 +194,11 @@ class PhraseSuggestionContext extends SuggestionContext { return postTag; } - CompiledScript getCollateQueryScript() { + Function, ExecutableScript> getCollateQueryScript() { return collateQueryScript; } - void setCollateQueryScript(CompiledScript collateQueryScript) { + void setCollateQueryScript( Function, ExecutableScript> collateQueryScript) { this.collateQueryScript = collateQueryScript; } diff --git a/core/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java b/core/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java index 062f8c01f42..2a961d58928 100644 --- a/core/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java +++ b/core/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java @@ -46,8 +46,7 @@ public class SearchSlowLogTests extends ESSingleNodeTestCase { protected SearchContext createSearchContext(IndexService indexService) { BigArrays bigArrays = indexService.getBigArrays(); ThreadPool threadPool = indexService.getThreadPool(); - ScriptService scriptService = node().injector().getInstance(ScriptService.class); - return new TestSearchContext(threadPool, bigArrays, scriptService, indexService) { + return new TestSearchContext(threadPool, bigArrays, indexService) { @Override public ShardSearchRequest request() { return new ShardSearchRequest() { diff --git a/core/src/test/java/org/elasticsearch/index/query/QueryShardContextTests.java b/core/src/test/java/org/elasticsearch/index/query/QueryShardContextTests.java index 3b3bdf31b75..35b9cc6fc6c 100644 --- a/core/src/test/java/org/elasticsearch/index/query/QueryShardContextTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/QueryShardContextTests.java @@ -47,8 +47,8 @@ public class QueryShardContextTests extends ESTestCase { MapperService mapperService = mock(MapperService.class); when(mapperService.getIndexSettings()).thenReturn(indexSettings); QueryShardContext context = new QueryShardContext( - indexSettings, null, null, mapperService, null, null, null, null, null, null - ); + indexSettings, null, null, mapperService, null, null, null, null, null, null, + System::currentTimeMillis); context.setAllowUnmappedFields(false); MappedFieldType fieldType = new TextFieldMapper.TextFieldType(); diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/histogram/ExtendedBoundsTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/histogram/ExtendedBoundsTests.java index 6b7e51cc90f..f65d7eb1f7b 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/histogram/ExtendedBoundsTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/histogram/ExtendedBoundsTests.java @@ -29,6 +29,7 @@ import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.search.DocValueFormat; import org.elasticsearch.search.SearchParseException; import org.elasticsearch.search.aggregations.bucket.histogram.ExtendedBounds; @@ -93,7 +94,9 @@ public class ExtendedBoundsTests extends ESTestCase { public void testParseAndValidate() { long now = randomLong(); SearchContext context = mock(SearchContext.class); - when(context.nowInMillis()).thenReturn(now); + QueryShardContext qsc = mock(QueryShardContext.class); + when(context.getQueryShardContext()).thenReturn(qsc); + when(qsc.nowInMillis()).thenReturn(now); FormatDateTimeFormatter formatter = Joda.forPattern("dateOptionalTime"); DocValueFormat format = new DocValueFormat.DateTime(formatter, DateTimeZone.UTC); diff --git a/core/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java b/core/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java index ed272040f51..78e05d4207b 100644 --- a/core/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java @@ -49,9 +49,6 @@ import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.query.TermQueryBuilder; import org.elasticsearch.indices.query.IndicesQueriesRegistry; import org.elasticsearch.search.SearchModule; -import org.elasticsearch.search.fetch.subphase.highlight.AbstractHighlighterBuilder; -import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilder; -import org.elasticsearch.search.fetch.subphase.highlight.SearchContextHighlight; import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilder.Field; import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilder.Order; import org.elasticsearch.search.fetch.subphase.highlight.SearchContextHighlight.FieldOptions; @@ -294,7 +291,7 @@ public class HighlightBuilderTests extends ESTestCase { IndexSettings idxSettings = IndexSettingsModule.newIndexSettings(index, indexSettings); // shard context will only need indicesQueriesRegistry for building Query objects nested in highlighter QueryShardContext mockShardContext = new QueryShardContext(idxSettings, null, null, null, null, null, indicesQueriesRegistry, - null, null, null) { + null, null, null, System::currentTimeMillis) { @Override public MappedFieldType fieldMapper(String name) { TextFieldMapper.Builder builder = new TextFieldMapper.Builder(name); diff --git a/core/src/test/java/org/elasticsearch/search/rescore/QueryRescoreBuilderTests.java b/core/src/test/java/org/elasticsearch/search/rescore/QueryRescoreBuilderTests.java index 1af4a2b1788..86caeefaae1 100644 --- a/core/src/test/java/org/elasticsearch/search/rescore/QueryRescoreBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/rescore/QueryRescoreBuilderTests.java @@ -161,7 +161,7 @@ public class QueryRescoreBuilderTests extends ESTestCase { IndexSettings idxSettings = IndexSettingsModule.newIndexSettings(randomAsciiOfLengthBetween(1, 10), indexSettings); // shard context will only need indicesQueriesRegistry for building Query objects nested in query rescorer QueryShardContext mockShardContext = new QueryShardContext(idxSettings, null, null, null, null, null, indicesQueriesRegistry, - null, null, null) { + null, null, null, System::currentTimeMillis) { @Override public MappedFieldType fieldMapper(String name) { TextFieldMapper.Builder builder = new TextFieldMapper.Builder(name); diff --git a/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java b/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java index 84f83c46e1a..6f45a320fbc 100644 --- a/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java +++ b/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java @@ -24,7 +24,6 @@ import org.apache.lucene.util.Accountable; import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.ParseFieldMatcher; -import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; @@ -236,7 +235,7 @@ public abstract class AbstractSortTestCase> extends EST } }); return new QueryShardContext(idxSettings, bitsetFilterCache, ifds, null, null, scriptService, - indicesQueriesRegistry, null, null, null) { + indicesQueriesRegistry, null, null, null, System::currentTimeMillis) { @Override public MappedFieldType fieldMapper(String name) { return provideMappedFieldType(name); diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TemplateQueryBuilder.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TemplateQueryBuilder.java index 0ed8b03ede3..f81d5c7e053 100644 --- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TemplateQueryBuilder.java +++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TemplateQueryBuilder.java @@ -120,10 +120,7 @@ public class TemplateQueryBuilder extends AbstractQueryBuilder> QueryShardContext createShardContext() { ClusterState state = ClusterState.builder(new ClusterName("_name")).build(); return new QueryShardContext(idxSettings, bitsetFilterCache, indexFieldDataService, mapperService, similarityService, - scriptService, indicesQueriesRegistry, this.client, null, state); + scriptService, indicesQueriesRegistry, this.client, null, state, System::currentTimeMillis); } ScriptModule createScriptModule(List scriptPlugins) { diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java index 4916d7df2f6..e440af4e9b9 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java @@ -283,7 +283,7 @@ public abstract class ESSingleNodeTestCase extends ESTestCase { BigArrays bigArrays = indexService.getBigArrays(); ThreadPool threadPool = indexService.getThreadPool(); ScriptService scriptService = node().injector().getInstance(ScriptService.class); - return new TestSearchContext(threadPool, bigArrays, scriptService, indexService); + return new TestSearchContext(threadPool, bigArrays, indexService); } /** diff --git a/test/framework/src/main/java/org/elasticsearch/test/TestSearchContext.java b/test/framework/src/main/java/org/elasticsearch/test/TestSearchContext.java index c6a1f64820b..c8feb8242be 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/TestSearchContext.java +++ b/test/framework/src/main/java/org/elasticsearch/test/TestSearchContext.java @@ -37,7 +37,6 @@ import org.elasticsearch.index.query.ParsedQuery; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.similarity.SimilarityService; -import org.elasticsearch.script.ScriptService; import org.elasticsearch.search.SearchExtBuilder; import org.elasticsearch.search.SearchShardTarget; import org.elasticsearch.search.aggregations.SearchContextAggregations; @@ -77,7 +76,6 @@ public class TestSearchContext extends SearchContext { final Counter timeEstimateCounter = Counter.newCounter(); final QuerySearchResult queryResult = new QuerySearchResult(); final QueryShardContext queryShardContext; - ScriptService scriptService; ParsedQuery originalQuery; ParsedQuery postFilter; Query query; @@ -91,7 +89,7 @@ public class TestSearchContext extends SearchContext { private final long originNanoTime = System.nanoTime(); private final Map searchExtBuilders = new HashMap<>(); - public TestSearchContext(ThreadPool threadPool, BigArrays bigArrays, ScriptService scriptService, IndexService indexService) { + public TestSearchContext(ThreadPool threadPool, BigArrays bigArrays, IndexService indexService) { super(ParseFieldMatcher.STRICT); this.bigArrays = bigArrays.withCircuitBreaking(); this.indexService = indexService; @@ -99,7 +97,6 @@ public class TestSearchContext extends SearchContext { this.fixedBitSetFilterCache = indexService.cache().bitsetFilterCache(); this.threadPool = threadPool; this.indexShard = indexService.getShardOrNull(0); - this.scriptService = scriptService; queryShardContext = indexService.newQueryShardContext(); } @@ -111,7 +108,6 @@ public class TestSearchContext extends SearchContext { this.threadPool = null; this.fixedBitSetFilterCache = null; this.indexShard = null; - scriptService = null; this.queryShardContext = queryShardContext; } @@ -169,11 +165,6 @@ public class TestSearchContext extends SearchContext { return originNanoTime; } - @Override - protected long nowInMillisImpl() { - return 0; - } - @Override public ScrollContext scrollContext() { return null; @@ -299,11 +290,6 @@ public class TestSearchContext extends SearchContext { return null; } - @Override - public ScriptService scriptService() { - return scriptService; - } - @Override public BigArrays bigArrays() { return bigArrays; diff --git a/test/framework/src/test/java/org/elasticsearch/search/MockSearchServiceTests.java b/test/framework/src/test/java/org/elasticsearch/search/MockSearchServiceTests.java index d6cd3eea5ac..f61a3a2b48e 100644 --- a/test/framework/src/test/java/org/elasticsearch/search/MockSearchServiceTests.java +++ b/test/framework/src/test/java/org/elasticsearch/search/MockSearchServiceTests.java @@ -34,7 +34,7 @@ import org.elasticsearch.test.TestSearchContext; public class MockSearchServiceTests extends ESTestCase { public void testAssertNoInFlightContext() { SearchContext s = new TestSearchContext(new QueryShardContext(new IndexSettings(IndexMetaData.PROTO, Settings.EMPTY), null, null, - null, null, null, null, null, null, null)) { + null, null, null, null, null, null, null, System::currentTimeMillis)) { @Override public SearchShardTarget shardTarget() { return new SearchShardTarget("node", new Index("idx", "ignored"), 0); From cbb3cc625e592beeac8fd6ad0f700770c0a86be5 Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Wed, 5 Oct 2016 09:05:58 +0100 Subject: [PATCH 04/20] move extended bounds parse and validate to date hitso factory --- .../bucket/histogram/DateHistogramAggregationBuilder.java | 5 +++++ .../bucket/histogram/DateHistogramAggregatorFactory.java | 3 +-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java index d3b8857ccab..191d4e62ab4 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java @@ -245,6 +245,11 @@ public class DateHistogramAggregationBuilder @Override protected ValuesSourceAggregatorFactory innerBuild(AggregationContext context, ValuesSourceConfig config, AggregatorFactory parent, Builder subFactoriesBuilder) throws IOException { + ExtendedBounds extendedBounds = null; + if (this.extendedBounds != null) { + // parse any string bounds to longs + extendedBounds = this.extendedBounds.parseAndValidate(name, context.searchContext(), config.format()); + } return new DateHistogramAggregatorFactory(name, type, config, interval, dateHistogramInterval, offset, order, keyed, minDocCount, extendedBounds, context, parent, subFactoriesBuilder, metaData); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java index 79f81e28374..52b0a77829f 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java @@ -134,8 +134,7 @@ public final class DateHistogramAggregatorFactory // code so we won't need to do that ExtendedBounds roundedBounds = null; if (extendedBounds != null) { - // parse any string bounds to longs and round them - roundedBounds = extendedBounds.parseAndValidate(name, context.searchContext(), config.format()).round(rounding); + roundedBounds = extendedBounds.round(rounding); } return new DateHistogramAggregator(name, factories, rounding, offset, order, keyed, minDocCount, roundedBounds, valuesSource, config.format(), context, parent, pipelineAggregators, metaData); From 5a308f8a5e5b792aba56e722e1f3458af618d3e2 Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Wed, 5 Oct 2016 09:15:00 +0100 Subject: [PATCH 05/20] move extended bounds rounding to date histo agg builder --- .../DateHistogramAggregationBuilder.java | 60 +++++++++++++++-- .../DateHistogramAggregatorFactory.java | 66 ++----------------- .../DerivativePipelineAggregationBuilder.java | 3 +- 3 files changed, 62 insertions(+), 67 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java index 191d4e62ab4..b20cf1b346a 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java @@ -21,6 +21,8 @@ package org.elasticsearch.search.aggregations.bucket.histogram; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.rounding.DateTimeUnit; +import org.elasticsearch.common.rounding.Rounding; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.search.aggregations.AggregatorFactories.Builder; @@ -35,8 +37,12 @@ import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; import org.elasticsearch.search.aggregations.support.ValuesSourceType; import java.io.IOException; +import java.util.HashMap; +import java.util.Map; import java.util.Objects; +import static java.util.Collections.unmodifiableMap; + /** * A builder for histograms on date fields. */ @@ -44,6 +50,29 @@ public class DateHistogramAggregationBuilder extends ValuesSourceAggregationBuilder { public static final String NAME = InternalDateHistogram.TYPE.name(); + public static final Map DATE_FIELD_UNITS; + + static { + Map dateFieldUnits = new HashMap<>(); + dateFieldUnits.put("year", DateTimeUnit.YEAR_OF_CENTURY); + dateFieldUnits.put("1y", DateTimeUnit.YEAR_OF_CENTURY); + dateFieldUnits.put("quarter", DateTimeUnit.QUARTER); + dateFieldUnits.put("1q", DateTimeUnit.QUARTER); + dateFieldUnits.put("month", DateTimeUnit.MONTH_OF_YEAR); + dateFieldUnits.put("1M", DateTimeUnit.MONTH_OF_YEAR); + dateFieldUnits.put("week", DateTimeUnit.WEEK_OF_WEEKYEAR); + dateFieldUnits.put("1w", DateTimeUnit.WEEK_OF_WEEKYEAR); + dateFieldUnits.put("day", DateTimeUnit.DAY_OF_MONTH); + dateFieldUnits.put("1d", DateTimeUnit.DAY_OF_MONTH); + dateFieldUnits.put("hour", DateTimeUnit.HOUR_OF_DAY); + dateFieldUnits.put("1h", DateTimeUnit.HOUR_OF_DAY); + dateFieldUnits.put("minute", DateTimeUnit.MINUTES_OF_HOUR); + dateFieldUnits.put("1m", DateTimeUnit.MINUTES_OF_HOUR); + dateFieldUnits.put("second", DateTimeUnit.SECOND_OF_MINUTE); + dateFieldUnits.put("1s", DateTimeUnit.SECOND_OF_MINUTE); + DATE_FIELD_UNITS = unmodifiableMap(dateFieldUnits); + } + private long interval; private DateHistogramInterval dateHistogramInterval; private long offset = 0; @@ -245,13 +274,36 @@ public class DateHistogramAggregationBuilder @Override protected ValuesSourceAggregatorFactory innerBuild(AggregationContext context, ValuesSourceConfig config, AggregatorFactory parent, Builder subFactoriesBuilder) throws IOException { - ExtendedBounds extendedBounds = null; + Rounding rounding = createRounding(); + ExtendedBounds roundedBounds = null; if (this.extendedBounds != null) { - // parse any string bounds to longs - extendedBounds = this.extendedBounds.parseAndValidate(name, context.searchContext(), config.format()); + // parse any string bounds to longs and round + roundedBounds = this.extendedBounds.parseAndValidate(name, context.searchContext(), config.format()).round(rounding); } return new DateHistogramAggregatorFactory(name, type, config, interval, dateHistogramInterval, offset, order, keyed, minDocCount, - extendedBounds, context, parent, subFactoriesBuilder, metaData); + rounding, roundedBounds, context, parent, subFactoriesBuilder, metaData); + } + + private Rounding createRounding() { + Rounding.Builder tzRoundingBuilder; + if (dateHistogramInterval != null) { + DateTimeUnit dateTimeUnit = DATE_FIELD_UNITS.get(dateHistogramInterval.toString()); + if (dateTimeUnit != null) { + tzRoundingBuilder = Rounding.builder(dateTimeUnit); + } else { + // the interval is a time value? + tzRoundingBuilder = Rounding.builder( + TimeValue.parseTimeValue(dateHistogramInterval.toString(), null, getClass().getSimpleName() + ".interval")); + } + } else { + // the interval is an integer time value in millis? + tzRoundingBuilder = Rounding.builder(TimeValue.timeValueMillis(interval)); + } + if (timeZone() != null) { + tzRoundingBuilder.timeZone(timeZone()); + } + Rounding rounding = tzRoundingBuilder.build(); + return rounding; } @Override diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java index 52b0a77829f..f1c4a6b4fae 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java @@ -19,9 +19,7 @@ package org.elasticsearch.search.aggregations.bucket.histogram; -import org.elasticsearch.common.rounding.DateTimeUnit; import org.elasticsearch.common.rounding.Rounding; -import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.search.aggregations.Aggregator; import org.elasticsearch.search.aggregations.AggregatorFactories; import org.elasticsearch.search.aggregations.AggregatorFactory; @@ -30,12 +28,9 @@ import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; import org.elasticsearch.search.aggregations.support.ValuesSource.Numeric; import java.io.IOException; -import java.util.HashMap; import java.util.List; import java.util.Map; -import static java.util.Collections.unmodifiableMap; - import org.elasticsearch.search.aggregations.support.AggregationContext; import org.elasticsearch.search.aggregations.support.ValuesSource; import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory; @@ -44,29 +39,6 @@ import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; public final class DateHistogramAggregatorFactory extends ValuesSourceAggregatorFactory { - public static final Map DATE_FIELD_UNITS; - - static { - Map dateFieldUnits = new HashMap<>(); - dateFieldUnits.put("year", DateTimeUnit.YEAR_OF_CENTURY); - dateFieldUnits.put("1y", DateTimeUnit.YEAR_OF_CENTURY); - dateFieldUnits.put("quarter", DateTimeUnit.QUARTER); - dateFieldUnits.put("1q", DateTimeUnit.QUARTER); - dateFieldUnits.put("month", DateTimeUnit.MONTH_OF_YEAR); - dateFieldUnits.put("1M", DateTimeUnit.MONTH_OF_YEAR); - dateFieldUnits.put("week", DateTimeUnit.WEEK_OF_WEEKYEAR); - dateFieldUnits.put("1w", DateTimeUnit.WEEK_OF_WEEKYEAR); - dateFieldUnits.put("day", DateTimeUnit.DAY_OF_MONTH); - dateFieldUnits.put("1d", DateTimeUnit.DAY_OF_MONTH); - dateFieldUnits.put("hour", DateTimeUnit.HOUR_OF_DAY); - dateFieldUnits.put("1h", DateTimeUnit.HOUR_OF_DAY); - dateFieldUnits.put("minute", DateTimeUnit.MINUTES_OF_HOUR); - dateFieldUnits.put("1m", DateTimeUnit.MINUTES_OF_HOUR); - dateFieldUnits.put("second", DateTimeUnit.SECOND_OF_MINUTE); - dateFieldUnits.put("1s", DateTimeUnit.SECOND_OF_MINUTE); - DATE_FIELD_UNITS = unmodifiableMap(dateFieldUnits); - } - private final DateHistogramInterval dateHistogramInterval; private final long interval; private final long offset; @@ -74,10 +46,11 @@ public final class DateHistogramAggregatorFactory private final boolean keyed; private final long minDocCount; private final ExtendedBounds extendedBounds; + private Rounding rounding; public DateHistogramAggregatorFactory(String name, Type type, ValuesSourceConfig config, long interval, DateHistogramInterval dateHistogramInterval, long offset, InternalOrder order, boolean keyed, long minDocCount, - ExtendedBounds extendedBounds, AggregationContext context, AggregatorFactory parent, + Rounding rounding, ExtendedBounds extendedBounds, AggregationContext context, AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder, Map metaData) throws IOException { super(name, type, config, context, parent, subFactoriesBuilder, metaData); this.interval = interval; @@ -87,34 +60,13 @@ public final class DateHistogramAggregatorFactory this.keyed = keyed; this.minDocCount = minDocCount; this.extendedBounds = extendedBounds; + this.rounding = rounding; } public long minDocCount() { return minDocCount; } - private Rounding createRounding() { - Rounding.Builder tzRoundingBuilder; - if (dateHistogramInterval != null) { - DateTimeUnit dateTimeUnit = DATE_FIELD_UNITS.get(dateHistogramInterval.toString()); - if (dateTimeUnit != null) { - tzRoundingBuilder = Rounding.builder(dateTimeUnit); - } else { - // the interval is a time value? - tzRoundingBuilder = Rounding.builder( - TimeValue.parseTimeValue(dateHistogramInterval.toString(), null, getClass().getSimpleName() + ".interval")); - } - } else { - // the interval is an integer time value in millis? - tzRoundingBuilder = Rounding.builder(TimeValue.timeValueMillis(interval)); - } - if (timeZone() != null) { - tzRoundingBuilder.timeZone(timeZone()); - } - Rounding rounding = tzRoundingBuilder.build(); - return rounding; - } - @Override protected Aggregator doCreateInternal(ValuesSource.Numeric valuesSource, Aggregator parent, boolean collectsFromSingleBucket, List pipelineAggregators, Map metaData) throws IOException { @@ -126,17 +78,7 @@ public final class DateHistogramAggregatorFactory private Aggregator createAggregator(ValuesSource.Numeric valuesSource, Aggregator parent, List pipelineAggregators, Map metaData) throws IOException { - Rounding rounding = createRounding(); - // we need to round the bounds given by the user and we have to do it - // for every aggregator we create - // as the rounding is not necessarily an idempotent operation. - // todo we need to think of a better structure to the factory/agtor - // code so we won't need to do that - ExtendedBounds roundedBounds = null; - if (extendedBounds != null) { - roundedBounds = extendedBounds.round(rounding); - } - return new DateHistogramAggregator(name, factories, rounding, offset, order, keyed, minDocCount, roundedBounds, valuesSource, + return new DateHistogramAggregator(name, factories, rounding, offset, order, keyed, minDocCount, extendedBounds, valuesSource, config.format(), context, parent, pipelineAggregators, metaData); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/derivative/DerivativePipelineAggregationBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/derivative/DerivativePipelineAggregationBuilder.java index 5ffc77669b8..ded2d110206 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/derivative/DerivativePipelineAggregationBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/derivative/DerivativePipelineAggregationBuilder.java @@ -32,6 +32,7 @@ import org.elasticsearch.search.DocValueFormat; import org.elasticsearch.search.aggregations.AggregatorFactory; import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregatorFactory; +import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; import org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregatorFactory; import org.elasticsearch.search.aggregations.pipeline.AbstractPipelineAggregationBuilder; @@ -141,7 +142,7 @@ public class DerivativePipelineAggregationBuilder extends AbstractPipelineAggreg } Long xAxisUnits = null; if (units != null) { - DateTimeUnit dateTimeUnit = DateHistogramAggregatorFactory.DATE_FIELD_UNITS.get(units); + DateTimeUnit dateTimeUnit = DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(units); if (dateTimeUnit != null) { xAxisUnits = dateTimeUnit.field(DateTimeZone.UTC).getDurationField().getUnitMillis(); } else { From 3ba0bd6ec97a874dc5f4c8371a2b93dac9a12d6a Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Wed, 5 Oct 2016 09:32:31 +0100 Subject: [PATCH 06/20] fix check style errors --- .../index/query/QueryShardContext.java | 6 ++++-- .../scripted/ScriptedMetricAggregationBuilder.java | 11 ++--------- .../scripted/ScriptedMetricAggregatorFactory.java | 12 +++++------- .../metrics/tophits/TopHitsAggregatorFactory.java | 13 ++++--------- 4 files changed, 15 insertions(+), 27 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index 671a89a3884..200d5fde0c7 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -341,7 +341,8 @@ public class QueryShardContext extends QueryRewriteContext { return scriptService.search(lookup(), script, context, params); } - public Function, SearchScript> getLazySearchScript(Script script, ScriptContext context, Map params) { + public Function, SearchScript> getLazySearchScript(Script script, ScriptContext context, + Map params) { failIfExecutionMode(); markAsNotCachable(); CompiledScript compile = scriptService.compile(script, context, params); @@ -354,7 +355,8 @@ public class QueryShardContext extends QueryRewriteContext { return scriptService.executable(script, context, params); } - public Function, ExecutableScript> getLazyExecutableScript(Script script, ScriptContext context, Map params) { + public Function, ExecutableScript> getLazyExecutableScript(Script script, ScriptContext context, + Map params) { failIfExecutionMode(); markAsNotCachable(); CompiledScript executable = scriptService.compile(script, context, params); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java index 208fae15701..f044e94f05b 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java @@ -27,25 +27,18 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.index.query.QueryShardContext; -import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ExecutableScript; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.SearchScript; -import org.elasticsearch.search.SearchParseException; import org.elasticsearch.search.aggregations.AbstractAggregationBuilder; import org.elasticsearch.search.aggregations.AggregatorFactories.Builder; import org.elasticsearch.search.aggregations.AggregatorFactory; import org.elasticsearch.search.aggregations.InternalAggregation.Type; import org.elasticsearch.search.aggregations.support.AggregationContext; -import org.elasticsearch.search.internal.SearchContext; - import java.io.IOException; -import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; -import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -212,8 +205,8 @@ public class ScriptedMetricAggregationBuilder extends AbstractAggregationBuilder } else { executableCombineScript = (p) -> null; } - return new ScriptedMetricAggregatorFactory(name, type, searchMapScript, executableInitScript, executableCombineScript, reduceScript, params, context, - parent, subfactoriesBuilder, metaData); + return new ScriptedMetricAggregatorFactory(name, type, searchMapScript, executableInitScript, executableCombineScript, reduceScript, + params, context, parent, subfactoriesBuilder, metaData); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorFactory.java index 1dc02ea42dc..d6150fa8743 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorFactory.java @@ -19,8 +19,6 @@ package org.elasticsearch.search.aggregations.metrics.scripted; -import org.elasticsearch.index.query.QueryShardContext; -import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ExecutableScript; import org.elasticsearch.script.Script; import org.elasticsearch.script.SearchScript; @@ -38,7 +36,6 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.function.Function; public class ScriptedMetricAggregatorFactory extends AggregatorFactory { @@ -49,9 +46,10 @@ public class ScriptedMetricAggregatorFactory extends AggregatorFactory params; private final Function, ExecutableScript> initScript; - public ScriptedMetricAggregatorFactory(String name, Type type, Function, SearchScript> mapScript, Function, ExecutableScript> initScript, Function, ExecutableScript> combineScript, - Script reduceScript, Map params, AggregationContext context, AggregatorFactory parent, - AggregatorFactories.Builder subFactories, Map metaData) throws IOException { + public ScriptedMetricAggregatorFactory(String name, Type type, Function, SearchScript> mapScript, + Function, ExecutableScript> initScript, Function, ExecutableScript> combineScript, + Script reduceScript, Map params, AggregationContext context, AggregatorFactory parent, + AggregatorFactories.Builder subFactories, Map metaData) throws IOException { super(name, type, context, parent, subFactories, metaData); this.mapScript = mapScript; this.initScript = initScript; @@ -111,7 +109,7 @@ public class ScriptedMetricAggregatorFactory extends AggregatorFactory originalList = (List) original; - List clonedList = new ArrayList(); + List clonedList = new ArrayList<>(); for (Object o : originalList) { clonedList.add(deepCopyParams(o, context)); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java index 42477c3212f..8e1c0c51095 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java @@ -19,15 +19,12 @@ package org.elasticsearch.search.aggregations.metrics.tophits; -import org.elasticsearch.script.ScriptContext; -import org.elasticsearch.script.SearchScript; import org.elasticsearch.search.aggregations.Aggregator; import org.elasticsearch.search.aggregations.AggregatorFactories; import org.elasticsearch.search.aggregations.AggregatorFactory; import org.elasticsearch.search.aggregations.InternalAggregation.Type; import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; import org.elasticsearch.search.aggregations.support.AggregationContext; -import org.elasticsearch.search.builder.SearchSourceBuilder.ScriptField; import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; @@ -38,11 +35,9 @@ import org.elasticsearch.search.sort.SortAndFormats; import org.elasticsearch.search.sort.SortBuilder; import java.io.IOException; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.Set; public class TopHitsAggregatorFactory extends AggregatorFactory { @@ -59,10 +54,10 @@ public class TopHitsAggregatorFactory extends AggregatorFactory> sorts, HighlightBuilder highlightBuilder, StoredFieldsContext storedFieldsContext, - List docValueFields, List scriptFields, FetchSourceContext fetchSourceContext, - AggregationContext context, AggregatorFactory parent, AggregatorFactories.Builder subFactories, - Map metaData) throws IOException { + List> sorts, HighlightBuilder highlightBuilder, StoredFieldsContext storedFieldsContext, + List docValueFields, List scriptFields, FetchSourceContext fetchSourceContext, + AggregationContext context, AggregatorFactory parent, AggregatorFactories.Builder subFactories, Map metaData) + throws IOException { super(name, type, context, parent, subFactories, metaData); this.from = from; this.size = size; From 04f5d4766dece37e29384bd4848d43a60e150131 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Tue, 4 Oct 2016 23:24:29 +0200 Subject: [PATCH 07/20] Make getter for bulk shard requests items visible (#20743) --- .../java/org/elasticsearch/action/bulk/BulkShardRequest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/elasticsearch/action/bulk/BulkShardRequest.java b/core/src/main/java/org/elasticsearch/action/bulk/BulkShardRequest.java index ffc2407b8a4..b9d7f876dc1 100644 --- a/core/src/main/java/org/elasticsearch/action/bulk/BulkShardRequest.java +++ b/core/src/main/java/org/elasticsearch/action/bulk/BulkShardRequest.java @@ -45,7 +45,7 @@ public class BulkShardRequest extends ReplicatedWriteRequest { setRefreshPolicy(refreshPolicy); } - BulkItemRequest[] items() { + public BulkItemRequest[] items() { return items; } From 4b82703bf75baa033fe6febcdff2cc138b9609d2 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 4 Oct 2016 19:47:47 -0400 Subject: [PATCH 08/20] Clarify wording for the strict REST params message This commit changes the strict REST parameters message to say that unconsumed parameters are unrecognized rather than unused. Additionally, the test is beefed up to include two unused parameters. Relates #20745 --- .../java/org/elasticsearch/rest/BaseRestHandler.java | 8 +++++++- .../org/elasticsearch/rest/BaseRestHandlerTests.java | 11 +++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/rest/BaseRestHandler.java b/core/src/main/java/org/elasticsearch/rest/BaseRestHandler.java index 10f5359803b..fd5ff846529 100644 --- a/core/src/main/java/org/elasticsearch/rest/BaseRestHandler.java +++ b/core/src/main/java/org/elasticsearch/rest/BaseRestHandler.java @@ -30,6 +30,7 @@ import org.elasticsearch.plugins.ActionPlugin; import java.io.IOException; import java.util.Collections; import java.util.List; +import java.util.Locale; import java.util.Set; import java.util.stream.Collectors; @@ -63,7 +64,12 @@ public abstract class BaseRestHandler extends AbstractComponent implements RestH // validate the non-response params if (!unconsumedParams.isEmpty()) { - throw new IllegalArgumentException("request [" + request.path() + "] contains unused params: " + unconsumedParams.toString()); + final String message = String.format( + Locale.ROOT, + "request [%s] contains unrecognized parameters: %s", + request.path(), + unconsumedParams.toString()); + throw new IllegalArgumentException(message); } // execute the action diff --git a/core/src/test/java/org/elasticsearch/rest/BaseRestHandlerTests.java b/core/src/test/java/org/elasticsearch/rest/BaseRestHandlerTests.java index 6019ab8aacf..fc5af192b74 100644 --- a/core/src/test/java/org/elasticsearch/rest/BaseRestHandlerTests.java +++ b/core/src/test/java/org/elasticsearch/rest/BaseRestHandlerTests.java @@ -33,6 +33,7 @@ import java.util.HashMap; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; +import static org.hamcrest.core.AnyOf.anyOf; import static org.hamcrest.core.StringContains.containsString; import static org.hamcrest.object.HasToString.hasToString; import static org.mockito.Mockito.mock; @@ -51,12 +52,18 @@ public class BaseRestHandlerTests extends ESTestCase { final HashMap params = new HashMap<>(); params.put("consumed", randomAsciiOfLength(8)); - params.put("unconsumed", randomAsciiOfLength(8)); + params.put("unconsumed-first", randomAsciiOfLength(8)); + params.put("unconsumed-second", randomAsciiOfLength(8)); RestRequest request = new FakeRestRequest.Builder().withParams(params).build(); RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1); final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> handler.handleRequest(request, channel, mock(NodeClient.class))); - assertThat(e, hasToString(containsString("request [/] contains unused params: [unconsumed]"))); + assertThat( + e, + // we can not rely on ordering of the unconsumed parameters here + anyOf( + hasToString(containsString("request [/] contains unrecognized parameters: [unconsumed-first, unconsumed-second]")), + hasToString(containsString("request [/] contains unrecognized parameters: [unconsumed-second, unconsumed-first]")))); assertFalse(executed.get()); } From d7dca159704bbc47a74d7e6b61f88f44d0baf509 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 4 Oct 2016 20:45:07 -0400 Subject: [PATCH 09/20] Add did you mean to strict REST params This commit adds a did you mean feature to the strict REST params error message. This works by comparing any unconsumed parameters to all of the consumer parameters, comparing the Levenstein distance between those parameters, and taking any consumed parameters that are close to an unconsumed parameter as candiates for the did you mean. * Fix pluralization in strict REST params message This commit fixes the pluralization in the strict REST parameters error message so that the word "parameter" is not unconditionally written as "parameters" even when there is only one unrecognized parameter. * Strength strict REST params did you mean test This commit adds an unconsumed parameter that is too far from every consumed parameter to have any candidate suggestions. Relates #20747 --- .../elasticsearch/rest/BaseRestHandler.java | 44 +++++++++++-- .../org/elasticsearch/rest/RestRequest.java | 11 +++- .../admin/indices/RestAnalyzeAction.java | 5 +- .../rest/BaseRestHandlerTests.java | 62 +++++++++++++++++-- 4 files changed, 108 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/rest/BaseRestHandler.java b/core/src/main/java/org/elasticsearch/rest/BaseRestHandler.java index fd5ff846529..3bc91dc48e2 100644 --- a/core/src/main/java/org/elasticsearch/rest/BaseRestHandler.java +++ b/core/src/main/java/org/elasticsearch/rest/BaseRestHandler.java @@ -19,8 +19,11 @@ package org.elasticsearch.rest; +import org.apache.lucene.search.spell.LevensteinDistance; +import org.apache.lucene.util.CollectionUtil; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.ParseFieldMatcher; +import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; @@ -28,10 +31,13 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.plugins.ActionPlugin; import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.Set; +import java.util.SortedSet; +import java.util.TreeSet; import java.util.stream.Collectors; /** @@ -59,16 +65,44 @@ public abstract class BaseRestHandler extends AbstractComponent implements RestH final RestChannelConsumer action = prepareRequest(request, client); // validate unconsumed params, but we must exclude params used to format the response - final List unconsumedParams = - request.unconsumedParams().stream().filter(p -> !responseParams().contains(p)).collect(Collectors.toList()); + // use a sorted set so the unconsumed parameters appear in a reliable sorted order + final SortedSet unconsumedParams = + request.unconsumedParams().stream().filter(p -> !responseParams().contains(p)).collect(Collectors.toCollection(TreeSet::new)); // validate the non-response params if (!unconsumedParams.isEmpty()) { - final String message = String.format( + String message = String.format( Locale.ROOT, - "request [%s] contains unrecognized parameters: %s", + "request [%s] contains unrecognized parameter%s: ", request.path(), - unconsumedParams.toString()); + unconsumedParams.size() > 1 ? "s" : ""); + boolean first = true; + for (final String unconsumedParam : unconsumedParams) { + final LevensteinDistance ld = new LevensteinDistance(); + final List> scoredParams = new ArrayList<>(); + for (String consumedParam : request.consumedParams()) { + final float distance = ld.getDistance(unconsumedParam, consumedParam); + if (distance > 0.5f) { + scoredParams.add(new Tuple<>(distance, consumedParam)); + } + } + CollectionUtil.timSort(scoredParams, (a, b) -> { + // sort by distance in reverse order, then parameter name for equal distances + int compare = a.v1().compareTo(b.v1()); + if (compare != 0) return -compare; + else return a.v2().compareTo(b.v2()); + }); + if (first == false) { + message += ", "; + } + message += "[" + unconsumedParam + "]"; + final List keys = scoredParams.stream().map(Tuple::v2).collect(Collectors.toList()); + if (keys.isEmpty() == false) { + message += " -> did you mean " + (keys.size() == 1 ? "[" + keys.get(0) + "]": "any of " + keys.toString()) + "?"; + } + first = false; + } + throw new IllegalArgumentException(message); } diff --git a/core/src/main/java/org/elasticsearch/rest/RestRequest.java b/core/src/main/java/org/elasticsearch/rest/RestRequest.java index 5960fc8979e..3f0f32fff37 100644 --- a/core/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/core/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -28,7 +28,6 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.ToXContent; import java.net.SocketAddress; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -129,6 +128,16 @@ public abstract class RestRequest implements ToXContent.Params { return params; } + /** + * Returns a list of parameters that have been consumed. This method returns a copy, callers + * are free to modify the returned list. + * + * @return the list of currently consumed parameters. + */ + List consumedParams() { + return consumedParams.stream().collect(Collectors.toList()); + } + /** * Returns a list of parameters that have not yet been consumed. This method returns a copy, * callers are free to modify the returned list. diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestAnalyzeAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestAnalyzeAction.java index 4b3035e08b5..247df1a380e 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestAnalyzeAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestAnalyzeAction.java @@ -73,8 +73,9 @@ public class RestAnalyzeAction extends BaseRestHandler { analyzeRequest.text(texts); analyzeRequest.analyzer(request.param("analyzer")); analyzeRequest.field(request.param("field")); - if (request.hasParam("tokenizer")) { - analyzeRequest.tokenizer(request.param("tokenizer")); + final String tokenizer = request.param("tokenizer"); + if (tokenizer != null) { + analyzeRequest.tokenizer(tokenizer); } for (String filter : request.paramAsStringArray("filter", Strings.EMPTY_ARRAY)) { analyzeRequest.addTokenFilter(filter); diff --git a/core/src/test/java/org/elasticsearch/rest/BaseRestHandlerTests.java b/core/src/test/java/org/elasticsearch/rest/BaseRestHandlerTests.java index fc5af192b74..8f9dd445369 100644 --- a/core/src/test/java/org/elasticsearch/rest/BaseRestHandlerTests.java +++ b/core/src/test/java/org/elasticsearch/rest/BaseRestHandlerTests.java @@ -33,14 +33,34 @@ import java.util.HashMap; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; -import static org.hamcrest.core.AnyOf.anyOf; import static org.hamcrest.core.StringContains.containsString; import static org.hamcrest.object.HasToString.hasToString; import static org.mockito.Mockito.mock; public class BaseRestHandlerTests extends ESTestCase { - public void testUnconsumedParameters() throws Exception { + public void testOneUnconsumedParameters() throws Exception { + final AtomicBoolean executed = new AtomicBoolean(); + BaseRestHandler handler = new BaseRestHandler(Settings.EMPTY) { + @Override + protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + request.param("consumed"); + return channel -> executed.set(true); + } + }; + + final HashMap params = new HashMap<>(); + params.put("consumed", randomAsciiOfLength(8)); + params.put("unconsumed", randomAsciiOfLength(8)); + RestRequest request = new FakeRestRequest.Builder().withParams(params).build(); + RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1); + final IllegalArgumentException e = + expectThrows(IllegalArgumentException.class, () -> handler.handleRequest(request, channel, mock(NodeClient.class))); + assertThat(e, hasToString(containsString("request [/] contains unrecognized parameter: [unconsumed]"))); + assertFalse(executed.get()); + } + + public void testMultipleUnconsumedParameters() throws Exception { final AtomicBoolean executed = new AtomicBoolean(); BaseRestHandler handler = new BaseRestHandler(Settings.EMPTY) { @Override @@ -56,14 +76,44 @@ public class BaseRestHandlerTests extends ESTestCase { params.put("unconsumed-second", randomAsciiOfLength(8)); RestRequest request = new FakeRestRequest.Builder().withParams(params).build(); RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1); + final IllegalArgumentException e = + expectThrows(IllegalArgumentException.class, () -> handler.handleRequest(request, channel, mock(NodeClient.class))); + assertThat(e, hasToString(containsString("request [/] contains unrecognized parameters: [unconsumed-first], [unconsumed-second]"))); + assertFalse(executed.get()); + } + + public void testUnconsumedParametersDidYouMean() throws Exception { + final AtomicBoolean executed = new AtomicBoolean(); + BaseRestHandler handler = new BaseRestHandler(Settings.EMPTY) { + @Override + protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + request.param("consumed"); + request.param("field"); + request.param("tokenizer"); + request.param("very_close_to_parameter_1"); + request.param("very_close_to_parameter_2"); + return channel -> executed.set(true); + } + }; + + final HashMap params = new HashMap<>(); + params.put("consumed", randomAsciiOfLength(8)); + params.put("flied", randomAsciiOfLength(8)); + params.put("tokenzier", randomAsciiOfLength(8)); + params.put("very_close_to_parametre", randomAsciiOfLength(8)); + params.put("very_far_from_every_consumed_parameter", randomAsciiOfLength(8)); + RestRequest request = new FakeRestRequest.Builder().withParams(params).build(); + RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1); final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> handler.handleRequest(request, channel, mock(NodeClient.class))); assertThat( e, - // we can not rely on ordering of the unconsumed parameters here - anyOf( - hasToString(containsString("request [/] contains unrecognized parameters: [unconsumed-first, unconsumed-second]")), - hasToString(containsString("request [/] contains unrecognized parameters: [unconsumed-second, unconsumed-first]")))); + hasToString(containsString( + "request [/] contains unrecognized parameters: " + + "[flied] -> did you mean [field]?, " + + "[tokenzier] -> did you mean [tokenizer]?, " + + "[very_close_to_parametre] -> did you mean any of [very_close_to_parameter_1, very_close_to_parameter_2]?, " + + "[very_far_from_every_consumed_parameter]"))); assertFalse(executed.get()); } From 0c7860f53391bfc516bb9275b2e991b7ce11f339 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Fri, 30 Sep 2016 15:24:26 +0200 Subject: [PATCH 10/20] ingest: Upgrade geoip2 dependency Closes #20563 --- plugins/ingest-geoip/build.gradle | 31 +++++++++++-------- .../licenses/geoip2-2.7.0.jar.sha1 | 1 - .../licenses/geoip2-2.8.0.jar.sha1 | 1 + .../jackson-annotations-2.7.1.jar.sha1 | 1 - .../jackson-annotations-2.8.2.jar.sha1 | 1 + .../licenses/jackson-databind-2.7.1.jar.sha1 | 1 - .../licenses/jackson-databind-2.8.2.jar.sha1 | 1 + 7 files changed, 21 insertions(+), 16 deletions(-) delete mode 100644 plugins/ingest-geoip/licenses/geoip2-2.7.0.jar.sha1 create mode 100644 plugins/ingest-geoip/licenses/geoip2-2.8.0.jar.sha1 delete mode 100644 plugins/ingest-geoip/licenses/jackson-annotations-2.7.1.jar.sha1 create mode 100644 plugins/ingest-geoip/licenses/jackson-annotations-2.8.2.jar.sha1 delete mode 100644 plugins/ingest-geoip/licenses/jackson-databind-2.7.1.jar.sha1 create mode 100644 plugins/ingest-geoip/licenses/jackson-databind-2.8.2.jar.sha1 diff --git a/plugins/ingest-geoip/build.gradle b/plugins/ingest-geoip/build.gradle index d9e90a61d40..e74a27844db 100644 --- a/plugins/ingest-geoip/build.gradle +++ b/plugins/ingest-geoip/build.gradle @@ -23,10 +23,10 @@ esplugin { } dependencies { - compile ('com.maxmind.geoip2:geoip2:2.7.0') + compile ('com.maxmind.geoip2:geoip2:2.8.0') // geoip2 dependencies: - compile('com.fasterxml.jackson.core:jackson-annotations:2.7.1') - compile('com.fasterxml.jackson.core:jackson-databind:2.7.1') + compile('com.fasterxml.jackson.core:jackson-annotations:2.8.2') + compile('com.fasterxml.jackson.core:jackson-databind:2.8.2') compile('com.maxmind.db:maxmind-db:1.2.1') testCompile 'org.elasticsearch:geolite2-databases:20160608' @@ -50,14 +50,19 @@ bundlePlugin { } thirdPartyAudit.excludes = [ - // geoip WebServiceClient needs Google http client, but we're not using WebServiceClient: - 'com.google.api.client.http.HttpTransport', - 'com.google.api.client.http.GenericUrl', - 'com.google.api.client.http.HttpResponse', - 'com.google.api.client.http.HttpRequestFactory', - 'com.google.api.client.http.HttpRequest', - 'com.google.api.client.http.HttpHeaders', - 'com.google.api.client.http.HttpResponseException', - 'com.google.api.client.http.javanet.NetHttpTransport', - 'com.google.api.client.http.javanet.NetHttpTransport', + // geoip WebServiceClient needs apache http client, but we're not using WebServiceClient: + 'org.apache.http.HttpEntity', + 'org.apache.http.HttpHost', + 'org.apache.http.HttpResponse', + 'org.apache.http.StatusLine', + 'org.apache.http.auth.UsernamePasswordCredentials', + 'org.apache.http.client.config.RequestConfig$Builder', + 'org.apache.http.client.config.RequestConfig', + 'org.apache.http.client.methods.CloseableHttpResponse', + 'org.apache.http.client.methods.HttpGet', + 'org.apache.http.client.utils.URIBuilder', + 'org.apache.http.impl.auth.BasicScheme', + 'org.apache.http.impl.client.CloseableHttpClient', + 'org.apache.http.impl.client.HttpClientBuilder', + 'org.apache.http.util.EntityUtils' ] diff --git a/plugins/ingest-geoip/licenses/geoip2-2.7.0.jar.sha1 b/plugins/ingest-geoip/licenses/geoip2-2.7.0.jar.sha1 deleted file mode 100644 index 2015e311d60..00000000000 --- a/plugins/ingest-geoip/licenses/geoip2-2.7.0.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -2010d922191f5801939b462a5703ab79a7829626 \ No newline at end of file diff --git a/plugins/ingest-geoip/licenses/geoip2-2.8.0.jar.sha1 b/plugins/ingest-geoip/licenses/geoip2-2.8.0.jar.sha1 new file mode 100644 index 00000000000..c6036686601 --- /dev/null +++ b/plugins/ingest-geoip/licenses/geoip2-2.8.0.jar.sha1 @@ -0,0 +1 @@ +46226778ec32b776e80f282c5bf65b88d36cc0a0 \ No newline at end of file diff --git a/plugins/ingest-geoip/licenses/jackson-annotations-2.7.1.jar.sha1 b/plugins/ingest-geoip/licenses/jackson-annotations-2.7.1.jar.sha1 deleted file mode 100644 index 69b45742d84..00000000000 --- a/plugins/ingest-geoip/licenses/jackson-annotations-2.7.1.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -8b93f301823b79033fcbe873779b3d84f9730fc1 \ No newline at end of file diff --git a/plugins/ingest-geoip/licenses/jackson-annotations-2.8.2.jar.sha1 b/plugins/ingest-geoip/licenses/jackson-annotations-2.8.2.jar.sha1 new file mode 100644 index 00000000000..c3b701dbb86 --- /dev/null +++ b/plugins/ingest-geoip/licenses/jackson-annotations-2.8.2.jar.sha1 @@ -0,0 +1 @@ +a38d544583e90cf163b2e45e4a57f5c54de670d3 \ No newline at end of file diff --git a/plugins/ingest-geoip/licenses/jackson-databind-2.7.1.jar.sha1 b/plugins/ingest-geoip/licenses/jackson-databind-2.7.1.jar.sha1 deleted file mode 100644 index d9b4ca6a79b..00000000000 --- a/plugins/ingest-geoip/licenses/jackson-databind-2.7.1.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -14d88822bca655de7aa6ed3e4c498d115505710a \ No newline at end of file diff --git a/plugins/ingest-geoip/licenses/jackson-databind-2.8.2.jar.sha1 b/plugins/ingest-geoip/licenses/jackson-databind-2.8.2.jar.sha1 new file mode 100644 index 00000000000..7e90b5f8e97 --- /dev/null +++ b/plugins/ingest-geoip/licenses/jackson-databind-2.8.2.jar.sha1 @@ -0,0 +1 @@ +1f12816593c1422be957471c98a80bfbace60fa2 \ No newline at end of file From 764a5fbb374877678855e554f6ddef849808c401 Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Tue, 4 Oct 2016 19:53:28 +0100 Subject: [PATCH 11/20] Fix date_range aggregation to not cache if now is used Before this change the processing of the ranges in the date range (and other range type) aggregations was done when the Aggregator was created. This meant that the SearchContext did not know that now had been used in a range until after the decision to cache was made. This change moves the processing of the ranges to the aggregation builders so that the search context is made aware that now has been used before it decides if the request should be cached --- .../range/AbstractRangeAggregatorFactory.java | 6 +-- .../bucket/range/AbstractRangeBuilder.java | 38 +++++++++++++++++ .../bucket/range/RangeAggregationBuilder.java | 2 + .../bucket/range/RangeAggregator.java | 41 +++---------------- .../bucket/range/RangeAggregatorFactory.java | 3 +- .../date/DateRangeAggregationBuilder.java | 3 ++ .../date/DateRangeAggregatorFactory.java | 3 +- .../GeoDistanceAggregationBuilder.java | 1 + .../GeoDistanceRangeAggregatorFactory.java | 6 +-- .../indices/IndicesRequestCacheIT.java | 30 ++++++++++++-- 10 files changed, 85 insertions(+), 48 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/AbstractRangeAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/AbstractRangeAggregatorFactory.java index f4103d87fbd..f67bec631bc 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/AbstractRangeAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/AbstractRangeAggregatorFactory.java @@ -40,10 +40,10 @@ public class AbstractRangeAggregatorFactory { private final InternalRange.Factory rangeFactory; - private final List ranges; + private final R[] ranges; private final boolean keyed; - public AbstractRangeAggregatorFactory(String name, Type type, ValuesSourceConfig config, List ranges, boolean keyed, + public AbstractRangeAggregatorFactory(String name, Type type, ValuesSourceConfig config, R[] ranges, boolean keyed, InternalRange.Factory rangeFactory, AggregationContext context, AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder, Map metaData) throws IOException { super(name, type, config, context, parent, subFactoriesBuilder, metaData); @@ -55,7 +55,7 @@ public class AbstractRangeAggregatorFactory pipelineAggregators, Map metaData) throws IOException { - return new Unmapped(name, ranges, keyed, config.format(), context, parent, rangeFactory, pipelineAggregators, metaData); + return new Unmapped<>(name, ranges, keyed, config.format(), context, parent, rangeFactory, pipelineAggregators, metaData); } @Override diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/AbstractRangeBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/AbstractRangeBuilder.java index 13d10bd0a0c..0692b0ed304 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/AbstractRangeBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/AbstractRangeBuilder.java @@ -19,13 +19,17 @@ package org.elasticsearch.search.aggregations.bucket.range; +import org.apache.lucene.util.InPlaceMergeSorter; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.search.aggregations.bucket.range.RangeAggregator.Range; +import org.elasticsearch.search.aggregations.support.AggregationContext; import org.elasticsearch.search.aggregations.support.ValuesSource; import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder; +import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; +import org.elasticsearch.search.aggregations.support.ValuesSource.Numeric; import java.io.IOException; import java.util.ArrayList; @@ -55,6 +59,40 @@ public abstract class AbstractRangeBuilder config) { + Range[] ranges = new Range[this.ranges.size()]; + for (int i = 0; i < ranges.length; i++) { + ranges[i] = this.ranges.get(i).process(config.format(), context.searchContext()); + } + sortRanges(ranges); + return ranges; + } + + private static void sortRanges(final Range[] ranges) { + new InPlaceMergeSorter() { + + @Override + protected void swap(int i, int j) { + final Range tmp = ranges[i]; + ranges[i] = ranges[j]; + ranges[j] = tmp; + } + + @Override + protected int compare(int i, int j) { + int cmp = Double.compare(ranges[i].from, ranges[j].from); + if (cmp == 0) { + cmp = Double.compare(ranges[i].to, ranges[j].to); + } + return cmp; + } + }.sort(0, ranges.length); + } + @Override protected void innerWriteTo(StreamOutput out) throws IOException { out.writeVInt(ranges.size()); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregationBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregationBuilder.java index c815ae9d3cf..73a4d86819a 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregationBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregationBuilder.java @@ -114,6 +114,8 @@ public class RangeAggregationBuilder extends AbstractRangeBuilder config, AggregatorFactory parent, Builder subFactoriesBuilder) throws IOException { + // We need to call processRanges here so they are parsed before we make the decision of whether to cache the request + Range[] ranges = processRanges(context, config); return new RangeAggregatorFactory(name, type, config, ranges, keyed, rangeFactory, context, parent, subFactoriesBuilder, metaData); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregator.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregator.java index 2eeb8db02b9..e35e4af9e78 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregator.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregator.java @@ -19,7 +19,6 @@ package org.elasticsearch.search.aggregations.bucket.range; import org.apache.lucene.index.LeafReaderContext; -import org.apache.lucene.util.InPlaceMergeSorter; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.io.stream.StreamInput; @@ -210,7 +209,7 @@ public class RangeAggregator extends BucketsAggregator { final double[] maxTo; public RangeAggregator(String name, AggregatorFactories factories, ValuesSource.Numeric valuesSource, DocValueFormat format, - InternalRange.Factory rangeFactory, List ranges, boolean keyed, AggregationContext aggregationContext, + InternalRange.Factory rangeFactory, Range[] ranges, boolean keyed, AggregationContext aggregationContext, Aggregator parent, List pipelineAggregators, Map metaData) throws IOException { super(name, factories, aggregationContext, parent, pipelineAggregators, metaData); @@ -220,11 +219,7 @@ public class RangeAggregator extends BucketsAggregator { this.keyed = keyed; this.rangeFactory = rangeFactory; - this.ranges = new Range[ranges.size()]; - for (int i = 0; i < this.ranges.length; i++) { - this.ranges[i] = ranges.get(i).process(format, context.searchContext()); - } - sortRanges(this.ranges); + this.ranges = ranges; maxTo = new double[this.ranges.length]; maxTo[0] = this.ranges[0].to; @@ -337,45 +332,21 @@ public class RangeAggregator extends BucketsAggregator { return rangeFactory.create(name, buckets, format, keyed, pipelineAggregators(), metaData()); } - private static void sortRanges(final Range[] ranges) { - new InPlaceMergeSorter() { - - @Override - protected void swap(int i, int j) { - final Range tmp = ranges[i]; - ranges[i] = ranges[j]; - ranges[j] = tmp; - } - - @Override - protected int compare(int i, int j) { - int cmp = Double.compare(ranges[i].from, ranges[j].from); - if (cmp == 0) { - cmp = Double.compare(ranges[i].to, ranges[j].to); - } - return cmp; - } - }.sort(0, ranges.length); - } - public static class Unmapped extends NonCollectingAggregator { - private final List ranges; + private final R[] ranges; private final boolean keyed; private final InternalRange.Factory factory; private final DocValueFormat format; @SuppressWarnings("unchecked") - public Unmapped(String name, List ranges, boolean keyed, DocValueFormat format, + public Unmapped(String name, R[] ranges, boolean keyed, DocValueFormat format, AggregationContext context, Aggregator parent, InternalRange.Factory factory, List pipelineAggregators, Map metaData) throws IOException { super(name, context, parent, pipelineAggregators, metaData); - this.ranges = new ArrayList<>(); - for (R range : ranges) { - this.ranges.add((R) range.process(format, context.searchContext())); - } + this.ranges = ranges; this.keyed = keyed; this.format = format; this.factory = factory; @@ -384,7 +355,7 @@ public class RangeAggregator extends BucketsAggregator { @Override public InternalAggregation buildEmptyAggregation() { InternalAggregations subAggs = buildEmptySubAggregations(); - List buckets = new ArrayList<>(ranges.size()); + List buckets = new ArrayList<>(ranges.length); for (RangeAggregator.Range range : ranges) { buckets.add(factory.createBucket(range.key, range.from, range.to, 0, subAggs, keyed, format)); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregatorFactory.java index 5dec4c40c45..b3297401457 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregatorFactory.java @@ -29,12 +29,11 @@ import org.elasticsearch.search.aggregations.support.AggregationContext; import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; import java.io.IOException; -import java.util.List; import java.util.Map; public class RangeAggregatorFactory extends AbstractRangeAggregatorFactory { - public RangeAggregatorFactory(String name, Type type, ValuesSourceConfig config, List ranges, boolean keyed, + public RangeAggregatorFactory(String name, Type type, ValuesSourceConfig config, Range[] ranges, boolean keyed, Factory rangeFactory, AggregationContext context, AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder, Map metaData) throws IOException { super(name, type, config, ranges, keyed, rangeFactory, context, parent, subFactoriesBuilder, metaData); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/date/DateRangeAggregationBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/date/DateRangeAggregationBuilder.java index a75b071569c..c8a8e16640b 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/date/DateRangeAggregationBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/date/DateRangeAggregationBuilder.java @@ -259,6 +259,9 @@ public class DateRangeAggregationBuilder extends AbstractRangeBuilder config, AggregatorFactory parent, Builder subFactoriesBuilder) throws IOException { + // We need to call processRanges here so they are parsed and we know whether `now` has been used before we make + // the decision of whether to cache the request + Range[] ranges = processRanges(context, config); return new DateRangeAggregatorFactory(name, type, config, ranges, keyed, rangeFactory, context, parent, subFactoriesBuilder, metaData); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/date/DateRangeAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/date/DateRangeAggregatorFactory.java index d3bb7ac6238..d5d16123ec3 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/date/DateRangeAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/date/DateRangeAggregatorFactory.java @@ -30,12 +30,11 @@ import org.elasticsearch.search.aggregations.support.AggregationContext; import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; import java.io.IOException; -import java.util.List; import java.util.Map; public class DateRangeAggregatorFactory extends AbstractRangeAggregatorFactory { - public DateRangeAggregatorFactory(String name, Type type, ValuesSourceConfig config, List ranges, boolean keyed, + public DateRangeAggregatorFactory(String name, Type type, ValuesSourceConfig config, Range[] ranges, boolean keyed, Factory rangeFactory, AggregationContext context, AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder, Map metaData) throws IOException { super(name, type, config, ranges, keyed, rangeFactory, context, parent, subFactoriesBuilder, metaData); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/geodistance/GeoDistanceAggregationBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/geodistance/GeoDistanceAggregationBuilder.java index 4a4cab2affa..583bc83feb4 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/geodistance/GeoDistanceAggregationBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/geodistance/GeoDistanceAggregationBuilder.java @@ -215,6 +215,7 @@ public class GeoDistanceAggregationBuilder extends ValuesSourceAggregationBuilde protected ValuesSourceAggregatorFactory innerBuild(AggregationContext context, ValuesSourceConfig config, AggregatorFactory parent, Builder subFactoriesBuilder) throws IOException { + Range[] ranges = this.ranges.toArray(new Range[this.range().size()]); return new GeoDistanceRangeAggregatorFactory(name, type, config, origin, ranges, unit, distanceType, keyed, context, parent, subFactoriesBuilder, metaData); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/geodistance/GeoDistanceRangeAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/geodistance/GeoDistanceRangeAggregatorFactory.java index 32c3592a8fc..62aa18b168a 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/geodistance/GeoDistanceRangeAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/range/geodistance/GeoDistanceRangeAggregatorFactory.java @@ -51,13 +51,13 @@ public class GeoDistanceRangeAggregatorFactory private final InternalRange.Factory rangeFactory = InternalGeoDistance.FACTORY; private final GeoPoint origin; - private final List ranges; + private final Range[] ranges; private final DistanceUnit unit; private final GeoDistance distanceType; private final boolean keyed; public GeoDistanceRangeAggregatorFactory(String name, Type type, ValuesSourceConfig config, GeoPoint origin, - List ranges, DistanceUnit unit, GeoDistance distanceType, boolean keyed, AggregationContext context, + Range[] ranges, DistanceUnit unit, GeoDistance distanceType, boolean keyed, AggregationContext context, AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder, Map metaData) throws IOException { super(name, type, config, context, parent, subFactoriesBuilder, metaData); this.origin = origin; @@ -70,7 +70,7 @@ public class GeoDistanceRangeAggregatorFactory @Override protected Aggregator createUnmapped(Aggregator parent, List pipelineAggregators, Map metaData) throws IOException { - return new Unmapped(name, ranges, keyed, config.format(), context, parent, rangeFactory, pipelineAggregators, metaData); + return new Unmapped<>(name, ranges, keyed, config.format(), context, parent, rangeFactory, pipelineAggregators, metaData); } @Override diff --git a/core/src/test/java/org/elasticsearch/indices/IndicesRequestCacheIT.java b/core/src/test/java/org/elasticsearch/indices/IndicesRequestCacheIT.java index 7b9f2dbb7cf..078bf499ff4 100644 --- a/core/src/test/java/org/elasticsearch/indices/IndicesRequestCacheIT.java +++ b/core/src/test/java/org/elasticsearch/indices/IndicesRequestCacheIT.java @@ -34,6 +34,8 @@ import org.joda.time.chrono.ISOChronology; import java.util.List; import static org.elasticsearch.search.aggregations.AggregationBuilders.dateHistogram; +import static org.elasticsearch.search.aggregations.AggregationBuilders.filter; +import static org.elasticsearch.search.aggregations.AggregationBuilders.dateRange; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.hamcrest.Matchers.equalTo; @@ -406,11 +408,33 @@ public class IndicesRequestCacheIT extends ESIntegTestCase { assertThat(client().admin().indices().prepareStats("index").setRequestCache(true).get().getTotal().getRequestCache().getMissCount(), equalTo(0L)); - // If size > 1 and cache flag is set on the request we should cache - final SearchResponse r4 = client().prepareSearch("index").setSearchType(SearchType.QUERY_THEN_FETCH).setSize(1) - .setRequestCache(true).setQuery(QueryBuilders.rangeQuery("s").gte("2016-03-21").lte("2016-03-27")).get(); + // If the request has an aggregation containng now we should not cache + final SearchResponse r4 = client().prepareSearch("index").setSearchType(SearchType.QUERY_THEN_FETCH).setSize(0) + .setRequestCache(true).setQuery(QueryBuilders.rangeQuery("s").gte("2016-03-20").lte("2016-03-26")) + .addAggregation(filter("foo", QueryBuilders.rangeQuery("s").from("now-10y").to("now"))).get(); assertSearchResponse(r4); assertThat(r4.getHits().getTotalHits(), equalTo(7L)); + assertThat(client().admin().indices().prepareStats("index").setRequestCache(true).get().getTotal().getRequestCache().getHitCount(), + equalTo(0L)); + assertThat(client().admin().indices().prepareStats("index").setRequestCache(true).get().getTotal().getRequestCache().getMissCount(), + equalTo(0L)); + + // If the request has an aggregation containng now we should not cache + final SearchResponse r5 = client().prepareSearch("index").setSearchType(SearchType.QUERY_THEN_FETCH).setSize(0) + .setRequestCache(true).setQuery(QueryBuilders.rangeQuery("s").gte("2016-03-20").lte("2016-03-26")) + .addAggregation(dateRange("foo").field("s").addRange("now-10y", "now")).get(); + assertSearchResponse(r5); + assertThat(r5.getHits().getTotalHits(), equalTo(7L)); + assertThat(client().admin().indices().prepareStats("index").setRequestCache(true).get().getTotal().getRequestCache().getHitCount(), + equalTo(0L)); + assertThat(client().admin().indices().prepareStats("index").setRequestCache(true).get().getTotal().getRequestCache().getMissCount(), + equalTo(0L)); + + // If size > 1 and cache flag is set on the request we should cache + final SearchResponse r6 = client().prepareSearch("index").setSearchType(SearchType.QUERY_THEN_FETCH).setSize(1) + .setRequestCache(true).setQuery(QueryBuilders.rangeQuery("s").gte("2016-03-21").lte("2016-03-27")).get(); + assertSearchResponse(r6); + assertThat(r6.getHits().getTotalHits(), equalTo(7L)); assertThat(client().admin().indices().prepareStats("index").setRequestCache(true).get().getTotal().getRequestCache().getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("index").setRequestCache(true).get().getTotal().getRequestCache().getMissCount(), From 7ba22bb75bd54e2a70938f686a17d63cea950228 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 5 Oct 2016 10:45:14 +0200 Subject: [PATCH 12/20] fix random score function builder to deal with empty seeds --- .../index/query/QueryRewriteContext.java | 7 +++++-- .../index/query/QueryShardContext.java | 4 ---- .../RandomScoreFunctionBuilder.java | 11 ++++++++-- .../search/internal/SearchContext.java | 3 +-- .../index/query/QueryShardContextTests.java | 3 ++- .../FunctionScoreQueryBuilderTests.java | 21 ++++++++++++------- .../rescore/QueryRescoreBuilderTests.java | 3 ++- .../search/sort/AbstractSortTestCase.java | 3 ++- .../test/AbstractQueryTestCase.java | 17 +++++++-------- .../search/MockSearchServiceTests.java | 3 ++- 10 files changed, 45 insertions(+), 30 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java index bc3c6ab52ab..48e9d134a77 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java @@ -48,7 +48,7 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { protected final Client client; protected final IndexReader reader; protected final ClusterState clusterState; - protected boolean cachable; + protected boolean cachable = true; private final SetOnce executionMode = new SetOnce<>(); public QueryRewriteContext(IndexSettings indexSettings, MapperService mapperService, ScriptService scriptService, @@ -127,7 +127,9 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { return cachable; } - public void setCachabe(boolean cachabe) { this.cachable = cachabe; } + public void setCachable(boolean cachabe) { + this.cachable = cachabe; + } public BytesReference getTemplateBytes(Script template) { failIfExecutionMode(); @@ -141,6 +143,7 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { } protected void failIfExecutionMode() { + markAsNotCachable(); if (executionMode.get() == Boolean.TRUE) { throw new IllegalArgumentException("features that prevent cachability are disabled on this context"); } else { diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index 200d5fde0c7..3869ecfd2c1 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -337,28 +337,24 @@ public class QueryShardContext extends QueryRewriteContext { public SearchScript getSearchScript(Script script, ScriptContext context, Map params) { failIfExecutionMode(); - markAsNotCachable(); return scriptService.search(lookup(), script, context, params); } public Function, SearchScript> getLazySearchScript(Script script, ScriptContext context, Map params) { failIfExecutionMode(); - markAsNotCachable(); CompiledScript compile = scriptService.compile(script, context, params); return (p) -> scriptService.search(lookup(), compile, p); } public ExecutableScript getExecutableScript(Script script, ScriptContext context, Map params) { failIfExecutionMode(); - markAsNotCachable(); return scriptService.executable(script, context, params); } public Function, ExecutableScript> getLazyExecutableScript(Script script, ScriptContext context, Map params) { failIfExecutionMode(); - markAsNotCachable(); CompiledScript executable = scriptService.compile(script, context, params); return (p) -> scriptService.executable(executable, p); } diff --git a/core/src/main/java/org/elasticsearch/index/query/functionscore/RandomScoreFunctionBuilder.java b/core/src/main/java/org/elasticsearch/index/query/functionscore/RandomScoreFunctionBuilder.java index 4f4a3b54a21..f630c0c9b76 100644 --- a/core/src/main/java/org/elasticsearch/index/query/functionscore/RandomScoreFunctionBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/functionscore/RandomScoreFunctionBuilder.java @@ -49,12 +49,19 @@ public class RandomScoreFunctionBuilder extends ScoreFunctionBuilder nowInMillis); context.setAllowUnmappedFields(false); MappedFieldType fieldType = new TextFieldMapper.TextFieldType(); diff --git a/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java index bbc7d945046..a799a5af6a9 100644 --- a/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java @@ -172,12 +172,14 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase nowInMillis) { @Override public MappedFieldType fieldMapper(String name) { TextFieldMapper.Builder builder = new TextFieldMapper.Builder(name); diff --git a/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java b/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java index 6f45a320fbc..6293401ebdd 100644 --- a/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java +++ b/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java @@ -234,8 +234,9 @@ public abstract class AbstractSortTestCase> extends EST public void onCache(ShardId shardId, Accountable accountable) { } }); + long nowInMillis = randomPositiveLong(); return new QueryShardContext(idxSettings, bitsetFilterCache, ifds, null, null, scriptService, - indicesQueriesRegistry, null, null, null, System::currentTimeMillis) { + indicesQueriesRegistry, null, null, null, () -> nowInMillis) { @Override public MappedFieldType fieldMapper(String name) { return provideMappedFieldType(name); diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java index dd93615a27d..70915eeafa4 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -580,11 +580,17 @@ public abstract class AbstractQueryTestCase> public void testToQuery() throws IOException { for (int runs = 0; runs < NUMBER_OF_TESTQUERIES; runs++) { QueryShardContext context = createShardContext(); + assert context.isCachable(); context.setAllowUnmappedFields(true); QB firstQuery = createTestQueryBuilder(); QB controlQuery = copyQuery(firstQuery); setSearchContext(randomTypes, context); // only set search context for toQuery to be more realistic Query firstLuceneQuery = rewriteQuery(firstQuery, context).toQuery(context); + if (isCachable(firstQuery)) { + assert context.isCachable() : firstQuery.toString(); + } else { + assert context.isCachable() == false : firstQuery.toString(); + } assertNotNull("toQuery should not return null", firstLuceneQuery); assertLuceneQuery(firstQuery, firstLuceneQuery, context); //remove after assertLuceneQuery since the assertLuceneQuery impl might access the context as well @@ -630,15 +636,7 @@ public abstract class AbstractQueryTestCase> } private QueryBuilder rewriteQuery(QB queryBuilder, QueryRewriteContext rewriteContext) throws IOException { - final boolean wasCachable = rewriteContext.isCachable(); QueryBuilder rewritten = QueryBuilder.rewriteQuery(queryBuilder, rewriteContext); - if (wasCachable == true) { // it's not resettable so we can only check it once - if (isCachable(queryBuilder)) { - assert rewriteContext.isCachable() : queryBuilder.toString(); - } else { - assert rewriteContext.isCachable() == false; - } - } // extra safety to fail fast - serialize the rewritten version to ensure it's serializable. assertSerialization(rewritten); return rewritten; @@ -1032,6 +1030,7 @@ public abstract class AbstractQueryTestCase> private final BitsetFilterCache bitsetFilterCache; private final ScriptService scriptService; private final Client client; + private final long nowInMillis = randomPositiveLong(); ServiceHolder(Settings nodeSettings, Settings indexSettings, Collection> plugins, AbstractQueryTestCase testCase) throws IOException { @@ -1110,7 +1109,7 @@ public abstract class AbstractQueryTestCase> QueryShardContext createShardContext() { ClusterState state = ClusterState.builder(new ClusterName("_name")).build(); return new QueryShardContext(idxSettings, bitsetFilterCache, indexFieldDataService, mapperService, similarityService, - scriptService, indicesQueriesRegistry, this.client, null, state, System::currentTimeMillis); + scriptService, indicesQueriesRegistry, this.client, null, state, () -> nowInMillis); } ScriptModule createScriptModule(List scriptPlugins) { diff --git a/test/framework/src/test/java/org/elasticsearch/search/MockSearchServiceTests.java b/test/framework/src/test/java/org/elasticsearch/search/MockSearchServiceTests.java index f61a3a2b48e..8353a58e969 100644 --- a/test/framework/src/test/java/org/elasticsearch/search/MockSearchServiceTests.java +++ b/test/framework/src/test/java/org/elasticsearch/search/MockSearchServiceTests.java @@ -33,8 +33,9 @@ import org.elasticsearch.test.TestSearchContext; public class MockSearchServiceTests extends ESTestCase { public void testAssertNoInFlightContext() { + final long nowInMillis = randomPositiveLong(); SearchContext s = new TestSearchContext(new QueryShardContext(new IndexSettings(IndexMetaData.PROTO, Settings.EMPTY), null, null, - null, null, null, null, null, null, null, System::currentTimeMillis)) { + null, null, null, null, null, null, null, () -> nowInMillis)) { @Override public SearchShardTarget shardTarget() { return new SearchShardTarget("node", new Index("idx", "ignored"), 0); From e556c289b99a4d86ff9793da1f41f92484db56a0 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 5 Oct 2016 11:41:49 +0200 Subject: [PATCH 13/20] use a private rewrite context to prevent exposing isCachable --- .../index/query/QueryRewriteContext.java | 13 ++++--------- .../org/elasticsearch/search/SearchService.java | 8 +------- .../search/internal/SearchContext.java | 4 ---- .../elasticsearch/test/AbstractQueryTestCase.java | 6 +++++- 4 files changed, 10 insertions(+), 21 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java index 48e9d134a77..c3d27b5d615 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java @@ -119,18 +119,13 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { return new QueryParseContext(defaultScriptLanguage, indicesQueriesRegistry, parser, indexSettings.getParseFieldMatcher()); } - protected final void markAsNotCachable() { - this.cachable = false; - } - + /** + * Returns true iff the result of the processed search request is cachable. Otherwise false + */ public boolean isCachable() { return cachable; } - public void setCachable(boolean cachabe) { - this.cachable = cachabe; - } - public BytesReference getTemplateBytes(Script template) { failIfExecutionMode(); ExecutableScript executable = scriptService.executable(template, @@ -143,7 +138,7 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { } protected void failIfExecutionMode() { - markAsNotCachable(); + this.cachable = false; if (executionMode.get() == Boolean.TRUE) { throw new IllegalArgumentException("features that prevent cachability are disabled on this context"); } else { diff --git a/core/src/main/java/org/elasticsearch/search/SearchService.java b/core/src/main/java/org/elasticsearch/search/SearchService.java index 3a9d0d58dc5..186ea231847 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchService.java +++ b/core/src/main/java/org/elasticsearch/search/SearchService.java @@ -521,13 +521,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv DefaultSearchContext context = createSearchContext(request, defaultSearchTimeout, searcher); SearchContext.setCurrent(context); try { - 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.resetCanCache(); + request.rewrite(new QueryShardContext(context.getQueryShardContext())); if (request.scroll() != null) { context.scrollContext(new ScrollContext()); context.scrollContext().scroll = request.scroll(); 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 f10344c6866..78fbc492c50 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java @@ -163,10 +163,6 @@ public abstract class SearchContext extends AbstractRefCounted implements Releas return getQueryShardContext().isCachable(); } - public final void resetCanCache() { - getQueryShardContext().setCachable(true); - } - public abstract ScrollContext scrollContext(); public abstract SearchContext scrollContext(ScrollContext scroll); diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java index 70915eeafa4..16fd64dc941 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -585,7 +585,11 @@ public abstract class AbstractQueryTestCase> QB firstQuery = createTestQueryBuilder(); QB controlQuery = copyQuery(firstQuery); setSearchContext(randomTypes, context); // only set search context for toQuery to be more realistic - Query firstLuceneQuery = rewriteQuery(firstQuery, context).toQuery(context); + /* we use a private rewrite context here since we want the most realistic way of asserting that we are cachabel or not. + * We do it this way in SearchService where + * we first rewrite the query with a private context, then reset the context and then build the actual lucene query*/ + QueryBuilder rewritten = rewriteQuery(firstQuery, new QueryShardContext(context)); + Query firstLuceneQuery = rewritten.toQuery(context); if (isCachable(firstQuery)) { assert context.isCachable() : firstQuery.toString(); } else { From 5687549ad8791b15db54206fe8124cf6c0d6b4f5 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 5 Oct 2016 12:14:48 +0200 Subject: [PATCH 14/20] clone the entire serach context for rewriting --- .../index/query/QueryRewriteContext.java | 4 ++++ .../index/query/QueryShardContext.java | 15 ++++++++++++++- .../search/DefaultSearchContext.java | 7 +++++++ .../org/elasticsearch/search/SearchService.java | 15 +++++++++++---- 4 files changed, 36 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java index c3d27b5d615..55ba778d012 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java @@ -137,6 +137,10 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { this.executionMode.set(Boolean.TRUE); } + /** + * This method fails if {@link #setExecutionMode()} is called before on this context. + * This is used to seal + */ protected void failIfExecutionMode() { this.cachable = false; if (executionMode.get() == Boolean.TRUE) { diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index 3869ecfd2c1..d15c69e088e 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -335,11 +335,17 @@ public class QueryShardContext extends QueryRewriteContext { return indexSettings.getIndex(); } + /** + * Compiles (or retrieves from cache) and executes the provided script + */ public SearchScript getSearchScript(Script script, ScriptContext context, Map params) { failIfExecutionMode(); return scriptService.search(lookup(), script, context, params); } - + /** + * Returns a lazily created {@link SearchScript} that is compiled immediately but can be pulled later once all + * parameters are available. + */ public Function, SearchScript> getLazySearchScript(Script script, ScriptContext context, Map params) { failIfExecutionMode(); @@ -347,11 +353,18 @@ public class QueryShardContext extends QueryRewriteContext { return (p) -> scriptService.search(lookup(), compile, p); } + /** + * Compiles (or retrieves from cache) and executes the provided script + */ public ExecutableScript getExecutableScript(Script script, ScriptContext context, Map params) { failIfExecutionMode(); return scriptService.executable(script, context, params); } + /** + * Returns a lazily created {@link ExecutableScript} that is compiled immediately but can be pulled later once all + * parameters are available. + */ public Function, ExecutableScript> getLazyExecutableScript(Script script, ScriptContext context, Map params) { failIfExecutionMode(); diff --git a/core/src/main/java/org/elasticsearch/search/DefaultSearchContext.java b/core/src/main/java/org/elasticsearch/search/DefaultSearchContext.java index 26af6d85d06..f8299693dcf 100644 --- a/core/src/main/java/org/elasticsearch/search/DefaultSearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/DefaultSearchContext.java @@ -172,6 +172,13 @@ final class DefaultSearchContext extends SearchContext { queryShardContext.setTypes(request.types()); } + DefaultSearchContext(DefaultSearchContext source) { + this(source.id(), source.request(), source.shardTarget(), source.engineSearcher, source.indexService, source.indexShard(), + source.bigArrays(), source.timeEstimateCounter(), source.parseFieldMatcher(), source.timeout(), source.fetchPhase()); + } + + + @Override public void doClose() { // clear and scope phase we have diff --git a/core/src/main/java/org/elasticsearch/search/SearchService.java b/core/src/main/java/org/elasticsearch/search/SearchService.java index 186ea231847..ff6a359f0b0 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchService.java +++ b/core/src/main/java/org/elasticsearch/search/SearchService.java @@ -517,11 +517,18 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv } final SearchContext createContext(ShardSearchRequest request, @Nullable Engine.Searcher searcher) throws IOException { - - DefaultSearchContext context = createSearchContext(request, defaultSearchTimeout, searcher); - SearchContext.setCurrent(context); + final DefaultSearchContext context = createSearchContext(request, defaultSearchTimeout, searcher); try { - request.rewrite(new QueryShardContext(context.getQueryShardContext())); + // we clone the search context here just for rewriting otherwise we + // might end up with incorrect state since we are using now() or script services + // during rewrite and normalized / evaluate templates etc. + // NOTE this context doesn't need to be closed - the outer context will + // take care of this. + DefaultSearchContext rewriteContext = new DefaultSearchContext(context); + SearchContext.setCurrent(rewriteContext); + request.rewrite(rewriteContext.getQueryShardContext()); + SearchContext.setCurrent(context); + assert context.getQueryShardContext().isCachable(); if (request.scroll() != null) { context.scrollContext(new ScrollContext()); context.scrollContext().scroll = request.scroll(); From a008959f7a1b9fbe970a9631a96c73ac7a1458df Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 5 Oct 2016 12:22:54 +0200 Subject: [PATCH 15/20] cleanup freeze methods and move them down to QueryShardContext --- .../index/query/QueryRewriteContext.java | 25 ---------- .../index/query/QueryShardContext.java | 48 +++++++++++++++++-- .../elasticsearch/indices/IndicesService.java | 2 +- .../elasticsearch/search/SearchService.java | 2 +- .../search/internal/SearchContext.java | 4 -- 5 files changed, 45 insertions(+), 36 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java index 55ba778d012..d7637f46370 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java @@ -48,8 +48,6 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { protected final Client client; protected final IndexReader reader; protected final ClusterState clusterState; - protected boolean cachable = true; - private final SetOnce executionMode = new SetOnce<>(); public QueryRewriteContext(IndexSettings indexSettings, MapperService mapperService, ScriptService scriptService, IndicesQueriesRegistry indicesQueriesRegistry, Client client, IndexReader reader, @@ -119,34 +117,11 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { return new QueryParseContext(defaultScriptLanguage, indicesQueriesRegistry, parser, indexSettings.getParseFieldMatcher()); } - /** - * Returns true iff the result of the processed search request is cachable. Otherwise false - */ - public boolean isCachable() { - return cachable; - } - public BytesReference getTemplateBytes(Script template) { - failIfExecutionMode(); ExecutableScript executable = scriptService.executable(template, ScriptContext.Standard.SEARCH, Collections.emptyMap()); return (BytesReference) executable.run(); } - public void setExecutionMode() { - this.executionMode.set(Boolean.TRUE); - } - /** - * This method fails if {@link #setExecutionMode()} is called before on this context. - * This is used to seal - */ - protected void failIfExecutionMode() { - this.cachable = false; - if (executionMode.get() == Boolean.TRUE) { - throw new IllegalArgumentException("features that prevent cachability are disabled on this context"); - } else { - assert executionMode.get() == null : executionMode.get(); - } - } } diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index d15c69e088e..abe573c0767 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -36,11 +36,13 @@ import org.apache.lucene.queryparser.classic.QueryParserSettings; import org.apache.lucene.search.Query; import org.apache.lucene.search.join.BitSetProducer; import org.apache.lucene.search.similarities.Similarity; +import org.apache.lucene.util.SetOnce; import org.elasticsearch.Version; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexSettings; @@ -77,6 +79,8 @@ public class QueryShardContext extends QueryRewriteContext { private final IndexFieldDataService indexFieldDataService; private final IndexSettings indexSettings; private String[] types = Strings.EMPTY_ARRAY; + private boolean cachable = true; + private final SetOnce frozen = new SetOnce<>(); public void setTypes(String... types) { this.types = types; @@ -271,7 +275,7 @@ public class QueryShardContext extends QueryRewriteContext { } public long nowInMillis() { - failIfExecutionMode(); + failIfFrozen(); return nowInMillis.getAsLong(); } @@ -339,7 +343,7 @@ public class QueryShardContext extends QueryRewriteContext { * Compiles (or retrieves from cache) and executes the provided script */ public SearchScript getSearchScript(Script script, ScriptContext context, Map params) { - failIfExecutionMode(); + failIfFrozen(); return scriptService.search(lookup(), script, context, params); } /** @@ -348,7 +352,7 @@ public class QueryShardContext extends QueryRewriteContext { */ public Function, SearchScript> getLazySearchScript(Script script, ScriptContext context, Map params) { - failIfExecutionMode(); + failIfFrozen(); CompiledScript compile = scriptService.compile(script, context, params); return (p) -> scriptService.search(lookup(), compile, p); } @@ -357,7 +361,7 @@ public class QueryShardContext extends QueryRewriteContext { * Compiles (or retrieves from cache) and executes the provided script */ public ExecutableScript getExecutableScript(Script script, ScriptContext context, Map params) { - failIfExecutionMode(); + failIfFrozen(); return scriptService.executable(script, context, params); } @@ -367,8 +371,42 @@ public class QueryShardContext extends QueryRewriteContext { */ public Function, ExecutableScript> getLazyExecutableScript(Script script, ScriptContext context, Map params) { - failIfExecutionMode(); + failIfFrozen(); CompiledScript executable = scriptService.compile(script, context, params); return (p) -> scriptService.executable(executable, p); } + + /** + * if this method is called the query context will throw exception if methods are accessed + * that could yield different results across executions like {@link #getTemplateBytes(Script)} + */ + public void freezeContext() { + this.frozen.set(Boolean.TRUE); + } + + /** + * This method fails if {@link #freezeContext()} is called before on this context. + * This is used to seal + */ + protected void failIfFrozen() { + this.cachable = false; + if (frozen.get() == Boolean.TRUE) { + throw new IllegalArgumentException("features that prevent cachability are disabled on this context"); + } else { + assert frozen.get() == null : frozen.get(); + } + } + + @Override + public BytesReference getTemplateBytes(Script template) { + failIfFrozen(); + return super.getTemplateBytes(template); + } + + /** + * Returns true iff the result of the processed search request is cachable. Otherwise false + */ + public boolean isCachable() { + return cachable; + } } diff --git a/core/src/main/java/org/elasticsearch/indices/IndicesService.java b/core/src/main/java/org/elasticsearch/indices/IndicesService.java index b658f70ed58..48947373601 100644 --- a/core/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/core/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -1082,7 +1082,7 @@ public class IndicesService extends AbstractLifecycleComponent } // if now in millis is used (or in the future, a more generic "isDeterministic" flag // then we can't cache based on "now" key within the search request, as it is not deterministic - if (context.isCachable() == false) { + if (context.getQueryShardContext().isCachable() == false) { return false; } return true; diff --git a/core/src/main/java/org/elasticsearch/search/SearchService.java b/core/src/main/java/org/elasticsearch/search/SearchService.java index ff6a359f0b0..c3fbd123fc8 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchService.java +++ b/core/src/main/java/org/elasticsearch/search/SearchService.java @@ -231,7 +231,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv */ private void loadOrExecuteQueryPhase(final ShardSearchRequest request, final SearchContext context) throws Exception { final boolean canCache = indicesService.canCache(request, context); - context.getQueryShardContext().setExecutionMode(); + context.getQueryShardContext().freezeContext(); if (canCache) { indicesService.loadIntoContext(request, context, queryPhase); } else { 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 78fbc492c50..35bfb00dc46 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java @@ -159,10 +159,6 @@ public abstract class SearchContext extends AbstractRefCounted implements Releas public abstract long getOriginNanoTime(); - public final boolean isCachable() { - return getQueryShardContext().isCachable(); - } - public abstract ScrollContext scrollContext(); public abstract SearchContext scrollContext(ScrollContext scroll); From 7bffe95025dab28ebff25b73f8b6e7bd9c434d17 Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Wed, 5 Oct 2016 15:03:29 +0100 Subject: [PATCH 16/20] Fix percolator queries to not be cacheable --- .../percolator/PercolateQueryBuilder.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java index 74ce3a5be1e..4a0d8383f55 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -366,6 +366,9 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder Date: Wed, 5 Oct 2016 16:38:47 +0200 Subject: [PATCH 17/20] PercolateQuery is never cacheable --- .../elasticsearch/percolator/PercolateQueryBuilderTests.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java index abb75b4dbd6..92147a63f53 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java @@ -248,4 +248,9 @@ public class PercolateQueryBuilderTests extends AbstractQueryTestCase Date: Wed, 5 Oct 2016 20:43:46 +0200 Subject: [PATCH 18/20] add percolate with script query test --- .../percolator/PercolatorIT.java | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorIT.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorIT.java index 7d10b831bc8..a3384d2844a 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorIT.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorIT.java @@ -36,12 +36,16 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.engine.VersionConflictEngineException; +import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.index.mapper.MapperParsingException; import org.elasticsearch.index.query.Operator; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.index.query.QueryShardException; import org.elasticsearch.index.query.functionscore.WeightBuilder; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.script.MockScriptPlugin; +import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptService; import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilder; import org.elasticsearch.test.ESIntegTestCase; @@ -59,6 +63,7 @@ import java.util.Map; import java.util.NavigableSet; import java.util.Set; import java.util.TreeSet; +import java.util.function.Function; import static org.elasticsearch.percolator.PercolateSourceBuilder.docBuilder; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -96,7 +101,16 @@ public class PercolatorIT extends ESIntegTestCase { @Override protected Collection> nodePlugins() { - return Collections.singleton(PercolatorPlugin.class); + return Arrays.asList(PercolatorPlugin.class, CustomScriptPlugin.class); + } + + public static class CustomScriptPlugin extends MockScriptPlugin { + @Override + protected Map, Object>> pluginScripts() { + Map, Object>> scripts = new HashMap<>(); + scripts.put("1==1", vars -> Boolean.TRUE); + return scripts; + } } @Override @@ -104,6 +118,23 @@ public class PercolatorIT extends ESIntegTestCase { return Collections.singleton(PercolatorPlugin.class); } + public void testPercolateScriptQuery() throws IOException { + client().admin().indices().prepareCreate(INDEX_NAME).addMapping(TYPE_NAME, "query", "type=percolator").get(); + ensureGreen(); + client().prepareIndex(INDEX_NAME, TYPE_NAME, "1") + .setSource(jsonBuilder().startObject().field("query", QueryBuilders.scriptQuery( + new Script("1==1", ScriptService.ScriptType.INLINE, CustomScriptPlugin.NAME, null))).endObject()) + .execute().actionGet(); + refresh(); + PercolateResponse response = preparePercolate(client()) + .setIndices(INDEX_NAME).setDocumentType(TYPE_NAME) + .setPercolateDoc(docBuilder().setDoc(jsonBuilder().startObject().field("field1", "b").endObject())) + .execute().actionGet(); + assertMatchCount(response, 1L); + assertThat(response.getMatches(), arrayWithSize(1)); + assertThat(convertFromTextArray(response.getMatches(), INDEX_NAME), arrayContainingInAnyOrder("1")); + } + public void testSimple1() throws Exception { client().admin().indices().prepareCreate(INDEX_NAME).addMapping(TYPE_NAME, "query", "type=percolator").get(); ensureGreen(); From ce21b607bb697540f59f7e2c9437a9e56baf307f Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 5 Oct 2016 21:55:50 +0200 Subject: [PATCH 19/20] move test to a single node test --- .../percolator/PercolatorIT.java | 29 +------------ .../percolator/PercolatorQuerySearchIT.java | 43 ++++++++++++++++++- 2 files changed, 43 insertions(+), 29 deletions(-) diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorIT.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorIT.java index a3384d2844a..197a82f2ccc 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorIT.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorIT.java @@ -36,7 +36,6 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.engine.VersionConflictEngineException; -import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.index.mapper.MapperParsingException; import org.elasticsearch.index.query.Operator; import org.elasticsearch.index.query.QueryBuilders; @@ -70,7 +69,6 @@ import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.common.xcontent.XContentFactory.smileBuilder; import static org.elasticsearch.common.xcontent.XContentFactory.yamlBuilder; import static org.elasticsearch.index.query.QueryBuilders.boolQuery; -import static org.elasticsearch.index.query.QueryBuilders.constantScoreQuery; import static org.elasticsearch.index.query.QueryBuilders.functionScoreQuery; import static org.elasticsearch.index.query.QueryBuilders.geoShapeQuery; import static org.elasticsearch.index.query.QueryBuilders.hasChildQuery; @@ -101,40 +99,15 @@ public class PercolatorIT extends ESIntegTestCase { @Override protected Collection> nodePlugins() { - return Arrays.asList(PercolatorPlugin.class, CustomScriptPlugin.class); + return Collections.singleton(PercolatorPlugin.class); } - public static class CustomScriptPlugin extends MockScriptPlugin { - @Override - protected Map, Object>> pluginScripts() { - Map, Object>> scripts = new HashMap<>(); - scripts.put("1==1", vars -> Boolean.TRUE); - return scripts; - } - } @Override protected Collection> transportClientPlugins() { return Collections.singleton(PercolatorPlugin.class); } - public void testPercolateScriptQuery() throws IOException { - client().admin().indices().prepareCreate(INDEX_NAME).addMapping(TYPE_NAME, "query", "type=percolator").get(); - ensureGreen(); - client().prepareIndex(INDEX_NAME, TYPE_NAME, "1") - .setSource(jsonBuilder().startObject().field("query", QueryBuilders.scriptQuery( - new Script("1==1", ScriptService.ScriptType.INLINE, CustomScriptPlugin.NAME, null))).endObject()) - .execute().actionGet(); - refresh(); - PercolateResponse response = preparePercolate(client()) - .setIndices(INDEX_NAME).setDocumentType(TYPE_NAME) - .setPercolateDoc(docBuilder().setDoc(jsonBuilder().startObject().field("field1", "b").endObject())) - .execute().actionGet(); - assertMatchCount(response, 1L); - assertThat(response.getMatches(), arrayWithSize(1)); - assertThat(convertFromTextArray(response.getMatches(), INDEX_NAME), arrayContainingInAnyOrder("1")); - } - public void testSimple1() throws Exception { client().admin().indices().prepareCreate(INDEX_NAME).addMapping(TYPE_NAME, "query", "type=percolator").get(); ensureGreen(); diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java index b21c131a625..828f800f34e 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java @@ -21,6 +21,7 @@ package org.elasticsearch.percolator; import org.apache.lucene.search.join.ScoreMode; import org.elasticsearch.action.search.SearchPhaseExecutionException; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -31,12 +32,20 @@ import org.elasticsearch.index.query.MultiMatchQueryBuilder; import org.elasticsearch.index.query.Operator; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.script.MockScriptPlugin; +import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptService; import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilder; import org.elasticsearch.search.sort.SortOrder; import org.elasticsearch.test.ESSingleNodeTestCase; +import java.io.IOException; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.function.Function; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.index.query.QueryBuilders.boolQuery; @@ -49,7 +58,13 @@ import static org.elasticsearch.index.query.QueryBuilders.spanNearQuery; import static org.elasticsearch.index.query.QueryBuilders.spanNotQuery; import static org.elasticsearch.index.query.QueryBuilders.spanTermQuery; import static org.elasticsearch.index.query.QueryBuilders.termQuery; +import static org.elasticsearch.percolator.PercolateSourceBuilder.docBuilder; +import static org.elasticsearch.percolator.PercolatorTestUtil.assertMatchCount; +import static org.elasticsearch.percolator.PercolatorTestUtil.convertFromTextArray; +import static org.elasticsearch.percolator.PercolatorTestUtil.preparePercolate; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; +import static org.hamcrest.Matchers.arrayContainingInAnyOrder; +import static org.hamcrest.Matchers.arrayWithSize; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; @@ -58,7 +73,33 @@ public class PercolatorQuerySearchIT extends ESSingleNodeTestCase { @Override protected Collection> getPlugins() { - return Collections.singleton(PercolatorPlugin.class); + return Arrays.asList(PercolatorPlugin.class, CustomScriptPlugin.class); + } + + public static class CustomScriptPlugin extends MockScriptPlugin { + @Override + protected Map, Object>> pluginScripts() { + Map, Object>> scripts = new HashMap<>(); + scripts.put("1==1", vars -> Boolean.TRUE); + return scripts; + } + } + + public void testPercolateScriptQuery() throws IOException { + client().admin().indices().prepareCreate("index").addMapping("type", "query", "type=percolator").get(); + ensureGreen(); + client().prepareIndex("index", "type", "1") + .setSource(jsonBuilder().startObject().field("query", QueryBuilders.scriptQuery( + new Script("1==1", ScriptService.ScriptType.INLINE, CustomScriptPlugin.NAME, null))).endObject()) + .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) + .execute().actionGet(); + PercolateResponse response = preparePercolate(client()) + .setIndices("index").setDocumentType("type") + .setPercolateDoc(docBuilder().setDoc(jsonBuilder().startObject().field("field1", "b").endObject())) + .execute().actionGet(); + assertMatchCount(response, 1L); + assertThat(response.getMatches(), arrayWithSize(1)); + assertThat(convertFromTextArray(response.getMatches(), "index"), arrayContainingInAnyOrder("1")); } public void testPercolatorQuery() throws Exception { From ce6f6d3835d682dda631f7373e5de2588d8c0512 Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Thu, 6 Oct 2016 08:55:31 +0100 Subject: [PATCH 20/20] Review comments --- .../org/elasticsearch/index/query/QueryShardContext.java | 6 ++++-- .../main/java/org/elasticsearch/script/ScriptService.java | 6 +++++- .../java/org/elasticsearch/test/AbstractQueryTestCase.java | 6 ++++-- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index abe573c0767..11af6f919ae 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -340,7 +340,8 @@ public class QueryShardContext extends QueryRewriteContext { } /** - * Compiles (or retrieves from cache) and executes the provided script + * Compiles (or retrieves from cache) and binds the parameters to the + * provided script */ public SearchScript getSearchScript(Script script, ScriptContext context, Map params) { failIfFrozen(); @@ -358,7 +359,8 @@ public class QueryShardContext extends QueryRewriteContext { } /** - * Compiles (or retrieves from cache) and executes the provided script + * Compiles (or retrieves from cache) and binds the parameters to the + * provided script */ public ExecutableScript getExecutableScript(Script script, ScriptContext context, Map params) { failIfFrozen(); diff --git a/core/src/main/java/org/elasticsearch/script/ScriptService.java b/core/src/main/java/org/elasticsearch/script/ScriptService.java index e63a29aac50..24cb816fb15 100644 --- a/core/src/main/java/org/elasticsearch/script/ScriptService.java +++ b/core/src/main/java/org/elasticsearch/script/ScriptService.java @@ -249,7 +249,7 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust long timePassed = now - lastInlineCompileTime; lastInlineCompileTime = now; - scriptsPerMinCounter += ((double) timePassed) * compilesAllowedPerNano; + scriptsPerMinCounter += (timePassed) * compilesAllowedPerNano; // It's been over the time limit anyway, readjust the bucket to be level if (scriptsPerMinCounter > totalCompilesPerMinute) { @@ -491,6 +491,10 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust return search(lookup, compiledScript, script.getParams()); } + /** + * Binds provided parameters to a compiled script returning a + * {@link SearchScript} ready for execution + */ public SearchScript search(SearchLookup lookup, CompiledScript compiledScript, Map params) { return getScriptEngineServiceForLang(compiledScript.lang()).search(compiledScript, lookup, params); } diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java index 16fd64dc941..711c74375f1 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -591,9 +591,11 @@ public abstract class AbstractQueryTestCase> QueryBuilder rewritten = rewriteQuery(firstQuery, new QueryShardContext(context)); Query firstLuceneQuery = rewritten.toQuery(context); if (isCachable(firstQuery)) { - assert context.isCachable() : firstQuery.toString(); + assertTrue("query was marked as not cacheable in the context but this test indicates it should be cacheable: " + + firstQuery.toString(), context.isCachable()); } else { - assert context.isCachable() == false : firstQuery.toString(); + assertFalse("query was marked as cacheable in the context but this test indicates it should not be cacheable: " + + firstQuery.toString(), context.isCachable()); } assertNotNull("toQuery should not return null", firstLuceneQuery); assertLuceneQuery(firstQuery, firstLuceneQuery, context);