honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-03-24 17:09:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

Query refactoring: set has_parent & has_child types context properly

Browse Source 
While refactoring has_child and has_parent query we lost an important detail around types. The types that the inner query gets executed against shouldn't be the main types of the search request but the parent or child type set to the parent query. We used to use QueryParseContext#setTypesWithPrevious as part of XContentStructure class which has been deleted, without taking care though of setting the types and restoring them as part of the innerQuery#toQuery call.

Meanwhile also we make sure that the original context types are restored in PercolatorQueriesRegistry

Closes #13863
This commit is contained in:
javanna 2015-09-29 23:54:53 +02:00 committed by Luca Cavanna
parent bb96dcd3e6
commit a3abfab865
7 changed files with 107 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
core/src/main/java/org/elasticsearch/index/percolator/PercolatorQueriesRegistry.java
View File

@ -187,7 +187,7 @@ public class PercolatorQueriesRegistry extends AbstractIndexShardComponent imple
private Query parseQuery(String type, XContentParser parser) {
String[] previousTypes = null;
if (type != null) {
QueryShardContext.setTypesWithPrevious(new String[]{type});
previousTypes = QueryShardContext.setTypesWithPrevious(type);
}
QueryShardContext context = queryParserService.getShardContext();
try {

12
core/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java
View File

@ -203,7 +203,13 @@ public class HasChildQueryBuilder extends AbstractQueryBuilder<HasChildQueryBuil
@Override
protected Query doToQuery(QueryShardContext context) throws IOException {
Query innerQuery = query.toQuery(context);
String[] previousTypes = QueryShardContext.setTypesWithPrevious(type);
Query innerQuery;
try {
innerQuery = query.toQuery(context);
} finally {
QueryShardContext.setTypes(previousTypes);
}
if (innerQuery == null) {
return null;
}
@ -333,6 +339,10 @@ public class HasChildQueryBuilder extends AbstractQueryBuilder<HasChildQueryBuil
public ScoreMode getScoreMode() {
return scoreMode;
}
public Query getInnerQuery() {
return innerQuery;
}
}
@Override

9
core/src/main/java/org/elasticsearch/index/query/HasParentQueryBuilder.java
View File

@ -118,7 +118,14 @@ public class HasParentQueryBuilder extends AbstractQueryBuilder<HasParentQueryBu
@Override
protected Query doToQuery(QueryShardContext context) throws IOException {
Query innerQuery = query.toQuery(context);
Query innerQuery;
String[] previousTypes = QueryShardContext.setTypesWithPrevious(type);
try {
innerQuery = query.toQuery(context);
} finally {
QueryShardContext.setTypes(previousTypes);
}
if (innerQuery == null) {
return null;
}

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

@ -69,7 +69,7 @@ public class QueryShardContext {
return typesContext.get();
}
public static String[] setTypesWithPrevious(String[] types) {
public static String[] setTypesWithPrevious(String... types) {
String[] old = typesContext.get();
setTypes(types);
return old;

51
core/src/test/java/org/elasticsearch/index/query/HasChildQueryBuilderTests.java
View File

@ -20,8 +20,10 @@
package org.elasticsearch.index.query;
import com.carrotsearch.randomizedtesting.generators.RandomPicks;
import org.apache.lucene.search.Query;
import org.apache.lucene.queries.TermsQuery;
import org.apache.lucene.search.*;
import org.apache.lucene.search.join.ScoreMode;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.xcontent.ToXContent;
@ -29,6 +31,9 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.index.fielddata.IndexFieldDataService;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.Uid;
import org.elasticsearch.index.mapper.internal.TypeFieldMapper;
import org.elasticsearch.index.mapper.internal.UidFieldMapper;
import org.elasticsearch.index.query.support.QueryInnerHits;
import org.elasticsearch.search.fetch.innerhits.InnerHitsBuilder;
import org.elasticsearch.search.fetch.innerhits.InnerHitsContext;
@ -37,8 +42,10 @@ import org.elasticsearch.search.sort.SortOrder;
import org.elasticsearch.test.TestSearchContext;
import java.io.IOException;
import java.util.Collections;
import static org.elasticsearch.test.StreamsUtils.copyToStringFromClasspath;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.instanceOf;
public class HasChildQueryBuilderTests extends AbstractQueryTestCase<HasChildQueryBuilder> {
@ -199,4 +206,46 @@ public class HasChildQueryBuilderTests extends AbstractQueryTestCase<HasChildQue
assertEquals(query, builder.string());
}
public void testToQueryInnerQueryType() throws IOException {
String[] searchTypes = new String[]{PARENT_TYPE};
QueryShardContext.setTypes(searchTypes);
HasChildQueryBuilder hasChildQueryBuilder = new HasChildQueryBuilder(CHILD_TYPE, new IdsQueryBuilder().addIds("id"));
Query query = hasChildQueryBuilder.toQuery(createShardContext());
//verify that the context types are still the same as the ones we previously set
assertThat(QueryShardContext.getTypes(), equalTo(searchTypes));
assertLateParsingQuery(query, CHILD_TYPE, "id");
}
static void assertLateParsingQuery(Query query, String type, String id) throws IOException {
assertThat(query, instanceOf(HasChildQueryBuilder.LateParsingQuery.class));
HasChildQueryBuilder.LateParsingQuery lateParsingQuery = (HasChildQueryBuilder.LateParsingQuery) query;
assertThat(lateParsingQuery.getInnerQuery(), instanceOf(BooleanQuery.class));
BooleanQuery booleanQuery = (BooleanQuery) lateParsingQuery.getInnerQuery();
assertThat(booleanQuery.clauses().size(), equalTo(2));
//check the inner ids query, we have to call rewrite to get to check the type it's executed against
assertThat(booleanQuery.clauses().get(0).getOccur(), equalTo(BooleanClause.Occur.MUST));
assertThat(booleanQuery.clauses().get(0).getQuery(), instanceOf(TermsQuery.class));
TermsQuery termsQuery = (TermsQuery) booleanQuery.clauses().get(0).getQuery();
Query rewrittenTermsQuery = termsQuery.rewrite(null);
assertThat(rewrittenTermsQuery, instanceOf(ConstantScoreQuery.class));
ConstantScoreQuery constantScoreQuery = (ConstantScoreQuery) rewrittenTermsQuery;
assertThat(constantScoreQuery.getQuery(), instanceOf(BooleanQuery.class));
BooleanQuery booleanTermsQuery = (BooleanQuery) constantScoreQuery.getQuery();
assertThat(booleanTermsQuery.clauses().size(), equalTo(1));
assertThat(booleanTermsQuery.clauses().get(0).getOccur(), equalTo(BooleanClause.Occur.SHOULD));
assertThat(booleanTermsQuery.clauses().get(0).getQuery(), instanceOf(TermQuery.class));
TermQuery termQuery = (TermQuery) booleanTermsQuery.clauses().get(0).getQuery();
assertThat(termQuery.getTerm().field(), equalTo(UidFieldMapper.NAME));
//we want to make sure that the inner ids query gets executed against the child type rather than the main type we initially set to the context
BytesRef[] ids = Uid.createUidsForTypesAndIds(Collections.singletonList(type), Collections.singletonList(id));
assertThat(termQuery.getTerm().bytes(), equalTo(ids[0]));
//check the type filter
assertThat(booleanQuery.clauses().get(1).getOccur(), equalTo(BooleanClause.Occur.FILTER));
assertThat(booleanQuery.clauses().get(1).getQuery(), instanceOf(ConstantScoreQuery.class));
ConstantScoreQuery typeConstantScoreQuery = (ConstantScoreQuery) booleanQuery.clauses().get(1).getQuery();
assertThat(typeConstantScoreQuery.getQuery(), instanceOf(TermQuery.class));
TermQuery typeTermQuery = (TermQuery) typeConstantScoreQuery.getQuery();
assertThat(typeTermQuery.getTerm().field(), equalTo(TypeFieldMapper.NAME));
assertThat(typeTermQuery.getTerm().text(), equalTo(type));
}
}

15
core/src/test/java/org/elasticsearch/index/query/HasParentQueryBuilderTests.java
View File

@ -25,7 +25,9 @@ import org.apache.lucene.search.join.ScoreMode;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
import org.elasticsearch.common.ParseFieldMatcher;
import org.elasticsearch.common.compress.CompressedXContent;
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.index.fielddata.IndexFieldDataService;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.query.support.QueryInnerHits;
@ -38,6 +40,7 @@ import org.elasticsearch.test.TestSearchContext;
import java.io.IOException;
import java.util.Arrays;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.instanceOf;
public class HasParentQueryBuilderTests extends AbstractQueryTestCase<HasParentQueryBuilder> {
@ -191,4 +194,14 @@ public class HasParentQueryBuilderTests extends AbstractQueryTestCase<HasParentQ
queryBuilder = (HasParentQueryBuilder) parseQuery(builder.string(), ParseFieldMatcher.EMPTY);
assertEquals(score, queryBuilder.score());
}
public void testToQueryInnerQueryType() throws IOException {
String[] searchTypes = new String[]{CHILD_TYPE};
QueryShardContext.setTypes(searchTypes);
HasParentQueryBuilder hasParentQueryBuilder = new HasParentQueryBuilder(PARENT_TYPE, new IdsQueryBuilder().addIds("id"));
Query query = hasParentQueryBuilder.toQuery(createShardContext());
//verify that the context types are still the same as the ones we previously set
assertThat(QueryShardContext.getTypes(), equalTo(searchTypes));
HasChildQueryBuilderTests.assertLateParsingQuery(query, PARENT_TYPE, "id");
}
}

25
plugins/lang-groovy/src/test/java/org/elasticsearch/messy/tests/ChildQuerySearchTests.java
View File

@ -34,9 +34,7 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.index.cache.IndexCacheModule;
import org.elasticsearch.index.mapper.MergeMappingException;
import org.elasticsearch.index.query.HasChildQueryBuilder;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.index.query.*;
import org.elasticsearch.index.query.functionscore.FunctionScoreQueryBuilder;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.rest.RestStatus;
@ -1938,4 +1936,25 @@ public class ChildQuerySearchTests extends ESIntegTestCase {
return hasChildQueryBuilder;
}
public void testHasParentInnerQueryType() {
assertAcked(prepareCreate("test").addMapping("parent-type").addMapping("child-type", "_parent", "type=parent-type"));
client().prepareIndex("test", "child-type", "child-id").setParent("parent-id").setSource("{}").get();
client().prepareIndex("test", "parent-type", "parent-id").setSource("{}").get();
refresh();
//make sure that when we explicitly set a type, the inner query is executed in the context of the parent type instead
SearchResponse searchResponse = client().prepareSearch("test").setTypes("child-type").setQuery(
QueryBuilders.hasParentQuery("parent-type", new IdsQueryBuilder().addIds("parent-id"))).get();
assertSearchHits(searchResponse, "child-id");
}
public void testHasChildInnerQueryType() {
assertAcked(prepareCreate("test").addMapping("parent-type").addMapping("child-type", "_parent", "type=parent-type"));
client().prepareIndex("test", "child-type", "child-id").setParent("parent-id").setSource("{}").get();
client().prepareIndex("test", "parent-type", "parent-id").setSource("{}").get();
refresh();
//make sure that when we explicitly set a type, the inner query is executed in the context of the child type instead
SearchResponse searchResponse = client().prepareSearch("test").setTypes("parent-type").setQuery(
QueryBuilders.hasChildQuery("child-type", new IdsQueryBuilder().addIds("child-id"))).get();
assertSearchHits(searchResponse, "parent-id");
}
}