honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Revert fd3a46a because fix is to fragile 

Reverting fix for #13884 because it was discussed to be too
fragile with respect to future changes in lucene simple query
string parsing. Undoes fix and removes test.
This commit is contained in:
Christoph Büscher 2015-11-03 11:48:11 +01:00
parent a7bf06ee3a
commit 2fe519c3e7
5 changed files with 48 additions and 140 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
core/src/main/java/org/elasticsearch/index/query/SimpleQueryParser.java
View File

@ -70,7 +70,7 @@ public class SimpleQueryParser extends org.apache.lucene.queryparser.simple.Simp
rethrowUnlessLenient(e);
}
}
return simplify(bq.build());
return super.simplify(bq.build());
}
/**
@ -93,7 +93,7 @@ public class SimpleQueryParser extends org.apache.lucene.queryparser.simple.Simp
rethrowUnlessLenient(e);
}
}
return simplify(bq.build());
return super.simplify(bq.build());
}
@Override
@ -111,7 +111,7 @@ public class SimpleQueryParser extends org.apache.lucene.queryparser.simple.Simp
rethrowUnlessLenient(e);
}
}
return simplify(bq.build());
return super.simplify(bq.build());
}
/**
@ -140,19 +140,7 @@ public class SimpleQueryParser extends org.apache.lucene.queryparser.simple.Simp
return rethrowUnlessLenient(e);
}
}
return simplify(bq.build());
}
/**
* Override of lucenes SimpleQueryParser that doesn't simplify for the 1-clause case.
*/
@Override
protected Query simplify(BooleanQuery bq) {
if (bq.clauses().isEmpty()) {
return null;
} else {
return bq;
}
return super.simplify(bq.build());
}
/**

18
core/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java
View File

@ -20,8 +20,6 @@
package org.elasticsearch.index.query;
import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.search.BooleanClause;
import org.apache.lucene.search.BooleanClause.Occur;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.Query;
import org.elasticsearch.common.Strings;
@ -287,20 +285,8 @@ public class SimpleQueryStringBuilder extends AbstractQueryBuilder<SimpleQuerySt
sqp.setDefaultOperator(defaultOperator.toBooleanClauseOccur());
Query query = sqp.parse(queryText);
if (query instanceof BooleanQuery) {
BooleanQuery booleanQuery = (BooleanQuery) query;
if (booleanQuery.clauses().size() > 1
&& ((booleanQuery.clauses().iterator().next().getQuery() instanceof BooleanQuery) == false)) {
// special case for one term query and more than one field: (f1:t1 f2:t1 f3:t1)
// we need to wrap this in additional BooleanQuery so minimum_should_match is applied correctly
BooleanQuery.Builder builder = new BooleanQuery.Builder();
builder.add(new BooleanClause(booleanQuery, Occur.SHOULD));
booleanQuery = builder.build();
}
if (minimumShouldMatch != null) {
booleanQuery = Queries.applyMinimumShouldMatch(booleanQuery, minimumShouldMatch);
}
query = booleanQuery;
if (minimumShouldMatch != null && query instanceof BooleanQuery) {
query = Queries.applyMinimumShouldMatch((BooleanQuery) query, minimumShouldMatch);
}
return query;
}

2
core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java
View File

@ -523,7 +523,7 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
testQuery.writeTo(output);
try (StreamInput in = new NamedWriteableAwareStreamInput(StreamInput.wrap(output.bytes()), namedWriteableRegistry)) {
QueryBuilder<?> prototype = queryParser(testQuery.getName()).getBuilderPrototype();
QueryBuilder<?> deserializedQuery = prototype.readFrom(in);
QueryBuilder deserializedQuery = prototype.readFrom(in);
assertEquals(deserializedQuery, testQuery);
assertEquals(deserializedQuery.hashCode(), testQuery.hashCode());
assertNotSame(deserializedQuery, testQuery);

131
core/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java
View File

@ -38,29 +38,19 @@ import java.util.HashSet;
import java.util.Iterator;
import java.util.Locale;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.TreeMap;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase<SimpleQueryStringBuilder> {
private String[] queryTerms;
@Override
protected SimpleQueryStringBuilder doCreateTestQueryBuilder() {
int numberOfTerms = randomIntBetween(1, 5);
queryTerms = new String[numberOfTerms];
StringBuilder queryString = new StringBuilder();
for (int i = 0; i < numberOfTerms; i++) {
queryTerms[i] = randomAsciiOfLengthBetween(1, 10);
queryString.append(queryTerms[i] + " ");
}
SimpleQueryStringBuilder result = new SimpleQueryStringBuilder(queryString.toString().trim());
SimpleQueryStringBuilder result = new SimpleQueryStringBuilder(randomAsciiOfLengthBetween(1, 10));
if (randomBoolean()) {
result.analyzeWildcard(randomBoolean());
}
@ -84,13 +74,9 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase<SimpleQ
}
if (randomBoolean()) {
Set<SimpleQueryStringFlag> flagSet = new HashSet<>();
if (numberOfTerms > 1) {
flagSet.add(SimpleQueryStringFlag.WHITESPACE);
}
int size = randomIntBetween(0, SimpleQueryStringFlag.values().length);
for (int i = 0; i < size; i++) {
SimpleQueryStringFlag randomFlag = randomFrom(SimpleQueryStringFlag.values());
flagSet.add(randomFlag);
flagSet.add(randomFrom(SimpleQueryStringFlag.values()));
}
if (flagSet.size() > 0) {
result.flags(flagSet.toArray(new SimpleQueryStringFlag[flagSet.size()]));
@ -101,12 +87,13 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase<SimpleQ
Map<String, Float> fields = new HashMap<>();
for (int i = 0; i < fieldCount; i++) {
if (randomBoolean()) {
fields.put("f" + i + "_" + randomAsciiOfLengthBetween(1, 10), AbstractQueryBuilder.DEFAULT_BOOST);
fields.put(randomAsciiOfLengthBetween(1, 10), AbstractQueryBuilder.DEFAULT_BOOST);
} else {
fields.put(randomBoolean() ? STRING_FIELD_NAME : "f" + i + "_" + randomAsciiOfLengthBetween(1, 10), 2.0f / randomIntBetween(1, 20));
fields.put(randomBoolean() ? STRING_FIELD_NAME : randomAsciiOfLengthBetween(1, 10), 2.0f / randomIntBetween(1, 20));
}
}
result.fields(fields);
return result;
}
@ -271,8 +258,8 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase<SimpleQ
// no strict field resolution (version before V_1_4_0_Beta1)
if (getCurrentTypes().length > 0 || shardContext.indexQueryParserService().getIndexCreatedVersion().before(Version.V_1_4_0_Beta1)) {
Query luceneQuery = queryBuilder.toQuery(shardContext);
assertThat(luceneQuery, instanceOf(BooleanQuery.class));
TermQuery termQuery = (TermQuery) ((BooleanQuery) luceneQuery).clauses().get(0).getQuery();
assertThat(luceneQuery, instanceOf(TermQuery.class));
TermQuery termQuery = (TermQuery) luceneQuery;
assertThat(termQuery.getTerm(), equalTo(new Term(MetaData.ALL, query)));
}
}
@ -290,7 +277,7 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase<SimpleQ
if ("".equals(queryBuilder.value())) {
assertTrue("Query should have been MatchNoDocsQuery but was " + query.getClass().getName(), query instanceof MatchNoDocsQuery);
} else {
} else if (queryBuilder.fields().size() > 1) {
assertTrue("Query should have been BooleanQuery but was " + query.getClass().getName(), query instanceof BooleanQuery);
BooleanQuery boolQuery = (BooleanQuery) query;
@ -303,42 +290,32 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase<SimpleQ
}
}
assertThat(boolQuery.clauses().size(), equalTo(queryTerms.length));
Map<String, Float> expectedFields = new TreeMap<String, Float>(queryBuilder.fields());
if (expectedFields.size() == 0) {
expectedFields.put(MetaData.ALL, AbstractQueryBuilder.DEFAULT_BOOST);
}
for (int i = 0; i < queryTerms.length; i++) {
BooleanClause booleanClause = boolQuery.clauses().get(i);
Iterator<Entry<String, Float>> fieldsIter = expectedFields.entrySet().iterator();
if (queryTerms.length == 1 && expectedFields.size() == 1) {
assertThat(booleanClause.getQuery(), instanceOf(TermQuery.class));
TermQuery termQuery = (TermQuery) booleanClause.getQuery();
Entry<String, Float> entry = fieldsIter.next();
assertThat(termQuery.getTerm().field(), equalTo(entry.getKey()));
assertThat(termQuery.getBoost(), equalTo(entry.getValue()));
assertThat(termQuery.getTerm().text().toLowerCase(Locale.ROOT), equalTo(queryTerms[i].toLowerCase(Locale.ROOT)));
} else {
assertThat(booleanClause.getQuery(), instanceOf(BooleanQuery.class));
for (BooleanClause clause : ((BooleanQuery) booleanClause.getQuery()).clauses()) {
TermQuery termQuery = (TermQuery) clause.getQuery();
Entry<String, Float> entry = fieldsIter.next();
assertThat(termQuery.getTerm().field(), equalTo(entry.getKey()));
assertThat(termQuery.getBoost(), equalTo(entry.getValue()));
assertThat(termQuery.getTerm().text().toLowerCase(Locale.ROOT), equalTo(queryTerms[i].toLowerCase(Locale.ROOT)));
}
}
assertThat(boolQuery.clauses().size(), equalTo(queryBuilder.fields().size()));
Iterator<String> fields = queryBuilder.fields().keySet().iterator();
for (BooleanClause booleanClause : boolQuery) {
assertThat(booleanClause.getQuery(), instanceOf(TermQuery.class));
TermQuery termQuery = (TermQuery) booleanClause.getQuery();
assertThat(termQuery.getTerm().field(), equalTo(fields.next()));
assertThat(termQuery.getTerm().text().toLowerCase(Locale.ROOT), equalTo(queryBuilder.value().toLowerCase(Locale.ROOT)));
}
if (queryBuilder.minimumShouldMatch() != null) {
int optionalClauses = queryTerms.length;
if (queryBuilder.defaultOperator().equals(Operator.AND) && queryTerms.length > 1) {
optionalClauses = 0;
}
int expectedMinimumShouldMatch = Queries.calculateMinShouldMatch(optionalClauses, queryBuilder.minimumShouldMatch());
assertEquals(expectedMinimumShouldMatch, boolQuery.getMinimumNumberShouldMatch());
assertThat(boolQuery.getMinimumNumberShouldMatch(), greaterThan(0));
}
} else if (queryBuilder.fields().size() <= 1) {
assertTrue("Query should have been TermQuery but was " + query.getClass().getName(), query instanceof TermQuery);
TermQuery termQuery = (TermQuery) query;
String field;
if (queryBuilder.fields().size() == 0) {
field = MetaData.ALL;
} else {
field = queryBuilder.fields().keySet().iterator().next();
}
assertThat(termQuery.getTerm().field(), equalTo(field));
assertThat(termQuery.getTerm().text().toLowerCase(Locale.ROOT), equalTo(queryBuilder.value().toLowerCase(Locale.ROOT)));
} else {
fail("Encountered lucene query type we do not have a validation implementation for in our " + SimpleQueryStringBuilderTests.class.getSimpleName());
}
}
@ -364,18 +341,15 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase<SimpleQ
SimpleQueryStringBuilder simpleQueryStringBuilder = new SimpleQueryStringBuilder("test");
simpleQueryStringBuilder.field(STRING_FIELD_NAME, 5);
Query query = simpleQueryStringBuilder.toQuery(shardContext);
assertThat(query, instanceOf(BooleanQuery.class));
TermQuery wrappedQuery = (TermQuery) ((BooleanQuery) query).clauses().get(0).getQuery();
assertThat(wrappedQuery.getBoost(), equalTo(5f));
assertThat(query, instanceOf(TermQuery.class));
assertThat(query.getBoost(), equalTo(5f));
simpleQueryStringBuilder = new SimpleQueryStringBuilder("test");
simpleQueryStringBuilder.field(STRING_FIELD_NAME, 5);
simpleQueryStringBuilder.boost(2);
query = simpleQueryStringBuilder.toQuery(shardContext);
assertThat(query.getBoost(), equalTo(2f));
assertThat(query, instanceOf(BooleanQuery.class));
wrappedQuery = (TermQuery) ((BooleanQuery) query).clauses().get(0).getQuery();
assertThat(wrappedQuery.getBoost(), equalTo(5f));
assertThat(query, instanceOf(TermQuery.class));
assertThat(query.getBoost(), equalTo(10f));
}
public void testNegativeFlags() throws IOException {
@ -387,39 +361,4 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase<SimpleQ
otherBuilder.flags(-1);
assertThat(builder, equalTo(otherBuilder));
}
public void testMinimumShouldMatch() throws IOException {
QueryShardContext shardContext = createShardContext();
int numberOfTerms = randomIntBetween(1, 4);
int numberOfFields = randomIntBetween(1, 4);
StringBuilder queryString = new StringBuilder();
for (int i = 0; i < numberOfTerms; i++) {
queryString.append("t" + i + " ");
}
SimpleQueryStringBuilder simpleQueryStringBuilder = new SimpleQueryStringBuilder(queryString.toString().trim());
if (randomBoolean()) {
simpleQueryStringBuilder.defaultOperator(Operator.AND);
}
for (int i = 0; i < numberOfFields; i++) {
simpleQueryStringBuilder.field("f" + i);
}
int percent = randomIntBetween(1, 100);
simpleQueryStringBuilder.minimumShouldMatch(percent + "%");
BooleanQuery query = (BooleanQuery) simpleQueryStringBuilder.toQuery(shardContext);
assertEquals("query should have one should clause per term", numberOfTerms, query.clauses().size());
int expectedMinimumShouldMatch = numberOfTerms * percent / 100;
if (simpleQueryStringBuilder.defaultOperator().equals(Operator.AND) && numberOfTerms > 1) {
expectedMinimumShouldMatch = 0;
}
assertEquals(expectedMinimumShouldMatch, query.getMinimumNumberShouldMatch());
for (BooleanClause clause : query.clauses()) {
if (numberOfFields == 1 && numberOfTerms == 1) {
assertTrue(clause.getQuery() instanceof TermQuery);
} else {
assertEquals(numberOfFields, ((BooleanQuery) clause.getQuery()).clauses().size());
}
}
}
}

15
core/src/test/java/org/elasticsearch/search/query/SimpleQueryStringIT.java
View File

@ -109,6 +109,7 @@ public class SimpleQueryStringIT extends ESIntegTestCase {
client().prepareIndex("test", "type1", "3").setSource("body", "foo bar"),
client().prepareIndex("test", "type1", "4").setSource("body", "foo baz bar"));
logger.info("--> query 1");
SearchResponse searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar").minimumShouldMatch("2")).get();
assertHitCount(searchResponse, 2l);
@ -119,13 +120,7 @@ public class SimpleQueryStringIT extends ESIntegTestCase {
assertHitCount(searchResponse, 2l);
assertSearchHits(searchResponse, "3", "4");
logger.info("--> query 3"); // test case from #13884
searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo")
.field("body").field("body2").field("body3").minimumShouldMatch("-50%")).get();
assertHitCount(searchResponse, 3l);
assertSearchHits(searchResponse, "1", "3", "4");
logger.info("--> query 4");
logger.info("--> query 3");
searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar baz").field("body").field("body2").minimumShouldMatch("70%")).get();
assertHitCount(searchResponse, 2l);
assertSearchHits(searchResponse, "3", "4");
@ -136,17 +131,17 @@ public class SimpleQueryStringIT extends ESIntegTestCase {
client().prepareIndex("test", "type1", "7").setSource("body2", "foo bar", "other", "foo"),
client().prepareIndex("test", "type1", "8").setSource("body2", "foo baz bar", "other", "foo"));
logger.info("--> query 5");
logger.info("--> query 4");
searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar").field("body").field("body2").minimumShouldMatch("2")).get();
assertHitCount(searchResponse, 4l);
assertSearchHits(searchResponse, "3", "4", "7", "8");
logger.info("--> query 6");
logger.info("--> query 5");
searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar").minimumShouldMatch("2")).get();
assertHitCount(searchResponse, 5l);
assertSearchHits(searchResponse, "3", "4", "6", "7", "8");
logger.info("--> query 7");
logger.info("--> query 6");
searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar baz").field("body2").field("other").minimumShouldMatch("70%")).get();
assertHitCount(searchResponse, 3l);
assertSearchHits(searchResponse, "6", "7", "8");