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

Make sure equivalent geohashCellQueries are equal after toQuery called 

Previous to this change if to equal geohash cell query builders were created and then toQuery was called on one, they would no longer be equal.

This change also adds a test to AbstractQueryTestCase to make sure calling toQuery on any query builder does not affect the query builder's equality
This commit is contained in:
Colin Goodheart-Smithe 2015-09-25 14:23:26 +01:00
parent 2f1b3ccde2
commit 8c7c7652f6
14 changed files with 133 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
core/src/main/java/org/elasticsearch/index/query/FuzzyQueryBuilder.java
View File

@ -233,6 +233,7 @@ public class FuzzyQueryBuilder extends AbstractQueryBuilder<FuzzyQueryBuilder> i
@Override
public Query doToQuery(QueryShardContext context) throws IOException {
Query query = null;
String rewrite = this.rewrite;
if (rewrite == null && context.isFilter()) {
rewrite = QueryParsers.CONSTANT_SCORE.getPreferredName();
}

10
core/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java
View File

@ -72,7 +72,10 @@ public class GeoPolygonQueryBuilder extends AbstractQueryBuilder<GeoPolygonQuery
}
}
this.fieldName = fieldName;
this.shell = points;
this.shell = new ArrayList<>(points);
if (!shell.get(shell.size() - 1).equals(shell.get(0))) {
shell.add(shell.get(0));
}
}
public String fieldName() {
@ -97,8 +100,9 @@ public class GeoPolygonQueryBuilder extends AbstractQueryBuilder<GeoPolygonQuery
@Override
protected Query doToQuery(QueryShardContext context) throws IOException {
if (!shell.get(shell.size() - 1).equals(shell.get(0))) {
shell.add(shell.get(0));
List<GeoPoint> shell = new ArrayList<GeoPoint>();
for (GeoPoint geoPoint : this.shell) {
shell.add(new GeoPoint(geoPoint));
}
final boolean indexCreatedBeforeV2_0 = context.indexVersionCreated().before(Version.V_2_0_0);

1
core/src/main/java/org/elasticsearch/index/query/GeohashCellQuery.java
View File

@ -197,6 +197,7 @@ public class GeohashCellQuery {
fieldName);
}
String geohash = this.geohash;
if (levels != null) {
int len = Math.min(levels, geohash.length());
geohash = geohash.substring(0, len);

47
core/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java
View File

@ -28,7 +28,11 @@ import org.apache.lucene.search.Query;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.action.termvectors.*;
import org.elasticsearch.action.termvectors.MultiTermVectorsItemResponse;
import org.elasticsearch.action.termvectors.MultiTermVectorsRequest;
import org.elasticsearch.action.termvectors.MultiTermVectorsResponse;
import org.elasticsearch.action.termvectors.TermVectorsRequest;
import org.elasticsearch.action.termvectors.TermVectorsResponse;
import org.elasticsearch.client.Client;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseField;
@ -41,7 +45,11 @@ import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.lucene.search.MoreLikeThisQuery;
import org.elasticsearch.common.lucene.search.XMoreLikeThis;
import org.elasticsearch.common.lucene.uid.Versions;
import org.elasticsearch.common.xcontent.*;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.VersionType;
import org.elasticsearch.index.analysis.Analysis;
import org.elasticsearch.index.mapper.MappedFieldType;
@ -49,7 +57,17 @@ import org.elasticsearch.index.mapper.internal.UidFieldMapper;
import org.elasticsearch.search.internal.SearchContext;
import java.io.IOException;
import java.util.*;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.index.mapper.Uid.createUidAsBytes;
@ -135,6 +153,20 @@ public class MoreLikeThisQueryBuilder extends AbstractQueryBuilder<MoreLikeThisQ
}
Item(Item copy) {
if (copy.id == null && copy.doc == null) {
throw new IllegalArgumentException("Item requires either id or doc to be non-null");
}
this.index = copy.index;
this.type = copy.type;
this.id = copy.id;
this.doc = copy.doc;
this.fields = copy.fields;
this.perFieldAnalyzer = copy.perFieldAnalyzer;
this.version = copy.version;
this.versionType = copy.versionType;
}
/**
* Constructor for a given item / document request
*
@ -731,6 +763,15 @@ public class MoreLikeThisQueryBuilder extends AbstractQueryBuilder<MoreLikeThisQ
@Override
protected Query doToQuery(QueryShardContext context) throws IOException {
Item[] likeItems = new Item[this.likeItems.length];
for (int i = 0; i < likeItems.length; i++) {
likeItems[i] = new Item(this.likeItems[i]);
}
Item[] unlikeItems = new Item[this.unlikeItems.length];
for (int i = 0; i < unlikeItems.length; i++) {
unlikeItems[i] = new Item(this.unlikeItems[i]);
}
MoreLikeThisQuery mltQuery = new MoreLikeThisQuery();
// set similarity

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

@ -41,7 +41,10 @@ import org.elasticsearch.indices.cache.query.terms.TermsLookup;
import org.elasticsearch.search.internal.SearchContext;
import java.io.IOException;
import java.util.*;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
@ -267,7 +270,9 @@ public class TermsQueryBuilder extends AbstractQueryBuilder<TermsQueryBuilder> {
@Override
protected Query doToQuery(QueryShardContext context) throws IOException {
List<Object> terms;
TermsLookup termsLookup = null;
if (this.termsLookup != null) {
termsLookup = new TermsLookup(this.termsLookup);
if (termsLookup.index() == null) {
termsLookup.index(context.index().name());
}

6
core/src/main/java/org/elasticsearch/index/query/support/InnerHitsQueryParserHelper.java
View File

@ -21,10 +21,6 @@ package org.elasticsearch.index.query.support;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.index.query.QueryShardException;
import org.elasticsearch.index.query.QueryParseContext;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.search.fetch.fielddata.FieldDataFieldsParseElement;
import org.elasticsearch.search.fetch.innerhits.InnerHitsSubSearchContext;
import org.elasticsearch.search.fetch.script.ScriptFieldsParseElement;
@ -73,7 +69,7 @@ public class InnerHitsQueryParserHelper {
}
}
} catch (Exception e) {
throw new IOException("Failed to parse [_inner_hits]");
throw new IOException("Failed to parse [_inner_hits]", e);
}
return new InnerHitsSubSearchContext(innerHitName, subSearchContext);
}

5
core/src/main/java/org/elasticsearch/indices/cache/query/terms/TermsLookup.java vendored
View File

@ -42,6 +42,11 @@ public class TermsLookup implements Writeable<TermsLookup>, ToXContent {
private final String path;
private String routing;
public TermsLookup(TermsLookup copy) {
this(copy.index, copy.type, copy.id, copy.path);
this.routing = copy.routing;
}
public TermsLookup(String index, String type, String id, String path) {
if (id == null) {
throw new IllegalArgumentException("[terms] query lookup element requires specifying the id.");

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

@ -20,6 +20,7 @@
package org.elasticsearch.index.query;
import com.carrotsearch.randomizedtesting.generators.CodepointSetGenerator;
import org.apache.lucene.search.Query;
import org.elasticsearch.Version;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
@ -69,7 +70,12 @@ import org.elasticsearch.indices.IndicesModule;
import org.elasticsearch.indices.analysis.IndicesAnalysisService;
import org.elasticsearch.indices.breaker.CircuitBreakerService;
import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
import org.elasticsearch.script.*;
import org.elasticsearch.script.MockScriptEngine;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptContextRegistry;
import org.elasticsearch.script.ScriptEngineService;
import org.elasticsearch.script.ScriptModule;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.script.mustache.MustacheScriptEngineService;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.test.ESTestCase;
@ -80,13 +86,22 @@ import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.threadpool.ThreadPoolModule;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.junit.*;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import java.io.IOException;
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import java.util.*;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import static org.hamcrest.Matchers.equalTo;
@ -356,10 +371,17 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
context.setAllowUnmappedFields(true);
QB firstQuery = createTestQueryBuilder();
QB controlQuery = copyQuery(firstQuery);
setSearchContext(randomTypes); // only set search context for toQuery to be more realistic
Query firstLuceneQuery = firstQuery.toQuery(context);
assertLuceneQuery(firstQuery, firstLuceneQuery, context);
SearchContext.removeCurrent(); // remove after assertLuceneQuery since the assertLuceneQuery impl might access the context as well
assertTrue("query is not equal to its copy after calling toQuery, firstQuery: " + firstQuery + ", secondQuery: " + controlQuery,
firstQuery.equals(controlQuery));
assertTrue("equals is not symmetric after calling toQuery, firstQuery: " + firstQuery + ", secondQuery: " + controlQuery,
controlQuery.equals(firstQuery));
assertThat("query copy's hashcode is different from original hashcode after calling toQuery, firstQuery: " + firstQuery
+ ", secondQuery: " + controlQuery, controlQuery.hashCode(), equalTo(firstQuery.hashCode()));
QB secondQuery = copyQuery(firstQuery);
@ -473,7 +495,8 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
assertTrue("equals is not symmetric", thirdQuery.equals(firstQuery));
if (randomBoolean()) {
secondQuery.queryName(secondQuery.queryName() == null ? randomAsciiOfLengthBetween(1, 30) : secondQuery.queryName() + randomAsciiOfLengthBetween(1, 10));
secondQuery.queryName(secondQuery.queryName() == null ? randomAsciiOfLengthBetween(1, 30) : secondQuery.queryName()
+ randomAsciiOfLengthBetween(1, 10));
} else {
secondQuery.boost(firstQuery.boost() + 1f + randomFloat());
}

14
core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java
View File

@ -19,12 +19,20 @@
package org.elasticsearch.index.query;
import org.apache.lucene.search.*;
import org.apache.lucene.search.BooleanClause;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.MatchAllDocsQuery;
import org.apache.lucene.search.Query;
import org.hamcrest.Matchers;
import org.junit.Test;
import java.io.IOException;
import java.util.*;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.index.query.QueryBuilders.boolQuery;
@ -98,7 +106,7 @@ public class BoolQueryBuilderTests extends AbstractQueryTestCase<BoolQueryBuilde
assertThat(booleanQuery.clauses().size(), equalTo(clauses.size()));
Iterator<BooleanClause> clauseIterator = clauses.iterator();
for (BooleanClause booleanClause : booleanQuery.getClauses()) {
assertThat(booleanClause, equalTo(clauseIterator.next()));
assertThat(booleanClause, instanceOf(clauseIterator.next().getClass()));
}
}
}

5
core/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTests.java
View File

@ -26,7 +26,8 @@ import org.junit.Test;
import java.io.IOException;
import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.nullValue;
public class ConstantScoreQueryBuilderTests extends AbstractQueryTestCase<ConstantScoreQueryBuilder> {
@ -46,7 +47,7 @@ public class ConstantScoreQueryBuilderTests extends AbstractQueryTestCase<Consta
} else {
assertThat(query, instanceOf(ConstantScoreQuery.class));
ConstantScoreQuery constantScoreQuery = (ConstantScoreQuery) query;
assertThat(constantScoreQuery.getQuery(), equalTo(innerQuery));
assertThat(constantScoreQuery.getQuery(), instanceOf(innerQuery.getClass()));
}
}

5
core/src/test/java/org/elasticsearch/index/query/GeoDistanceQueryBuilderTests.java
View File

@ -20,6 +20,7 @@
package org.elasticsearch.index.query;
import com.spatial4j.core.shape.Point;
import org.apache.lucene.search.Query;
import org.elasticsearch.common.geo.GeoDistance;
import org.elasticsearch.common.geo.GeoPoint;
@ -30,7 +31,9 @@ import org.junit.Test;
import java.io.IOException;
import static org.hamcrest.Matchers.*;
import static org.hamcrest.Matchers.closeTo;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
public class GeoDistanceQueryBuilderTests extends AbstractQueryTestCase<GeoDistanceQueryBuilder> {

4
core/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java
View File

@ -52,7 +52,9 @@ import java.util.Map;
import java.util.stream.Stream;
import static org.elasticsearch.index.query.QueryBuilders.moreLikeThisQuery;
import static org.hamcrest.Matchers.*;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
public class MoreLikeThisQueryBuilderTests extends AbstractQueryTestCase<MoreLikeThisQueryBuilder> {

6
core/src/test/java/org/elasticsearch/index/query/NotQueryBuilderTests.java
View File

@ -31,7 +31,9 @@ import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.nullValue;
public class NotQueryBuilderTests extends AbstractQueryTestCase<NotQueryBuilder> {
@ -55,7 +57,7 @@ public class NotQueryBuilderTests extends AbstractQueryTestCase<NotQueryBuilder>
assertThat(booleanQuery.clauses().get(0).getOccur(), equalTo(BooleanClause.Occur.MUST));
assertThat(booleanQuery.clauses().get(0).getQuery(), instanceOf(MatchAllDocsQuery.class));
assertThat(booleanQuery.clauses().get(1).getOccur(), equalTo(BooleanClause.Occur.MUST_NOT));
assertThat(booleanQuery.clauses().get(1).getQuery(), equalTo(filter));
assertThat(booleanQuery.clauses().get(1).getQuery(), instanceOf(filter.getClass()));
}
}

19
core/src/test/java/org/elasticsearch/search/geo/GeoFilterIT.java
View File

@ -23,6 +23,7 @@ import com.spatial4j.core.context.SpatialContext;
import com.spatial4j.core.distance.DistanceUtils;
import com.spatial4j.core.exception.InvalidShapeException;
import com.spatial4j.core.shape.Shape;
import org.apache.lucene.spatial.prefix.RecursivePrefixTreeStrategy;
import org.apache.lucene.spatial.prefix.tree.GeohashPrefixTree;
import org.apache.lucene.spatial.query.SpatialArgs;
@ -43,8 +44,8 @@ import org.elasticsearch.common.geo.builders.PolygonBuilder;
import org.elasticsearch.common.geo.builders.ShapeBuilder;
import org.elasticsearch.common.io.Streams;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.index.query.GeohashCellQuery;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.test.ESIntegTestCase;
import org.junit.BeforeClass;
@ -65,9 +66,19 @@ import java.util.Random;
import java.util.zip.GZIPInputStream;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.index.query.QueryBuilders.*;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.*;
import static org.hamcrest.Matchers.*;
import static org.elasticsearch.index.query.QueryBuilders.geoBoundingBoxQuery;
import static org.elasticsearch.index.query.QueryBuilders.geoDistanceQuery;
import static org.elasticsearch.index.query.QueryBuilders.geoHashCellQuery;
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
import static org.elasticsearch.index.query.QueryBuilders.matchQuery;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFirstHit;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.hasId;
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
/**
*