Skip to content

Commit 46a269e

Browse files
authored
Do not throw exception when flat_object field is explicitly null (opensearch-project#15375)
It is valid for a flat_object field to have an explicit value of null. (It's functionally the same as not specifying the field at all.) Prior to this fix, though, we would erroneously advance the context parser to the next token, violating the contract with DocumentParser (which says that a call to parseCreateField with a null value should complete with the parser still pointing at the null value -- it is DocumentParser's responsibility to advance). Signed-off-by: Michael Froh <froh@amazon.com> * Fix unit test Signed-off-by: Michael Froh <froh@amazon.com> * Add changelog entry Signed-off-by: Michael Froh <froh@amazon.com> --------- Signed-off-by: Michael Froh <froh@amazon.com>
1 parent f247d8f commit 46a269e

File tree

4 files changed

+17
-11
lines changed

4 files changed

+17
-11
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
6868
- Fixed array field name omission in flat_object function for nested JSON ([#13620](https://github.com/opensearch-project/OpenSearch/pull/13620))
6969
- Fix range aggregation optimization ignoring top level queries ([#15194](https://github.com/opensearch-project/OpenSearch/pull/15194))
7070
- Fix incorrect parameter names in MinHash token filter configuration handling ([#15233](https://github.com/opensearch-project/OpenSearch/pull/15233))
71+
- Fix indexing error when flat_object field is explicitly null ([#15375](https://github.com/opensearch-project/OpenSearch/pull/15375))
7172
- Fix split response processor not included in allowlist ([#15393](https://github.com/opensearch-project/OpenSearch/pull/15393))
7273
- Fix unchecked cast in dynamic action map getter ([#15394](https://github.com/opensearch-project/OpenSearch/pull/15394))
7374

rest-api-spec/src/main/resources/rest-api-spec/test/index/100_partial_flat_object.yml

+11-2
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,16 @@ setup:
8888
} ]
8989
}
9090
}
91-
91+
- do:
92+
index:
93+
index: test_partial_flat_object
94+
id: 4
95+
body: {
96+
"issue": {
97+
"number": 999,
98+
"labels": null
99+
}
100+
}
92101
- do:
93102
indices.refresh:
94103
index: test_partial_flat_object
@@ -135,7 +144,7 @@ teardown:
135144
}
136145
}
137146

138-
- length: { hits.hits: 3 }
147+
- length: { hits.hits: 4 }
139148

140149
# Match Query with exact dot path.
141150
- do:

server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java

+1-5
Original file line numberDiff line numberDiff line change
@@ -568,11 +568,7 @@ protected void parseCreateField(ParseContext context) throws IOException {
568568
if (context.externalValueSet()) {
569569
String value = context.externalValue().toString();
570570
parseValueAddFields(context, value, fieldType().name());
571-
} else if (context.parser().currentToken() == XContentParser.Token.VALUE_NULL) {
572-
context.parser().nextToken(); // This triggers an exception in DocumentParser.
573-
// We could remove the above nextToken() call to skip the null value, but the existing
574-
// behavior (since 2.7) throws the exception.
575-
} else {
571+
} else if (context.parser().currentToken() != XContentParser.Token.VALUE_NULL) {
576572
JsonToStringXContentParser jsonToStringParser = new JsonToStringXContentParser(
577573
NamedXContentRegistry.EMPTY,
578574
DeprecationHandler.IGNORE_DEPRECATIONS,

server/src/test/java/org/opensearch/index/mapper/FlatObjectFieldMapperTests.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525

2626
import static org.hamcrest.Matchers.instanceOf;
2727
import static org.hamcrest.core.IsEqual.equalTo;
28-
import static org.hamcrest.core.StringContains.containsString;
2928

3029
public class FlatObjectFieldMapperTests extends MapperTestCase {
3130
private static final String FIELD_TYPE = "flat_object";
@@ -133,9 +132,10 @@ public void testDefaults() throws Exception {
133132

134133
public void testNullValue() throws IOException {
135134
DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping));
136-
MapperParsingException e = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> b.nullField("field"))));
137-
assertThat(e.getMessage(), containsString("object mapping for [_doc] tried to parse field [field] as object"));
138-
135+
ParsedDocument parsedDocument = mapper.parse(source(b -> b.nullField("field")));
136+
assertEquals(1, parsedDocument.docs().size());
137+
IndexableField[] fields = parsedDocument.rootDoc().getFields("field");
138+
assertEquals(0, fields.length);
139139
}
140140

141141
@Override

0 commit comments

Comments
 (0)