Skip to content

Commit

Permalink
Add support for missing value fetchers. (elastic#63515)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jtibshirani committed Oct 12, 2020
1 parent 4eee270 commit 9334899
Show file tree
Hide file tree
Showing 11 changed files with 145 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ private ShingleFieldType shingleFieldForPositions(int positions) {

@Override
public ValueFetcher valueFetcher(MapperService mapperService, SearchLookup searchLookup, String format) {
throw new UnsupportedOperationException();
return SourceValueFetcher.toString(name(), mapperService, format);
}

@Override
Expand Down Expand Up @@ -376,7 +376,9 @@ public Query prefixQuery(String value, MultiTermQuery.RewriteMethod method, bool

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

@Override
Expand Down Expand Up @@ -480,7 +482,9 @@ void setPrefixFieldType(PrefixFieldType prefixFieldType) {

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

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.lucene.analysis.TokenStream;
import org.apache.lucene.analysis.tokenattributes.PositionIncrementAttribute;
import org.elasticsearch.index.analysis.NamedAnalyzer;
import org.elasticsearch.search.lookup.SearchLookup;

import java.io.IOException;
import java.util.Arrays;
Expand Down Expand Up @@ -72,19 +73,33 @@ public TokenCountFieldMapper build(BuilderContext context) {
if (analyzer.getValue() == null) {
throw new MapperParsingException("Analyzer must be set for field [" + name + "] but wasn't.");
}
MappedFieldType ft = new NumberFieldMapper.NumberFieldType(
MappedFieldType ft = new TokenCountFieldType(
buildFullName(context),
NumberFieldMapper.NumberType.INTEGER,
index.getValue(),
store.getValue(),
hasDocValues.getValue(),
false,
nullValue.getValue(),
meta.getValue());
return new TokenCountFieldMapper(name, ft, multiFieldsBuilder.build(this, context), copyTo.build(), this);
}
}

static class TokenCountFieldType extends NumberFieldMapper.NumberFieldType {

TokenCountFieldType(String name, boolean isSearchable, boolean isStored,
boolean hasDocValues, Number nullValue, Map<String, String> meta) {
super(name, NumberFieldMapper.NumberType.INTEGER, isSearchable, isStored, hasDocValues, false, nullValue, meta);
}

@Override
public ValueFetcher valueFetcher(MapperService mapperService, SearchLookup searchLookup, String format) {
if (hasDocValues() == false) {
return lookup -> org.elasticsearch.common.collect.List.of();
}
return new DocValueFetcher(docValueFormat(format, null), searchLookup.doc().getForField(this));
}
}

public static TypeParser PARSER = new TypeParser((n, c) -> new Builder(n));

private final boolean index;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.elasticsearch.index.mapper.SearchAsYouTypeFieldMapper.SearchAsYouTypeFieldType;
import org.elasticsearch.index.mapper.SearchAsYouTypeFieldMapper.ShingleFieldType;

import java.io.IOException;
import java.util.Collections;

import static java.util.Arrays.asList;
Expand Down Expand Up @@ -109,4 +110,25 @@ public void testPrefixQuery() {
assertEquals("[prefix] queries cannot be executed when 'search.allow_expensive_queries' is set to false. " +
"For optimised prefix queries on text fields please enable [index_prefixes].", ee.getMessage());
}

public void testFetchSourceValue() throws IOException {
SearchAsYouTypeFieldType fieldType = createFieldType();
fieldType.setIndexAnalyzer(Lucene.STANDARD_ANALYZER);

assertEquals(org.elasticsearch.common.collect.List.of("value"), fetchSourceValue(fieldType, "value"));
assertEquals(org.elasticsearch.common.collect.List.of("42"), fetchSourceValue(fieldType, 42L));
assertEquals(org.elasticsearch.common.collect.List.of("true"), fetchSourceValue(fieldType, true));

SearchAsYouTypeFieldMapper.PrefixFieldType prefixFieldType = new SearchAsYouTypeFieldMapper.PrefixFieldType(
fieldType.name(), fieldType.getTextSearchInfo(), 2, 10);
assertEquals(org.elasticsearch.common.collect.List.of("value"), fetchSourceValue(prefixFieldType, "value"));
assertEquals(org.elasticsearch.common.collect.List.of("42"), fetchSourceValue(prefixFieldType, 42L));
assertEquals(org.elasticsearch.common.collect.List.of("true"), fetchSourceValue(prefixFieldType, true));

SearchAsYouTypeFieldMapper.ShingleFieldType shingleFieldType = new SearchAsYouTypeFieldMapper.ShingleFieldType(
fieldType.name(), 5, fieldType.getTextSearchInfo());
assertEquals(org.elasticsearch.common.collect.List.of("value"), fetchSourceValue(shingleFieldType, "value"));
assertEquals(org.elasticsearch.common.collect.List.of("42"), fetchSourceValue(shingleFieldType, 42L));
assertEquals(org.elasticsearch.common.collect.List.of("true"), fetchSourceValue(shingleFieldType, true));
}
}
4 changes: 4 additions & 0 deletions rest-api-spec/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,9 @@ artifacts {
restTests(new File(projectDir, "src/main/resources/rest-api-spec/test"))
}

testClusters.all {
module ':modules:mapper-extras'
}

test.enabled = false
jarHell.enabled = false
Original file line number Diff line number Diff line change
Expand Up @@ -259,3 +259,39 @@ setup:
- is_true: hits.hits.0._id
- is_true: hits.hits.0._source
- match: { hits.hits.0.fields.date.0: "1990/12/29" }

---
"Test token count":
- skip:
version: " - 7.99.99"
reason: "support for token_count isn't yet backported"
- do:
indices.create:
index: test
body:
mappings:
properties:
count:
type: token_count
analyzer: standard
count_without_dv:
type: token_count
analyzer: standard
doc_values: false

- do:
index:
index: test
id: 1
refresh: true
body:
count: "some text"
- do:
search:
index: test
body:
fields: [count, count_without_dv]

- is_true: hits.hits.0._id
- match: { hits.hits.0.fields.count: [2] }
- is_false: hits.hits.0.fields.count_without_dv
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, S

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -889,7 +889,7 @@ public static Query longRangeQuery(
}
}

public static final class NumberFieldType extends SimpleMappedFieldType {
public static class NumberFieldType extends SimpleMappedFieldType {

private final NumberType type;
private final boolean coerce;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -447,11 +447,11 @@ protected TokenStreamComponents wrapComponents(String fieldName, TokenStreamComp
}
}

private static final class PhraseFieldType extends StringFieldType {
static final class PhraseFieldType extends StringFieldType {

final TextFieldType parent;

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

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

@Override
Expand Down Expand Up @@ -496,7 +498,9 @@ static final class PrefixFieldType extends StringFieldType {

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

void setAnalyzer(NamedAnalyzer delegate) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,9 @@
import org.apache.lucene.util.automaton.Automaton;
import org.apache.lucene.util.automaton.Operations;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.common.lucene.BytesRefs;
import org.elasticsearch.common.lucene.Lucene;
import org.elasticsearch.common.lucene.search.AutomatonQueries;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.Fuzziness;
import org.elasticsearch.index.mapper.TextFieldMapper.TextFieldType;

Expand Down Expand Up @@ -167,13 +164,21 @@ public void testIndexPrefixes() {
}

public void testFetchSourceValue() throws IOException {
Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build();
Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath());

MappedFieldType mapper = new TextFieldMapper.Builder("field", () -> Lucene.STANDARD_ANALYZER).build(context).fieldType();

assertEquals(Collections.singletonList("value"), fetchSourceValue(mapper, "value"));
assertEquals(Collections.singletonList("42"), fetchSourceValue(mapper, 42L));
assertEquals(Collections.singletonList("true"), fetchSourceValue(mapper, true));
TextFieldType fieldType = createFieldType();
fieldType.setIndexAnalyzer(Lucene.STANDARD_ANALYZER);

assertEquals(org.elasticsearch.common.collect.List.of("value"), fetchSourceValue(fieldType, "value"));
assertEquals(org.elasticsearch.common.collect.List.of("42"), fetchSourceValue(fieldType, 42L));
assertEquals(org.elasticsearch.common.collect.List.of("true"), fetchSourceValue(fieldType, true));

TextFieldMapper.PrefixFieldType prefixFieldType = new TextFieldMapper.PrefixFieldType(fieldType, "field._index_prefix", 2, 10);
assertEquals(org.elasticsearch.common.collect.List.of("value"), fetchSourceValue(prefixFieldType, "value"));
assertEquals(org.elasticsearch.common.collect.List.of("42"), fetchSourceValue(prefixFieldType, 42L));
assertEquals(org.elasticsearch.common.collect.List.of("true"), fetchSourceValue(prefixFieldType, true));

TextFieldMapper.PhraseFieldType phraseFieldType = new TextFieldMapper.PhraseFieldType(fieldType);
assertEquals(org.elasticsearch.common.collect.List.of("value"), fetchSourceValue(phraseFieldType, "value"));
assertEquals(org.elasticsearch.common.collect.List.of("42"), fetchSourceValue(phraseFieldType, 42L));
assertEquals(org.elasticsearch.common.collect.List.of("true"), fetchSourceValue(phraseFieldType, true));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.List;
import java.util.Map;

import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasItems;

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

public void testTextSubFields() throws IOException {
XContentBuilder mapping = XContentFactory.jsonBuilder().startObject()
.startObject("properties")
.startObject("field")
.field("type", "text")
.startObject("index_prefixes").endObject()
.field("index_phrases", true)
.endObject()
.endObject()
.endObject();

IndexService indexService = createIndex("index", Settings.EMPTY, MapperService.SINGLE_MAPPING_NAME, mapping);
MapperService mapperService = indexService.mapperService();

XContentBuilder source = XContentFactory.jsonBuilder().startObject()
.array("field", "some text")
.endObject();

Map<String, DocumentField> fields = fetchFields(mapperService, source, "*");
assertThat(fields.size(), equalTo(3));
assertThat(fields.keySet(), containsInAnyOrder("field", "field._index_prefix", "field._index_phrase"));

for (DocumentField field : fields.values()) {
assertThat(field.getValues().size(), equalTo(1));
assertThat(field.getValue(), equalTo("some text"));
}
}

private Map<String, DocumentField> fetchFields(MapperService mapperService, XContentBuilder source, String fieldPattern)
throws IOException {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ public static List<?> fetchSourceValue(MappedFieldType fieldType, Object sourceV

public static List<?> fetchSourceValue(MappedFieldType fieldType, Object sourceValue, String format) throws IOException {
String field = fieldType.name();

MapperService mapperService = mock(MapperService.class);
when(mapperService.sourcePath(field)).thenReturn(org.elasticsearch.common.collect.Set.of(field));

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

}

0 comments on commit 9334899

Please sign in to comment.