Skip to content

Commit

Permalink
Make NestedObjectMapper its own class (elastic#74410)
Browse files Browse the repository at this point in the history
Nested objects are implemented via a Nested class directly on object mappers,
even though nested and non-nested objects have quite different semantics. In
addition, most call-sites that need to get an object mapper in fact need a nested
object mapper. To make it clearer that nested and object mappers are different
beasts with different implementations and different requirements, we should
split them into different classes.
  • Loading branch information
romseygeek committed Jul 19, 2021
1 parent dbfd86a commit cf575f4
Show file tree
Hide file tree
Showing 40 changed files with 527 additions and 386 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ private FieldCapabilitiesIndexResponse shardOperation(final FieldCapabilitiesInd
if (searchExecutionContext.getFieldType(parentField) == null) {
// no field type, it must be an object field
ObjectMapper mapper = searchExecutionContext.getObjectMapper(parentField);
String type = mapper.nested().isNested() ? "nested" : "object";
String type = mapper.isNested() ? "nested" : "object";
IndexFieldCapabilities fieldCap = new IndexFieldCapabilities(parentField, type,
false, false, false, Collections.emptyMap());
responseMap.put(parentField, fieldCap);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
import org.elasticsearch.index.IndexWarmer.TerminationHandle;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.MappingLookup;
import org.elasticsearch.index.mapper.ObjectMapper;
import org.elasticsearch.index.mapper.NestedObjectMapper;
import org.elasticsearch.index.shard.IndexShard;
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.index.shard.ShardUtils;
Expand Down Expand Up @@ -232,7 +232,7 @@ public IndexWarmer.TerminationHandle warmReader(final IndexShard indexShard, fin
MappingLookup lookup = mapperService.mappingLookup();
if (lookup.hasNested()) {
warmUp.add(Queries.newNonNestedFilter());
lookup.getNestedParentMappers().stream().map(ObjectMapper::nestedTypeFilter).forEach(warmUp::add);
lookup.getNestedParentMappers().stream().map(NestedObjectMapper::nestedTypeFilter).forEach(warmUp::add);
}

final CountDownLatch latch = new CountDownLatch(reader.leaves().size() * warmUp.size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

package org.elasticsearch.index.mapper;

import org.elasticsearch.Version;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.index.IndexSettings;

Expand All @@ -24,7 +23,7 @@ public class DocumentMapper {
* @return the newly created document mapper
*/
public static DocumentMapper createEmpty(MapperService mapperService) {
RootObjectMapper root = new RootObjectMapper.Builder(MapperService.SINGLE_MAPPING_NAME, Version.CURRENT).build(new ContentPath(1));
RootObjectMapper root = new RootObjectMapper.Builder(MapperService.SINGLE_MAPPING_NAME).build(new ContentPath(1));
MetadataFieldMapper[] metadata = mapperService.getMetadataMappers().values().toArray(new MetadataFieldMapper[0]);
Mapping mapping = new Mapping(root, metadata, null);
return new DocumentMapper(mapperService.documentParser(), mapping);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -427,9 +427,8 @@ static void parseObjectOrNested(DocumentParserContext context, ObjectMapper mapp
+ "] as object, but found a concrete value");
}

ObjectMapper.Nested nested = mapper.nested();
if (nested.isNested()) {
context = nestedContext(context, mapper);
if (mapper.isNested()) {
context = nestedContext(context, (NestedObjectMapper) mapper);
}

// if we are at the end of the previous object, advance
Expand All @@ -443,8 +442,8 @@ static void parseObjectOrNested(DocumentParserContext context, ObjectMapper mapp

innerParseObject(context, mapper, parser, currentFieldName, token);
// restore the enable path flag
if (nested.isNested()) {
nested(context, nested);
if (mapper.isNested()) {
nested(context, (NestedObjectMapper) mapper);
}
}

Expand Down Expand Up @@ -476,7 +475,7 @@ private static void innerParseObject(DocumentParserContext context, ObjectMapper
}
}

private static void nested(DocumentParserContext context, ObjectMapper.Nested nested) {
private static void nested(DocumentParserContext context, NestedObjectMapper nested) {
LuceneDocument nestedDoc = context.doc();
LuceneDocument parentDoc = nestedDoc.getParent();
Version indexVersion = context.indexSettings().getIndexVersionCreated();
Expand All @@ -501,7 +500,7 @@ private static void addFields(Version indexCreatedVersion, LuceneDocument nested
}
}

private static DocumentParserContext nestedContext(DocumentParserContext context, ObjectMapper mapper) {
private static DocumentParserContext nestedContext(DocumentParserContext context, NestedObjectMapper mapper) {
context = context.createNestedContext(mapper.fullPath());
LuceneDocument nestedDoc = context.doc();
LuceneDocument parentDoc = nestedDoc.getParent();
Expand Down Expand Up @@ -792,7 +791,7 @@ private static Tuple<Integer, ObjectMapper> getDynamicParentMapper(DocumentParse
context.sourceToParse().dynamicTemplates().get(currentPath) + "]");
}
mapper = (ObjectMapper) fieldMapper;
if (mapper.nested() != ObjectMapper.Nested.NO) {
if (mapper.isNested()) {
throw new MapperParsingException("It is forbidden to create dynamic nested objects (["
+ currentPath + "]) through `copy_to` or dots in field names");
}
Expand Down Expand Up @@ -854,7 +853,7 @@ private static Mapper getMapper(final DocumentParserContext context,
return null;
}
objectMapper = (ObjectMapper)mapper;
if (objectMapper.nested().isNested()) {
if (objectMapper.isNested()) {
throw new MapperParsingException("Cannot add a value for field ["
+ fieldName + "] since one of the intermediate objects is mapped as a nested object: ["
+ mapper.name() + "]");
Expand Down Expand Up @@ -966,7 +965,7 @@ protected String contentType() {

private static class NoOpObjectMapper extends ObjectMapper {
NoOpObjectMapper(String name, String fullPath) {
super(name, fullPath, new Explicit<>(true, false), Nested.NO, Dynamic.RUNTIME, Collections.emptyMap(), Version.CURRENT);
super(name, fullPath, new Explicit<>(true, false), Dynamic.RUNTIME, Collections.emptyMap());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,7 @@ void createDynamicFieldFromValue(final DocumentParserContext context,
*/
Mapper createDynamicObjectMapper(DocumentParserContext context, String name) {
Mapper mapper = createObjectMapperFromTemplate(context, name);
return mapper != null ? mapper :
new ObjectMapper.Builder(name, context.indexSettings().getIndexVersionCreated()).enabled(true).build(context.path());
return mapper != null ? mapper : new ObjectMapper.Builder(name).enabled(true).build(context.path());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
package org.elasticsearch.index.mapper;

import org.elasticsearch.ElasticsearchGenerationException;
import org.elasticsearch.Version;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.xcontent.ToXContent;
Expand All @@ -36,7 +35,7 @@
public final class Mapping implements ToXContentFragment {

public static final Mapping EMPTY = new Mapping(
new RootObjectMapper.Builder("_doc", Version.CURRENT).build(new ContentPath()), new MetadataFieldMapper[0], null);
new RootObjectMapper.Builder("_doc").build(new ContentPath()), new MetadataFieldMapper[0], null);

private final RootObjectMapper root;
private final Map<String, Object> meta;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ private MappingLookup(Mapping mapping,
if (objects.put(mapper.fullPath(), mapper) != null) {
throw new MapperParsingException("Object mapper [" + mapper.fullPath() + "] is defined more than once");
}
if (mapper.nested().isNested()) {
if (mapper.isNested()) {
hasNested = true;
}
}
Expand Down Expand Up @@ -259,7 +259,7 @@ private void checkFieldNameLengthLimit(long limit) {
private void checkNestedLimit(long limit) {
long actualNestedFields = 0;
for (ObjectMapper objectMapper : objectMappers.values()) {
if (objectMapper.nested().isNested()) {
if (objectMapper.isNested()) {
actualNestedFields++;
}
}
Expand Down Expand Up @@ -288,7 +288,7 @@ public boolean isObjectField(String field) {
public String getNestedScope(String path) {
for (String parentPath = parentObject(path); parentPath != null; parentPath = parentObject(parentPath)) {
ObjectMapper objectMapper = objectMappers.get(parentPath);
if (objectMapper != null && objectMapper.nested().isNested()) {
if (objectMapper != null && objectMapper.isNested()) {
return parentPath;
}
}
Expand Down Expand Up @@ -369,13 +369,13 @@ public Mapping getMapping() {
/**
* Returns all nested object mappers
*/
public List<ObjectMapper> getNestedMappers() {
List<ObjectMapper> childMappers = new ArrayList<>();
public List<NestedObjectMapper> getNestedMappers() {
List<NestedObjectMapper> childMappers = new ArrayList<>();
for (ObjectMapper mapper : objectMappers().values()) {
if (mapper.nested().isNested() == false) {
if (mapper.isNested() == false) {
continue;
}
childMappers.add(mapper);
childMappers.add((NestedObjectMapper) mapper);
}
return childMappers;
}
Expand All @@ -385,16 +385,16 @@ public List<ObjectMapper> getNestedMappers() {
*
* Used by BitSetProducerWarmer
*/
public List<ObjectMapper> getNestedParentMappers() {
List<ObjectMapper> parents = new ArrayList<>();
public List<NestedObjectMapper> getNestedParentMappers() {
List<NestedObjectMapper> parents = new ArrayList<>();
for (ObjectMapper mapper : objectMappers().values()) {
String nestedParentPath = getNestedParent(mapper.fullPath());
if (nestedParentPath == null) {
continue;
}
ObjectMapper parent = objectMappers().get(nestedParentPath);
if (parent.nested().isNested()) {
parents.add(parent);
if (parent.isNested()) {
parents.add((NestedObjectMapper)parent);
}
}
return parents;
Expand All @@ -421,7 +421,7 @@ public String getNestedParent(String path) {
if (mapper == null) {
return null;
}
if (mapper.nested().isNested()) {
if (mapper.isNested()) {
return path;
}
if (path.contains(".") == false) {
Expand Down
Loading

0 comments on commit cf575f4

Please sign in to comment.