Skip to content

Commit

Permalink
Fix collapse interaction with stored fields (elastic#112761)
Browse files Browse the repository at this point in the history
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 elastic#112646
  • Loading branch information
benwtrent committed Sep 26, 2024
1 parent cbce0a6 commit d3ea060
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 1 deletion.
6 changes: 6 additions & 0 deletions docs/changelog/112761.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 112761
summary: Fix collapse interaction with stored fields
area: Search
type: bug
issues:
- 112646
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.elasticsearch.index.query.MatchAllQueryBuilder;
import org.elasticsearch.search.collapse.CollapseBuilder;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.xcontent.XContentType;

import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -85,4 +86,33 @@ public void testCollapseWithFields() {
}
);
}

public void testCollapseWithStoredFields() {
final String indexName = "test_collapse";
createIndex(indexName);
final String collapseField = "collapse_field";
assertAcked(indicesAdmin().preparePutMapping(indexName).setSource("""
{
"dynamic": "strict",
"properties": {
"collapse_field": { "type": "keyword", "store": true },
"ts": { "type": "date", "store": true }
}
}
""", XContentType.JSON));
index(indexName, "id_1_0", Map.of(collapseField, "value1", "ts", 0));
index(indexName, "id_1_1", Map.of(collapseField, "value1", "ts", 1));
index(indexName, "id_2_0", Map.of(collapseField, "value2", "ts", 2));
refresh(indexName);

assertNoFailuresAndResponse(
prepareSearch(indexName).setQuery(new MatchAllQueryBuilder())
.setFetchSource(false)
.storedFields("*")
.setCollapse(new CollapseBuilder(collapseField)),
searchResponse -> {
assertEquals(collapseField, searchResponse.getHits().getCollapseField());
}
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

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

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

0 comments on commit d3ea060

Please sign in to comment.