Skip to content

Commit facc1cc

Browse files
jtibshiranijtibshirani
committed
Resolve field names only one time for all hits.
1 parent e6c86a7 commit facc1cc

File tree

3 files changed

+85
-111
lines changed

3 files changed

+85
-111
lines changed

‎server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchFieldsPhase.java‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,11 @@
1919

2020
package org.elasticsearch.search.fetch.subphase;
2121

22+
import org.apache.lucene.index.LeafReaderContext;
23+
import org.apache.lucene.index.ReaderUtil;
2224
import org.elasticsearch.common.document.DocumentField;
2325
import org.elasticsearch.index.mapper.DocumentMapper;
26+
import org.elasticsearch.search.SearchHit;
2427
import org.elasticsearch.search.fetch.FetchSubPhase;
2528
import org.elasticsearch.search.internal.SearchContext;
2629
import org.elasticsearch.search.lookup.SourceLookup;
@@ -39,7 +42,7 @@ public final class FetchFieldsPhase implements FetchSubPhase {
3942

4043
@Override
4144
@SuppressWarnings("unchecked")
42-
public void hitExecute(SearchContext context, HitContext hitContext) {
45+
public void hitsExecute(SearchContext context, SearchHit[] hits) {
4346
FetchFieldsContext fetchFieldsContext = context.fetchFieldsContext();
4447
if (fetchFieldsContext == null || fetchFieldsContext.fields().isEmpty()) {
4548
return;
@@ -61,17 +64,24 @@ public void hitExecute(SearchContext context, HitContext hitContext) {
6164
}
6265

6366
SourceLookup sourceLookup = context.lookup().source();
64-
Map<String, Object> valuesByField = sourceLookup.extractValues(fields);
6567

66-
for (Map.Entry<String, Object> entry : valuesByField.entrySet()) {
67-
String field = entry.getKey();
68-
Object value = entry.getValue();
69-
List<Object> values = value instanceof List
70-
? (List<Object>) value
71-
: List.of(value);
68+
for (SearchHit hit : hits) {
69+
int readerIndex = ReaderUtil.subIndex(hit.docId(), context.searcher().getIndexReader().leaves());
70+
LeafReaderContext readerContext = context.searcher().getIndexReader().leaves().get(readerIndex);
71+
sourceLookup.setSegmentAndDocument(readerContext, hit.docId());
7272

73-
DocumentField documentField = new DocumentField(field, values);
74-
hitContext.hit().setField(field, documentField);
73+
Map<String, Object> valuesByField = sourceLookup.extractValues(fields);
74+
75+
for (Map.Entry<String, Object> entry : valuesByField.entrySet()) {
76+
String field = entry.getKey();
77+
Object value = entry.getValue();
78+
List<Object> values = value instanceof List
79+
? (List<Object>) value
80+
: List.of(value);
81+
82+
DocumentField documentField = new DocumentField(field, values);
83+
hit.setField(field, documentField);
84+
}
7585
}
7686
}
7787
}

‎server/src/main/java/org/elasticsearch/search/lookup/SourceLookup.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,9 @@ public List<Object> extractRawValues(String path) {
129129
return XContentMapValues.extractRawValues(path, loadSourceIfNeeded());
130130
}
131131

132+
/**
133+
* For each of the provided paths, return its value in the source. Note that
134+
*/
132135
public Map<String, Object> extractValues(Collection<String> paths) {
133136
Map<String, Object> source = loadSourceIfNeeded();
134137
Map<String, Object> result = new HashMap<>(paths.size());

‎server/src/test/java/org/elasticsearch/search/fetch/subphase/FetchFieldsPhaseTests.java‎

Lines changed: 62 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -19,22 +19,18 @@
1919

2020
package org.elasticsearch.search.fetch.subphase;
2121

22-
import org.elasticsearch.common.bytes.BytesReference;
22+
import org.elasticsearch.action.search.SearchPhaseExecutionException;
23+
import org.elasticsearch.action.search.SearchRequestBuilder;
24+
import org.elasticsearch.action.search.SearchResponse;
25+
import org.elasticsearch.action.support.WriteRequest;
2326
import org.elasticsearch.common.document.DocumentField;
24-
import org.elasticsearch.common.settings.Settings;
2527
import org.elasticsearch.common.xcontent.XContentBuilder;
2628
import org.elasticsearch.common.xcontent.XContentFactory;
27-
import org.elasticsearch.index.IndexService;
2829
import org.elasticsearch.search.SearchHit;
29-
import org.elasticsearch.search.fetch.FetchSubPhase;
30-
import org.elasticsearch.search.internal.SearchContext;
31-
import org.elasticsearch.search.lookup.SearchLookup;
3230
import org.elasticsearch.test.ESSingleNodeTestCase;
33-
import org.elasticsearch.test.TestSearchContext;
3431
import org.junit.Before;
3532

3633
import java.io.IOException;
37-
import java.util.List;
3834
import java.util.Map;
3935

4036
import static org.hamcrest.Matchers.containsString;
@@ -43,10 +39,8 @@
4339

4440
public class FetchFieldsPhaseTests extends ESSingleNodeTestCase {
4541

46-
private IndexService indexService;
47-
4842
@Before
49-
public void setupIndex() throws IOException {
43+
public void createIndex() throws IOException {
5044
XContentBuilder mapping = XContentFactory.jsonBuilder().startObject()
5145
.startObject("properties")
5246
.startObject("field").field("type", "keyword").endObject()
@@ -60,71 +54,72 @@ public void setupIndex() throws IOException {
6054
.startObject("field_that_does_not_match").field("type", "keyword").endObject()
6155
.endObject()
6256
.endObject();
63-
indexService = createIndex("index", Settings.EMPTY, mapping);
57+
58+
client().admin().indices().prepareCreate("index").setMapping(mapping).get();
6459
}
6560

6661
public void testLeafValues() throws IOException {
67-
XContentBuilder source = XContentFactory.jsonBuilder().startObject()
68-
.array("field", "first", "second")
69-
.startObject("object")
70-
.field("field", "third")
71-
.endObject()
72-
.endObject();
62+
indexDocument(XContentFactory.jsonBuilder().startObject()
63+
.array("field", "first", "second")
64+
.startObject("object")
65+
.field("field", "third")
66+
.endObject()
67+
.endObject());
7368

74-
FetchSubPhase.HitContext hitContext = hitExecute(indexService, source, List.of("field", "object.field"));
75-
assertThat(hitContext.hit().getFields().size(), equalTo(2));
69+
SearchHit hit = fetchFields("field", "object.field");
70+
assertThat(hit.getFields().size(), equalTo(2));
7671

77-
DocumentField field = hitContext.hit().getFields().get("field");
72+
DocumentField field = hit.getFields().get("field");
7873
assertNotNull(field);
7974
assertThat(field.getValues().size(), equalTo(2));
8075
assertThat(field.getValues(), hasItems("first", "second"));
8176

82-
DocumentField objectField = hitContext.hit().getFields().get("object.field");
77+
DocumentField objectField = hit.getFields().get("object.field");
8378
assertNotNull(objectField);
8479
assertThat(objectField.getValues().size(), equalTo(1));
8580
assertThat(objectField.getValues(), hasItems("third"));
8681
}
8782

8883
public void testObjectValues() throws IOException {
89-
XContentBuilder source = XContentFactory.jsonBuilder().startObject()
90-
.startObject("float_range")
91-
.field("gte", 0.0)
92-
.field("lte", 2.718)
93-
.endObject()
94-
.endObject();
84+
indexDocument(XContentFactory.jsonBuilder().startObject()
85+
.startObject("float_range")
86+
.field("gte", 0.0)
87+
.field("lte", 2.718)
88+
.endObject()
89+
.endObject());
9590

96-
FetchSubPhase.HitContext hitContext = hitExecute(indexService, source, "float_range");
97-
assertThat(hitContext.hit().getFields().size(), equalTo(1));
91+
SearchHit hit = fetchFields("float_range");
92+
assertThat(hit.getFields().size(), equalTo(1));
9893

99-
DocumentField rangeField = hitContext.hit().getFields().get("float_range");
94+
DocumentField rangeField = hit.getFields().get("float_range");
10095
assertNotNull(rangeField);
10196
assertThat(rangeField.getValues().size(), equalTo(1));
10297
assertThat(rangeField.getValue(), equalTo(Map.of("gte", 0.0, "lte", 2.718)));
10398
}
10499

105100
public void testFieldNamesWithWildcard() throws IOException {
106-
XContentBuilder source = XContentFactory.jsonBuilder().startObject()
107-
.array("field", "first", "second")
108-
.field("integer_field", "third")
109-
.startObject("object")
110-
.field("field", "fourth")
111-
.endObject()
112-
.endObject();
101+
indexDocument(XContentFactory.jsonBuilder().startObject()
102+
.array("field", "first", "second")
103+
.field("integer_field", 42)
104+
.startObject("object")
105+
.field("field", "fourth")
106+
.endObject()
107+
.endObject());
113108

114-
FetchSubPhase.HitContext hitContext = hitExecute(indexService, source, "*field");
115-
assertThat(hitContext.hit().getFields().size(), equalTo(3));
109+
SearchHit hit = fetchFields("*field");
110+
assertThat(hit.getFields().size(), equalTo(3));
116111

117-
DocumentField field = hitContext.hit().getFields().get("field");
112+
DocumentField field = hit.getFields().get("field");
118113
assertNotNull(field);
119114
assertThat(field.getValues().size(), equalTo(2));
120115
assertThat(field.getValues(), hasItems("first", "second"));
121116

122-
DocumentField otherField = hitContext.hit().getFields().get("integer_field");
117+
DocumentField otherField = hit.getFields().get("integer_field");
123118
assertNotNull(otherField);
124119
assertThat(otherField.getValues().size(), equalTo(1));
125-
assertThat(otherField.getValues(), hasItems("third"));
120+
assertThat(otherField.getValues(), hasItems(42));
126121

127-
DocumentField objectField = hitContext.hit().getFields().get("object.field");
122+
DocumentField objectField = hit.getFields().get("object.field");
128123
assertNotNull(objectField);
129124
assertThat(objectField.getValues().size(), equalTo(1));
130125
assertThat(objectField.getValues(), hasItems("fourth"));
@@ -138,77 +133,43 @@ public void testSourceDisabled() throws IOException {
138133
.endObject()
139134
.endObject();
140135

141-
IndexService sourceDisabledIndexService = createIndex("disabled-index", Settings.EMPTY, mapping);
142-
XContentBuilder source = XContentFactory.jsonBuilder().startObject()
143-
.field("field", "value")
144-
.endObject();
145-
146-
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
147-
() -> hitExecute(sourceDisabledIndexService, source, "field"));
148-
assertThat(e.getMessage(), containsString("Unable to retrieve the requested [fields] since _source is disabled in the mapping"));
149-
}
150-
151-
public void testNonExistentFields() throws IOException {
152-
XContentBuilder source = XContentFactory.jsonBuilder().startObject()
153-
.field("field", "value")
154-
.endObject();
136+
client().admin().indices().prepareCreate("source-disabled")
137+
.setMapping(mapping)
138+
.get();
155139

156-
FetchSubPhase.HitContext hitContext = hitExecute(indexService, source, "non_existent");
157-
assertFalse(hitContext.hit().getFields().containsKey("non_existent"));
140+
SearchPhaseExecutionException e = expectThrows(SearchPhaseExecutionException.class, () ->
141+
client().prepareSearch("source-disabled").addFetchField("field").get());
142+
assertThat(e.getCause().getMessage(), containsString(
143+
"Unable to retrieve the requested [fields] since _source is disabled in the mapping"));
158144
}
159145

160146
public void testObjectFields() throws IOException {
161-
XContentBuilder source = XContentFactory.jsonBuilder().startObject()
147+
indexDocument(XContentFactory.jsonBuilder().startObject()
162148
.array("field", "first", "second")
163149
.startObject("object")
164150
.field("field", "third")
165151
.endObject()
166-
.endObject();
152+
.endObject());
167153

168-
FetchSubPhase.HitContext hitContext = hitExecute(indexService, source, "object");
169-
assertFalse(hitContext.hit().getFields().containsKey("object"));
154+
SearchHit hit = fetchFields("object");
155+
assertFalse(hit.getFields().containsKey("object"));
170156
}
171157

172-
private FetchSubPhase.HitContext hitExecute(IndexService indexService, XContentBuilder source, String field) {
173-
return hitExecute(indexService, source, List.of(field));
158+
private void indexDocument(XContentBuilder source) {
159+
client().prepareIndex("index")
160+
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
161+
.setSource(source)
162+
.get();
174163
}
175164

176-
private FetchSubPhase.HitContext hitExecute(IndexService indexService, XContentBuilder source, List<String> fields) {
177-
FetchFieldsContext fetchFieldsContext = new FetchFieldsContext(fields);
178-
BytesReference sourceAsBytes = source != null ? BytesReference.bytes(source) : null;
179-
SearchContext searchContext = new FetchFieldsPhaseTestSearchContext(indexService, fetchFieldsContext, sourceAsBytes);
180-
181-
FetchSubPhase.HitContext hitContext = new FetchSubPhase.HitContext();
182-
SearchHit searchHit = new SearchHit(1, null, null, null, null);
183-
hitContext.reset(searchHit, null, 1, null);
184-
185-
FetchFieldsPhase phase = new FetchFieldsPhase();
186-
phase.hitExecute(searchContext, hitContext);
187-
return hitContext;
188-
}
189-
190-
private static class FetchFieldsPhaseTestSearchContext extends TestSearchContext {
191-
private final FetchFieldsContext context;
192-
private final BytesReference source;
193-
194-
FetchFieldsPhaseTestSearchContext(IndexService indexService,
195-
FetchFieldsContext context,
196-
BytesReference source) {
197-
super(indexService.getBigArrays(), indexService);
198-
this.context = context;
199-
this.source = source;
165+
private SearchHit fetchFields(String... fields) {
166+
SearchRequestBuilder builder = client().prepareSearch("index");
167+
for (String field : fields) {
168+
builder.addFetchField(field);
200169
}
170+
SearchResponse response = builder.get();
201171

202-
@Override
203-
public FetchFieldsContext fetchFieldsContext() {
204-
return context;
205-
}
206-
207-
@Override
208-
public SearchLookup lookup() {
209-
SearchLookup lookup = new SearchLookup(this.mapperService(), this::getForField);
210-
lookup.source().setSource(source);
211-
return lookup;
212-
}
172+
assertEquals(1, response.getHits().getHits().length);
173+
return response.getHits().getHits()[0];
213174
}
214175
}

0 commit comments

Comments
 (0)