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

Shrink slow log for has_child query #83870

Merged
merged 2 commits into from
Feb 14, 2022
+93 −9
Merged

Shrink slow log for has_child query #83870

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
28e91e5
Shrink slow log for has_child query
nik9000 Feb 11, 2022
1d5df5b
Speeling
nik9000 Feb 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions modules/parent-join/src/main/java/org/elasticsearch/join/query/HasChildQueryBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public class HasChildQueryBuilder extends AbstractQueryBuilder<HasChildQueryBuil
* The default minimum number of children that are required to match for the parent to be considered a match.
*/
public static final int DEFAULT_MIN_CHILDREN = 1;
private static final ScoreMode DEFAULT_SCORE_MODE = ScoreMode.None;

/**
* The default value for ignore_unmapped.
Expand All @@ -80,7 +81,7 @@ public class HasChildQueryBuilder extends AbstractQueryBuilder<HasChildQueryBuil
private InnerHitBuilder innerHitBuilder;
private int minChildren = DEFAULT_MIN_CHILDREN;
private int maxChildren = DEFAULT_MAX_CHILDREN;
private boolean ignoreUnmapped = false;
private boolean ignoreUnmapped = DEFAULT_IGNORE_UNMAPPED;

public HasChildQueryBuilder(String type, QueryBuilder query, ScoreMode scoreMode) {
this(type, query, DEFAULT_MIN_CHILDREN, DEFAULT_MAX_CHILDREN, scoreMode, null);
Expand Down Expand Up @@ -224,11 +225,19 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
builder.field(QUERY_FIELD.getPreferredName());
query.toXContent(builder, params);
builder.field(TYPE_FIELD.getPreferredName(), type);
builder.field(SCORE_MODE_FIELD.getPreferredName(), NestedQueryBuilder.scoreModeAsString(scoreMode));
builder.field(MIN_CHILDREN_FIELD.getPreferredName(), minChildren);
builder.field(MAX_CHILDREN_FIELD.getPreferredName(), maxChildren);
builder.field(IGNORE_UNMAPPED_FIELD.getPreferredName(), ignoreUnmapped);
printBoostAndQueryName(builder);
if (false == scoreMode.equals(DEFAULT_SCORE_MODE)) {
builder.field(SCORE_MODE_FIELD.getPreferredName(), NestedQueryBuilder.scoreModeAsString(scoreMode));
}
if (minChildren != DEFAULT_MIN_CHILDREN) {
builder.field(MIN_CHILDREN_FIELD.getPreferredName(), minChildren);
}
if (maxChildren != DEFAULT_MAX_CHILDREN) {
builder.field(MAX_CHILDREN_FIELD.getPreferredName(), maxChildren);
}
if (ignoreUnmapped != DEFAULT_IGNORE_UNMAPPED) {
builder.field(IGNORE_UNMAPPED_FIELD.getPreferredName(), ignoreUnmapped);
}
boostAndQueryNameToXContent(builder);
if (innerHitBuilder != null) {
builder.field(INNER_HITS_FIELD.getPreferredName(), innerHitBuilder, params);
}
Expand All @@ -238,7 +247,7 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
public static HasChildQueryBuilder fromXContent(XContentParser parser) throws IOException {
float boost = AbstractQueryBuilder.DEFAULT_BOOST;
String childType = null;
ScoreMode scoreMode = ScoreMode.None;
ScoreMode scoreMode = DEFAULT_SCORE_MODE;
int minChildren = HasChildQueryBuilder.DEFAULT_MIN_CHILDREN;
int maxChildren = HasChildQueryBuilder.DEFAULT_MAX_CHILDREN;
boolean ignoreUnmapped = DEFAULT_IGNORE_UNMAPPED;
Expand Down
79 changes: 77 additions & 2 deletions ...les/parent-join/src/test/java/org/elasticsearch/join/query/HasChildQueryBuilderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import org.elasticsearch.index.query.WrapperQueryBuilder;
import org.elasticsearch.index.similarity.SimilarityService;
import org.elasticsearch.join.ParentJoinPlugin;
import org.elasticsearch.join.query.HasChildQueryBuilder.LateParsingQuery;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.search.sort.FieldSortBuilder;
import org.elasticsearch.search.sort.SortOrder;
Expand All @@ -55,7 +56,6 @@
import java.util.HashMap;
import java.util.Map;

import static org.elasticsearch.join.query.HasChildQueryBuilder.LateParsingQuery;
import static org.elasticsearch.join.query.JoinQueryBuilders.hasChildQuery;
import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
import static org.hamcrest.CoreMatchers.containsString;
Expand Down Expand Up @@ -245,7 +245,14 @@ public void testFromJson() throws IOException {
}
}""";
HasChildQueryBuilder queryBuilder = (HasChildQueryBuilder) parseQuery(query);
checkGeneratedJson(query, queryBuilder);
checkGeneratedJson(
/*
* Ignoring unampped is the default and we don't dump it and can't
Copy link
Contributor

Choose a reason for hiding this comment

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

s/unampped/unmapped/

* change it if we're going to use inner_hits.
*/
query.replaceFirst("\"ignore_unmapped\" : false,", ""),
queryBuilder
);
assertEquals(query, queryBuilder.maxChildren(), 1217235442);
assertEquals(query, queryBuilder.minChildren(), 883170873);
assertEquals(query, queryBuilder.boost(), 2.0f, 0.0f);
Expand All @@ -259,6 +266,74 @@ public void testFromJson() throws IOException {
assertEquals(query, queryBuilder.innerHit(), expected);
}

public void testParseDefaultsRemoved() throws IOException {
String query = """
{
"has_child" : {
"query" : {
"range" : {
"mapped_string" : {
"gte" : "agJhRET",
"lte" : "zvqIq",
"boost" : 1.0
}
}
},
"type" : "child",
"score_mode" : "none",
"min_children" : 1,
"max_children" : MAX_CHILDREN,
"ignore_unmapped" : false,
"boost" : 1.0,
"inner_hits" : {
"name" : "inner_hits_name",
"ignore_unmapped" : false,
"from" : 0,
"size" : 100,
"version" : false,
"seq_no_primary_term" : false,
"explain" : false,
"track_scores" : false,
"sort" : [ {
"mapped_string" : {
"order" : "asc"
}
} ]
}
}
}""".replaceAll("MAX_CHILDREN", Integer.toString(Integer.MAX_VALUE));
checkGeneratedJson("""
{
"has_child" : {
"query" : {
"range" : {
"mapped_string" : {
"gte" : "agJhRET",
"lte" : "zvqIq",
"boost" : 1.0
}
}
},
"type" : "child",
"inner_hits" : {
"name" : "inner_hits_name",
"ignore_unmapped" : false,
"from" : 0,
"size" : 100,
"version" : false,
"seq_no_primary_term" : false,
"explain" : false,
"track_scores" : false,
"sort" : [ {
"mapped_string" : {
"order" : "asc"
}
} ]
}
}
}""", parseQuery(query));
}

Copy link
Contributor

Choose a reason for hiding this comment

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

❤️ text blocks

public void testToQueryInnerQueryType() throws IOException {
SearchExecutionContext searchExecutionContext = createSearchExecutionContext();
HasChildQueryBuilder hasChildQueryBuilder = hasChildQuery(CHILD_DOC, new IdsQueryBuilder().addIds("id"), ScoreMode.None);
Expand Down