Skip to content

Commit

Permalink
Return null_value when the source contains a 'null' for the field. (#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
jtibshirani committed Jul 14, 2020
1 parent b704f0e commit 50150f2
Show file tree
Hide file tree
Showing 22 changed files with 301 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,11 @@ protected ScaledFloatFieldMapper clone() {
return (ScaledFloatFieldMapper) super.clone();
}

@Override
protected Double nullValue() {
return nullValue;
}

@Override
protected void parseCreateField(ParseContext context) throws IOException {

Expand Down Expand Up @@ -479,7 +484,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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));

Expand All @@ -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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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("\\."));
}
Expand All @@ -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;
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,4 +281,8 @@ protected String contentType() {
return CONTENT_TYPE;
}

@Override
protected Object nullValue() {
return nullValue;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,11 @@ protected DateFieldMapper clone() {
return (DateFieldMapper) super.clone();
}

@Override
protected String nullValue() {
return nullValueAsString;
}

@Override
protected void parseCreateField(ParseContext context) throws IOException {
String dateAsString;
Expand Down Expand Up @@ -588,8 +593,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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -338,6 +345,7 @@ protected FieldMapper clone() {
}
}


@Override
public FieldMapper merge(Mapper mergeWith) {
FieldMapper merged = clone();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,11 @@ protected String contentType() {
return fieldType().typeName();
}

@Override
protected Object nullValue() {
return nullValue;
}

@Override
protected IpFieldMapper clone() {
return (IpFieldMapper) super.clone();
Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit 50150f2

Please sign in to comment.