From b416205f7b79ea428b44826e216540938d0ac605 Mon Sep 17 00:00:00 2001 From: Michael Froh Date: Thu, 22 Aug 2024 17:42:01 -0700 Subject: [PATCH 1/3] Do not throw exception when flat_object field is explicitly null 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 --- .../test/index/100_partial_flat_object.yml | 13 +++++++++++-- .../index/mapper/FlatObjectFieldMapper.java | 6 +----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/index/100_partial_flat_object.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/index/100_partial_flat_object.yml index 91e4127da9c32..e1bc86f1c9f3d 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/index/100_partial_flat_object.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/index/100_partial_flat_object.yml @@ -88,7 +88,16 @@ setup: } ] } } - + - do: + index: + index: test_partial_flat_object + id: 4 + body: { + "issue": { + "number": 999, + "labels": null + } + } - do: indices.refresh: index: test_partial_flat_object @@ -135,7 +144,7 @@ teardown: } } - - length: { hits.hits: 3 } + - length: { hits.hits: 4 } # Match Query with exact dot path. - do: diff --git a/server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java index b82fa3999612a..bf8f83e1b95df 100644 --- a/server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java @@ -568,11 +568,7 @@ protected void parseCreateField(ParseContext context) throws IOException { if (context.externalValueSet()) { String value = context.externalValue().toString(); parseValueAddFields(context, value, fieldType().name()); - } else if (context.parser().currentToken() == XContentParser.Token.VALUE_NULL) { - context.parser().nextToken(); // This triggers an exception in DocumentParser. - // We could remove the above nextToken() call to skip the null value, but the existing - // behavior (since 2.7) throws the exception. - } else { + } else if (context.parser().currentToken() != XContentParser.Token.VALUE_NULL) { JsonToStringXContentParser jsonToStringParser = new JsonToStringXContentParser( NamedXContentRegistry.EMPTY, DeprecationHandler.IGNORE_DEPRECATIONS, From 0a952c336a156a52f89c744f639c0a298efed66a Mon Sep 17 00:00:00 2001 From: Michael Froh Date: Thu, 22 Aug 2024 19:28:35 -0700 Subject: [PATCH 2/3] Fix unit test Signed-off-by: Michael Froh --- .../index/mapper/FlatObjectFieldMapperTests.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/test/java/org/opensearch/index/mapper/FlatObjectFieldMapperTests.java b/server/src/test/java/org/opensearch/index/mapper/FlatObjectFieldMapperTests.java index 637072c8886c1..5b5ca378ee7ff 100644 --- a/server/src/test/java/org/opensearch/index/mapper/FlatObjectFieldMapperTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/FlatObjectFieldMapperTests.java @@ -25,7 +25,6 @@ import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.core.IsEqual.equalTo; -import static org.hamcrest.core.StringContains.containsString; public class FlatObjectFieldMapperTests extends MapperTestCase { private static final String FIELD_TYPE = "flat_object"; @@ -133,9 +132,10 @@ public void testDefaults() throws Exception { public void testNullValue() throws IOException { DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping)); - MapperParsingException e = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> b.nullField("field")))); - assertThat(e.getMessage(), containsString("object mapping for [_doc] tried to parse field [field] as object")); - + ParsedDocument parsedDocument = mapper.parse(source(b -> b.nullField("field"))); + assertEquals(1, parsedDocument.docs().size()); + IndexableField[] fields = parsedDocument.rootDoc().getFields("field"); + assertEquals(0, fields.length); } @Override From 3d6e90802c13d3cb5dc74e9c47d0cc0e22fa9e09 Mon Sep 17 00:00:00 2001 From: Michael Froh Date: Mon, 26 Aug 2024 10:15:34 -0700 Subject: [PATCH 3/3] Add changelog entry Signed-off-by: Michael Froh --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd02af4f625b9..df212b05f9001 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,6 +65,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fixed array field name omission in flat_object function for nested JSON ([#13620](https://github.com/opensearch-project/OpenSearch/pull/13620)) - Fix range aggregation optimization ignoring top level queries ([#15194](https://github.com/opensearch-project/OpenSearch/pull/15194)) - Fix incorrect parameter names in MinHash token filter configuration handling ([#15233](https://github.com/opensearch-project/OpenSearch/pull/15233)) +- Fix indexing error when flat_object field is explicitly null ([#15375](https://github.com/opensearch-project/OpenSearch/pull/15375)) ### Security