Java api: don't mutate fields in SimpleQueryStringBuilder during toQuery execution

We used to mutate the map of fields and weights while transforming to lucene query (toQuery method), by adding the default field if the map is empty. That is an anti-pattern, we should leave the original query as is instead, but still generate the proper lucene query. Also took the chance to fix a couple of issues in SimpleQueryStringBuilderTest#doAssertLuceneQuery.
This commit is contained in:
javanna 2015-08-31 17:15:33 +02:00 committed by Luca Cavanna
parent faf526dccb
commit 11b41f1d68
2 changed files with 25 additions and 29 deletions

View File

@ -263,24 +263,19 @@ public class SimpleQueryStringBuilder extends AbstractQueryBuilder<SimpleQuerySt
@Override
protected Query doToQuery(QueryShardContext context) throws IOException {
// Use the default field if no fields specified
if (fieldsAndWeights.isEmpty()) {
fieldsAndWeights.put(context.defaultField(), AbstractQueryBuilder.DEFAULT_BOOST);
}
// field names in builder can have wildcards etc, need to resolve them here
Map<String, Float> resolvedFieldsAndWeights = new TreeMap<>();
for (String fField : fieldsAndWeights.keySet()) {
if (Regex.isSimpleMatchPattern(fField)) {
for (String fieldName : context.mapperService().simpleMatchToIndexNames(fField)) {
resolvedFieldsAndWeights.put(fieldName, fieldsAndWeights.get(fField));
}
} else {
MappedFieldType fieldType = context.fieldMapper(fField);
if (fieldType != null) {
resolvedFieldsAndWeights.put(fieldType.names().indexName(), fieldsAndWeights.get(fField));
// Use the default field if no fields specified
if (fieldsAndWeights.isEmpty()) {
resolvedFieldsAndWeights.put(resolveIndexName(context.defaultField(), context), AbstractQueryBuilder.DEFAULT_BOOST);
} else {
for (Map.Entry<String, Float> fieldEntry : fieldsAndWeights.entrySet()) {
if (Regex.isSimpleMatchPattern(fieldEntry.getKey())) {
for (String fieldName : context.mapperService().simpleMatchToIndexNames(fieldEntry.getKey())) {
resolvedFieldsAndWeights.put(fieldName, fieldEntry.getValue());
}
} else {
resolvedFieldsAndWeights.put(fField, fieldsAndWeights.get(fField));
resolvedFieldsAndWeights.put(resolveIndexName(fieldEntry.getKey(), context), fieldEntry.getValue());
}
}
}
@ -308,6 +303,14 @@ public class SimpleQueryStringBuilder extends AbstractQueryBuilder<SimpleQuerySt
return query;
}
private static String resolveIndexName(String fieldName, QueryShardContext context) {
MappedFieldType fieldType = context.fieldMapper(fieldName);
if (fieldType != null) {
return fieldType.names().indexName();
}
return fieldName;
}
@Override
protected void doXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(NAME);

View File

@ -20,11 +20,7 @@
package org.elasticsearch.index.query;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.BooleanClause;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.MatchNoDocsQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.search.*;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.common.xcontent.XContentFactory;
@ -258,7 +254,9 @@ public class SimpleQueryStringBuilderTest extends BaseQueryTestCase<SimpleQueryS
protected void doAssertLuceneQuery(SimpleQueryStringBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
assertThat(query, notNullValue());
if (queryBuilder.fields().size() > 1 && (!"".equals(queryBuilder.value()))) {
if ("".equals(queryBuilder.value())) {
assertTrue("Query should have been MatchNoDocsQuery but was " + query.getClass().getName(), query instanceof MatchNoDocsQuery);
} else if (queryBuilder.fields().size() > 1) {
assertTrue("Query should have been BooleanQuery but was " + query.getClass().getName(), query instanceof BooleanQuery);
BooleanQuery boolQuery = (BooleanQuery) query;
@ -280,8 +278,8 @@ public class SimpleQueryStringBuilderTest extends BaseQueryTestCase<SimpleQueryS
}
if (queryBuilder.minimumShouldMatch() != null) {
Collection<String> minMatchAlways = Arrays.asList(new String[] { "1", "-1", "75%", "-25%" });
Collection<String> minMatchLarger = Arrays.asList(new String[] { "2<75%", "2<-25%" });
Collection<String> minMatchAlways = Arrays.asList("1", "-1", "75%", "-25%");
Collection<String> minMatchLarger = Arrays.asList("2<75%", "2<-25%");
if (minMatchAlways.contains(queryBuilder.minimumShouldMatch())) {
assertThat(boolQuery.getMinimumNumberShouldMatch(), greaterThan(0));
@ -293,12 +291,10 @@ public class SimpleQueryStringBuilderTest extends BaseQueryTestCase<SimpleQueryS
assertEquals(0, boolQuery.getMinimumNumberShouldMatch());
}
}
} else if (queryBuilder.fields().size() == 1 && (!"".equals(queryBuilder.value()))) {
} else if (queryBuilder.fields().size() <= 1) {
assertTrue("Query should have been TermQuery but was " + query.getClass().getName(), query instanceof TermQuery);
TermQuery termQuery = (TermQuery) query;
assertThat(termQuery.getTerm().field(), is(queryBuilder.fields().keySet().iterator().next()));
String field;
if (queryBuilder.fields().size() == 0) {
field = MetaData.ALL;
@ -310,9 +306,6 @@ public class SimpleQueryStringBuilderTest extends BaseQueryTestCase<SimpleQueryS
if (queryBuilder.lowercaseExpandedTerms()) {
assertThat(termQuery.getTerm().bytes().toString(), is(termQuery.getTerm().bytes().toString().toLowerCase(Locale.ROOT)));
}
} else if ("".equals(queryBuilder.value())) {
assertTrue("Query should have been MatchNoDocsQuery but was " + query.getClass().getName(), query instanceof MatchNoDocsQuery);
} else {
fail("Encountered lucene query type we do not have a validation implementation for in our SimpleQueryStringBuilderTest");
}