Skip to content

Commit f6047ff

Browse files
kkrik-eskkrik-es
authored
[8.x] Skip storing ignored source for single-element leaf arrays (elastic#113937) (elastic#114384)
* Skip storing ignored source for single-element leaf arrays (elastic#113937) * Minimize storing array source * restrict to fields * revert changes for `addIgnoredFieldFromContext` * fix test * spotless * count nulls (cherry picked from commit f79705d) * fix list api * fix tests * Update MapperTestCase.java * Update MapperTestCase.java * Update MapperTestCase.java
1 parent 37125f2 commit f6047ff

File tree

4 files changed

+71
-11
lines changed

4 files changed

+71
-11
lines changed

‎server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java‎

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -799,17 +799,30 @@ private static void parseNonDynamicArray(
799799
String fullPath = context.path().pathAsText(arrayFieldName);
800800

801801
// Check if we need to record the array source. This only applies to synthetic source.
802+
boolean canRemoveSingleLeafElement = false;
802803
if (context.canAddIgnoredField()) {
803-
boolean objectRequiresStoringSource = mapper instanceof ObjectMapper objectMapper
804-
&& (getSourceKeepMode(context, objectMapper.sourceKeepMode()) == Mapper.SourceKeepMode.ALL
805-
|| (getSourceKeepMode(context, objectMapper.sourceKeepMode()) == Mapper.SourceKeepMode.ARRAYS
806-
&& objectMapper instanceof NestedObjectMapper == false));
807-
boolean fieldWithFallbackSyntheticSource = mapper instanceof FieldMapper fieldMapper
808-
&& fieldMapper.syntheticSourceMode() == FieldMapper.SyntheticSourceMode.FALLBACK;
809-
boolean fieldWithStoredArraySource = mapper instanceof FieldMapper fieldMapper
810-
&& getSourceKeepMode(context, fieldMapper.sourceKeepMode()) != Mapper.SourceKeepMode.NONE;
804+
Mapper.SourceKeepMode mode = Mapper.SourceKeepMode.NONE;
805+
boolean objectWithFallbackSyntheticSource = false;
806+
if (mapper instanceof ObjectMapper objectMapper) {
807+
mode = getSourceKeepMode(context, objectMapper.sourceKeepMode());
808+
objectWithFallbackSyntheticSource = (mode == Mapper.SourceKeepMode.ALL
809+
|| (mode == Mapper.SourceKeepMode.ARRAYS && objectMapper instanceof NestedObjectMapper == false));
810+
}
811+
boolean fieldWithFallbackSyntheticSource = false;
812+
boolean fieldWithStoredArraySource = false;
813+
if (mapper instanceof FieldMapper fieldMapper) {
814+
mode = getSourceKeepMode(context, fieldMapper.sourceKeepMode());
815+
fieldWithFallbackSyntheticSource = fieldMapper.syntheticSourceMode() == FieldMapper.SyntheticSourceMode.FALLBACK;
816+
fieldWithStoredArraySource = mode != Mapper.SourceKeepMode.NONE;
817+
}
811818
boolean copyToFieldHasValuesInDocument = context.isWithinCopyTo() == false && context.isCopyToDestinationField(fullPath);
812-
if (objectRequiresStoringSource
819+
820+
canRemoveSingleLeafElement = mapper instanceof FieldMapper
821+
&& mode == Mapper.SourceKeepMode.ARRAYS
822+
&& fieldWithFallbackSyntheticSource == false
823+
&& copyToFieldHasValuesInDocument == false;
824+
825+
if (objectWithFallbackSyntheticSource
813826
|| fieldWithFallbackSyntheticSource
814827
|| fieldWithStoredArraySource
815828
|| copyToFieldHasValuesInDocument) {
@@ -829,20 +842,28 @@ private static void parseNonDynamicArray(
829842

830843
XContentParser parser = context.parser();
831844
XContentParser.Token token;
845+
int elements = 0;
832846
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
833847
if (token == XContentParser.Token.START_OBJECT) {
848+
elements = Integer.MAX_VALUE;
834849
parseObject(context, lastFieldName);
835850
} else if (token == XContentParser.Token.START_ARRAY) {
851+
elements = Integer.MAX_VALUE;
836852
parseArray(context, lastFieldName);
837853
} else if (token == XContentParser.Token.VALUE_NULL) {
854+
elements++;
838855
parseNullValue(context, lastFieldName);
839856
} else if (token == null) {
840857
throwEOFOnParseArray(arrayFieldName, context);
841858
} else {
842859
assert token.isValue();
860+
elements++;
843861
parseValue(context, lastFieldName);
844862
}
845863
}
864+
if (elements <= 1 && canRemoveSingleLeafElement) {
865+
context.removeLastIgnoredField(fullPath);
866+
}
846867
postProcessDynamicArrayMapping(context, lastFieldName);
847868
}
848869

‎server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,12 @@ public final void addIgnoredField(IgnoredSourceFieldMapper.NameValue values) {
296296
}
297297
}
298298

299+
final void removeLastIgnoredField(String name) {
300+
if (ignoredFieldValues.isEmpty() == false && ignoredFieldValues.get(ignoredFieldValues.size() - 1).name().equals(name)) {
301+
ignoredFieldValues.remove(ignoredFieldValues.size() - 1);
302+
}
303+
}
304+
299305
/**
300306
* Return the collection of values for fields that have been ignored so far.
301307
*/

‎server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,38 @@ public void testIndexStoredArraySourceRootValueArrayDisabled() throws IOExceptio
532532
{"bool_value":true,"int_value":[10,20,30]}""", syntheticSource);
533533
}
534534

535+
public void testIndexStoredArraySourceSingleLeafElement() throws IOException {
536+
DocumentMapper documentMapper = createMapperServiceWithStoredArraySource(syntheticSourceMapping(b -> {
537+
b.startObject("int_value").field("type", "integer").endObject();
538+
})).documentMapper();
539+
var syntheticSource = syntheticSource(documentMapper, b -> b.array("int_value", new int[] { 10 }));
540+
assertEquals("{\"int_value\":10}", syntheticSource);
541+
ParsedDocument doc = documentMapper.parse(source(syntheticSource));
542+
assertNull(doc.rootDoc().getField("_ignored_source"));
543+
}
544+
545+
public void testIndexStoredArraySourceSingleLeafElementAndNull() throws IOException {
546+
DocumentMapper documentMapper = createMapperServiceWithStoredArraySource(syntheticSourceMapping(b -> {
547+
b.startObject("value").field("type", "keyword").endObject();
548+
})).documentMapper();
549+
var syntheticSource = syntheticSource(documentMapper, b -> b.array("value", new String[] { "foo", null }));
550+
assertEquals("{\"value\":[\"foo\",null]}", syntheticSource);
551+
}
552+
553+
public void testIndexStoredArraySourceSingleObjectElement() throws IOException {
554+
DocumentMapper documentMapper = createMapperServiceWithStoredArraySource(syntheticSourceMapping(b -> {
555+
b.startObject("path").startObject("properties");
556+
{
557+
b.startObject("int_value").field("type", "integer").endObject();
558+
}
559+
b.endObject().endObject();
560+
})).documentMapper();
561+
var syntheticSource = syntheticSource(documentMapper, b -> {
562+
b.startArray("path").startObject().field("int_value", 10).endObject().endArray();
563+
});
564+
assertEquals("{\"path\":[{\"int_value\":10}]}", syntheticSource);
565+
}
566+
535567
public void testFieldStoredArraySourceRootValueArray() throws IOException {
536568
DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> {
537569
b.startObject("int_value").field("type", "integer").field(Mapper.SYNTHETIC_SOURCE_KEEP_PARAM, "arrays").endObject();

‎test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1586,7 +1586,7 @@ public void testSyntheticSourceKeepArrays() throws IOException {
15861586
b.endObject();
15871587
}));
15881588

1589-
int elementCount = randomIntBetween(1, 5);
1589+
int elementCount = randomIntBetween(2, 5);
15901590
CheckedConsumer<XContentBuilder, IOException> buildInput = (XContentBuilder builder) -> {
15911591
example.buildInputArray(builder, elementCount);
15921592
};
@@ -1596,7 +1596,8 @@ public void testSyntheticSourceKeepArrays() throws IOException {
15961596
buildInput.accept(builder);
15971597
builder.endObject();
15981598
String expected = Strings.toString(builder);
1599-
assertThat(syntheticSource(mapperAll, buildInput), equalTo(expected));
1599+
String actual = syntheticSource(mapperAll, buildInput);
1600+
assertThat(actual, equalTo(expected));
16001601
}
16011602

16021603
@Override

0 commit comments

Comments
 (0)