Skip to content

Commit 9e52513

Browse files
authored
Add support for missing value fetchers. (#63585)
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 3f210e2 commit 9e52513

File tree

11 files changed

+145
-24
lines changed

11 files changed

+145
-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 -> org.elasticsearch.common.collect.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: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
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;
3839

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

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
@@ -102,6 +102,10 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, S
102102

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -889,7 +889,7 @@ public static Query longRangeQuery(
889889
}
890890
}
891891

892-
public static final class NumberFieldType extends SimpleMappedFieldType {
892+
public static class NumberFieldType extends SimpleMappedFieldType {
893893

894894
private final NumberType type;
895895
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
@@ -447,11 +447,11 @@ protected TokenStreamComponents wrapComponents(String fieldName, TokenStreamComp
447447
}
448448
}
449449

450-
private static final class PhraseFieldType extends StringFieldType {
450+
static final class PhraseFieldType extends StringFieldType {
451451

452452
final TextFieldType parent;
453453

454-
private PhraseFieldType(TextFieldType parent) {
454+
PhraseFieldType(TextFieldType parent) {
455455
super(parent.name() + FAST_PHRASE_SUFFIX, true, false, false, parent.getTextSearchInfo(), Collections.emptyMap());
456456
setAnalyzer(parent.indexAnalyzer().name(), parent.indexAnalyzer().analyzer());
457457
this.parent = parent;
@@ -468,7 +468,9 @@ public String typeName() {
468468

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

474476
@Override
@@ -496,7 +498,9 @@ static final class PrefixFieldType extends StringFieldType {
496498

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

502506
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(Collections.singletonList("value"), fetchSourceValue(mapper, "value"));
176-
assertEquals(Collections.singletonList("42"), fetchSourceValue(mapper, 42L));
177-
assertEquals(Collections.singletonList("true"), fetchSourceValue(mapper, true));
167+
TextFieldType fieldType = createFieldType();
168+
fieldType.setIndexAnalyzer(Lucene.STANDARD_ANALYZER);
169+
170+
assertEquals(org.elasticsearch.common.collect.List.of("value"), fetchSourceValue(fieldType, "value"));
171+
assertEquals(org.elasticsearch.common.collect.List.of("42"), fetchSourceValue(fieldType, 42L));
172+
assertEquals(org.elasticsearch.common.collect.List.of("true"), fetchSourceValue(fieldType, true));
173+
174+
TextFieldMapper.PrefixFieldType prefixFieldType = new TextFieldMapper.PrefixFieldType(fieldType, "field._index_prefix", 2, 10);
175+
assertEquals(org.elasticsearch.common.collect.List.of("value"), fetchSourceValue(prefixFieldType, "value"));
176+
assertEquals(org.elasticsearch.common.collect.List.of("42"), fetchSourceValue(prefixFieldType, 42L));
177+
assertEquals(org.elasticsearch.common.collect.List.of("true"), fetchSourceValue(prefixFieldType, true));
178+
179+
TextFieldMapper.PhraseFieldType phraseFieldType = new TextFieldMapper.PhraseFieldType(fieldType);
180+
assertEquals(org.elasticsearch.common.collect.List.of("value"), fetchSourceValue(phraseFieldType, "value"));
181+
assertEquals(org.elasticsearch.common.collect.List.of("42"), fetchSourceValue(phraseFieldType, 42L));
182+
assertEquals(org.elasticsearch.common.collect.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
@@ -33,6 +33,7 @@
3333
import java.util.List;
3434
import java.util.Map;
3535

36+
import static org.hamcrest.Matchers.containsInAnyOrder;
3637
import static org.hamcrest.Matchers.equalTo;
3738
import static org.hamcrest.Matchers.hasItems;
3839

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

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

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
@@ -51,7 +51,6 @@ public static List<?> fetchSourceValue(MappedFieldType fieldType, Object sourceV
5151

5252
public static List<?> fetchSourceValue(MappedFieldType fieldType, Object sourceValue, String format) throws IOException {
5353
String field = fieldType.name();
54-
5554
MapperService mapperService = mock(MapperService.class);
5655
when(mapperService.sourcePath(field)).thenReturn(org.elasticsearch.common.collect.Set.of(field));
5756

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

0 commit comments

Comments
 (0)