Skip to content

Commit

Permalink
Read subobjects mapping parameter in advance (#86894)
Browse files Browse the repository at this point in the history
As part of #86166 we added support for a new mapping parameter called `subobjects` that
can be set to the `object` field type. Such parameter will affect not only how incoming
documents will be dynamically mapped in the future, but also how field names are treated
as part of the mappings themselves.

Mappings get parsed into a map, where keys ordering is not guaranteed. In case a mappings call
contains `subobjects` set to `false` and also sub-fields for that same object, we need to make
sure that we read the parameter in advance in order to know how to treat the sub-field and decide
whether we need to expand dots in field names or leave them as they are.
  • Loading branch information
javanna committed May 23, 2022
1 parent ca542fe commit bff9113
Show file tree
Hide file tree
Showing 17 changed files with 245 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import org.elasticsearch.index.mapper.Mapping;
import org.elasticsearch.index.mapper.MappingLookup;
import org.elasticsearch.index.mapper.MetadataFieldMapper;
import org.elasticsearch.index.mapper.ObjectMapper;
import org.elasticsearch.index.mapper.RootObjectMapper;
import org.elasticsearch.indices.EmptySystemIndices;
import org.elasticsearch.indices.IndicesService;
Expand Down Expand Up @@ -218,7 +219,7 @@ public void setup() throws Exception {
false,
Version.CURRENT
).build(MapperBuilderContext.ROOT);
RootObjectMapper.Builder root = new RootObjectMapper.Builder("_doc");
RootObjectMapper.Builder root = new RootObjectMapper.Builder("_doc", ObjectMapper.Defaults.SUBOBJECTS);
root.add(
new DateFieldMapper.Builder(
"@timestamp",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
"Metrics indexing":
"Metrics object indexing":
- skip:
features: allowed_warnings_regex
version: " - 8.2.99"
Expand All @@ -17,6 +17,9 @@
mapping:
type: object
subobjects: false
properties:
host.name:
type: keyword

- do:
allowed_warnings_regex:
Expand All @@ -26,15 +29,62 @@
id: 1
refresh: true
body:
{ metrics.time: 10, metrics.time.max: 100, metrics.time.min: 1 }
{ metrics.host.name: localhost, metrics.host.id: 1, metrics.time: 10, metrics.time.max: 100, metrics.time.min: 1 }

- do:
field_caps:
index: test-1
fields: metrics.time*
fields: metrics*
- match: {fields.metrics\.host\.id.long.searchable: true}
- match: {fields.metrics\.host\.id.long.aggregatable: true}
- match: {fields.metrics\.host\.name.keyword.searchable: true}
- match: {fields.metrics\.host\.name.keyword.aggregatable: true}
- match: {fields.metrics\.time.long.searchable: true}
- match: {fields.metrics\.time.long.aggregatable: true}
- match: {fields.metrics\.time\.max.long.searchable: true}
- match: {fields.metrics\.time\.max.long.aggregatable: true}
- match: {fields.metrics\.time\.min.long.searchable: true}
- match: {fields.metrics\.time\.min.long.aggregatable: true}

---
"Root without subobjects":
- skip:
features: allowed_warnings_regex
version: " - 8.2.99"
reason: added in 8.3.0

- do:
indices.put_template:
name: test
body:
index_patterns: test-*
mappings:
subobjects: false
properties:
host.name:
type: keyword

- do:
allowed_warnings_regex:
- "index \\[test-1\\] matches multiple legacy templates \\[global, test\\], composable templates will only match a single template"
index:
index: test-1
id: 1
refresh: true
body:
{ host.name: localhost, host.id: 1, time: 10, time.max: 100, time.min: 1 }

- do:
field_caps:
index: test-1
fields: [host*, time*]
- match: {fields.host\.name.keyword.searchable: true}
- match: {fields.host\.name.keyword.aggregatable: true}
- match: {fields.host\.id.long.searchable: true}
- match: {fields.host\.id.long.aggregatable: true}
- match: {fields.time.long.searchable: true}
- match: {fields.time.long.aggregatable: true}
- match: {fields.time\.max.long.searchable: true}
- match: {fields.time\.max.long.aggregatable: true}
- match: {fields.time\.min.long.searchable: true}
- match: {fields.time\.min.long.aggregatable: true}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.elasticsearch.action.bulk.BulkResponse;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.action.index.IndexResponse;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.cluster.ClusterState;
Expand Down Expand Up @@ -494,4 +495,77 @@ public void testDynamicRuntimeObjectFields() {
e.getMessage()
);
}

public void testSubobjectsFalseAtRoot() {
assertAcked(client().admin().indices().prepareCreate("test").setMapping("""
{
"_doc": {
"subobjects" : false,
"properties": {
"host.name": {
"type": "keyword"
}
}
}
}""").get());

IndexRequest request = new IndexRequest("test").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
.source("host.name", "localhost", "host.id", 111, "time", 100, "time.max", 1000);
IndexResponse indexResponse = client().index(request).actionGet();
assertEquals(RestStatus.CREATED, indexResponse.status());

Map<String, Object> mappings = client().admin().indices().prepareGetMappings("test").get().mappings().get("test").getSourceAsMap();
@SuppressWarnings("unchecked")
Map<String, Object> properties = (Map<String, Object>) mappings.get("properties");
assertEquals(4, properties.size());
assertNotNull(properties.get("host.name"));
assertNotNull(properties.get("host.id"));
assertNotNull(properties.get("time"));
assertNotNull(properties.get("time.max"));
}

@SuppressWarnings("unchecked")
public void testSubobjectsFalse() {
assertAcked(client().admin().indices().prepareCreate("test").setMapping("""
{
"_doc": {
"properties": {
"foo.metrics" : {
"subobjects" : false,
"properties" : {
"host.name": {
"type": "keyword"
}
}
}
}
}
}""").get());

IndexRequest request = new IndexRequest("test").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
.source(
"foo.metrics.host.name",
"localhost",
"foo.metrics.host.id",
111,
"foo.metrics.time",
100,
"foo.metrics.time.max",
1000
);
IndexResponse indexResponse = client().index(request).actionGet();
assertEquals(RestStatus.CREATED, indexResponse.status());

Map<String, Object> mappings = client().admin().indices().prepareGetMappings("test").get().mappings().get("test").getSourceAsMap();
Map<String, Object> properties = (Map<String, Object>) mappings.get("properties");
Map<String, Object> foo = (Map<String, Object>) properties.get("foo");
properties = (Map<String, Object>) foo.get("properties");
Map<String, Object> metrics = (Map<String, Object>) properties.get("metrics");
properties = (Map<String, Object>) metrics.get("properties");
assertEquals(4, properties.size());
assertNotNull(properties.get("host.name"));
assertNotNull(properties.get("host.id"));
assertNotNull(properties.get("time"));
assertNotNull(properties.get("time.max"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ public class DocumentMapper {
* @return the newly created document mapper
*/
public static DocumentMapper createEmpty(MapperService mapperService) {
RootObjectMapper root = new RootObjectMapper.Builder(MapperService.SINGLE_MAPPING_NAME).build(MapperBuilderContext.ROOT);
RootObjectMapper root = new RootObjectMapper.Builder(MapperService.SINGLE_MAPPING_NAME, ObjectMapper.Defaults.SUBOBJECTS).build(
MapperBuilderContext.ROOT
);
MetadataFieldMapper[] metadata = mapperService.getMetadataMappers().values().toArray(new MetadataFieldMapper[0]);
Mapping mapping = new Mapping(root, metadata, null);
return new DocumentMapper(mapperService.documentParser(), mapping, mapping.toCompressedXContent());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@ static Mapper createDynamicObjectMapper(DocumentParserContext context, String na
Mapper mapper = createObjectMapperFromTemplate(context, name);
return mapper != null
? mapper
: new ObjectMapper.Builder(name).enabled(ObjectMapper.Defaults.ENABLED).build(context.createMapperBuilderContext());
: new ObjectMapper.Builder(name, ObjectMapper.Defaults.SUBOBJECTS).enabled(ObjectMapper.Defaults.ENABLED)
.build(context.createMapperBuilderContext());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
public final class Mapping implements ToXContentFragment {

public static final Mapping EMPTY = new Mapping(
new RootObjectMapper.Builder("_doc").build(MapperBuilderContext.ROOT),
new RootObjectMapper.Builder("_doc", ObjectMapper.Defaults.SUBOBJECTS).build(MapperBuilderContext.ROOT),
new MetadataFieldMapper[0],
null
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public static class Builder extends ObjectMapper.Builder {
private final Version indexCreatedVersion;

public Builder(String name, Version indexCreatedVersion) {
super(name);
super(name, Explicit.IMPLICIT_TRUE);
this.indexCreatedVersion = indexCreatedVersion;
}

Expand All @@ -57,6 +57,9 @@ public static class TypeParser extends ObjectMapper.TypeParser {
@Override
public Mapper.Builder parse(String name, Map<String, Object> node, MappingParserContext parserContext)
throws MapperParsingException {
if (parseSubobjects(node).explicit()) {
throw new MapperParsingException("Nested type [" + name + "] does not support [subobjects] parameter");
}
NestedObjectMapper.Builder builder = new NestedObjectMapper.Builder(name, parserContext.indexVersionCreated());
parseNested(name, node, builder);
for (Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator(); iterator.hasNext();) {
Expand All @@ -67,9 +70,6 @@ public Mapper.Builder parse(String name, Map<String, Object> node, MappingParser
iterator.remove();
}
}
if (builder.subobjects.explicit()) {
throw new MapperParsingException("Nested type [" + name + "] does not support [subobjects] parameter");
}
return builder;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public class ObjectMapper extends Mapper implements Cloneable {

public static class Defaults {
public static final boolean ENABLED = true;
public static final Explicit<Boolean> SUBOBJECTS = Explicit.IMPLICIT_TRUE;
}

public enum Dynamic {
Expand All @@ -61,26 +62,21 @@ DynamicFieldsBuilder getDynamicFieldsBuilder() {
}

public static class Builder extends Mapper.Builder {

protected final Explicit<Boolean> subobjects;
protected Explicit<Boolean> enabled = Explicit.IMPLICIT_TRUE;
protected Explicit<Boolean> subobjects = Explicit.IMPLICIT_TRUE;
protected Dynamic dynamic;
protected final List<Mapper.Builder> mappersBuilders = new ArrayList<>();

public Builder(String name) {
public Builder(String name, Explicit<Boolean> subobjects) {
super(name);
this.subobjects = subobjects;
}

public Builder enabled(boolean enabled) {
this.enabled = Explicit.explicitBoolean(enabled);
return this;
}

public Builder subobjects(boolean subobjects) {
this.subobjects = Explicit.explicitBoolean(subobjects);
return this;
}

public Builder dynamic(Dynamic dynamic) {
this.dynamic = dynamic;
return this;
Expand Down Expand Up @@ -186,7 +182,8 @@ public boolean supportsVersion(Version indexCreatedVersion) {
@Override
public Mapper.Builder parse(String name, Map<String, Object> node, MappingParserContext parserContext)
throws MapperParsingException {
ObjectMapper.Builder builder = new Builder(name);
Explicit<Boolean> subobjects = parseSubobjects(node);
ObjectMapper.Builder builder = new Builder(name, subobjects);
for (Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator(); iterator.hasNext();) {
Map.Entry<String, Object> entry = iterator.next();
String fieldName = entry.getKey();
Expand Down Expand Up @@ -219,9 +216,6 @@ protected static boolean parseObjectOrDocumentTypeProperties(
} else if (fieldName.equals("enabled")) {
builder.enabled(XContentMapValues.nodeBooleanValue(fieldNode, fieldName + ".enabled"));
return true;
} else if (fieldName.equals("subobjects")) {
builder.subobjects(XContentMapValues.nodeBooleanValue(fieldNode, fieldName + ".subobjects"));
return true;
} else if (fieldName.equals("properties")) {
if (fieldNode instanceof Collection && ((Collection) fieldNode).isEmpty()) {
// nothing to do here, empty (to support "properties: []" case)
Expand All @@ -242,6 +236,14 @@ protected static boolean parseObjectOrDocumentTypeProperties(
return false;
}

protected static Explicit<Boolean> parseSubobjects(Map<String, Object> node) {
Object subobjectsNode = node.remove("subobjects");
if (subobjectsNode != null) {
return Explicit.explicitBoolean(XContentMapValues.nodeBooleanValue(subobjectsNode, "subobjects.subobjects"));
}
return Explicit.IMPLICIT_TRUE;
}

protected static void parseProperties(
ObjectMapper.Builder objBuilder,
Map<String, Object> propsNode,
Expand Down Expand Up @@ -298,7 +300,7 @@ protected static void parseProperties(
String realFieldName = fieldNameParts[fieldNameParts.length - 1];
fieldBuilder = typeParser.parse(realFieldName, propNode, parserContext);
for (int i = fieldNameParts.length - 2; i >= 0; --i) {
ObjectMapper.Builder intermediate = new ObjectMapper.Builder(fieldNameParts[i]);
ObjectMapper.Builder intermediate = new ObjectMapper.Builder(fieldNameParts[i], Defaults.SUBOBJECTS);
intermediate.add(fieldBuilder);
fieldBuilder = intermediate;
}
Expand Down Expand Up @@ -367,9 +369,8 @@ protected ObjectMapper clone() {
* @return a Builder that will produce an empty ObjectMapper with the same configuration as this one
*/
public ObjectMapper.Builder newBuilder(Version indexVersionCreated) {
ObjectMapper.Builder builder = new ObjectMapper.Builder(simpleName());
ObjectMapper.Builder builder = new ObjectMapper.Builder(simpleName(), subobjects);
builder.enabled = this.enabled;
builder.subobjects = this.subobjects;
builder.dynamic = this.dynamic;
return builder;
}
Expand Down Expand Up @@ -515,7 +516,7 @@ void toXContent(XContentBuilder builder, Params params, ToXContent custom) throw
if (isEnabled() != Defaults.ENABLED) {
builder.field("enabled", enabled.value());
}
if (subobjects() == false) {
if (subobjects != Defaults.SUBOBJECTS) {
builder.field("subobjects", subobjects.value());
}
if (custom != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ public static class Builder extends ObjectMapper.Builder {
protected Explicit<Boolean> dateDetection = Defaults.DATE_DETECTION;
protected Explicit<Boolean> numericDetection = Defaults.NUMERIC_DETECTION;

public Builder(String name) {
super(name);
public Builder(String name, Explicit<Boolean> subobjects) {
super(name, subobjects);
}

public Builder dynamicDateTimeFormatter(Collection<DateFormatter> dateTimeFormatters) {
Expand Down Expand Up @@ -152,7 +152,8 @@ static final class TypeParser extends ObjectMapper.TypeParser {
@Override
public RootObjectMapper.Builder parse(String name, Map<String, Object> node, MappingParserContext parserContext)
throws MapperParsingException {
RootObjectMapper.Builder builder = new Builder(name);
Explicit<Boolean> subobjects = parseSubobjects(node);
RootObjectMapper.Builder builder = new Builder(name, subobjects);
Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator();
while (iterator.hasNext()) {
Map.Entry<String, Object> entry = iterator.next();
Expand Down Expand Up @@ -278,9 +279,8 @@ protected ObjectMapper clone() {

@Override
public RootObjectMapper.Builder newBuilder(Version indexVersionCreated) {
RootObjectMapper.Builder builder = new RootObjectMapper.Builder(name());
RootObjectMapper.Builder builder = new RootObjectMapper.Builder(name(), subobjects);
builder.enabled = enabled;
builder.subobjects = subobjects;
builder.dynamic = dynamic;
return builder;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.elasticsearch.index.mapper.MapperBuilderContext;
import org.elasticsearch.index.mapper.Mapping;
import org.elasticsearch.index.mapper.MetadataFieldMapper;
import org.elasticsearch.index.mapper.ObjectMapper;
import org.elasticsearch.index.mapper.RootObjectMapper;
import org.elasticsearch.test.ESTestCase;

Expand Down Expand Up @@ -146,7 +147,9 @@ public void testSendUpdateMappingUsingAutoPutMappingAction() {
);
mua.setClient(client);

RootObjectMapper rootObjectMapper = new RootObjectMapper.Builder("name").build(MapperBuilderContext.ROOT);
RootObjectMapper rootObjectMapper = new RootObjectMapper.Builder("name", ObjectMapper.Defaults.SUBOBJECTS).build(
MapperBuilderContext.ROOT
);
Mapping update = new Mapping(rootObjectMapper, new MetadataFieldMapper[0], Map.of());

mua.sendUpdateMapping(new Index("name", "uuid"), update, ActionListener.noop());
Expand Down

0 comments on commit bff9113

Please sign in to comment.