Skip to content

Commit 62857b4

Browse files
authored
Add support for missing value fetchers. (#63515)
This PR implements value fetching for the following field types: * `text` phrase and prefix subfields * `search_as_you_type`, plus its subfields * `token_count`, which is implemented by fetching doc values Supporting these types helps ensure that retrieving all fields through `"fields": ["*"]` doesn't fail because of unsupported value fetchers.
1 parent 5ec70e5 commit 62857b4

File tree

11 files changed

+146
-24
lines changed

11 files changed

+146
-24
lines changed

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapper.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ private ShingleFieldType shingleFieldForPositions(int positions) {
265265

266266
@Override
267267
public ValueFetcher valueFetcher(MapperService mapperService, SearchLookup searchLookup, String format) {
268-
throw new UnsupportedOperationException();
268+
return SourceValueFetcher.toString(name(), mapperService, format);
269269
}
270270

271271
@Override
@@ -376,7 +376,9 @@ public Query prefixQuery(String value, MultiTermQuery.RewriteMethod method, bool
376376

377377
@Override
378378
public ValueFetcher valueFetcher(MapperService mapperService, SearchLookup searchLookup, String format) {
379-
throw new UnsupportedOperationException();
379+
// Because this internal field is modelled as a multi-field, SourceValueFetcher will look up its
380+
// parent field in _source. So we don't need to use the parent field name here.
381+
return SourceValueFetcher.toString(name(), mapperService, format);
380382
}
381383

382384
@Override
@@ -480,7 +482,9 @@ void setPrefixFieldType(PrefixFieldType prefixFieldType) {
480482

481483
@Override
482484
public ValueFetcher valueFetcher(MapperService mapperService, SearchLookup searchLookup, String format) {
483-
throw new UnsupportedOperationException();
485+
// Because this internal field is modelled as a multi-field, SourceValueFetcher will look up its
486+
// parent field in _source. So we don't need to use the parent field name here.
487+
return SourceValueFetcher.toString(name(), mapperService, format);
484488
}
485489

486490
@Override

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/TokenCountFieldMapper.java

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.apache.lucene.analysis.TokenStream;
2424
import org.apache.lucene.analysis.tokenattributes.PositionIncrementAttribute;
2525
import org.elasticsearch.index.analysis.NamedAnalyzer;
26+
import org.elasticsearch.search.lookup.SearchLookup;
2627

2728
import java.io.IOException;
2829
import java.util.Arrays;
@@ -72,19 +73,33 @@ public TokenCountFieldMapper build(BuilderContext context) {
7273
if (analyzer.getValue() == null) {
7374
throw new MapperParsingException("Analyzer must be set for field [" + name + "] but wasn't.");
7475
}
75-
MappedFieldType ft = new NumberFieldMapper.NumberFieldType(
76+
MappedFieldType ft = new TokenCountFieldType(
7677
buildFullName(context),
77-
NumberFieldMapper.NumberType.INTEGER,
7878
index.getValue(),
7979
store.getValue(),
8080
hasDocValues.getValue(),
81-
false,
8281
nullValue.getValue(),
8382
meta.getValue());
8483
return new TokenCountFieldMapper(name, ft, multiFieldsBuilder.build(this, context), copyTo.build(), this);
8584
}
8685
}
8786

87+
static class TokenCountFieldType extends NumberFieldMapper.NumberFieldType {
88+
89+
TokenCountFieldType(String name, boolean isSearchable, boolean isStored,
90+
boolean hasDocValues, Number nullValue, Map<String, String> meta) {
91+
super(name, NumberFieldMapper.NumberType.INTEGER, isSearchable, isStored, hasDocValues, false, nullValue, meta);
92+
}
93+
94+
@Override
95+
public ValueFetcher valueFetcher(MapperService mapperService, SearchLookup searchLookup, String format) {
96+
if (hasDocValues() == false) {
97+
return lookup -> List.of();
98+
}
99+
return new DocValueFetcher(docValueFormat(format, null), searchLookup.doc().getForField(this));
100+
}
101+
}
102+
88103
public static TypeParser PARSER = new TypeParser((n, c) -> new Builder(n));
89104

90105
private final boolean index;

modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldTypeTests.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@
3434
import org.elasticsearch.index.mapper.SearchAsYouTypeFieldMapper.SearchAsYouTypeFieldType;
3535
import org.elasticsearch.index.mapper.SearchAsYouTypeFieldMapper.ShingleFieldType;
3636

37+
import java.io.IOException;
3738
import java.util.Collections;
39+
import java.util.List;
3840

3941
import static java.util.Arrays.asList;
4042
import static org.apache.lucene.search.MultiTermQuery.CONSTANT_SCORE_REWRITE;
@@ -109,4 +111,25 @@ public void testPrefixQuery() {
109111
assertEquals("[prefix] queries cannot be executed when 'search.allow_expensive_queries' is set to false. " +
110112
"For optimised prefix queries on text fields please enable [index_prefixes].", ee.getMessage());
111113
}
114+
115+
public void testFetchSourceValue() throws IOException {
116+
SearchAsYouTypeFieldType fieldType = createFieldType();
117+
fieldType.setIndexAnalyzer(Lucene.STANDARD_ANALYZER);
118+
119+
assertEquals(List.of("value"), fetchSourceValue(fieldType, "value"));
120+
assertEquals(List.of("42"), fetchSourceValue(fieldType, 42L));
121+
assertEquals(List.of("true"), fetchSourceValue(fieldType, true));
122+
123+
SearchAsYouTypeFieldMapper.PrefixFieldType prefixFieldType = new SearchAsYouTypeFieldMapper.PrefixFieldType(
124+
fieldType.name(), fieldType.getTextSearchInfo(), 2, 10);
125+
assertEquals(List.of("value"), fetchSourceValue(prefixFieldType, "value"));
126+
assertEquals(List.of("42"), fetchSourceValue(prefixFieldType, 42L));
127+
assertEquals(List.of("true"), fetchSourceValue(prefixFieldType, true));
128+
129+
SearchAsYouTypeFieldMapper.ShingleFieldType shingleFieldType = new SearchAsYouTypeFieldMapper.ShingleFieldType(
130+
fieldType.name(), 5, fieldType.getTextSearchInfo());
131+
assertEquals(List.of("value"), fetchSourceValue(shingleFieldType, "value"));
132+
assertEquals(List.of("42"), fetchSourceValue(shingleFieldType, 42L));
133+
assertEquals(List.of("true"), fetchSourceValue(shingleFieldType, true));
134+
}
112135
}

rest-api-spec/build.gradle

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,9 @@ artifacts {
1515
restTests(new File(projectDir, "src/main/resources/rest-api-spec/test"))
1616
}
1717

18+
testClusters.all {
19+
module ':modules:mapper-extras'
20+
}
21+
1822
test.enabled = false
1923
jarHell.enabled = false

rest-api-spec/src/main/resources/rest-api-spec/test/search/330_fetch_fields.yml

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,3 +259,39 @@ setup:
259259
- is_true: hits.hits.0._id
260260
- is_true: hits.hits.0._source
261261
- match: { hits.hits.0.fields.date.0: "1990/12/29" }
262+
263+
---
264+
"Test token count":
265+
- skip:
266+
version: " - 7.99.99"
267+
reason: "support for token_count isn't yet backported"
268+
- do:
269+
indices.create:
270+
index: test
271+
body:
272+
mappings:
273+
properties:
274+
count:
275+
type: token_count
276+
analyzer: standard
277+
count_without_dv:
278+
type: token_count
279+
analyzer: standard
280+
doc_values: false
281+
282+
- do:
283+
index:
284+
index: test
285+
id: 1
286+
refresh: true
287+
body:
288+
count: "some text"
289+
- do:
290+
search:
291+
index: test
292+
body:
293+
fields: [count, count_without_dv]
294+
295+
- is_true: hits.hits.0._id
296+
- match: { hits.hits.0.fields.count: [2] }
297+
- is_false: hits.hits.0.fields.count_without_dv

server/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, S
100100

101101
/**
102102
* Create a helper class to fetch field values during the {@link FetchFieldsPhase}.
103+
*
104+
* New field types must implement this method in order to support the search 'fields' option. Except
105+
* for metadata fields, field types should not throw {@link UnsupportedOperationException} since this
106+
* could cause a search retrieving multiple fields (like "fields": ["*"]) to fail.
103107
*/
104108
public abstract ValueFetcher valueFetcher(MapperService mapperService, SearchLookup searchLookup, @Nullable String format);
105109

server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -886,7 +886,7 @@ public static Query longRangeQuery(
886886
}
887887
}
888888

889-
public static final class NumberFieldType extends SimpleMappedFieldType {
889+
public static class NumberFieldType extends SimpleMappedFieldType {
890890

891891
private final NumberType type;
892892
private final boolean coerce;

server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -445,11 +445,11 @@ protected TokenStreamComponents wrapComponents(String fieldName, TokenStreamComp
445445
}
446446
}
447447

448-
private static final class PhraseFieldType extends StringFieldType {
448+
static final class PhraseFieldType extends StringFieldType {
449449

450450
final TextFieldType parent;
451451

452-
private PhraseFieldType(TextFieldType parent) {
452+
PhraseFieldType(TextFieldType parent) {
453453
super(parent.name() + FAST_PHRASE_SUFFIX, true, false, false, parent.getTextSearchInfo(), Collections.emptyMap());
454454
setAnalyzer(parent.indexAnalyzer().name(), parent.indexAnalyzer().analyzer());
455455
this.parent = parent;
@@ -466,7 +466,9 @@ public String typeName() {
466466

467467
@Override
468468
public ValueFetcher valueFetcher(MapperService mapperService, SearchLookup searchLookup, String format) {
469-
throw new UnsupportedOperationException();
469+
// Because this internal field is modelled as a multi-field, SourceValueFetcher will look up its
470+
// parent field in _source. So we don't need to use the parent field name here.
471+
return SourceValueFetcher.toString(name(), mapperService, format);
470472
}
471473

472474
@Override
@@ -494,7 +496,9 @@ static final class PrefixFieldType extends StringFieldType {
494496

495497
@Override
496498
public ValueFetcher valueFetcher(MapperService mapperService, SearchLookup searchLookup, String format) {
497-
throw new UnsupportedOperationException();
499+
// Because this internal field is modelled as a multi-field, SourceValueFetcher will look up its
500+
// parent field in _source. So we don't need to use the parent field name here.
501+
return SourceValueFetcher.toString(name(), mapperService, format);
498502
}
499503

500504
void setAnalyzer(NamedAnalyzer delegate) {

server/src/test/java/org/elasticsearch/index/mapper/TextFieldTypeTests.java

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,9 @@
3535
import org.apache.lucene.util.automaton.Automaton;
3636
import org.apache.lucene.util.automaton.Operations;
3737
import org.elasticsearch.ElasticsearchException;
38-
import org.elasticsearch.Version;
39-
import org.elasticsearch.cluster.metadata.IndexMetadata;
4038
import org.elasticsearch.common.lucene.BytesRefs;
4139
import org.elasticsearch.common.lucene.Lucene;
4240
import org.elasticsearch.common.lucene.search.AutomatonQueries;
43-
import org.elasticsearch.common.settings.Settings;
4441
import org.elasticsearch.common.unit.Fuzziness;
4542
import org.elasticsearch.index.mapper.TextFieldMapper.TextFieldType;
4643

@@ -167,13 +164,21 @@ public void testIndexPrefixes() {
167164
}
168165

169166
public void testFetchSourceValue() throws IOException {
170-
Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build();
171-
Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath());
172-
173-
MappedFieldType mapper = new TextFieldMapper.Builder("field", () -> Lucene.STANDARD_ANALYZER).build(context).fieldType();
174-
175-
assertEquals(List.of("value"), fetchSourceValue(mapper, "value"));
176-
assertEquals(List.of("42"), fetchSourceValue(mapper, 42L));
177-
assertEquals(List.of("true"), fetchSourceValue(mapper, true));
167+
TextFieldType fieldType = createFieldType();
168+
fieldType.setIndexAnalyzer(Lucene.STANDARD_ANALYZER);
169+
170+
assertEquals(List.of("value"), fetchSourceValue(fieldType, "value"));
171+
assertEquals(List.of("42"), fetchSourceValue(fieldType, 42L));
172+
assertEquals(List.of("true"), fetchSourceValue(fieldType, true));
173+
174+
TextFieldMapper.PrefixFieldType prefixFieldType = new TextFieldMapper.PrefixFieldType(fieldType, "field._index_prefix", 2, 10);
175+
assertEquals(List.of("value"), fetchSourceValue(prefixFieldType, "value"));
176+
assertEquals(List.of("42"), fetchSourceValue(prefixFieldType, 42L));
177+
assertEquals(List.of("true"), fetchSourceValue(prefixFieldType, true));
178+
179+
TextFieldMapper.PhraseFieldType phraseFieldType = new TextFieldMapper.PhraseFieldType(fieldType);
180+
assertEquals(List.of("value"), fetchSourceValue(phraseFieldType, "value"));
181+
assertEquals(List.of("42"), fetchSourceValue(phraseFieldType, 42L));
182+
assertEquals(List.of("true"), fetchSourceValue(phraseFieldType, true));
178183
}
179184
}

server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import java.util.Map;
3535
import java.util.Set;
3636

37+
import static org.hamcrest.Matchers.containsInAnyOrder;
3738
import static org.hamcrest.Matchers.equalTo;
3839
import static org.hamcrest.Matchers.hasItems;
3940

@@ -360,6 +361,34 @@ public void testObjectFields() throws IOException {
360361
assertFalse(fields.containsKey("object"));
361362
}
362363

364+
public void testTextSubFields() throws IOException {
365+
XContentBuilder mapping = XContentFactory.jsonBuilder().startObject()
366+
.startObject("properties")
367+
.startObject("field")
368+
.field("type", "text")
369+
.startObject("index_prefixes").endObject()
370+
.field("index_phrases", true)
371+
.endObject()
372+
.endObject()
373+
.endObject();
374+
375+
IndexService indexService = createIndex("index", Settings.EMPTY, mapping);
376+
MapperService mapperService = indexService.mapperService();
377+
378+
XContentBuilder source = XContentFactory.jsonBuilder().startObject()
379+
.array("field", "some text")
380+
.endObject();
381+
382+
Map<String, DocumentField> fields = fetchFields(mapperService, source, "*");
383+
assertThat(fields.size(), equalTo(3));
384+
assertThat(fields.keySet(), containsInAnyOrder("field", "field._index_prefix", "field._index_phrase"));
385+
386+
for (DocumentField field : fields.values()) {
387+
assertThat(field.getValues().size(), equalTo(1));
388+
assertThat(field.getValue(), equalTo("some text"));
389+
}
390+
}
391+
363392
private Map<String, DocumentField> fetchFields(MapperService mapperService, XContentBuilder source, String fieldPattern)
364393
throws IOException {
365394

test/framework/src/main/java/org/elasticsearch/index/mapper/FieldTypeTestCase.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ public static List<?> fetchSourceValue(MappedFieldType fieldType, Object sourceV
5252

5353
public static List<?> fetchSourceValue(MappedFieldType fieldType, Object sourceValue, String format) throws IOException {
5454
String field = fieldType.name();
55-
5655
MapperService mapperService = mock(MapperService.class);
5756
when(mapperService.sourcePath(field)).thenReturn(Set.of(field));
5857

@@ -61,5 +60,4 @@ public static List<?> fetchSourceValue(MappedFieldType fieldType, Object sourceV
6160
lookup.setSource(Collections.singletonMap(field, sourceValue));
6261
return fetcher.fetchValues(lookup);
6362
}
64-
6563
}

0 commit comments

Comments
 (0)