Skip to content

Commit 83b492c

Browse files
romseygeekromseygeek
committed
Runtime fields that shadow an object shouldn't shadow its children (#65820)
If you define a runtime field, and we encounter a field in a document at index-time that shares its name with the runtime field, then that field is 'shadowed' and does not get indexed. Currently, if that field happens to be an object, we end up not consuming its children correctly in the parser and we can end up with a mapper parsing exception. This commit changes the parsing logic so that the shadowing is not applied to fields that are defined as objects. The object children will continue to be indexed, but queries against the object parent will be redirected to the runtime field.
1 parent 279bf21 commit 83b492c

File tree

2 files changed

+143
-8
lines changed

2 files changed

+143
-8
lines changed

‎server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ private static void parseArray(ParseContext context, ObjectMapper parentMapper,
539539
String[] paths) throws IOException {
540540
String arrayFieldName = lastFieldName;
541541

542-
Mapper mapper = getMapper(context, parentMapper, lastFieldName, paths);
542+
Mapper mapper = getLeafMapper(context, parentMapper, lastFieldName, paths);
543543
if (mapper != null) {
544544
// There is a concrete mapper for this field already. Need to check if the mapper
545545
// expects an array, if so we pass the context straight to the mapper and if not
@@ -615,7 +615,7 @@ private static void parseValue(final ParseContext context, ObjectMapper parentMa
615615
throw new MapperParsingException("object mapping [" + parentMapper.name() + "] trying to serialize a value with"
616616
+ " no field associated with it, current value [" + context.parser().textOrNull() + "]");
617617
}
618-
Mapper mapper = getMapper(context, parentMapper, currentFieldName, paths);
618+
Mapper mapper = getLeafMapper(context, parentMapper, currentFieldName, paths);
619619
if (mapper != null) {
620620
parseObjectOrField(context, mapper);
621621
} else {
@@ -632,7 +632,7 @@ private static void parseValue(final ParseContext context, ObjectMapper parentMa
632632
private static void parseNullValue(ParseContext context, ObjectMapper parentMapper, String lastFieldName,
633633
String[] paths) throws IOException {
634634
// we can only handle null values if we have mappings for them
635-
Mapper mapper = getMapper(context, parentMapper, lastFieldName, paths);
635+
Mapper mapper = getLeafMapper(context, parentMapper, lastFieldName, paths);
636636
if (mapper != null) {
637637
// TODO: passing null to an object seems bogus?
638638
parseObjectOrField(context, mapper);
@@ -898,7 +898,13 @@ private static ObjectMapper.Dynamic dynamicOrDefault(ObjectMapper parentMapper,
898898
}
899899

900900
// looks up a child mapper, but takes into account field names that expand to objects
901-
private static Mapper getMapper(final ParseContext context, ObjectMapper objectMapper, String fieldName, String[] subfields) {
901+
// returns null if no such child mapper exists - note that unlike getLeafMapper,
902+
// we do not check for shadowing runtime fields because they only apply to leaf
903+
// fields
904+
private static Mapper getMapper(final ParseContext context,
905+
ObjectMapper objectMapper,
906+
String fieldName,
907+
String[] subfields) {
902908
String fieldPath = context.path().pathAsText(fieldName);
903909
// Check if mapper is a metadata mapper first
904910
Mapper mapper = context.docMapper().mapping().getMetadataMapper(fieldPath);
@@ -919,15 +925,27 @@ private static Mapper getMapper(final ParseContext context, ObjectMapper objectM
919925
}
920926
}
921927
String leafName = subfields[subfields.length - 1];
922-
mapper = objectMapper.getMapper(leafName);
928+
return objectMapper.getMapper(leafName);
929+
}
930+
931+
// looks up a child mapper, taking into account field names that expand to objects
932+
// if no mapper is found, checks to see if a runtime field with the specified
933+
// field name exists and if so returns a no-op mapper to prevent indexing
934+
private static Mapper getLeafMapper(final ParseContext context,
935+
ObjectMapper objectMapper,
936+
String fieldName,
937+
String[] subfields) {
938+
Mapper mapper = getMapper(context, objectMapper, fieldName, subfields);
923939
if (mapper != null) {
924940
return mapper;
925941
}
926-
//concrete fields take the precedence over runtime fields when parsing documents, though when a field is defined as runtime field
927-
//only, and not under properties, it is ignored when it is sent as part of _source
942+
// concrete fields take precedence over runtime fields when parsing documents
943+
// if a leaf field is not mapped, and is defined as a runtime field, then we
944+
// don't create a dynamic mapping for it and don't index it.
945+
String fieldPath = context.path().pathAsText(fieldName);
928946
RuntimeFieldType runtimeFieldType = context.docMapper().mapping().root.getRuntimeFieldType(fieldPath);
929947
if (runtimeFieldType != null) {
930-
return new NoOpFieldMapper(leafName, runtimeFieldType);
948+
return new NoOpFieldMapper(subfields[subfields.length - 1], runtimeFieldType);
931949
}
932950
return null;
933951
}

‎server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,123 @@ public void testParseWithShadowedMultiField() throws Exception {
138138
assertNotNull(doc.rootDoc().getField("field.keyword"));
139139
}
140140

141+
public void testRuntimeFieldAndArrayChildren() throws IOException {
142+
DocumentMapper mapper = createDocumentMapper(topMapping(b -> {
143+
b.field("dynamic", "true");
144+
b.startObject("runtime");
145+
{
146+
b.startObject("object").field("type", "test").endObject();
147+
}
148+
b.endObject();
149+
}));
150+
151+
{
152+
ParsedDocument doc = mapper.parse(source(b -> {
153+
b.startObject("object");
154+
b.array("array", 1, 2, 3);
155+
b.field("foo", "bar");
156+
b.endObject();
157+
}));
158+
assertNotNull(doc.rootDoc().getField("object.foo"));
159+
assertNotNull(doc.rootDoc().getField("object.array"));
160+
}
161+
162+
{
163+
ParsedDocument doc = mapper.parse(source(b -> {
164+
b.startArray("object");
165+
{
166+
b.startObject().array("array", 1, 2, 3).endObject();
167+
b.startObject().field("foo", "bar").endObject();
168+
}
169+
b.endArray();
170+
}));
171+
assertNotNull(doc.rootDoc().getField("object.foo"));
172+
assertNotNull(doc.rootDoc().getField("object.array"));
173+
}
174+
}
175+
176+
public void testRuntimeFieldDoesNotShadowObjectChildren() throws IOException {
177+
DocumentMapper mapper = createDocumentMapper(topMapping(b -> {
178+
b.field("dynamic", "true");
179+
b.startObject("runtime");
180+
{
181+
b.startObject("location").field("type", "test").endObject();
182+
b.startObject("country").field("type", "test").endObject();
183+
}
184+
b.endObject();
185+
b.startObject("properties");
186+
{
187+
b.startObject("timestamp").field("type", "date").endObject();
188+
b.startObject("concrete").field("type", "keyword").endObject();
189+
}
190+
b.endObject();
191+
}));
192+
193+
{
194+
ParsedDocument doc = mapper.parse(source(b -> {
195+
b.field("timestamp", "1998-04-30T14:30:17-05:00");
196+
b.startObject("location");
197+
{
198+
b.field("lat", 13.5);
199+
b.field("lon", 34.89);
200+
}
201+
b.endObject();
202+
b.field("country", "de");
203+
b.field("concrete", "foo");
204+
}));
205+
206+
assertNotNull(doc.rootDoc().getField("timestamp"));
207+
assertNotNull(doc.rootDoc().getField("_source"));
208+
assertNotNull(doc.rootDoc().getField("location.lat"));
209+
assertNotNull(doc.rootDoc().getField("location.lon"));
210+
assertNotNull(doc.rootDoc().getField("concrete"));
211+
assertNull(doc.rootDoc().getField("country"));
212+
}
213+
214+
{
215+
ParsedDocument doc = mapper.parse(source(b -> {
216+
b.field("timestamp", "1998-04-30T14:30:17-05:00");
217+
b.startArray("location");
218+
{
219+
b.startObject().field("lat", 13.5).field("lon", 34.89).endObject();
220+
b.startObject().field("lat", 14.5).field("lon", 89.33).endObject();
221+
}
222+
b.endArray();
223+
b.field("country", "de");
224+
b.field("concrete", "foo");
225+
}));
226+
227+
assertNotNull(doc.rootDoc().getField("timestamp"));
228+
assertNotNull(doc.rootDoc().getField("_source"));
229+
assertThat(doc.rootDoc().getFields("location.lat").length, equalTo(4));
230+
assertThat(doc.rootDoc().getFields("location.lon").length, equalTo(4));
231+
assertNotNull(doc.rootDoc().getField("concrete"));
232+
assertNull(doc.rootDoc().getField("country"));
233+
}
234+
235+
{
236+
ParsedDocument doc = mapper.parse(source(b -> {
237+
b.field("timestamp", "1998-04-30T14:30:17-05:00");
238+
b.startObject("location");
239+
{
240+
b.array("lat", 13.5, 14.5);
241+
b.array("lon", 34.89, 89.33);
242+
}
243+
b.endObject();
244+
b.field("country", "de");
245+
b.field("concrete", "foo");
246+
}));
247+
248+
assertNotNull(doc.rootDoc().getField("timestamp"));
249+
assertNotNull(doc.rootDoc().getField("_source"));
250+
assertThat(doc.rootDoc().getFields("location.lat").length, equalTo(4));
251+
assertThat(doc.rootDoc().getFields("location.lon").length, equalTo(4));
252+
assertNotNull(doc.rootDoc().getField("concrete"));
253+
assertNull(doc.rootDoc().getField("country"));
254+
}
255+
256+
}
257+
141258
public void testFieldDisabled() throws Exception {
142259
DocumentMapper mapper = createDocumentMapper(mapping(b -> {
143260
b.startObject("foo").field("enabled", false).endObject();

0 commit comments

Comments
 (0)