diff --git a/server/src/test/java/org/elasticsearch/index/query/FullTextQueryTestCase.java b/server/src/test/java/org/elasticsearch/index/query/FullTextQueryTestCase.java deleted file mode 100644 index 7f9d85e3e26b7..0000000000000 --- a/server/src/test/java/org/elasticsearch/index/query/FullTextQueryTestCase.java +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.index.query; - -import org.elasticsearch.index.mapper.DateFieldMapper; -import org.elasticsearch.index.mapper.MappedFieldType; -import org.elasticsearch.test.AbstractQueryTestCase; - -import java.util.Collection; -import java.util.HashSet; -import java.util.Set; - -public abstract class FullTextQueryTestCase> extends AbstractQueryTestCase { - protected abstract boolean isCacheable(QB queryBuilder); - - /** - * Full text queries that start with "now" are not cacheable if they - * target a {@link DateFieldMapper.DateFieldType} field. - */ - protected final boolean isCacheable(Collection fields, String value) { - if (value.length() < 3 - || value.substring(0, 3).equalsIgnoreCase("now") == false) { - return true; - } - Set dateFields = new HashSet<>(); - getMapping().forEach(ft -> { - if (ft instanceof DateFieldMapper.DateFieldType) { - dateFields.add(ft.name()); - } - }); - for (MappedFieldType ft : getMapping()) { - if (ft instanceof DateFieldMapper.DateFieldType) { - dateFields.add(ft.name()); - } - } - if (fields.isEmpty()) { - // special case: all fields are requested - return dateFields.isEmpty(); - } - return fields.stream() - .anyMatch(dateFields::contains) == false; - } -} diff --git a/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java index 76ea5aa9dc6a0..99c7a1e7fdc14 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java @@ -45,6 +45,7 @@ import org.elasticsearch.index.search.MatchQuery.Type; import org.elasticsearch.index.search.MatchQuery.ZeroTermsQuery; import org.elasticsearch.search.internal.SearchContext; +import org.elasticsearch.test.AbstractQueryTestCase; import org.hamcrest.Matcher; import java.io.IOException; @@ -54,19 +55,13 @@ import java.util.Locale; import java.util.Map; -import static java.util.Collections.singletonList; import static org.hamcrest.CoreMatchers.either; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.notNullValue; -public class MatchQueryBuilderTests extends FullTextQueryTestCase { - @Override - protected boolean isCacheable(MatchQueryBuilder queryBuilder) { - return queryBuilder.fuzziness() != null - || isCacheable(singletonList(queryBuilder.fieldName()), queryBuilder.value().toString()); - } +public class MatchQueryBuilderTests extends AbstractQueryTestCase { @Override protected MatchQueryBuilder doCreateTestQueryBuilder() { @@ -513,4 +508,22 @@ private static CannedBinaryTokenStream.BinaryToken[] createGiantGraphMultiTerms( } return tokens.toArray(new CannedBinaryTokenStream.BinaryToken[0]); } + + /** + * "now" on date fields should make the query non-cachable. + */ + public void testCachingStrategiesWithNow() throws IOException { + // if we hit a date field with "now", this should diable cachability + MatchQueryBuilder queryBuilder = new MatchQueryBuilder(DATE_FIELD_NAME, "now"); + QueryShardContext context = createShardContext(); + assert context.isCacheable(); + /* + * We use a private rewrite context here since we want the most realistic way of asserting that we are cacheable 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(queryBuilder, new QueryShardContext(context)); + assertNotNull(rewritten.toQuery(context)); + assertFalse("query should not be cacheable: " + queryBuilder.toString(), context.isCacheable()); + } } diff --git a/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java index ef810d6686c4b..3bbcca56beca7 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java @@ -41,6 +41,7 @@ import org.elasticsearch.index.query.MultiMatchQueryBuilder.Type; import org.elasticsearch.index.search.MatchQuery; import org.elasticsearch.search.internal.SearchContext; +import org.elasticsearch.test.AbstractQueryTestCase; import java.io.IOException; import java.util.Arrays; @@ -58,16 +59,10 @@ import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.collection.IsCollectionWithSize.hasSize; -public class MultiMatchQueryBuilderTests extends FullTextQueryTestCase { +public class MultiMatchQueryBuilderTests extends AbstractQueryTestCase { private static final String MISSING_WILDCARD_FIELD_NAME = "missing_*"; private static final String MISSING_FIELD_NAME = "missing"; - @Override - protected boolean isCacheable(MultiMatchQueryBuilder queryBuilder) { - return queryBuilder.fuzziness() != null - || isCacheable(queryBuilder.fields().keySet(), queryBuilder.value().toString()); - } - @Override protected MultiMatchQueryBuilder doCreateTestQueryBuilder() { String fieldName = randomFrom(STRING_FIELD_NAME, INT_FIELD_NAME, DOUBLE_FIELD_NAME, BOOLEAN_FIELD_NAME, DATE_FIELD_NAME, @@ -555,4 +550,22 @@ private void assertQueryWithAllFieldsWildcard(Query query) { assertThat(disjunctionMaxQuery.getDisjuncts(), hasItems(new TermQuery(new Term(STRING_FIELD_NAME, "hello")), new TermQuery(new Term(STRING_FIELD_NAME_2, "hello")))); } + + /** + * "now" on date fields should make the query non-cachable. + */ + public void testCachingStrategiesWithNow() throws IOException { + // if we hit a date field with "now", this should diable cachability + MultiMatchQueryBuilder queryBuilder = new MultiMatchQueryBuilder("now", DATE_FIELD_NAME); + QueryShardContext context = createShardContext(); + assert context.isCacheable(); + /* + * We use a private rewrite context here since we want the most realistic way of asserting that we are cacheable 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(queryBuilder, new QueryShardContext(context)); + assertNotNull(rewritten.toQuery(context)); + assertFalse("query should not be cacheable: " + queryBuilder.toString(), context.isCacheable()); + } } diff --git a/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java index 723417fb85207..ad6152e96737f 100644 --- a/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java @@ -61,6 +61,7 @@ import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.search.QueryStringQueryParser; import org.elasticsearch.search.internal.SearchContext; +import org.elasticsearch.test.AbstractQueryTestCase; import org.hamcrest.CoreMatchers; import org.hamcrest.Matchers; @@ -71,6 +72,7 @@ import java.util.Arrays; import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -83,12 +85,7 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.instanceOf; -public class QueryStringQueryBuilderTests extends FullTextQueryTestCase { - @Override - protected boolean isCacheable(QueryStringQueryBuilder queryBuilder) { - return queryBuilder.fuzziness() != null - || isCacheable(queryBuilder.fields().keySet(), queryBuilder.queryString()); - } +public class QueryStringQueryBuilderTests extends AbstractQueryTestCase { @Override protected void initializeAdditionalMappings(MapperService mapperService) throws IOException { @@ -108,8 +105,11 @@ protected QueryStringQueryBuilder doCreateTestQueryBuilder() { int numTerms = randomIntBetween(0, 5); String query = ""; for (int i = 0; i < numTerms; i++) { - //min length 4 makes sure that the text is not an operator (AND/OR) so toQuery won't break - query += (randomBoolean() ? STRING_FIELD_NAME + ":" : "") + randomAlphaOfLengthBetween(4, 10) + " "; + // min length 4 makes sure that the text is not an operator (AND/OR) so toQuery won't break + // also avoid "now" since we might hit dqte fields later and this complicates caching checks + String term = randomValueOtherThanMany(s -> s.toLowerCase(Locale.ROOT).contains("now"), + () -> randomAlphaOfLengthBetween(4, 10)); + query += (randomBoolean() ? STRING_FIELD_NAME + ":" : "") + term + " "; } QueryStringQueryBuilder queryStringQueryBuilder = new QueryStringQueryBuilder(query); if (randomBoolean()) { @@ -1578,4 +1578,39 @@ private void assertQueryWithAllFieldsWildcard(Query query) { assertThat(disjunctionMaxQuery.getDisjuncts(), hasItems(new TermQuery(new Term(STRING_FIELD_NAME, "hello")), new TermQuery(new Term(STRING_FIELD_NAME_2, "hello")))); } + + /** + * Query terms that contain "now" can trigger a query to not be cacheable. + * This test checks the search context cacheable flag is updated accordingly. + */ + public void testCachingStrategiesWithNow() throws IOException { + // if we hit all fields, this should contain a date field and should diable cachability + String query = "now " + randomAlphaOfLengthBetween(4, 10); + QueryStringQueryBuilder queryStringQueryBuilder = new QueryStringQueryBuilder(query); + assertQueryCachability(queryStringQueryBuilder, false); + + // if we hit a date field with "now", this should diable cachability + queryStringQueryBuilder = new QueryStringQueryBuilder("now"); + queryStringQueryBuilder.field(DATE_FIELD_NAME); + assertQueryCachability(queryStringQueryBuilder, false); + + // everything else is fine on all fields + query = randomFrom("NoW", "nOw", "NOW") + " " + randomAlphaOfLengthBetween(4, 10); + queryStringQueryBuilder = new QueryStringQueryBuilder(query); + assertQueryCachability(queryStringQueryBuilder, true); + } + + private void assertQueryCachability(QueryStringQueryBuilder qb, boolean cachingExpected) throws IOException { + QueryShardContext context = createShardContext(); + assert context.isCacheable(); + /* + * We use a private rewrite context here since we want the most realistic way of asserting that we are cacheable 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(qb, new QueryShardContext(context)); + assertNotNull(rewritten.toQuery(context)); + assertEquals("query should " + (cachingExpected ? "" : "not") + " be cacheable: " + qb.toString(), cachingExpected, + context.isCacheable()); + } } diff --git a/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java index 2ac15af573adf..fbd707b4c2ae1 100644 --- a/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java @@ -43,6 +43,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.search.SimpleQueryStringQueryParser; import org.elasticsearch.search.internal.SearchContext; +import org.elasticsearch.test.AbstractQueryTestCase; import java.io.IOException; import java.util.ArrayList; @@ -64,15 +65,13 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; -public class SimpleQueryStringBuilderTests extends FullTextQueryTestCase { - @Override - protected boolean isCacheable(SimpleQueryStringBuilder queryBuilder) { - return isCacheable(queryBuilder.fields().keySet(), queryBuilder.value()); - } +public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase { @Override protected SimpleQueryStringBuilder doCreateTestQueryBuilder() { - String queryText = randomAlphaOfLengthBetween(1, 10); + // we avoid strings with "now" since those can have different caching policies that are checked elsewhere + String queryText = randomValueOtherThanMany(s -> s.toLowerCase(Locale.ROOT).contains("now"), + () -> randomAlphaOfLengthBetween(1, 10)); SimpleQueryStringBuilder result = new SimpleQueryStringBuilder(queryText); if (randomBoolean()) { result.analyzeWildcard(randomBoolean()); @@ -786,4 +785,39 @@ private void assertQueryWithAllFieldsWildcard(Query query) { assertThat(disjunctionMaxQuery.getDisjuncts(), hasItems(new TermQuery(new Term(STRING_FIELD_NAME, "hello")), new TermQuery(new Term(STRING_FIELD_NAME_2, "hello")))); } + + /** + * Query terms that contain "now" can trigger a query to not be cacheable. + * This test checks the search context cacheable flag is updated accordingly. + */ + public void testCachingStrategiesWithNow() throws IOException { + // if we hit all fields, this should contain a date field and should diable cachability + String query = "now " + randomAlphaOfLengthBetween(4, 10); + SimpleQueryStringBuilder queryBuilder = new SimpleQueryStringBuilder(query); + assertQueryCachability(queryBuilder, false); + + // if we hit a date field with "now", this should diable cachability + queryBuilder = new SimpleQueryStringBuilder("now"); + queryBuilder.field(DATE_FIELD_NAME); + assertQueryCachability(queryBuilder, false); + + // everything else is fine on all fields + query = randomFrom("NoW", "nOw", "NOW") + " " + randomAlphaOfLengthBetween(4, 10); + queryBuilder = new SimpleQueryStringBuilder(query); + assertQueryCachability(queryBuilder, true); + } + + private void assertQueryCachability(SimpleQueryStringBuilder qb, boolean cachingExpected) throws IOException { + QueryShardContext context = createShardContext(); + assert context.isCacheable(); + /* + * We use a private rewrite context here since we want the most realistic way of asserting that we are cacheable 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(qb, new QueryShardContext(context)); + assertNotNull(rewritten.toQuery(context)); + assertEquals("query should " + (cachingExpected ? "" : "not") + " be cacheable: " + qb.toString(), cachingExpected, + context.isCacheable()); + } } 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 847fb58eca3e7..0ce38f3d68144 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -20,6 +20,7 @@ package org.elasticsearch.test; import com.fasterxml.jackson.core.io.JsonStringEncoder; + import org.apache.lucene.search.BoostQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermQuery; @@ -470,7 +471,7 @@ public void testToQuery() throws IOException { } } - private QueryBuilder rewriteQuery(QB queryBuilder, QueryRewriteContext rewriteContext) throws IOException { + protected QueryBuilder rewriteQuery(QB queryBuilder, QueryRewriteContext rewriteContext) throws IOException { QueryBuilder rewritten = rewriteAndFetch(queryBuilder, rewriteContext); // extra safety to fail fast - serialize the rewritten version to ensure it's serializable. assertSerialization(rewritten);