Fix handling of null values in geo_point (#65307)

A bug was introduced in 7.10 that causes explicit `null` values to be indexed in the _field_names field. This change fixes this bug for newly ingested data but `null` values ingested with 7.10 will continue to match `exists` query so a reindex is required. Fixes #65306
elastic · Nov 24, 2020 · 88993e7 · 88993e7
1 parent 59ceba8
commit 88993e7
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 5 deletions.
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java
@@ -235,9 +235,8 @@ public List<P> parse(XContentParser parser) throws IOException, ParseException {
                 return points;
             } else if (parser.currentToken() == XContentParser.Token.VALUE_NULL) {
                 if (nullValue == null) {
-                    return Collections.emptyList();
-                }
-                else {
+                    return null;
+                } else {
                     return Collections.singletonList(nullValue);
                 }
             } else {

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java
@@ -232,14 +232,36 @@ public void testMultiField() throws Exception {
     }
 
     public void testNullValue() throws Exception {
-        DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "geo_point").field("null_value", "1,2")));
+        DocumentMapper mapper = createDocumentMapper(
+            fieldMapping(b -> b.field("type", "geo_point"))
+        );
         Mapper fieldMapper = mapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoPointFieldMapper.class));
 
+        ParsedDocument doc = mapper.parse(source(b -> b.nullField("field")));
+        assertThat(doc.rootDoc().getField("field"), nullValue());
+        assertThat(doc.rootDoc().getFields(FieldNamesFieldMapper.NAME).length, equalTo(0));
+
+        mapper = createDocumentMapper(
+            fieldMapping(b -> b.field("type", "geo_point").field("doc_values", false))
+        );
+        fieldMapper = mapper.mappers().getMapper("field");
+        assertThat(fieldMapper, instanceOf(GeoPointFieldMapper.class));
+
+        doc = mapper.parse(source(b -> b.nullField("field")));
+        assertThat(doc.rootDoc().getField("field"), nullValue());
+        assertThat(doc.rootDoc().getFields(FieldNamesFieldMapper.NAME).length, equalTo(0));
+
+        mapper = createDocumentMapper(
+            fieldMapping(b -> b.field("type", "geo_point").field("null_value", "1,2"))
+        );
+        fieldMapper = mapper.mappers().getMapper("field");
+        assertThat(fieldMapper, instanceOf(GeoPointFieldMapper.class));
+
         AbstractPointGeometryFieldMapper.ParsedPoint nullValue = ((GeoPointFieldMapper) fieldMapper).nullValue;
         assertThat(nullValue, equalTo(new GeoPoint(1, 2)));
 
-        ParsedDocument doc = mapper.parse(source(b -> b.nullField("field")));
+        doc = mapper.parse(source(b -> b.nullField("field")));
         assertThat(doc.rootDoc().getField("field"), notNullValue());
         BytesRef defaultValue = doc.rootDoc().getBinaryValue("field");