honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-03-30 20:08:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

Removed static versions of MatchAllDocsQuery

Browse Source 
If a static cached version of MatchAllDocsQuery is used through for
instanst the `query_string` together with a boost like `*:*^2.0` the
globally used version is modified since queries are not immutable and it's
boost variable can change at any time. Holding on to queries that are modifiable
is risky and should not be done in a global scope.
This commit also adds tests for constant scores from `constant_score` query.

Closes #3521
This commit is contained in:
Simon Willnauer 2013-08-16 14:42:42 +02:00
parent 57c0d29114
commit b11f81d744
12 changed files with 169 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/main/java/org/apache/lucene/queryparser/classic/MapperQueryParser.java
View File

@ -149,7 +149,7 @@ public class MapperQueryParser extends QueryParser {
@Override
protected Query newMatchAllDocsQuery() {
return Queries.MATCH_ALL_QUERY;
return Queries.newMatchAllQuery();
}
@Override

25
src/main/java/org/elasticsearch/common/lucene/search/ApplyAcceptedDocsFilter.java
View File

@ -142,4 +142,29 @@ public class ApplyAcceptedDocsFilter extends Filter {
return match.get(doc);
}
}
@Override
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + ((filter == null) ? 0 : filter.hashCode());
return result;
}
@Override
public boolean equals(Object obj) {
if (this == obj)
return true;
if (obj == null)
return false;
if (getClass() != obj.getClass())
return false;
ApplyAcceptedDocsFilter other = (ApplyAcceptedDocsFilter) obj;
if (filter == null) {
if (other.filter != null)
return false;
} else if (!filter.equals(other.filter))
return false;
return true;
}
}

21
src/main/java/org/elasticsearch/common/lucene/search/Queries.java
View File

@ -31,10 +31,12 @@ import java.util.regex.Pattern;
*/
public class Queries {
// We don't use MatchAllDocsQuery, its slower than the one below ... (much slower)
public final static Query MATCH_ALL_QUERY = new XConstantScoreQuery(new MatchAllDocsFilter());
/* In general we should never us a static query instance and share it.
* In this case the instance is immutable so that's ok.*/
public final static Query NO_MATCH_QUERY = MatchNoDocsQuery.INSTANCE;
private static final Filter MATCH_ALL_DOCS_FILTER = new MatchAllDocsFilter();
/**
* A match all docs filter. Note, requires no caching!.
*/
@ -54,6 +56,7 @@ public class Queries {
disjuncts = disjunctsX;
}
@SuppressWarnings("unchecked")
public static List<Query> disMaxClauses(DisjunctionMaxQuery query) {
try {
return (List<Query>) disjuncts.get(query);
@ -61,6 +64,13 @@ public class Queries {
return null;
}
}
public static Query newMatchAllQuery() {
// We don't use MatchAllDocsQuery, its slower than the one below ... (much slower)
// NEVER cache this XConstantScore Query it's not immutable and based on #3521
// some code might set a boost on this query.
return new XConstantScoreQuery(MATCH_ALL_DOCS_FILTER);
}
/**
* Optimizes the given query and returns the optimized version of it.
@ -103,16 +113,13 @@ public class Queries {
public static Query fixNegativeQueryIfNeeded(Query q) {
if (isNegativeQuery(q)) {
BooleanQuery newBq = (BooleanQuery) q.clone();
newBq.add(MATCH_ALL_QUERY, BooleanClause.Occur.MUST);
newBq.add(newMatchAllQuery(), BooleanClause.Occur.MUST);
return newBq;
}
return q;
}
public static boolean isConstantMatchAllQuery(Query query) {
if (query == Queries.MATCH_ALL_QUERY) {
return true;
}
if (query instanceof XConstantScoreQuery) {
XConstantScoreQuery scoreQuery = (XConstantScoreQuery) query;
if (scoreQuery.getFilter() instanceof MatchAllDocsFilter) {

2
src/main/java/org/elasticsearch/index/query/FilteredQueryParser.java
View File

@ -51,7 +51,7 @@ public class FilteredQueryParser implements QueryParser {
public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException {
XContentParser parser = parseContext.parser();
Query query = Queries.MATCH_ALL_QUERY;
Query query = Queries.newMatchAllQuery();
Filter filter = null;
boolean filterFound = false;
float boost = 1.0f;

4
src/main/java/org/elasticsearch/index/query/IndicesQueryParser.java
View File

@ -63,7 +63,7 @@ public class IndicesQueryParser implements QueryParser {
String currentFieldName = null;
XContentParser.Token token;
Query noMatchQuery = Queries.MATCH_ALL_QUERY;
Query noMatchQuery = Queries.newMatchAllQuery();
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
currentFieldName = parser.currentName();
@ -94,7 +94,7 @@ public class IndicesQueryParser implements QueryParser {
} else if ("no_match_query".equals(currentFieldName)) {
String type = parser.text();
if ("all".equals(type)) {
noMatchQuery = Queries.MATCH_ALL_QUERY;
noMatchQuery = Queries.newMatchAllQuery();
} else if ("none".equals(type)) {
noMatchQuery = MatchNoDocsQuery.INSTANCE;
}

7
src/main/java/org/elasticsearch/index/query/MatchAllQueryParser.java
View File

@ -49,7 +49,6 @@ public class MatchAllQueryParser implements QueryParser {
XContentParser parser = parseContext.parser();
float boost = 1.0f;
String normsField = null;
String currentFieldName = null;
XContentParser.Token token;
@ -59,16 +58,14 @@ public class MatchAllQueryParser implements QueryParser {
} else if (token.isValue()) {
if ("boost".equals(currentFieldName)) {
boost = parser.floatValue();
} else if ("norms_field".equals(currentFieldName) || "normsField".equals(currentFieldName)) {
normsField = parseContext.indexName(parser.text());
} else {
throw new QueryParsingException(parseContext.index(), "[match_all] query does not support [" + currentFieldName + "]");
}
}
}
if (boost == 1.0f && normsField == null) {
return Queries.MATCH_ALL_QUERY;
if (boost == 1.0f) {
return Queries.newMatchAllQuery();
}
//LUCENE 4 UPGRADE norms field is not supported anymore need to find another way or drop the functionality

6
src/main/java/org/elasticsearch/index/query/ParsedQuery.java
View File

@ -31,8 +31,6 @@ import org.elasticsearch.common.lucene.search.Queries;
*/
public class ParsedQuery {
public static ParsedQuery MATCH_ALL_PARSED_QUERY = new ParsedQuery(Queries.MATCH_ALL_QUERY, ImmutableMap.<String, Filter>of());
private final Query query;
private final ImmutableMap<String, Filter> namedFilters;
@ -57,4 +55,8 @@ public class ParsedQuery {
public ImmutableMap<String, Filter> namedFilters() {
return this.namedFilters;
}
public static ParsedQuery parsedMatchAllQuery() {
return new ParsedQuery(Queries.newMatchAllQuery(), ImmutableMap.<String, Filter>of());
}
}

2
src/main/java/org/elasticsearch/index/search/MatchQuery.java
View File

@ -367,6 +367,6 @@ public class MatchQuery {
}
protected Query zeroTermsQuery() {
return zeroTermsQuery == ZeroTermsQuery.NONE ? MatchNoDocsQuery.INSTANCE : Queries.MATCH_ALL_QUERY;
return zeroTermsQuery == ZeroTermsQuery.NONE ? MatchNoDocsQuery.INSTANCE : Queries.newMatchAllQuery();
}
}

2
src/main/java/org/elasticsearch/search/internal/SearchContext.java
View File

@ -234,7 +234,7 @@ public class SearchContext implements Releasable {
*/
public void preProcess() {
if (query() == null) {
parsedQuery(ParsedQuery.MATCH_ALL_PARSED_QUERY);
parsedQuery(ParsedQuery.parsedMatchAllQuery());
}
if (queryBoost() != 1.0f) {
parsedQuery(new ParsedQuery(new FunctionScoreQuery(query(), new BoostScoreFunction(queryBoost)), parsedQuery()));

109
src/test/java/org/elasticsearch/test/integration/search/query/SimpleQueryTests.java
View File

@ -19,7 +19,9 @@
package org.elasticsearch.test.integration.search.query;
import org.apache.lucene.util.English;
import org.elasticsearch.ElasticSearchException;
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.action.search.SearchPhaseExecutionException;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.search.SearchType;
@ -29,11 +31,14 @@ import org.elasticsearch.index.query.*;
import org.elasticsearch.index.query.CommonTermsQueryBuilder.Operator;
import org.elasticsearch.index.query.MatchQueryBuilder.Type;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.search.SearchHits;
import org.elasticsearch.search.facet.FacetBuilders;
import org.elasticsearch.test.integration.AbstractSharedClusterTest;
import org.junit.Test;
import java.io.IOException;
import java.util.Random;
import java.util.concurrent.ExecutionException;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
@ -111,6 +116,108 @@ public class SimpleQueryTests extends AbstractSharedClusterTest {
assertTrue("wrong exception message " + e.getMessage(), e.getMessage().endsWith("IllegalStateException[field \"field1\" was indexed without position data; cannot run PhraseQuery (term=quick)]; }"));
}
}
@Test // see #3521
public void testConstantScoreQuery() throws Exception {
Random random = getRandom();
createIndex("test");
indexRandom("test", true, client().prepareIndex("test", "type1", "1").setSource("field1", "quick brown fox", "field2", "quick brown fox"),
client().prepareIndex("test", "type1", "2").setSource("field1", "quick lazy huge brown fox", "field2", "quick lazy huge brown fox"));
refresh();
SearchResponse searchResponse = client().prepareSearch().setQuery(QueryBuilders.constantScoreQuery(QueryBuilders.matchQuery("field1", "quick"))).get();
SearchHits hits = searchResponse.getHits();
assertThat(hits.totalHits(), equalTo(2l));
for (SearchHit searchHit : hits) {
assertThat(searchHit.getScore(), equalTo(1.0f));
}
searchResponse = client().prepareSearch().setQuery(
QueryBuilders.boolQuery().must(QueryBuilders.matchAllQuery()).must(
QueryBuilders.constantScoreQuery(QueryBuilders.matchQuery("field1", "quick")).boost(1.0f + getRandom().nextFloat()))).get();
hits = searchResponse.getHits();
assertThat(hits.totalHits(), equalTo(2l));
assertThat(hits.getAt(0).score(), equalTo(hits.getAt(1).score()));
client().prepareSearch().setQuery(QueryBuilders.constantScoreQuery(QueryBuilders.matchQuery("field1", "quick")).boost(1.0f + getRandom().nextFloat())).get();
hits = searchResponse.getHits();
assertThat(hits.totalHits(), equalTo(2l));
assertThat(hits.getAt(0).score(), equalTo(hits.getAt(1).score()));
searchResponse = client().prepareSearch().setQuery(
QueryBuilders.constantScoreQuery(QueryBuilders.boolQuery().must(QueryBuilders.matchAllQuery()).must(
QueryBuilders.constantScoreQuery(QueryBuilders.matchQuery("field1", "quick")).boost(1.0f + (random.nextBoolean()? 0.0f : random.nextFloat()))))).get();
hits = searchResponse.getHits();
assertThat(hits.totalHits(), equalTo(2l));
assertThat(hits.getAt(0).score(), equalTo(hits.getAt(1).score()));
for (SearchHit searchHit : hits) {
assertThat(searchHit.getScore(), equalTo(1.0f));
}
int num = atLeast(100);
IndexRequestBuilder[] builders = new IndexRequestBuilder[num];
for (int i = 0; i < builders.length; i++) {
builders[i] = client().prepareIndex("test", "type", "" + i).setSource("f", English.intToEnglish(i));
}
createIndex("test_1");
indexRandom("test_1", true, builders);
int queryRounds = atLeast(10);
for (int i = 0; i < queryRounds; i++) {
MatchQueryBuilder matchQuery = QueryBuilders.matchQuery("f", English.intToEnglish(between(0, num)));
searchResponse = client().prepareSearch("test_1").setQuery(matchQuery).setSize(num).get();
long totalHits = searchResponse.getHits().totalHits();
hits = searchResponse.getHits();
for (SearchHit searchHit : hits) {
assertThat(searchHit.getScore(), equalTo(1.0f));
}
if (random.nextBoolean()) {
searchResponse = client().prepareSearch("test_1").setQuery(
QueryBuilders.boolQuery().must(QueryBuilders.matchAllQuery()).must(
QueryBuilders.constantScoreQuery(matchQuery).boost(1.0f + (random.nextBoolean()? 0.0f : random.nextFloat())))).setSize(num).get();
hits = searchResponse.getHits();
} else {
FilterBuilder filter = FilterBuilders.queryFilter(matchQuery);
searchResponse = client().prepareSearch("test_1").setQuery(
QueryBuilders.boolQuery().must(QueryBuilders.matchAllQuery()).must(
QueryBuilders.constantScoreQuery(filter).boost(1.0f + (random.nextBoolean()? 0.0f : random.nextFloat())))).setSize(num).get();
hits = searchResponse.getHits();
}
assertThat(hits.totalHits(), equalTo(totalHits));
if (totalHits > 1) {
float expected = hits.getAt(0).score();
for (SearchHit searchHit : hits) {
assertThat(searchHit.getScore(), equalTo(expected));
}
}
}
}
@Test // see #3521
public void testAllDocsQueryString() throws InterruptedException, ExecutionException {
client().admin().indices().prepareCreate("test")
.setSettings(ImmutableSettings.settingsBuilder().put("index.number_of_replicas", 0)).execute().actionGet();
indexRandom("test", true,
client().prepareIndex("test", "type1", "1").setSource("foo", "bar"),
client().prepareIndex("test", "type1", "2").setSource("foo", "bar")
);
int iters = atLeast(100);
for (int i = 0; i < iters; i++) {
SearchResponse searchResponse = client().prepareSearch("test")
.setQuery(queryString("*:*^10.0").boost(10.0f))
.execute().actionGet();
assertNoFailures(searchResponse);
assertThat(searchResponse.getHits().totalHits(), equalTo(2l));
searchResponse = client().prepareSearch("test")
.setQuery(QueryBuilders.boolQuery().must(QueryBuilders.matchAllQuery()).must(
QueryBuilders.constantScoreQuery(QueryBuilders.matchAllQuery())))
.execute().actionGet();
assertNoFailures(searchResponse);
assertThat(searchResponse.getHits().totalHits(), equalTo(2l));
assertThat((double)searchResponse.getHits().getAt(0).score(), closeTo(Math.sqrt(2), 0.1));
assertThat((double)searchResponse.getHits().getAt(1).score(),closeTo(Math.sqrt(2), 0.1));
}
}
@Test
public void testCommonTermsQuery() throws Exception {
@ -772,7 +879,7 @@ public class SimpleQueryTests extends AbstractSharedClusterTest {
assertThat(searchResponse.getHits().getAt(1).id(), equalTo("2"));
assertThat((double)searchResponse.getHits().getAt(0).score(), closeTo(boost * searchResponse.getHits().getAt(1).score(), .1));
}
@Test
public void testSpecialRangeSyntaxInQueryString() {
client().admin().indices().prepareCreate("test").setSettings(ImmutableSettings.settingsBuilder().put("index.number_of_shards", 1)).execute().actionGet();

2
src/test/java/org/elasticsearch/test/unit/index/engine/EngineSearcherTotalHitsMatcher.java
View File

@ -63,6 +63,6 @@ public final class EngineSearcherTotalHitsMatcher extends TypeSafeMatcher<Engine
}
public static Matcher<Engine.Searcher> engineSearcherTotalHits(int totalHits) {
return new EngineSearcherTotalHitsMatcher(Queries.MATCH_ALL_QUERY, totalHits);
return new EngineSearcherTotalHitsMatcher(Queries.newMatchAllQuery(), totalHits);
}
}

13
src/test/java/org/elasticsearch/test/unit/index/query/SimpleIndexQueryParserTests.java
View File

@ -66,6 +66,7 @@ import org.elasticsearch.index.settings.IndexSettingsModule;
import org.elasticsearch.index.similarity.SimilarityModule;
import org.elasticsearch.indices.query.IndicesQueriesModule;
import org.elasticsearch.script.ScriptModule;
import org.elasticsearch.test.integration.ElasticsearchTestCase;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.threadpool.ThreadPoolModule;
import org.hamcrest.Matchers;
@ -84,13 +85,12 @@ import static org.elasticsearch.index.query.FilterBuilders.*;
import static org.elasticsearch.index.query.QueryBuilders.*;
import static org.elasticsearch.index.query.RegexpFlag.*;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertBooleanSubQuery;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.*;
/**
*
*/
public class SimpleIndexQueryParserTests {
public class SimpleIndexQueryParserTests extends ElasticsearchTestCase {
private static Injector injector;
@ -135,6 +135,8 @@ public class SimpleIndexQueryParserTests {
@AfterClass
public static void close() {
injector.getInstance(ThreadPool.class).shutdownNow();
queryParser = null;
injector = null;
}
private IndexQueryParserService queryParser() throws IOException {
@ -301,7 +303,8 @@ public class SimpleIndexQueryParserTests {
IndexQueryParserService queryParser = queryParser();
String query = copyToStringFromClasspath("/org/elasticsearch/test/unit/index/query/match_all_empty1.json");
Query parsedQuery = queryParser.parse(query).query();
assertThat(parsedQuery, sameInstance(Queries.MATCH_ALL_QUERY));
assertThat(parsedQuery, equalTo(Queries.newMatchAllQuery()));
assertThat(parsedQuery, not(sameInstance(Queries.newMatchAllQuery())));
}
@Test
@ -309,7 +312,9 @@ public class SimpleIndexQueryParserTests {
IndexQueryParserService queryParser = queryParser();
String query = copyToStringFromClasspath("/org/elasticsearch/test/unit/index/query/match_all_empty2.json");
Query parsedQuery = queryParser.parse(query).query();
assertThat(parsedQuery, sameInstance(Queries.MATCH_ALL_QUERY));
assertThat(parsedQuery, equalTo(Queries.newMatchAllQuery()));
assertThat(parsedQuery, not(sameInstance(Queries.newMatchAllQuery())));
}
@Test