Skip to content

Commit

Permalink
Add null-field checks to shape field mappers (elastic#71999) (elastic…
Browse files Browse the repository at this point in the history
…#72025)

elastic#71696 introduced a regression to the various shape field mappers,
where they would no longer handle null values. This commit fixes
that regression and adds a testNullValues method to MapperTestCase
to ensure that all field mappers correctly handle nulls.

Fixes elastic#71874
  • Loading branch information
romseygeek committed Apr 22, 2021
1 parent 06eceff commit f9cd001
Show file tree
Hide file tree
Showing 9 changed files with 47 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -145,4 +145,9 @@ protected Object generateRandomInputValue(MappedFieldType ft) {
assumeFalse("Test implemented in a follow up", true);
return null;
}

@Override
protected boolean allowsNullValues() {
return false; // TODO should this allow null values?
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,9 @@ protected void checkIncomingMergeType(FieldMapper mergeWith) {

@Override
protected void index(ParseContext context, Geometry geometry) throws IOException {
if (geometry == null) {
return;
}
context.doc().addAll(indexer.indexShape(geometry));
createFieldNamesField(context);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,9 @@ String strategy() {

@Override
protected void index(ParseContext context, ShapeBuilder<?, ?, ?> shapeBuilder) throws IOException {
if (shapeBuilder == null) {
return;
}
Shape shape = shapeBuilder.buildS4J();
if (fieldType().pointsOnly()) {
// index configured for pointsOnly
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -684,4 +684,19 @@ private BiFunction<MappedFieldType, Supplier<SearchLookup>, IndexFieldData<?>> f
.fielddataBuilder("test", lookupSource)
.build(new IndexFieldDataCache.None(), new NoneCircuitBreakerService());
}

public final void testNullInput() throws Exception {
DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping));
if (allowsNullValues()) {
ParsedDocument doc = mapper.parse(source(b -> b.nullField("field")));
assertThat(doc.docs().get(0).getFields("field").length, equalTo(0));
assertThat(doc.docs().get(0).getFields("_field_names").length, equalTo(0));
} else {
expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> b.nullField("field"))));
}
}

protected boolean allowsNullValues() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -177,4 +177,9 @@ protected String generateRandomInputValue(MappedFieldType ft) {
protected void randomFetchTestFieldConfig(XContentBuilder b) throws IOException {
b.field("type", "constant_keyword").field("value", randomAlphaOfLengthBetween(1, 10));
}

@Override
protected boolean allowsNullValues() {
return false; // null is an error for constant keyword
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,9 @@ public GeoShapeWithDocValuesFieldMapper(String simpleName, MappedFieldType mappe

@Override
protected void index(ParseContext context, Geometry geometry) throws IOException {
if (geometry == null) {
return;
}
List<IndexableField> fields = indexer.indexShape(geometry);
if (fieldType().isSearchable()) {
context.doc().addAll(fields);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ public ShapeFieldMapper(String simpleName, MappedFieldType mappedFieldType,

@Override
protected void index(ParseContext context, Geometry geometry) throws IOException {
if (geometry == null) {
return;
}
context.doc().addAll(indexer.indexShape(geometry));
createFieldNamesField(context);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,4 +166,9 @@ protected Object generateRandomInputValue(MappedFieldType ft) {
assumeFalse("Test implemented in a follow up", true);
return null;
}

@Override
protected boolean allowsNullValues() {
return false; // TODO should this allow null values?
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -246,4 +246,9 @@ protected Object generateRandomInputValue(MappedFieldType ft) {
assumeFalse("doesn't support docvalues_fetcher", true);
return null;
}

@Override
protected boolean allowsNullValues() {
return false;
}
}

0 comments on commit f9cd001

Please sign in to comment.