Skip to content

Commit 328fe44

Browse files
jimczijimczi
authored
Delay the creation of SubSearchContext to the FetchSubPhase (#46598)
This change delays the creation of the SubSearchContext for nested and parent/child inner_hits to the fetch sub phase in order to ensure that a SearchContext can built entirely from a QueryShardContext. This commit also adds a validation step to the inner hits builder that ensures that we fail the request early if the inner hits path is invalid. Relates #46523
1 parent a84908c commit 328fe44

14 files changed

Lines changed: 141 additions & 118 deletions

File tree

‎modules/parent-join/src/main/java/org/elasticsearch/join/query/ParentChildInnerHitContextBuilder.java‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -67,29 +67,38 @@ class ParentChildInnerHitContextBuilder extends InnerHitContextBuilder {
6767
}
6868

6969
@Override
70-
protected void doBuild(SearchContext context, InnerHitsContext innerHitsContext) throws IOException {
70+
public void doValidate(QueryShardContext queryShardContext) {
71+
if (ParentJoinFieldMapper.getMapper(queryShardContext.getMapperService()) == null
72+
&& innerHitBuilder.isIgnoreUnmapped() == false) {
73+
throw new IllegalStateException("no join field has been configured");
74+
}
75+
}
76+
77+
@Override
78+
public void build(SearchContext context, InnerHitsContext innerHitsContext) throws IOException {
7179
QueryShardContext queryShardContext = context.getQueryShardContext();
7280
ParentJoinFieldMapper joinFieldMapper = ParentJoinFieldMapper.getMapper(context.mapperService());
73-
if (joinFieldMapper != null) {
74-
String name = innerHitBuilder.getName() != null ? innerHitBuilder.getName() : typeName;
75-
JoinFieldInnerHitSubContext joinFieldInnerHits = new JoinFieldInnerHitSubContext(name, context, typeName,
76-
fetchChildInnerHits, joinFieldMapper);
77-
setupInnerHitsContext(queryShardContext, joinFieldInnerHits);
78-
innerHitsContext.addInnerHitDefinition(joinFieldInnerHits);
79-
} else {
80-
if (innerHitBuilder.isIgnoreUnmapped() == false) {
81-
throw new IllegalStateException("no join field has been configured");
82-
}
81+
if (joinFieldMapper == null) {
82+
assert innerHitBuilder.isIgnoreUnmapped() : "should be validated first";
83+
return;
8384
}
85+
String name = innerHitBuilder.getName() != null ? innerHitBuilder.getName() : typeName;
86+
JoinFieldInnerHitSubContext joinFieldInnerHits =
87+
new JoinFieldInnerHitSubContext(name, context, typeName, fetchChildInnerHits, joinFieldMapper);
88+
setupInnerHitsContext(queryShardContext, joinFieldInnerHits);
89+
innerHitsContext.addInnerHitDefinition(joinFieldInnerHits);
8490
}
8591

8692
static final class JoinFieldInnerHitSubContext extends InnerHitsContext.InnerHitSubContext {
8793
private final String typeName;
8894
private final boolean fetchChildInnerHits;
8995
private final ParentJoinFieldMapper joinFieldMapper;
9096

91-
JoinFieldInnerHitSubContext(String name, SearchContext context, String typeName, boolean fetchChildInnerHits,
92-
ParentJoinFieldMapper joinFieldMapper) {
97+
JoinFieldInnerHitSubContext(String name,
98+
SearchContext context,
99+
String typeName,
100+
boolean fetchChildInnerHits,
101+
ParentJoinFieldMapper joinFieldMapper) {
93102
super(name, context);
94103
this.typeName = typeName;
95104
this.fetchChildInnerHits = fetchChildInnerHits;
@@ -102,13 +111,13 @@ public TopDocsAndMaxScore[] topDocs(SearchHit[] hits) throws IOException {
102111
TopDocsAndMaxScore[] result = new TopDocsAndMaxScore[hits.length];
103112
for (int i = 0; i < hits.length; i++) {
104113
SearchHit hit = hits[i];
105-
String joinName = getSortedDocValue(joinFieldMapper.name(), context, hit.docId());
114+
String joinName = getSortedDocValue(joinFieldMapper.name(), this, hit.docId());
106115
if (joinName == null) {
107116
result[i] = new TopDocsAndMaxScore(Lucene.EMPTY_TOP_DOCS, Float.NaN);
108117
continue;
109118
}
110119

111-
QueryShardContext qsc = context.getQueryShardContext();
120+
QueryShardContext qsc = getQueryShardContext();
112121
ParentIdFieldMapper parentIdFieldMapper =
113122
joinFieldMapper.getParentIdFieldMapper(typeName, fetchChildInnerHits == false);
114123
if (parentIdFieldMapper == null) {
@@ -126,14 +135,14 @@ public TopDocsAndMaxScore[] topDocs(SearchHit[] hits) throws IOException {
126135
.add(joinFieldMapper.fieldType().termQuery(typeName, qsc), BooleanClause.Occur.FILTER)
127136
.build();
128137
} else {
129-
String parentId = getSortedDocValue(parentIdFieldMapper.name(), context, hit.docId());
130-
q = context.mapperService().fullName(IdFieldMapper.NAME).termQuery(parentId, qsc);
138+
String parentId = getSortedDocValue(parentIdFieldMapper.name(), this, hit.docId());
139+
q = mapperService().fullName(IdFieldMapper.NAME).termQuery(parentId, qsc);
131140
}
132141

133-
Weight weight = context.searcher().createWeight(context.searcher().rewrite(q), ScoreMode.COMPLETE_NO_SCORES, 1f);
142+
Weight weight = searcher().createWeight(searcher().rewrite(q), ScoreMode.COMPLETE_NO_SCORES, 1f);
134143
if (size() == 0) {
135144
TotalHitCountCollector totalHitCountCollector = new TotalHitCountCollector();
136-
for (LeafReaderContext ctx : context.searcher().getIndexReader().leaves()) {
145+
for (LeafReaderContext ctx : searcher().getIndexReader().leaves()) {
137146
intersect(weight, innerHitQueryWeight, totalHitCountCollector, ctx);
138147
}
139148
result[i] = new TopDocsAndMaxScore(
@@ -142,7 +151,7 @@ public TopDocsAndMaxScore[] topDocs(SearchHit[] hits) throws IOException {
142151
Lucene.EMPTY_SCORE_DOCS
143152
), Float.NaN);
144153
} else {
145-
int topN = Math.min(from() + size(), context.searcher().getIndexReader().maxDoc());
154+
int topN = Math.min(from() + size(), searcher().getIndexReader().maxDoc());
146155
TopDocsCollector<?> topDocsCollector;
147156
MaxScoreCollector maxScoreCollector = null;
148157
if (sort() != null) {
@@ -155,7 +164,7 @@ public TopDocsAndMaxScore[] topDocs(SearchHit[] hits) throws IOException {
155164
maxScoreCollector = new MaxScoreCollector();
156165
}
157166
try {
158-
for (LeafReaderContext ctx : context.searcher().getIndexReader().leaves()) {
167+
for (LeafReaderContext ctx : searcher().getIndexReader().leaves()) {
159168
intersect(weight, innerHitQueryWeight, MultiCollector.wrap(topDocsCollector, maxScoreCollector), ctx);
160169
}
161170
} finally {

‎modules/parent-join/src/test/java/org/elasticsearch/join/query/HasChildQueryBuilderTests.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -177,14 +177,13 @@ protected void doAssertLuceneQuery(HasChildQueryBuilder queryBuilder, Query quer
177177
queryBuilder = (HasChildQueryBuilder) queryBuilder.rewrite(searchContext.getQueryShardContext());
178178
Map<String, InnerHitContextBuilder> innerHitBuilders = new HashMap<>();
179179
InnerHitContextBuilder.extractInnerHits(queryBuilder, innerHitBuilders);
180+
final InnerHitsContext innerHitsContext = new InnerHitsContext();
180181
for (InnerHitContextBuilder builder : innerHitBuilders.values()) {
181-
builder.build(searchContext, searchContext.innerHits());
182+
builder.build(searchContext, innerHitsContext);
182183
}
183-
assertNotNull(searchContext.innerHits());
184-
assertEquals(1, searchContext.innerHits().getInnerHits().size());
185-
assertTrue(searchContext.innerHits().getInnerHits().containsKey(queryBuilder.innerHit().getName()));
186-
InnerHitsContext.InnerHitSubContext innerHits =
187-
searchContext.innerHits().getInnerHits().get(queryBuilder.innerHit().getName());
184+
assertEquals(1, innerHitsContext.getInnerHits().size());
185+
assertTrue(innerHitsContext.getInnerHits().containsKey(queryBuilder.innerHit().getName()));
186+
InnerHitsContext.InnerHitSubContext innerHits = innerHitsContext.getInnerHits().get(queryBuilder.innerHit().getName());
188187
assertEquals(innerHits.size(), queryBuilder.innerHit().getSize());
189188
assertEquals(innerHits.sort().sort.getSort().length, 1);
190189
assertEquals(innerHits.sort().sort.getSort()[0].getField(), STRING_FIELD_NAME_2);

‎modules/parent-join/src/test/java/org/elasticsearch/join/query/HasParentQueryBuilderTests.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -148,17 +148,15 @@ protected void doAssertLuceneQuery(HasParentQueryBuilder queryBuilder, Query que
148148
// doCreateTestQueryBuilder)
149149
queryBuilder = (HasParentQueryBuilder) queryBuilder.rewrite(searchContext.getQueryShardContext());
150150

151-
assertNotNull(searchContext);
152151
Map<String, InnerHitContextBuilder> innerHitBuilders = new HashMap<>();
153152
InnerHitContextBuilder.extractInnerHits(queryBuilder, innerHitBuilders);
153+
final InnerHitsContext innerHitsContext = new InnerHitsContext();
154154
for (InnerHitContextBuilder builder : innerHitBuilders.values()) {
155-
builder.build(searchContext, searchContext.innerHits());
155+
builder.build(searchContext, innerHitsContext);
156156
}
157-
assertNotNull(searchContext.innerHits());
158-
assertEquals(1, searchContext.innerHits().getInnerHits().size());
159-
assertTrue(searchContext.innerHits().getInnerHits().containsKey(queryBuilder.innerHit().getName()));
160-
InnerHitsContext.InnerHitSubContext innerHits = searchContext.innerHits()
161-
.getInnerHits().get(queryBuilder.innerHit().getName());
157+
assertEquals(1, innerHitsContext.getInnerHits().size());
158+
assertTrue(innerHitsContext.getInnerHits().containsKey(queryBuilder.innerHit().getName()));
159+
InnerHitsContext.InnerHitSubContext innerHits = innerHitsContext.getInnerHits().get(queryBuilder.innerHit().getName());
162160
assertEquals(innerHits.size(), queryBuilder.innerHit().getSize());
163161
assertEquals(innerHits.sort().sort.getSort().length, 1);
164162
assertEquals(innerHits.sort().sort.getSort()[0].getField(), STRING_FIELD_NAME_2);

‎server/src/main/java/org/elasticsearch/index/query/InnerHitContextBuilder.java‎

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import org.elasticsearch.search.sort.SortBuilder;
3030

3131
import java.io.IOException;
32-
import java.util.HashMap;
3332
import java.util.Map;
3433
import java.util.Optional;
3534

@@ -47,9 +46,9 @@ protected InnerHitContextBuilder(QueryBuilder query, InnerHitBuilder innerHitBui
4746
this.query = query;
4847
}
4948

50-
public final void build(SearchContext parentSearchContext, InnerHitsContext innerHitsContext) throws IOException {
49+
public final void validate(QueryShardContext queryShardContext) {
5150
long innerResultWindow = innerHitBuilder.getFrom() + innerHitBuilder.getSize();
52-
int maxInnerResultWindow = parentSearchContext.mapperService().getIndexSettings().getMaxInnerResultWindow();
51+
int maxInnerResultWindow = queryShardContext.getIndexSettings().getMaxInnerResultWindow();
5352
if (innerResultWindow > maxInnerResultWindow) {
5453
throw new IllegalArgumentException(
5554
"Inner result window is too large, the inner hit definition's [" + innerHitBuilder.getName() +
@@ -58,10 +57,12 @@ public final void build(SearchContext parentSearchContext, InnerHitsContext inne
5857
"] index level setting."
5958
);
6059
}
61-
doBuild(parentSearchContext, innerHitsContext);
60+
doValidate(queryShardContext);
6261
}
6362

64-
protected abstract void doBuild(SearchContext parentSearchContext, InnerHitsContext innerHitsContext) throws IOException;
63+
protected abstract void doValidate(QueryShardContext queryShardContext);
64+
65+
public abstract void build(SearchContext parentSearchContext, InnerHitsContext innerHitsContext) throws IOException;
6566

6667
public static void extractInnerHits(QueryBuilder query, Map<String, InnerHitContextBuilder> innerHitBuilders) {
6768
if (query instanceof AbstractQueryBuilder) {
@@ -109,23 +110,6 @@ protected void setupInnerHitsContext(QueryShardContext queryShardContext,
109110
}
110111
ParsedQuery parsedQuery = new ParsedQuery(query.toQuery(queryShardContext), queryShardContext.copyNamedQueries());
111112
innerHitsContext.parsedQuery(parsedQuery);
112-
Map<String, InnerHitsContext.InnerHitSubContext> baseChildren =
113-
buildChildInnerHits(innerHitsContext.parentSearchContext(), children);
114-
innerHitsContext.setChildInnerHits(baseChildren);
115-
}
116-
117-
private static Map<String, InnerHitsContext.InnerHitSubContext> buildChildInnerHits(SearchContext parentSearchContext,
118-
Map<String, InnerHitContextBuilder> children) throws IOException {
119-
120-
Map<String, InnerHitsContext.InnerHitSubContext> childrenInnerHits = new HashMap<>();
121-
for (Map.Entry<String, InnerHitContextBuilder> entry : children.entrySet()) {
122-
InnerHitsContext childInnerHitsContext = new InnerHitsContext();
123-
entry.getValue().build(
124-
parentSearchContext, childInnerHitsContext);
125-
if (childInnerHitsContext.getInnerHits() != null) {
126-
childrenInnerHits.putAll(childInnerHitsContext.getInnerHits());
127-
}
128-
}
129-
return childrenInnerHits;
113+
innerHitsContext.innerHits(children);
130114
}
131115
}

‎server/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -332,28 +332,34 @@ public void extractInnerHitBuilders(Map<String, InnerHitContextBuilder> innerHit
332332
static class NestedInnerHitContextBuilder extends InnerHitContextBuilder {
333333
private final String path;
334334

335-
NestedInnerHitContextBuilder(String path, QueryBuilder query, InnerHitBuilder innerHitBuilder,
336-
Map<String, InnerHitContextBuilder> children) {
335+
NestedInnerHitContextBuilder(String path,
336+
QueryBuilder query,
337+
InnerHitBuilder innerHitBuilder,
338+
Map<String, InnerHitContextBuilder> children) {
337339
super(query, innerHitBuilder, children);
338340
this.path = path;
339341
}
340342

341343
@Override
342-
protected void doBuild(SearchContext parentSearchContext,
343-
InnerHitsContext innerHitsContext) throws IOException {
344-
QueryShardContext queryShardContext = parentSearchContext.getQueryShardContext();
344+
public void doValidate(QueryShardContext queryShardContext) {
345+
if (queryShardContext.getObjectMapper(path) == null
346+
&& innerHitBuilder.isIgnoreUnmapped() == false) {
347+
throw new IllegalStateException("[" + query.getName() + "] no mapping found for type [" + path + "]");
348+
}
349+
}
350+
351+
@Override
352+
public void build(SearchContext searchContext, InnerHitsContext innerHitsContext) throws IOException {
353+
QueryShardContext queryShardContext = searchContext.getQueryShardContext();
345354
ObjectMapper nestedObjectMapper = queryShardContext.getObjectMapper(path);
346355
if (nestedObjectMapper == null) {
347-
if (innerHitBuilder.isIgnoreUnmapped() == false) {
348-
throw new IllegalStateException("[" + query.getName() + "] no mapping found for type [" + path + "]");
349-
} else {
350-
return;
351-
}
356+
assert innerHitBuilder.isIgnoreUnmapped() : "should be validated first";
357+
return;
352358
}
353359
String name = innerHitBuilder.getName() != null ? innerHitBuilder.getName() : nestedObjectMapper.fullPath();
354360
ObjectMapper parentObjectMapper = queryShardContext.nestedScope().nextLevel(nestedObjectMapper);
355361
NestedInnerHitSubContext nestedInnerHits = new NestedInnerHitSubContext(
356-
name, parentSearchContext, parentObjectMapper, nestedObjectMapper
362+
name, searchContext, parentObjectMapper, nestedObjectMapper
357363
);
358364
setupInnerHitsContext(queryShardContext, nestedInnerHits);
359365
queryShardContext.nestedScope().previousLevel();
@@ -399,17 +405,17 @@ public TopDocsAndMaxScore[] topDocs(SearchHit[] hits) throws IOException {
399405
LeafReaderContext ctx = searcher().getIndexReader().leaves().get(readerIndex);
400406

401407
Query childFilter = childObjectMapper.nestedTypeFilter();
402-
BitSetProducer parentFilter = context.bitsetFilterCache().getBitSetProducer(rawParentFilter);
408+
BitSetProducer parentFilter = bitsetFilterCache().getBitSetProducer(rawParentFilter);
403409
Query q = new ParentChildrenBlockJoinQuery(parentFilter, childFilter, parentDocId);
404-
Weight weight = context.searcher().createWeight(context.searcher().rewrite(q),
410+
Weight weight = searcher().createWeight(searcher().rewrite(q),
405411
org.apache.lucene.search.ScoreMode.COMPLETE_NO_SCORES, 1f);
406412
if (size() == 0) {
407413
TotalHitCountCollector totalHitCountCollector = new TotalHitCountCollector();
408414
intersect(weight, innerHitQueryWeight, totalHitCountCollector, ctx);
409415
result[i] = new TopDocsAndMaxScore(new TopDocs(new TotalHits(totalHitCountCollector.getTotalHits(),
410416
TotalHits.Relation.EQUAL_TO), Lucene.EMPTY_SCORE_DOCS), Float.NaN);
411417
} else {
412-
int topN = Math.min(from() + size(), context.searcher().getIndexReader().maxDoc());
418+
int topN = Math.min(from() + size(), searcher().getIndexReader().maxDoc());
413419
TopDocsCollector<?> topDocsCollector;
414420
MaxScoreCollector maxScoreCollector = null;
415421
if (sort() != null) {

‎server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import org.elasticsearch.index.mapper.MapperService;
4545
import org.elasticsearch.index.mapper.ObjectMapper;
4646
import org.elasticsearch.index.query.AbstractQueryBuilder;
47+
import org.elasticsearch.index.query.InnerHitContextBuilder;
4748
import org.elasticsearch.index.query.ParsedQuery;
4849
import org.elasticsearch.index.query.QueryBuilder;
4950
import org.elasticsearch.index.query.QueryShardContext;
@@ -110,6 +111,7 @@ final class DefaultSearchContext extends SearchContext {
110111
private ScriptFieldsContext scriptFields;
111112
private FetchSourceContext fetchSourceContext;
112113
private DocValueFieldsContext docValueFieldsContext;
114+
private Map<String, InnerHitContextBuilder> innerHits = Collections.emptyMap();
113115
private int from = -1;
114116
private int size = -1;
115117
private SortAndFormats sort;
@@ -377,6 +379,16 @@ public void highlight(SearchContextHighlight highlight) {
377379
this.highlight = highlight;
378380
}
379381

382+
@Override
383+
public void innerHits(Map<String, InnerHitContextBuilder> innerHits) {
384+
this.innerHits = innerHits;
385+
}
386+
387+
@Override
388+
public Map<String, InnerHitContextBuilder> innerHits() {
389+
return innerHits;
390+
}
391+
380392
@Override
381393
public SuggestionSearchContext suggest() {
382394
return suggest;

‎server/src/main/java/org/elasticsearch/search/SearchService.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,7 @@ private void parseSource(DefaultSearchContext context, SearchSourceBuilder sourc
739739
context.from(source.from());
740740
context.size(source.size());
741741
Map<String, InnerHitContextBuilder> innerHitBuilders = new HashMap<>();
742+
context.innerHits(innerHitBuilders);
742743
if (source.query() != null) {
743744
InnerHitContextBuilder.extractInnerHits(source.query(), innerHitBuilders);
744745
context.parsedQuery(queryShardContext.toQuery(source.query()));
@@ -749,11 +750,7 @@ private void parseSource(DefaultSearchContext context, SearchSourceBuilder sourc
749750
}
750751
if (innerHitBuilders.size() > 0) {
751752
for (Map.Entry<String, InnerHitContextBuilder> entry : innerHitBuilders.entrySet()) {
752-
try {
753-
entry.getValue().build(context, context.innerHits());
754-
} catch (IOException e) {
755-
throw new SearchContextException(context, "failed to build inner_hits", e);
756-
}
753+
entry.getValue().validate(queryShardContext);
757754
}
758755
}
759756
if (source.sorts() != null) {

0 commit comments

Comments
 (0)