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

[8.x] Skip storing ignored source for single-element leaf arrays (#113937) #114384

Merged
merged 6 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -799,17 +799,30 @@ private static void parseNonDynamicArray(
String fullPath = context.path().pathAsText(arrayFieldName);

// Check if we need to record the array source. This only applies to synthetic source.
boolean canRemoveSingleLeafElement = false;
if (context.canAddIgnoredField()) {
boolean objectRequiresStoringSource = mapper instanceof ObjectMapper objectMapper
&& (getSourceKeepMode(context, objectMapper.sourceKeepMode()) == Mapper.SourceKeepMode.ALL
|| (getSourceKeepMode(context, objectMapper.sourceKeepMode()) == Mapper.SourceKeepMode.ARRAYS
&& objectMapper instanceof NestedObjectMapper == false));
boolean fieldWithFallbackSyntheticSource = mapper instanceof FieldMapper fieldMapper
&& fieldMapper.syntheticSourceMode() == FieldMapper.SyntheticSourceMode.FALLBACK;
boolean fieldWithStoredArraySource = mapper instanceof FieldMapper fieldMapper
&& getSourceKeepMode(context, fieldMapper.sourceKeepMode()) != Mapper.SourceKeepMode.NONE;
Mapper.SourceKeepMode mode = Mapper.SourceKeepMode.NONE;
boolean objectWithFallbackSyntheticSource = false;
if (mapper instanceof ObjectMapper objectMapper) {
mode = getSourceKeepMode(context, objectMapper.sourceKeepMode());
objectWithFallbackSyntheticSource = (mode == Mapper.SourceKeepMode.ALL
|| (mode == Mapper.SourceKeepMode.ARRAYS && objectMapper instanceof NestedObjectMapper == false));
}
boolean fieldWithFallbackSyntheticSource = false;
boolean fieldWithStoredArraySource = false;
if (mapper instanceof FieldMapper fieldMapper) {
mode = getSourceKeepMode(context, fieldMapper.sourceKeepMode());
fieldWithFallbackSyntheticSource = fieldMapper.syntheticSourceMode() == FieldMapper.SyntheticSourceMode.FALLBACK;
fieldWithStoredArraySource = mode != Mapper.SourceKeepMode.NONE;
}
boolean copyToFieldHasValuesInDocument = context.isWithinCopyTo() == false && context.isCopyToDestinationField(fullPath);
if (objectRequiresStoringSource

canRemoveSingleLeafElement = mapper instanceof FieldMapper
&& mode == Mapper.SourceKeepMode.ARRAYS
&& fieldWithFallbackSyntheticSource == false
&& copyToFieldHasValuesInDocument == false;

if (objectWithFallbackSyntheticSource
|| fieldWithFallbackSyntheticSource
|| fieldWithStoredArraySource
|| copyToFieldHasValuesInDocument) {
Expand All @@ -829,20 +842,28 @@ private static void parseNonDynamicArray(

XContentParser parser = context.parser();
XContentParser.Token token;
int elements = 0;
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
if (token == XContentParser.Token.START_OBJECT) {
elements = Integer.MAX_VALUE;
parseObject(context, lastFieldName);
} else if (token == XContentParser.Token.START_ARRAY) {
elements = Integer.MAX_VALUE;
parseArray(context, lastFieldName);
} else if (token == XContentParser.Token.VALUE_NULL) {
elements++;
parseNullValue(context, lastFieldName);
} else if (token == null) {
throwEOFOnParseArray(arrayFieldName, context);
} else {
assert token.isValue();
elements++;
parseValue(context, lastFieldName);
}
}
if (elements <= 1 && canRemoveSingleLeafElement) {
context.removeLastIgnoredField(fullPath);
}
postProcessDynamicArrayMapping(context, lastFieldName);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,12 @@ public final void addIgnoredField(IgnoredSourceFieldMapper.NameValue values) {
}
}

final void removeLastIgnoredField(String name) {
if (ignoredFieldValues.isEmpty() == false && ignoredFieldValues.get(ignoredFieldValues.size() - 1).name().equals(name)) {
ignoredFieldValues.remove(ignoredFieldValues.size() - 1);
}
}

/**
* Return the collection of values for fields that have been ignored so far.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,38 @@ public void testIndexStoredArraySourceRootValueArrayDisabled() throws IOExceptio
{"bool_value":true,"int_value":[10,20,30]}""", syntheticSource);
}

public void testIndexStoredArraySourceSingleLeafElement() throws IOException {
DocumentMapper documentMapper = createMapperServiceWithStoredArraySource(syntheticSourceMapping(b -> {
b.startObject("int_value").field("type", "integer").endObject();
})).documentMapper();
var syntheticSource = syntheticSource(documentMapper, b -> b.array("int_value", new int[] { 10 }));
assertEquals("{\"int_value\":10}", syntheticSource);
ParsedDocument doc = documentMapper.parse(source(syntheticSource));
assertNull(doc.rootDoc().getField("_ignored_source"));
}

public void testIndexStoredArraySourceSingleLeafElementAndNull() throws IOException {
DocumentMapper documentMapper = createMapperServiceWithStoredArraySource(syntheticSourceMapping(b -> {
b.startObject("value").field("type", "keyword").endObject();
})).documentMapper();
var syntheticSource = syntheticSource(documentMapper, b -> b.array("value", new String[] { "foo", null }));
assertEquals("{\"value\":[\"foo\",null]}", syntheticSource);
}

public void testIndexStoredArraySourceSingleObjectElement() throws IOException {
DocumentMapper documentMapper = createMapperServiceWithStoredArraySource(syntheticSourceMapping(b -> {
b.startObject("path").startObject("properties");
{
b.startObject("int_value").field("type", "integer").endObject();
}
b.endObject().endObject();
})).documentMapper();
var syntheticSource = syntheticSource(documentMapper, b -> {
b.startArray("path").startObject().field("int_value", 10).endObject().endArray();
});
assertEquals("{\"path\":[{\"int_value\":10}]}", syntheticSource);
}

public void testFieldStoredArraySourceRootValueArray() throws IOException {
DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> {
b.startObject("int_value").field("type", "integer").field(Mapper.SYNTHETIC_SOURCE_KEEP_PARAM, "arrays").endObject();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1586,7 +1586,7 @@ public void testSyntheticSourceKeepArrays() throws IOException {
b.endObject();
}));

int elementCount = randomIntBetween(1, 5);
int elementCount = randomIntBetween(2, 5);
CheckedConsumer<XContentBuilder, IOException> buildInput = (XContentBuilder builder) -> {
example.buildInputArray(builder, elementCount);
};
Expand All @@ -1596,7 +1596,8 @@ public void testSyntheticSourceKeepArrays() throws IOException {
buildInput.accept(builder);
builder.endObject();
String expected = Strings.toString(builder);
assertThat(syntheticSource(mapperAll, buildInput), equalTo(expected));
String actual = syntheticSource(mapperAll, buildInput);
assertThat(actual, equalTo(expected));
}

@Override
Expand Down