Skip to content

Commit

Permalink
Add ability to retrieve _id via fields option
Browse files Browse the repository at this point in the history
Currently we exclude metadata fields from being looked up using the `fields` option in search.
However, as issue like #75836 show, they can still be retrieved e.g. via aliases and then fetching
their values causes errors. 
With this change, we enable retrieval of metadata fields (like `_id`) using the fields option when
 the field is explicitely asked for. We still continue to exclude any metadata field from matching
wildcard patterns, but they should be retrievable via an exact name or if there is an alias definition
with a path to a metadata field. This change adds support for the `_id` field in particular.
  • Loading branch information
Christoph Büscher authored Oct 11, 2021
1 parent e7ab7c8 commit d713c95
Show file tree
Hide file tree
Showing 8 changed files with 145 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1007,3 +1007,58 @@ error for flattened includes whole path:
fields:
- field: flattened.bar
format: "yyyy/MM/dd"

---
test fetching metadata fields:
- skip:
version: ' - 7.99.99'
reason: 'fetching metadata via fields introduced in 8.0'

- do:
indices.create:
index: test
body:
settings:
index.number_of_shards: 1
mappings:
properties:
field:
type: keyword
idAlias:
type: alias
path: _id

- do:
index:
index: test
id: 1
refresh: true
body:
field: foo

- do:
search:
index: test
body:
fields: [ "*" ]

- length: { hits.hits.0.fields : 2 }
- match: { hits.hits.0.fields.field.0: "foo" }
- match: { hits.hits.0.fields.idAlias.0: "1" }

- do:
search:
index: test
body:
fields: [ "_*" ]

- is_false: hits.hits.0.fields

- do:
search:
index: test
body:
fields: [ "_id" ]

- length: { hits.hits.0.fields : 1 }
- match: { hits.hits.0.fields._id.0: "1" }
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public boolean isSearchable() {

@Override
public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
throw new UnsupportedOperationException("Cannot fetch values for internal field [" + name() + "].");
return new StoredValueFetcher(context.lookup(), NAME);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.index.mapper;

import org.apache.lucene.index.LeafReaderContext;
import org.elasticsearch.search.lookup.LeafSearchLookup;
import org.elasticsearch.search.lookup.SearchLookup;
import org.elasticsearch.search.lookup.SourceLookup;

import java.io.IOException;
import java.util.List;

/**
* Value fetcher that loads from stored values.
*/
public final class StoredValueFetcher implements ValueFetcher {

private final SearchLookup lookup;
private LeafSearchLookup leafSearchLookup;
private final String fieldname;

public StoredValueFetcher(SearchLookup lookup, String fieldname) {
this.lookup = lookup;
this.fieldname = fieldname;
}

@Override
public void setNextReader(LeafReaderContext context) {
this.leafSearchLookup = lookup.getLeafSearchLookup(context);
}

@Override
public List<Object> fetchValues(SourceLookup lookup) throws IOException {
leafSearchLookup.setDocument(lookup.docId());
return leafSearchLookup.fields().get(fieldname).getValues();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@
import org.apache.lucene.index.DocValuesType;
import org.apache.lucene.index.IndexableField;
import org.elasticsearch.Version;
import org.elasticsearch.jdk.JavaVersion;
import org.elasticsearch.common.time.DateFormatter;
import org.elasticsearch.common.time.DateUtils;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.index.mapper.DateFieldMapper.DateFieldType;
import org.elasticsearch.index.termvectors.TermVectorsService;
import org.elasticsearch.jdk.JavaVersion;
import org.elasticsearch.search.DocValueFormat;

import java.io.IOException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,19 @@

import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.index.IndexableField;
import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.search.IndexSearcher;
import org.elasticsearch.index.query.SearchExecutionContext;
import org.elasticsearch.indices.IndicesService;
import org.elasticsearch.search.lookup.SearchLookup;

import java.io.IOException;
import java.util.List;

import static org.elasticsearch.index.mapper.IdFieldMapper.ID_FIELD_DATA_DEPRECATION_MESSAGE;
import static org.hamcrest.Matchers.containsString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class IdFieldMapperTests extends MapperServiceTestCase {

Expand Down Expand Up @@ -61,4 +68,25 @@ public void testEnableFieldData() throws IOException {
assertTrue(ft.isAggregatable());
}

public void testFetchIdFieldValue() throws IOException {
MapperService mapperService = createMapperService(
fieldMapping(b -> b.field("type", "keyword"))
);
String id = randomAlphaOfLength(12);
withLuceneIndex(mapperService, iw -> {
iw.addDocument(mapperService.documentMapper().parse(source(id, b -> b.field("field", "value"), null)).rootDoc());
}, iw -> {
SearchLookup lookup = new SearchLookup(mapperService::fieldType, fieldDataLookup());
SearchExecutionContext searchExecutionContext = mock(SearchExecutionContext.class);
when(searchExecutionContext.lookup()).thenReturn(lookup);
IdFieldMapper.IdFieldType ft = (IdFieldMapper.IdFieldType) mapperService.fieldType("_id");
ValueFetcher valueFetcher = ft.valueFetcher(searchExecutionContext, null);
IndexSearcher searcher = newSearcher(iw);
LeafReaderContext context = searcher.getIndexReader().leaves().get(0);
lookup.source().setSegmentAndDocument(context, 0);
valueFetcher.setNextReader(context);
assertEquals(List.of(id), valueFetcher.fetchValues(lookup.source()));
});
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public void testMetadataFields() throws IOException {
assertTrue(fields.isEmpty());

// several other metadata fields throw exceptions via their value fetchers when trying to get them
for (String fieldname : List.of("_id", "_index", "_seq_no", "_routing", "_ignored")) {
for (String fieldname : List.of("_index", "_seq_no", "_routing", "_ignored")) {
expectThrows(UnsupportedOperationException.class, () -> fetchFields(mapperService, source, fieldname));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
import org.apache.lucene.store.Directory;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.core.CheckedConsumer;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.breaker.CircuitBreaker;
import org.elasticsearch.common.bytes.BytesArray;
Expand All @@ -29,11 +27,8 @@
import org.elasticsearch.common.util.BigArrays;
import org.elasticsearch.common.util.MockBigArrays;
import org.elasticsearch.common.util.MockPageCacheRecycler;
import org.elasticsearch.xcontent.ToXContent;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentFactory;
import org.elasticsearch.xcontent.XContentType;
import org.elasticsearch.xcontent.json.JsonXContent;
import org.elasticsearch.core.CheckedConsumer;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.analysis.AnalyzerScope;
import org.elasticsearch.index.analysis.IndexAnalyzers;
Expand Down Expand Up @@ -64,15 +59,22 @@
import org.elasticsearch.search.sort.SortAndFormats;
import org.elasticsearch.search.sort.SortBuilder;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xcontent.ToXContent;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentFactory;
import org.elasticsearch.xcontent.XContentType;
import org.elasticsearch.xcontent.json.JsonXContent;

import java.io.IOException;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.function.BiFunction;
import java.util.function.BooleanSupplier;
import java.util.function.Function;
import java.util.function.Supplier;

import static java.util.Collections.emptyList;
import static java.util.stream.Collectors.toList;
Expand Down Expand Up @@ -554,4 +556,10 @@ protected SearchExecutionContext createSearchExecutionContext(MapperService mapp

return searchExecutionContext;
}

protected BiFunction<MappedFieldType, Supplier<SearchLookup>, IndexFieldData<?>> fieldDataLookup() {
return (mft, lookupSource) -> mft
.fielddataBuilder("test", lookupSource)
.build(new IndexFieldDataCache.None(), new NoneCircuitBreakerService());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.xcontent.json.JsonXContent;
import org.elasticsearch.core.CheckedConsumer;
import org.elasticsearch.index.fielddata.IndexFieldData;
import org.elasticsearch.index.fielddata.IndexFieldDataCache;
import org.elasticsearch.index.fielddata.ScriptDocValues;
import org.elasticsearch.index.query.SearchExecutionContext;
Expand All @@ -44,10 +43,8 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.BiFunction;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Collectors;

import static java.util.stream.Collectors.toList;
Expand Down Expand Up @@ -737,12 +734,6 @@ public final void testIndexTimeStoredFieldsAccess() throws IOException {
});
}

private BiFunction<MappedFieldType, Supplier<SearchLookup>, IndexFieldData<?>> fieldDataLookup() {
return (mft, lookupSource) -> mft
.fielddataBuilder("test", lookupSource)
.build(new IndexFieldDataCache.None(), new NoneCircuitBreakerService());
}

public final void testNullInput() throws Exception {
DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping));
if (allowsNullValues()) {
Expand Down

0 comments on commit d713c95

Please sign in to comment.