Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Has child query forces default similarity #16611

Closed
wants to merge 1 commit into from
Closed

Has child query forces default similarity #16611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 9 additions & 2 deletions core/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java
View file
Open in desktop
Expand Up @@ -26,6 +26,7 @@
import org.apache.lucene.search.Query;
import org.apache.lucene.search.join.JoinUtil;
import org.apache.lucene.search.join.ScoreMode;
import org.apache.lucene.search.similarities.Similarity;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
Expand Down Expand Up @@ -262,7 +263,8 @@ protected Query doToQuery(QueryShardContext context) throws IOException {
if (maxChildren == 0) {
maxChildren = Integer.MAX_VALUE;
}
return new LateParsingQuery(parentDocMapper.typeFilter(), innerQuery, minChildren(), maxChildren, parentType, scoreMode, parentChildIndexFieldData);
return new LateParsingQuery(parentDocMapper.typeFilter(), innerQuery, minChildren(), maxChildren,
parentType, scoreMode, parentChildIndexFieldData, context.getSearchSimilarity());
}

final static class LateParsingQuery extends Query {
Expand All @@ -274,15 +276,19 @@ final static class LateParsingQuery extends Query {
private final String parentType;
private final ScoreMode scoreMode;
private final ParentChildIndexFieldData parentChildIndexFieldData;
private final Similarity similarity;

LateParsingQuery(Query toQuery, Query innerQuery, int minChildren, int maxChildren, String parentType, ScoreMode scoreMode, ParentChildIndexFieldData parentChildIndexFieldData) {
LateParsingQuery(Query toQuery, Query innerQuery, int minChildren, int maxChildren,
String parentType, ScoreMode scoreMode, ParentChildIndexFieldData parentChildIndexFieldData,
Similarity similarity) {
this.toQuery = toQuery;
this.innerQuery = innerQuery;
this.minChildren = minChildren;
this.maxChildren = maxChildren;
this.parentType = parentType;
this.scoreMode = scoreMode;
this.parentChildIndexFieldData = parentChildIndexFieldData;
this.similarity = similarity;
}

@Override
Expand All @@ -295,6 +301,7 @@ public Query rewrite(IndexReader reader) throws IOException {
String joinField = ParentFieldMapper.joinField(parentType);
IndexSearcher indexSearcher = new IndexSearcher(reader);
indexSearcher.setQueryCache(null);
indexSearcher.setSimilarity(similarity);
IndexParentChildFieldData indexParentChildFieldData = parentChildIndexFieldData.loadGlobal((DirectoryReader) reader);
MultiDocValues.OrdinalMap ordinalMap = ParentChildIndexFieldData.getOrdinalMap(indexParentChildFieldData, parentType);
return JoinUtil.createJoinQuery(joinField, innerQuery, toQuery, indexSearcher, scoreMode, ordinalMap, minChildren, maxChildren);
Expand Down
9 changes: 8 additions & 1 deletion core/src/main/java/org/elasticsearch/index/query/HasParentQueryBuilder.java
View file
Open in desktop
Expand Up @@ -195,7 +195,14 @@ protected Query doToQuery(QueryShardContext context) throws IOException {
// wrap the query with type query
innerQuery = Queries.filtered(innerQuery, parentDocMapper.typeFilter());
Query childrenFilter = Queries.not(parentTypeQuery);
return new HasChildQueryBuilder.LateParsingQuery(childrenFilter, innerQuery, HasChildQueryBuilder.DEFAULT_MIN_CHILDREN, HasChildQueryBuilder.DEFAULT_MAX_CHILDREN, type, score ? ScoreMode.Max : ScoreMode.None, parentChildIndexFieldData);
return new HasChildQueryBuilder.LateParsingQuery(childrenFilter,
innerQuery,
HasChildQueryBuilder.DEFAULT_MIN_CHILDREN,
HasChildQueryBuilder.DEFAULT_MAX_CHILDREN,
type,
score ? ScoreMode.Max : ScoreMode.None,
parentChildIndexFieldData,
context.getSearchSimilarity());
}

@Override
Expand Down
48 changes: 48 additions & 0 deletions core/src/test/java/org/elasticsearch/search/child/ChildQuerySearchIT.java
View file
Open in desktop
Expand Up @@ -51,6 +51,7 @@

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -1926,4 +1927,51 @@ public void testHasChildInnerQueryType() {
QueryBuilders.hasChildQuery("child-type", new IdsQueryBuilder().addIds("child-id"))).get();
assertSearchHits(searchResponse, "parent-id");
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@martijnvg can we do this with a unittest HasChildQueryBuilderTests should be able to verify this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, that is better. We should just test the integration part here (whether non default similarity gets applied) and not whether right scores get computed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add this test on top of this pr. For the 2.x branches we should still use the integ tests, since that is the only way to tests this stuff there.

// Tests #16550
public void testHasChildWithNonDefaultGlobalSimilarity() {
assertAcked(prepareCreate("test").setSettings(settingsBuilder().put(indexSettings())
.put("index.similarity.default.type", "BM25"))
.addMapping("parent")
.addMapping("child", "_parent", "type=parent", "c_field", "type=string"));
ensureGreen();

verifyNonDefaultSimilarity();
}

// Tests #16550
public void testHasChildWithNonDefaultFieldSimilarity() {
assertAcked(prepareCreate("test")
.addMapping("parent")
.addMapping("child", "_parent", "type=parent", "c_field", "type=string,similarity=BM25"));
ensureGreen();

verifyNonDefaultSimilarity();
}

// Tests #16550
private void verifyNonDefaultSimilarity() {
client().prepareIndex("test", "parent", "p1").setSource("p_field", "p_value1").get();
client().prepareIndex("test", "child", "c1").setSource("c_field", "c_value").setParent("p1").get();
client().prepareIndex("test", "child", "c2").setSource("c_field", "c_value").setParent("p1").get();
refresh();

// baseline: sum of scores of matching child docs outside of has_child query
SearchResponse searchResponse = client().prepareSearch("test")
.setTypes("child")
.setQuery(matchQuery("c_field", "c_value"))
.get();
assertSearchHits(searchResponse, "c1", "c2");
Float childSum = (float) Arrays.asList(searchResponse.getHits().getHits())
.stream()
.mapToDouble(hit -> hit.getScore())
.sum();

// compare baseline to has_child with 'total' score_mode
searchResponse = client().prepareSearch("test")
.setQuery(hasChildQuery("child", matchQuery("c_field", "c_value")).scoreMode(ScoreMode.Total))
.get();
assertSearchHits(searchResponse, "p1");
assertThat(searchResponse.getHits().hits()[0].score(), equalTo(childSum));
}
}