Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always support ignore_malformed in the same way #90565

Merged
merged 4 commits into from
Oct 3, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,6 @@ public void testValidateDefaultIgnoreMalformed() throws Exception {
}));
assertThat(mapperService, notNullValue());
assertThat(mapperService.documentMapper().mappers().getMapper("@timestamp"), notNullValue());
assertThat(((DateFieldMapper) mapperService.documentMapper().mappers().getMapper("@timestamp")).getIgnoreMalformed(), is(false));
assertThat(((DateFieldMapper) mapperService.documentMapper().mappers().getMapper("@timestamp")).ignoreMalformed(), is(false));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,6 @@ protected void registerParameters(ParameterChecker checker) throws IOException {
LegacyGeoShapeFieldMapper gsfm = (LegacyGeoShapeFieldMapper) m;
assertEquals(Orientation.RIGHT, gsfm.orientation());
});
checker.registerUpdateCheck(b -> b.field("ignore_malformed", true), m -> {
LegacyGeoShapeFieldMapper gpfm = (LegacyGeoShapeFieldMapper) m;
assertTrue(gpfm.ignoreMalformed());
});
checker.registerUpdateCheck(b -> b.field("ignore_z_value", false), m -> {
LegacyGeoShapeFieldMapper gpfm = (LegacyGeoShapeFieldMapper) m;
assertFalse(gpfm.ignoreZValue());
Expand Down Expand Up @@ -232,27 +228,14 @@ public void testIgnoreZValue() throws IOException {
assertFieldWarnings("strategy", "tree");
}

/**
* Test that ignore_malformed parameter correctly parses
*/
public void testIgnoreMalformedParsing() throws IOException {
DocumentMapper mapper = createDocumentMapper(
fieldMapping(b -> b.field("type", "geo_shape").field("tree", "quadtree").field("ignore_malformed", true))
);
Mapper fieldMapper = mapper.mappers().getMapper("field");
assertThat(fieldMapper, instanceOf(LegacyGeoShapeFieldMapper.class));
boolean ignoreMalformed = ((LegacyGeoShapeFieldMapper) fieldMapper).ignoreMalformed();
assertThat(ignoreMalformed, equalTo(true));
@Override
protected boolean supportsIgnoreMalformed() {
return true;
}

// explicit false ignore_malformed test
mapper = createDocumentMapper(
fieldMapping(b -> b.field("type", "geo_shape").field("tree", "quadtree").field("ignore_malformed", false))
);
fieldMapper = mapper.mappers().getMapper("field");
assertThat(fieldMapper, instanceOf(LegacyGeoShapeFieldMapper.class));
ignoreMalformed = ((LegacyGeoShapeFieldMapper) fieldMapper).ignoreMalformed();
assertThat(ignoreMalformed, equalTo(false));
assertFieldWarnings("tree", "strategy");
@Override
protected List<ExampleMalformedValue> exampleMalformedValues() {
return List.of();
}

public void testGeohashConfiguration() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,8 @@ boolean coerce() {
return coerce.value();
}

boolean ignoreMalformed() {
@Override
public boolean ignoreMalformed() {
return ignoreMalformed.value();
}

Expand Down Expand Up @@ -464,6 +465,7 @@ protected void parseCreateField(DocumentParserContext context) throws IOExceptio
numericValue = parse(parser, coerce.value());
} catch (IllegalArgumentException e) {
if (ignoreMalformed.value()) {
context.addIgnoredField(mappedFieldType.name());
return;
} else {
throw e;
Expand All @@ -487,6 +489,7 @@ protected void parseCreateField(DocumentParserContext context) throws IOExceptio
double doubleValue = numericValue.doubleValue();
if (Double.isFinite(doubleValue) == false) {
if (ignoreMalformed.value()) {
context.addIgnoredField(mappedFieldType.name());
return;
} else {
// since we encode to a long, we have no way to carry NaNs and infinities
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,11 @@ protected void randomFetchTestFieldConfig(XContentBuilder b) throws IOException
assumeFalse("We don't have a way to assert things here", true);
}

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

@Override
protected SyntheticSourceSupport syntheticSourceSupport() {
return new MatchOnlyTextSyntheticSourceSupport();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ protected boolean supportsStoredFields() {
return false;
}

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

@Override
protected Collection<? extends Plugin> getPlugins() {
return List.of(new MapperExtrasPlugin());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ protected boolean supportsStoredFields() {
return false;
}

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

@Override
protected void registerParameters(ParameterChecker checker) throws IOException {
checker.registerConflictCheck("positive_score_impact", b -> b.field("positive_score_impact", false));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import org.junit.AssumptionViolatedException;

import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;

Expand Down Expand Up @@ -64,10 +63,6 @@ protected void registerParameters(ParameterChecker checker) throws IOException {
checker.registerConflictCheck("store", b -> b.field("store", true));
checker.registerConflictCheck("null_value", b -> b.field("null_value", 1));
checker.registerUpdateCheck(b -> b.field("coerce", false), m -> assertFalse(((ScaledFloatFieldMapper) m).coerce()));
checker.registerUpdateCheck(
b -> b.field("ignore_malformed", true),
m -> assertTrue(((ScaledFloatFieldMapper) m).ignoreMalformed())
);
}

public void testExistsQueryDocValuesDisabled() throws IOException {
Expand Down Expand Up @@ -217,40 +212,19 @@ public void testCoerce() throws Exception {
assertThat(e.getCause().getMessage(), containsString("passed as String"));
}

public void testIgnoreMalformed() throws Exception {
doTestIgnoreMalformed("a", "For input string: \"a\"");

List<String> values = Arrays.asList("NaN", "Infinity", "-Infinity");
for (String value : values) {
doTestIgnoreMalformed(value, "[scaled_float] only supports finite values, but got [" + value + "]");
}
@Override
protected boolean supportsIgnoreMalformed() {
return true;
}

private void doTestIgnoreMalformed(String value, String exceptionMessageContains) throws Exception {
DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping));
ThrowingRunnable runnable = () -> mapper.parse(
new SourceToParse(
"1",
BytesReference.bytes(XContentFactory.jsonBuilder().startObject().field("field", value).endObject()),
XContentType.JSON
)
);
MapperParsingException e = expectThrows(MapperParsingException.class, runnable);
assertThat(e.getCause().getMessage(), containsString(exceptionMessageContains));

DocumentMapper mapper2 = createDocumentMapper(
fieldMapping(b -> b.field("type", "scaled_float").field("scaling_factor", 10.0).field("ignore_malformed", true))
);
ParsedDocument doc = mapper2.parse(
new SourceToParse(
"1",
BytesReference.bytes(XContentFactory.jsonBuilder().startObject().field("field", value).endObject()),
XContentType.JSON
)
@Override
protected List<ExampleMalformedValue> exampleMalformedValues() {
return List.of(
exampleMalformedValue("a").matcher("For input string: \"a\""),
exampleMalformedValue("NaN").matcher("[scaled_float] only supports finite values, but got [NaN]"),
exampleMalformedValue("Infinity").matcher("[scaled_float] only supports finite values, but got [Infinity]"),
exampleMalformedValue("-Infinity").matcher("[scaled_float] only supports finite values, but got [-Infinity]")
);

IndexableField[] fields = doc.rootDoc().getFields("field");
assertEquals(0, fields.length);
}

public void testNullValue() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,11 @@ protected Object generateRandomInputValue(MappedFieldType ft) {
return null;
}

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

@Override
protected SyntheticSourceSupport syntheticSourceSupport() {
throw new AssumptionViolatedException("not supported");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,11 @@ protected void randomFetchTestFieldConfig(XContentBuilder b) throws IOException
b.field("type", "token_count").field("analyzer", "standard");
}

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

@Override
protected SyntheticSourceSupport syntheticSourceSupport() {
throw new AssumptionViolatedException("not supported");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,11 @@ protected String generateRandomInputValue(MappedFieldType ft) {
return null;
}

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

@Override
protected SyntheticSourceSupport syntheticSourceSupport() {
throw new AssumptionViolatedException("not supported");
Expand Down
7 changes: 1 addition & 6 deletions plugins/mapper-annotated-text/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,18 @@ import org.elasticsearch.gradle.internal.info.BuildParams
*/
apply plugin: 'elasticsearch.internal-yaml-rest-test'
apply plugin: 'elasticsearch.yaml-rest-compat-test'
apply plugin: 'elasticsearch.internal-cluster-test'

esplugin {
description 'The Mapper Annotated_text plugin adds support for text fields with markup used to inject annotation tokens into the index.'
classname 'org.elasticsearch.index.mapper.annotatedtext.AnnotatedTextPlugin'
}

if (BuildParams.isSnapshotBuild() == false) {
tasks.named("internalClusterTest").configure {
tasks.named("test").configure {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been annoying me for ages, thanks for fixing

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙇

systemProperty 'es.index_mode_feature_flag_registered', 'true'
}
}

tasks.named('internalClusterTestTestingConventions').configure {
baseClass 'org.elasticsearch.index.mapper.MapperTestCase'
}

restResources {
restApi {
include '_common', 'indices', 'index', 'search'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,11 @@ protected Object generateRandomInputValue(MappedFieldType ft) {
return null;
}

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

@Override
protected SyntheticSourceSupport syntheticSourceSupport() {
throw new AssumptionViolatedException("not supported");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ protected Object generateRandomInputValue(MappedFieldType ft) {
return null;
}

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

@Override
protected SyntheticSourceSupport syntheticSourceSupport() {
throw new AssumptionViolatedException("not supported");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ public final void parse(DocumentParserContext context) throws IOException {
});
}

@Override
public boolean ignoreMalformed() {
return ignoreMalformed.value();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public void doValidate(MappingLookup lookup) {
"data stream timestamp field [" + DEFAULT_PATH + "] has disallowed [null_value] attribute specified"
);
}
if (dateFieldMapper.getIgnoreMalformed()) {
if (dateFieldMapper.ignoreMalformed()) {
throw new IllegalArgumentException(
"data stream timestamp field [" + DEFAULT_PATH + "] has disallowed [ignore_malformed] attribute specified"
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -926,7 +926,8 @@ protected void indexScriptValues(
this.scriptValues.valuesForDoc(searchLookup, readerContext, doc, v -> indexValue(documentParserContext, v));
}

public boolean getIgnoreMalformed() {
@Override
public boolean ignoreMalformed() {
return ignoreMalformed;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,16 @@ public MultiFields multiFields() {
return multiFields;
}

/**
* Will this field ignore malformed values for this field and accept the
* document ({@code true}) or will it reject documents with malformed
* values for this field ({@code false}). Some fields don't have a concept
* of "malformed" and will return {@code false} here.
*/
public boolean ignoreMalformed() {
return false;
}

/**
* 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
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,8 @@ private IpFieldMapper(String simpleName, MappedFieldType mappedFieldType, MultiF
this.dimension = builder.dimension.getValue();
}

boolean ignoreMalformed() {
@Override
public boolean ignoreMalformed() {
return ignoreMalformed;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1647,7 +1647,8 @@ boolean coerce() {
return coerce.value();
}

boolean ignoreMalformed() {
@Override
public boolean ignoreMalformed() {
return ignoreMalformed.value();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,11 @@ protected boolean dedupAfterFetch() {
return true;
}

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

@Override
protected SyntheticSourceSupport syntheticSourceSupport() {
throw new AssumptionViolatedException("not supported");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,11 @@ public void testScriptAndPrecludedParameters() {
assertThat(e.getMessage(), equalTo("Failed to parse mapping: Field [null_value] cannot be set in conjunction with field [script]"));
}

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

@Override
protected SyntheticSourceSupport syntheticSourceSupport() {
return new SyntheticSourceSupport() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ protected boolean supportsStoredFields() {
return false;
}

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

@Override
protected void registerParameters(ParameterChecker checker) throws IOException {
checker.registerConflictCheck("analyzer", b -> b.field("analyzer", "standard"));
Expand Down
Loading