Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor how to determine if a field is metafield (#57378) #57771

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@
package org.elasticsearch.index.mapper;

import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.IndexService;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.ESSingleNodeTestCase;
Expand All @@ -44,7 +46,7 @@ public void setup() {
protected Collection<Class<? extends Plugin>> getPlugins() {
return pluginList(MapperExtrasPlugin.class);
}

public void testBasics() throws Exception {
String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties").startObject("field").field("type", "rank_feature").endObject().endObject()
Expand All @@ -55,4 +57,18 @@ public void testBasics() throws Exception {
assertEquals(mapping, mapper.mappingSource().toString());
assertNotNull(mapper.metadataMapper(RankFeatureMetaFieldMapper.class));
}

/**
* Check that meta-fields are picked from plugins (in this case MapperExtrasPlugin),
* and parsing of a document fails if the document contains these meta-fields.
*/
public void testDocumentParsingFailsOnMetaField() throws Exception {
String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("_doc").endObject().endObject());
DocumentMapper mapper = parser.parse("_doc", new CompressedXContent(mapping));
String rfMetaField = RankFeatureMetaFieldMapper.CONTENT_TYPE;
BytesReference bytes = BytesReference.bytes(XContentFactory.jsonBuilder().startObject().field(rfMetaField, 0).endObject());
MapperParsingException e = expectThrows(MapperParsingException.class, () ->
mapper.parse(new SourceToParse("test", "_doc", "1", bytes, XContentType.JSON)));
assertTrue(e.getMessage().contains("Field ["+ rfMetaField + "] is a metadata field and cannot be added inside a document."));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@

import java.io.IOException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

Expand Down Expand Up @@ -104,13 +102,9 @@ static void innerHitsExecute(Query mainQuery, IndexSearcher indexSearcher, Searc
continue;
}

Map<String, DocumentField> fields = hit.fieldsOrNull();
if (fields == null) {
fields = new HashMap<>();
hit.fields(fields);
}
IntStream slots = convertTopDocsToSlots(topDocs, rootDocsBySlot);
hit.setField(fieldName, new DocumentField(fieldName, slots.boxed().collect(Collectors.toList())));
// _percolator_document_slot fields are document fields and should be under "fields" section in a hit
hit.setDocumentField(fieldName, new DocumentField(fieldName, slots.boxed().collect(Collectors.toList())));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import java.util.OptionalInt;

import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode;
import static org.elasticsearch.test.XContentTestUtils.insertRandomFields;

public class RatedSearchHitTests extends ESTestCase {

Expand Down Expand Up @@ -76,8 +75,7 @@ public void testXContentRoundtrip() throws IOException {
RatedSearchHit testItem = randomRatedSearchHit();
XContentType xContentType = randomFrom(XContentType.values());
BytesReference originalBytes = toShuffledXContent(testItem, xContentType, ToXContent.EMPTY_PARAMS, randomBoolean());
BytesReference withRandomFields = insertRandomFields(xContentType, originalBytes, null, random());
try (XContentParser parser = createParser(xContentType.xContent(), withRandomFields)) {
try (XContentParser parser = createParser(xContentType.xContent(), originalBytes)) {
RatedSearchHit parsedItem = RatedSearchHit.parse(parser);
assertNotSame(testItem, parsedItem);
assertEquals(testItem, parsedItem);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -596,14 +596,12 @@ public void testGetFieldsComplexField() throws Exception {
String field = "field1.field2.field3.field4";
GetResponse getResponse = client().prepareGet("my-index", "my-type", "1").setStoredFields(field).get();
assertThat(getResponse.isExists(), equalTo(true));
assertThat(getResponse.getField(field).isMetadataField(), equalTo(false));
assertThat(getResponse.getField(field).getValues().size(), equalTo(2));
assertThat(getResponse.getField(field).getValues().get(0).toString(), equalTo("value1"));
assertThat(getResponse.getField(field).getValues().get(1).toString(), equalTo("value2"));

getResponse = client().prepareGet("my-index", "my-type", "1").setStoredFields(field).get();
assertThat(getResponse.isExists(), equalTo(true));
assertThat(getResponse.getField(field).isMetadataField(), equalTo(false));
assertThat(getResponse.getField(field).getValues().size(), equalTo(2));
assertThat(getResponse.getField(field).getValues().get(0).toString(), equalTo("value1"));
assertThat(getResponse.getField(field).getValues().get(1).toString(), equalTo("value2"));
Expand Down Expand Up @@ -631,7 +629,6 @@ public void testGetFieldsComplexField() throws Exception {

getResponse = client().prepareGet("my-index", "my-type", "1").setStoredFields(field).get();
assertThat(getResponse.isExists(), equalTo(true));
assertThat(getResponse.getField(field).isMetadataField(), equalTo(false));
assertThat(getResponse.getField(field).getValues().size(), equalTo(2));
assertThat(getResponse.getField(field).getValues().get(0).toString(), equalTo("value1"));
assertThat(getResponse.getField(field).getValues().get(1).toString(), equalTo("value2"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,7 @@ public void testGetFieldsMetadataWithRouting() throws Exception {
.setStoredFields("field1")
.get();
assertThat(getResponse.isExists(), equalTo(true));
assertThat(getResponse.getField("field1").isMetadataField(), equalTo(false));
assertThat(getResponse.getField("field1").getValue().toString(), equalTo("value"));
assertThat(getResponse.getField("_routing").isMetadataField(), equalTo(true));
assertThat(getResponse.getField("_routing").getValue().toString(), equalTo("1"));
}

Expand All @@ -78,9 +76,7 @@ public void testGetFieldsMetadataWithRouting() throws Exception {
.setRouting("1")
.get();
assertThat(getResponse.isExists(), equalTo(true));
assertThat(getResponse.getField("field1").isMetadataField(), equalTo(false));
assertThat(getResponse.getField("field1").getValue().toString(), equalTo("value"));
assertThat(getResponse.getField("_routing").isMetadataField(), equalTo(true));
assertThat(getResponse.getField("_routing").getValue().toString(), equalTo("1"));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,10 @@ public void hitExecute(SearchContext context, HitContext hitContext) {
return;
}
String field = fetchSubPhaseBuilder.getField();
if (hitContext.hit().fieldsOrNull() == null) {
hitContext.hit().fields(new HashMap<>());
}
DocumentField hitField = hitContext.hit().getFields().get(NAME);
if (hitField == null) {
hitField = new DocumentField(NAME, new ArrayList<>(1));
hitContext.hit().setField(NAME, hitField);
hitContext.hit().setDocumentField(NAME, hitField);
}
TermVectorsRequest termVectorsRequest = new TermVectorsRequest(context.indexShard().shardId().getIndex().getName(),
hitContext.hit().getType(), hitContext.hit().getId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,6 @@ public void testSearchFieldsMetadata() throws Exception {

assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L));
assertThat(searchResponse.getHits().getAt(0).field("field1"), nullValue());
assertThat(searchResponse.getHits().getAt(0).field("_routing").isMetadataField(), equalTo(true));
assertThat(searchResponse.getHits().getAt(0).field("_routing").getValue().toString(), equalTo("1"));
}

Expand Down Expand Up @@ -736,7 +735,6 @@ public void testGetFieldsComplexField() throws Exception {

SearchResponse searchResponse = client().prepareSearch("my-index").addStoredField(field).get();
assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L));
assertThat(searchResponse.getHits().getAt(0).field(field).isMetadataField(), equalTo(false));
assertThat(searchResponse.getHits().getAt(0).field(field).getValues().size(), equalTo(2));
assertThat(searchResponse.getHits().getAt(0).field(field).getValues().get(0).toString(), equalTo("value1"));
assertThat(searchResponse.getHits().getAt(0).field(field).getValues().get(1).toString(), equalTo("value2"));
Expand Down Expand Up @@ -1223,7 +1221,6 @@ public void testLoadMetadata() throws Exception {
Map<String, DocumentField> fields = response.getHits().getAt(0).getFields();

assertThat(fields.get("field1"), nullValue());
assertThat(fields.get("_routing").isMetadataField(), equalTo(true));
assertThat(fields.get("_routing").getValue().toString(), equalTo("1"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.get.GetResult;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.search.SearchHit;

import java.io.IOException;
Expand Down Expand Up @@ -87,13 +86,6 @@ public List<Object> getValues() {
return values;
}

/**
* @return The field is a metadata field
*/
public boolean isMetadataField() {
return MapperService.isMetadataField(name);
}

@Override
public Iterator<Object> iterator() {
return values.iterator();
Expand Down
28 changes: 8 additions & 20 deletions server/src/main/java/org/elasticsearch/index/get/GetResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.mapper.IgnoredFieldMapper;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.SourceFieldMapper;
import org.elasticsearch.search.lookup.SourceLookup;

Expand Down Expand Up @@ -97,7 +98,8 @@ public GetResult(StreamInput in) throws IOException {
Map<String, DocumentField> fields = readFields(in);
documentFields = new HashMap<>();
metaFields = new HashMap<>();
splitFieldsByMetadata(fields, documentFields, metaFields);
fields.forEach((fieldName, docField) ->
(MapperService.isMetadataFieldStatic(fieldName) ? metaFields : documentFields).put(fieldName, docField));
}
} else {
metaFields = Collections.emptyMap();
Expand Down Expand Up @@ -363,7 +365,7 @@ public static GetResult fromXContentEmbedded(XContentParser parser, String index
} else if (FOUND.equals(currentFieldName)) {
found = parser.booleanValue();
} else {
metaFields.put(currentFieldName, new DocumentField(currentFieldName,
metaFields.put(currentFieldName, new DocumentField(currentFieldName,
Collections.singletonList(parser.objectText())));
}
} else if (token == XContentParser.Token.START_OBJECT) {
Expand Down Expand Up @@ -401,10 +403,10 @@ public static GetResult fromXContent(XContentParser parser) throws IOException {
}

private Map<String, DocumentField> readFields(StreamInput in) throws IOException {
Map<String, DocumentField> fields = null;
Map<String, DocumentField> fields;
int size = in.readVInt();
if (size == 0) {
fields = new HashMap<>();
fields = emptyMap();
} else {
fields = new HashMap<>(size);
for (int i = 0; i < size; i++) {
Expand All @@ -414,20 +416,6 @@ private Map<String, DocumentField> readFields(StreamInput in) throws IOException
}
return fields;
}

static void splitFieldsByMetadata(Map<String, DocumentField> fields, Map<String, DocumentField> outOther,
Map<String, DocumentField> outMetadata) {
if (fields == null) {
return;
}
for (Map.Entry<String, DocumentField> fieldEntry: fields.entrySet()) {
if (fieldEntry.getValue().isMetadataField()) {
outMetadata.put(fieldEntry.getKey(), fieldEntry.getValue());
} else {
outOther.put(fieldEntry.getKey(), fieldEntry.getValue());
}
}
}

@Override
public void writeTo(StreamOutput out) throws IOException {
Expand All @@ -446,11 +434,11 @@ public void writeTo(StreamOutput out) throws IOException {
writeFields(out, documentFields);
writeFields(out, metaFields);
} else {
writeFields(out, this.getFields());
writeFields(out, this.getFields());
}
}
}

private void writeFields(StreamOutput out, Map<String, DocumentField> fields) throws IOException {
if (fields == null) {
out.writeVInt(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ private GetResult innerGetLoadFromStoredFields(String type, String id, String[]
documentFields = new HashMap<>();
metadataFields = new HashMap<>();
for (Map.Entry<String, List<Object>> entry : fieldVisitor.fields().entrySet()) {
if (MapperService.isMetadataField(entry.getKey())) {
if (mapperService.isMetadataField(entry.getKey())) {
metadataFields.put(entry.getKey(), new DocumentField(entry.getKey(), entry.getValue()));
} else {
documentFields.put(entry.getKey(), new DocumentField(entry.getKey(), entry.getValue()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ private static void innerParseObject(ParseContext context, ObjectMapper mapper,
if (token == XContentParser.Token.FIELD_NAME) {
currentFieldName = parser.currentName();
paths = splitAndValidatePath(currentFieldName);
if (MapperService.isMetadataField(context.path().pathAsText(currentFieldName))) {
if (context.mapperService().isMetadataField(context.path().pathAsText(currentFieldName))) {
throw new MapperParsingException("Field [" + currentFieldName + "] is a metadata field and cannot be added inside"
+ " a document. Use the index API request parameters.");
} else if (containsDisabledObjectMapper(mapper, paths)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

package org.elasticsearch.index.mapper;

import com.carrotsearch.hppc.ObjectHashSet;
import com.carrotsearch.hppc.cursors.ObjectCursor;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.message.ParameterizedMessage;
Expand Down Expand Up @@ -56,14 +55,14 @@
import org.elasticsearch.index.mapper.Mapper.BuilderContext;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.index.similarity.SimilarityService;
import org.elasticsearch.indices.IndicesModule;
import org.elasticsearch.indices.InvalidTypeNameException;
import org.elasticsearch.indices.mapper.MapperRegistry;
import org.elasticsearch.search.suggest.completion.context.ContextMapping;

import java.io.Closeable;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -121,14 +120,6 @@ public enum MergeReason {
Setting.boolSetting("index.mapper.dynamic", INDEX_MAPPER_DYNAMIC_DEFAULT,
Property.Dynamic, Property.IndexScope, Property.Deprecated);

//TODO this needs to be cleaned up: _timestamp and _ttl are not supported anymore, _field_names, _seq_no, _version and _source are
//also missing, not sure if on purpose. See IndicesModule#getMetadataMappers
private static final String[] SORTED_META_FIELDS = new String[]{
"_id", IgnoredFieldMapper.NAME, "_index", "_routing", "_size", "_timestamp", "_ttl", "_type"
};

private static final ObjectHashSet<String> META_FIELDS = ObjectHashSet.from(SORTED_META_FIELDS);

private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(MapperService.class));
static final String DEFAULT_MAPPING_ERROR_MESSAGE = "[_default_] mappings are not allowed on new indices and should no " +
"longer be used. See [https://www.elastic.co/guide/en/elasticsearch/reference/current/breaking-changes-7.0.html" +
Expand All @@ -146,6 +137,7 @@ public enum MergeReason {
private boolean hasNested = false; // updated dynamically to true when a nested object is added

private final DocumentMapperParser documentParser;
private final Version indexVersionCreated;

private final MapperAnalyzerWrapper indexAnalyzer;
private final MapperAnalyzerWrapper searchAnalyzer;
Expand All @@ -161,6 +153,7 @@ public MapperService(IndexSettings indexSettings, IndexAnalyzers indexAnalyzers,
SimilarityService similarityService, MapperRegistry mapperRegistry,
Supplier<QueryShardContext> queryShardContextSupplier, BooleanSupplier idFieldDataEnabled) {
super(indexSettings);
this.indexVersionCreated = indexSettings.getIndexVersionCreated();
this.indexAnalyzers = indexAnalyzers;
this.fieldTypes = new FieldTypeLookup();
this.documentParser = new DocumentMapperParser(indexSettings, this, xContentRegistry, similarityService, mapperRegistry,
Expand Down Expand Up @@ -812,14 +805,27 @@ public void close() throws IOException {
}

/**
* @return Whether a field is a metadata field.
* @return Whether a field is a metadata field
* Deserialization of SearchHit objects sent from pre 7.8 nodes and GetResults objects sent from pre 7.3 nodes,
* uses this method to divide fields into meta and document fields.
* TODO: remove in v 9.0
* @deprecated Use an instance method isMetadataField instead
*/
public static boolean isMetadataField(String fieldName) {
return META_FIELDS.contains(fieldName);
@Deprecated
public static boolean isMetadataFieldStatic(String fieldName) {
if (IndicesModule.getBuiltInMetadataFields().contains(fieldName)) {
return true;
}
// if a node had Size Plugin installed, _size field should also be considered a meta-field
return fieldName.equals("_size");
}

public static String[] getAllMetaFields() {
return Arrays.copyOf(SORTED_META_FIELDS, SORTED_META_FIELDS.length);
/**
* @return Whether a field is a metadata field.
* this method considers all mapper plugins
*/
public boolean isMetadataField(String field) {
return mapperRegistry.isMetadataField(indexVersionCreated, field);
}

/**
Expand Down
Loading