The query string cache can't return the same instance, since Query is mutable changing the query else where in the execution path changes the instance in the cache too.

Instead the query parser cache should return a cloned instances.

Closes #2542
Closes #6733
This commit is contained in:
Martijn van Groningen 2014-07-04 14:44:43 +02:00
parent 80617612cb
commit 730b83c03c
3 changed files with 69 additions and 5 deletions

View File

@ -63,7 +63,12 @@ public class ResidentQueryParserCache extends AbstractIndexComponent implements
@Override @Override
public Query get(QueryParserSettings queryString) { public Query get(QueryParserSettings queryString) {
return cache.getIfPresent(queryString); Query value = cache.getIfPresent(queryString);
if (value != null) {
return value.clone();
} else {
return null;
}
} }
@Override @Override

View File

@ -0,0 +1,35 @@
package org.elasticsearch.index.cache.query.parser.resident;
import org.apache.lucene.index.Term;
import org.apache.lucene.queryparser.classic.QueryParserSettings;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermQuery;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.index.Index;
import org.elasticsearch.test.ElasticsearchTestCase;
import org.junit.Test;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.sameInstance;
/**
*/
public class ResidentQueryParserCacheTest extends ElasticsearchTestCase {
@Test
public void testCaching() throws Exception {
ResidentQueryParserCache cache = new ResidentQueryParserCache(new Index("test"), ImmutableSettings.EMPTY);
QueryParserSettings key = new QueryParserSettings();
key.queryString("abc");
key.defaultField("a");
key.boost(2.0f);
Query query = new TermQuery(new Term("a", "abc"));
cache.put(key, query);
assertThat(cache.get(key), not(sameInstance(query)));
assertThat(cache.get(key), equalTo(query));
}
}

View File

@ -47,10 +47,7 @@ import org.joda.time.format.ISODateTimeFormat;
import org.junit.Test; import org.junit.Test;
import java.io.IOException; import java.io.IOException;
import java.util.HashSet; import java.util.*;
import java.util.Locale;
import java.util.Random;
import java.util.Set;
import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutionException;
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS;
@ -2457,4 +2454,31 @@ public class SimpleQueryTests extends ElasticsearchIntegrationTest {
} }
} }
@Test
public void testQueryStringParserCache() throws Exception {
createIndex("test");
indexRandom(true, Arrays.asList(
client().prepareIndex("test", "type", "1").setSource("nameTokens", "xyz")
));
SearchResponse response = client().prepareSearch("test")
.setQuery(QueryBuilders.queryString("xyz").boost(100))
.get();
assertThat(response.getHits().totalHits(), equalTo(1l));
assertThat(response.getHits().getAt(0).id(), equalTo("1"));
float score = response.getHits().getAt(0).getScore();
for (int i = 0; i < 100; i++) {
response = client().prepareSearch("test")
.setQuery(QueryBuilders.queryString("xyz").boost(100))
.get();
assertThat(response.getHits().totalHits(), equalTo(1l));
assertThat(response.getHits().getAt(0).id(), equalTo("1"));
assertThat(Float.compare(score, response.getHits().getAt(0).getScore()), equalTo(0));
}
}
} }