Skip to content

Commit ac55e58

Browse files
authored
Fix collapse interaction with stored fields (#112761) (#113597)
Collapse dynamically will add values to the DocumentField values array. There are a few scenarios where this is immutable and most of these are OK. However, we get in trouble when we create an immutable set for StoredValues which collapse later tries to update. The other option for this fix was to make an array copy for `values` in every `DocumentField` ctor, this seemed very expensive and could get out of hand. So, I decided to fix this one bug instead. closes #112646
1 parent 09313b0 commit ac55e58

File tree

3 files changed

+39
-1
lines changed

3 files changed

+39
-1
lines changed

docs/changelog/112761.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 112761
2+
summary: Fix collapse interaction with stored fields
3+
area: Search
4+
type: bug
5+
issues:
6+
- 112646

server/src/internalClusterTest/java/org/elasticsearch/search/CollapseSearchResultsIT.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.elasticsearch.index.query.MatchAllQueryBuilder;
1515
import org.elasticsearch.search.collapse.CollapseBuilder;
1616
import org.elasticsearch.test.ESIntegTestCase;
17+
import org.elasticsearch.xcontent.XContentType;
1718

1819
import java.util.Map;
1920
import java.util.Set;
@@ -85,4 +86,33 @@ public void testCollapseWithFields() {
8586
}
8687
);
8788
}
89+
90+
public void testCollapseWithStoredFields() {
91+
final String indexName = "test_collapse";
92+
createIndex(indexName);
93+
final String collapseField = "collapse_field";
94+
assertAcked(indicesAdmin().preparePutMapping(indexName).setSource("""
95+
{
96+
"dynamic": "strict",
97+
"properties": {
98+
"collapse_field": { "type": "keyword", "store": true },
99+
"ts": { "type": "date", "store": true }
100+
}
101+
}
102+
""", XContentType.JSON));
103+
index(indexName, "id_1_0", Map.of(collapseField, "value1", "ts", 0));
104+
index(indexName, "id_1_1", Map.of(collapseField, "value1", "ts", 1));
105+
index(indexName, "id_2_0", Map.of(collapseField, "value2", "ts", 2));
106+
refresh(indexName);
107+
108+
assertNoFailuresAndResponse(
109+
prepareSearch(indexName).setQuery(new MatchAllQueryBuilder())
110+
.setFetchSource(false)
111+
.storedFields("*")
112+
.setCollapse(new CollapseBuilder(collapseField)),
113+
searchResponse -> {
114+
assertEquals(collapseField, searchResponse.getHits().getCollapseField());
115+
}
116+
);
117+
}
88118
}

server/src/main/java/org/elasticsearch/search/fetch/subphase/StoredFieldsPhase.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.util.List;
2828
import java.util.Map;
2929
import java.util.Set;
30+
import java.util.stream.Collectors;
3031

3132
/**
3233
* Process stored fields loaded from a HitContext into DocumentFields
@@ -42,7 +43,8 @@ List<Object> process(Map<String, List<Object>> loadedFields) {
4243
if (inputs == null) {
4344
return List.of();
4445
}
45-
return inputs.stream().map(ft::valueForDisplay).toList();
46+
// This is eventually provided to DocumentField, which needs this collection to be mutable
47+
return inputs.stream().map(ft::valueForDisplay).collect(Collectors.toList());
4648
}
4749

4850
boolean hasValue(Map<String, List<Object>> loadedFields) {

0 commit comments

Comments
 (0)