Return null_value when the source contains a 'null' for the field. (#…

…58623)

This PR adds a version of `XContentMapValues.extractValue` that accepts a
default value to return in place of 'null'. It then uses this method when
looking up source values to return the configured `null_value` instead of
'null' when retrieving fields.

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java

@@ -356,6 +356,11 @@ protected ScaledFloatFieldMapper clone() {
  return (ScaledFloatFieldMapper) super.clone();
  }
 
+ @Override
+ protected Double nullValue() {
+ return nullValue;
+ }
+
  @Override
  protected void parseCreateField(ParseContext context) throws IOException {
 
@@ -480,7 +485,16 @@ protected Double parseSourceValue(Object value, String format) {
  throw new IllegalArgumentException("Field [" + name() + "] of type [" + typeName() + "] doesn't support formats.");
  }
 
- double doubleValue = objectToDouble(value);
+ double doubleValue;
+ if (value.equals("")) {
+ if (nullValue == null) {
+ return null;
+ }
+ doubleValue = nullValue;
+ } else {
+ doubleValue = objectToDouble(value);
+ }
+
  double scalingFactor = fieldType().getScalingFactor();
  return Math.round(doubleValue * scalingFactor) / scalingFactor;
  }

modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapperTests.java

@@ -32,6 +32,7 @@
 import org.elasticsearch.index.IndexService;
 import org.elasticsearch.index.mapper.MapperService.MergeReason;
 import org.elasticsearch.plugins.Plugin;
+import org.elasticsearch.search.lookup.SourceLookup;
 import org.elasticsearch.test.InternalSettingsPlugin;
 import org.junit.Before;
 
@@ -405,11 +406,22 @@ public void testMeta() throws Exception {
  public void testParseSourceValue() {
  Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build();
  Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath());
+
  ScaledFloatFieldMapper mapper = new ScaledFloatFieldMapper.Builder("field")
  .scalingFactor(100)
  .build(context);
-
  assertEquals(3.14, mapper.parseSourceValue(3.1415926, null), 0.00001);
  assertEquals(3.14, mapper.parseSourceValue("3.1415", null), 0.00001);
+ assertNull(mapper.parseSourceValue("", null));
+
+ ScaledFloatFieldMapper nullValueMapper = new ScaledFloatFieldMapper.Builder("field")
+ .scalingFactor(100)
+ .nullValue(2.71)
+ .build(context);
+ assertEquals(2.71, nullValueMapper.parseSourceValue("", null), 0.00001);
+
+ SourceLookup sourceLookup = new SourceLookup();
+ sourceLookup.setSource(Collections.singletonMap("field", null));
+ assertEquals(List.of(2.71), nullValueMapper.lookupValues(sourceLookup, null));
  }
 }

plugins/analysis-icu/src/main/java/org/elasticsearch/index/mapper/ICUCollationKeywordFieldMapper.java

@@ -577,6 +577,11 @@ protected String contentType() {
  return CONTENT_TYPE;
  }
 
+ @Override
+ protected String nullValue() {
+ return nullValue;
+ }
+
  @Override
  protected void mergeOptions(FieldMapper other, List<String> conflicts) {
  ICUCollationKeywordFieldMapper icuMergeWith = (ICUCollationKeywordFieldMapper) other;

plugins/analysis-icu/src/test/java/org/elasticsearch/index/mapper/ICUCollationKeywordFieldMapperTests.java

@@ -38,12 +38,15 @@
 import org.elasticsearch.index.mapper.MapperService.MergeReason;
 import org.elasticsearch.plugin.analysis.icu.AnalysisICUPlugin;
 import org.elasticsearch.plugins.Plugin;
+import org.elasticsearch.search.lookup.SourceLookup;
 import org.elasticsearch.test.InternalSettingsPlugin;
 import org.junit.Before;
 
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
 import java.util.Set;
 
 import static org.hamcrest.Matchers.containsString;
@@ -489,9 +492,8 @@ public void testUpdateIgnoreAbove() throws IOException {
  public void testParseSourceValue() {
  Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build();
  Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath());
- ICUCollationKeywordFieldMapper mapper = new ICUCollationKeywordFieldMapper.Builder("field").build(context);
 
- assertEquals("value", mapper.parseSourceValue("value", null));
+ ICUCollationKeywordFieldMapper mapper = new ICUCollationKeywordFieldMapper.Builder("field").build(context);
  assertEquals("42", mapper.parseSourceValue(42L, null));
  assertEquals("true", mapper.parseSourceValue(true, null));
 
@@ -501,5 +503,12 @@ public void testParseSourceValue() {
  assertNull(ignoreAboveMapper.parseSourceValue("value", null));
  assertEquals("42", ignoreAboveMapper.parseSourceValue(42L, null));
  assertEquals("true", ignoreAboveMapper.parseSourceValue(true, null));
+
+ ICUCollationKeywordFieldMapper nullValueMapper = new ICUCollationKeywordFieldMapper.Builder("field")
+ .nullValue("NULL")
+ .build(context);
+ SourceLookup sourceLookup = new SourceLookup();
+ sourceLookup.setSource(Collections.singletonMap("field", null));
+ assertEquals(List.of("NULL"), nullValueMapper.lookupValues(sourceLookup, null));
  }
 }

server/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java

@@ -97,6 +97,16 @@ private static void extractRawValues(List values, List<Object> part, String[] pa
  }
  }
 
+ /**
+ * For the provided path, return its value in the xContent map.
+ *
+ * Note that in contrast with {@link XContentMapValues#extractRawValues}, array and object values
+ * can be returned.
+ *
+ * @param path the value's path in the map.
+ *
+ * @return the value associated with the path in the map or 'null' if the path does not exist.
+ */
  public static Object extractValue(String path, Map<?, ?> map) {
  return extractValue(map, path.split("\\."));
  }
@@ -105,19 +115,51 @@ public static Object extractValue(Map<?, ?> map, String... pathElements) {
  if (pathElements.length == 0) {
  return null;
  }
- return extractValue(pathElements, 0, map);
+ return XContentMapValues.extractValue(pathElements, 0, map, null);
  }
 
- @SuppressWarnings({"unchecked"})
- private static Object extractValue(String[] pathElements, int index, Object currentValue) {
- if (index == pathElements.length) {
- return currentValue;
- }
- if (currentValue == null) {
+ /**
+ * For the provided path, return its value in the xContent map.
+ *
+ * Note that in contrast with {@link XContentMapValues#extractRawValues}, array and object values
+ * can be returned.
+ *
+ * @param path the value's path in the map.
+ * @param nullValue a value to return if the path exists, but the value is 'null'. This helps
+ * in distinguishing between a path that doesn't exist vs. a value of 'null'.
+ *
+ * @return the value associated with the path in the map or 'null' if the path does not exist.
+ */
+ public static Object extractValue(String path, Map<?, ?> map, Object nullValue) {
+ String[] pathElements = path.split("\\.");
+ if (pathElements.length == 0) {
  return null;
  }
+ return extractValue(pathElements, 0, map, nullValue);
+ }
+
+ private static Object extractValue(String[] pathElements,
+ int index,
+ Object currentValue,
+ Object nullValue) {
+ if (currentValue instanceof List) {
+ List<?> valueList = (List<?>) currentValue;
+ List<Object> newList = new ArrayList<>(valueList.size());
+ for (Object o : valueList) {
+ Object listValue = extractValue(pathElements, index, o, nullValue);
+ if (listValue != null) {
+ newList.add(listValue);
+ }
+ }
+ return newList;
+ }
+
+ if (index == pathElements.length) {
+ return currentValue != null ? currentValue : nullValue;
+ }
+
  if (currentValue instanceof Map) {
- Map map = (Map) currentValue;
+ Map<?, ?> map = (Map<?, ?>) currentValue;
  String key = pathElements[index];
  Object mapValue = map.get(key);
  int nextIndex = index + 1;
@@ -126,18 +168,12 @@ private static Object extractValue(String[] pathElements, int index, Object curr
  mapValue = map.get(key);
  nextIndex++;
  }
- return extractValue(pathElements, nextIndex, mapValue);
- }
- if (currentValue instanceof List) {
- List valueList = (List) currentValue;
- List newList = new ArrayList(valueList.size());
- for (Object o : valueList) {
- Object listValue = extractValue(pathElements, index, o);
- if (listValue != null) {
- newList.add(listValue);
- }
+
+ if (map.containsKey(key) == false) {
+ return null;
  }
- return newList;
+
+ return extractValue(pathElements, nextIndex, mapValue, nullValue);
  }
  return null;
  }

server/src/main/java/org/elasticsearch/index/mapper/BooleanFieldMapper.java

@@ -273,4 +273,8 @@ protected String contentType() {
  return CONTENT_TYPE;
  }
 
+ @Override
+ protected Object nullValue() {
+ return nullValue;
+ }
 }

server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java

@@ -497,6 +497,11 @@ protected DateFieldMapper clone() {
  return (DateFieldMapper) super.clone();
  }
 
+ @Override
+ protected String nullValue() {
+ return nullValueAsString;
+ }
+
  @Override
  protected void parseCreateField(ParseContext context) throws IOException {
  String dateAsString;
@@ -547,8 +552,8 @@ protected void parseCreateField(ParseContext context) throws IOException {
  public String parseSourceValue(Object value, String format) {
  String date = value.toString();
  long timestamp = fieldType().parse(date);
- ZonedDateTime dateTime = fieldType().resolution().toInstant(timestamp).atZone(ZoneOffset.UTC);
 
+ ZonedDateTime dateTime = fieldType().resolution().toInstant(timestamp).atZone(ZoneOffset.UTC);
  DateFormatter dateTimeFormatter = fieldType().dateTimeFormatter();
  if (format != null) {
  dateTimeFormatter = DateFormatter.forPattern(format).withLocale(dateTimeFormatter.locale());

server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java

@@ -224,6 +224,13 @@ public CopyTo copyTo() {
  return copyTo;
  }
 
+ /**
+ * A value to use in place of a {@code null} value in the document source.
+ */
+ protected Object nullValue() {
+ return null;
+ }
+
  /**
  * Whether this mapper can handle an array value during document parsing. If true,
  * when an array is encountered during parsing, the document parser will pass the
@@ -286,7 +293,7 @@ public void parse(ParseContext context) throws IOException {
  * @return a list a standardized field values.
  */
  public List<?> lookupValues(SourceLookup lookup, @Nullable String format) {
- Object sourceValue = lookup.extractValue(name());
+ Object sourceValue = lookup.extractValue(name(), nullValue());
  if (sourceValue == null) {
  return List.of();
  }
@@ -338,6 +345,7 @@ protected FieldMapper clone() {
  }
  }
 
+
  @Override
  public FieldMapper merge(Mapper mergeWith) {
  FieldMapper merged = clone();

server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java

@@ -348,6 +348,11 @@ protected String contentType() {
  return fieldType().typeName();
  }
 
+ @Override
+ protected Object nullValue() {
+ return nullValue;
+ }
+
  @Override
  protected IpFieldMapper clone() {
  return (IpFieldMapper) super.clone();
@@ -406,7 +411,12 @@ protected String parseSourceValue(Object value, String format) {
  throw new IllegalArgumentException("Field [" + name() + "] of type [" + typeName() + "] doesn't support formats.");
  }
 
- InetAddress address = InetAddresses.forString(value.toString());
+ InetAddress address;
+ if (value instanceof InetAddress) {
+ address = (InetAddress) value;
+ } else {
+ address = InetAddresses.forString(value.toString());
+ }
  return InetAddresses.toAddrString(address);
  }
 

server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java

@@ -416,6 +416,11 @@ protected String contentType() {
  return CONTENT_TYPE;
  }
 
+ @Override
+ protected String nullValue() {
+ return nullValue;
+ }
+
  @Override
  protected void mergeOptions(FieldMapper other, List<String> conflicts) {
  KeywordFieldMapper k = (KeywordFieldMapper) other;

server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java

@@ -1036,6 +1036,11 @@ protected NumberFieldMapper clone() {
  return (NumberFieldMapper) super.clone();
  }
 
+ @Override
+ protected Number nullValue() {
+ return nullValue;
+ }
+
  @Override
  protected void parseCreateField(ParseContext context) throws IOException {
  XContentParser parser = context.parser();
@@ -1090,6 +1095,11 @@ protected Number parseSourceValue(Object value, String format) {
  if (format != null) {
  throw new IllegalArgumentException("Field [" + name() + "] of type [" + typeName() + "] doesn't support formats.");
  }
+
+ if (value.equals("")) {
+ return nullValue;
+ }
+
  return fieldType().type.parse(value, coerce.value());
  }
 

server/src/main/java/org/elasticsearch/search/lookup/SourceLookup.java

@@ -21,6 +21,7 @@
 import org.apache.lucene.index.LeafReader;
 import org.apache.lucene.index.LeafReaderContext;
 import org.elasticsearch.ElasticsearchParseException;
+import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.xcontent.XContentHelper;
@@ -133,11 +134,19 @@ public List<Object> extractRawValues(String path) {
  }
 
  /**
- * For the provided path, return its value in the source. Note that in contrast with
- * {@link SourceLookup#extractRawValues}, array and object values can be returned.
+ * For the provided path, return its value in the source.
+ *
+ * Note that in contrast with {@link SourceLookup#extractRawValues}, array and object values
+ * can be returned.
+ *
+ * @param path the value's path in the source.
+ * @param nullValue a value to return if the path exists, but the value is 'null'. This helps
+ * in distinguishing between a path that doesn't exist vs. a value of 'null'.
+ *
+ * @return the value associated with the path in the source or 'null' if the path does not exist.
  */
- public Object extractValue(String path) {
- return XContentMapValues.extractValue(path, loadSourceIfNeeded());
+ public Object extractValue(String path, @Nullable Object nullValue) {
+ return XContentMapValues.extractValue(path, loadSourceIfNeeded(), nullValue);
  }
 
  public Object filter(FetchSourceContext context) {