Skip to content

Commit 1b67dab

Browse files
authored
Fix collapse interaction with stored fields (#112761)
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 e2281a1 commit 1b67dab

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)