Skip to content

Commit a7c9857

Browse files
authored
Fix binary doc values fetching in _search (#29567)
Binary doc values are retrieved during the DocValueFetchSubPhase through an instance of ScriptDocValues. Since 6.0 ScriptDocValues instances are not allowed to reuse the object that they return (#26775) but BinaryScriptDocValues doesn't follow this restriction and reuses instances of BytesRefBuilder among different documents. This results in `field` values assigned to the wrong document in the response. This commit fixes this issue by recreating the BytesRef for each value that needs to be returned. Fixes #29565
1 parent 8b34066 commit a7c9857

File tree

2 files changed

+33
-20
lines changed

2 files changed

+33
-20
lines changed

server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -633,7 +633,12 @@ public String get(int index) {
633633

634634
public BytesRef getBytesValue() {
635635
if (size() > 0) {
636-
return values[0].get();
636+
/**
637+
* We need to make a copy here because {@link BinaryScriptDocValues} might reuse the
638+
* returned value and the same instance might be used to
639+
* return values from multiple documents.
640+
**/
641+
return values[0].toBytesRef();
637642
} else {
638643
return null;
639644
}
@@ -658,14 +663,19 @@ public BytesRefs(SortedBinaryDocValues in) {
658663

659664
@Override
660665
public BytesRef get(int index) {
661-
return values[index].get();
666+
/**
667+
* We need to make a copy here because {@link BinaryScriptDocValues} might reuse the
668+
* returned value and the same instance might be used to
669+
* return values from multiple documents.
670+
**/
671+
return values[index].toBytesRef();
662672
}
663673

664674
public BytesRef getValue() {
665675
if (count == 0) {
666676
return new BytesRef();
667677
}
668-
return values[0].get();
678+
return values[0].toBytesRef();
669679
}
670680

671681
}

server/src/test/java/org/elasticsearch/index/fielddata/BinaryDVFieldDataTests.java

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ public void testDocValue() throws Exception {
5252

5353
final DocumentMapper mapper = mapperService.documentMapperParser().parse("test", new CompressedXContent(mapping));
5454

55-
5655
List<BytesRef> bytesList1 = new ArrayList<>(2);
5756
bytesList1.add(randomBytes());
5857
bytesList1.add(randomBytes());
@@ -123,22 +122,26 @@ public void testDocValue() throws Exception {
123122
// Test whether ScriptDocValues.BytesRefs makes a deepcopy
124123
fieldData = indexFieldData.load(reader);
125124
ScriptDocValues<?> scriptValues = fieldData.getScriptValues();
126-
scriptValues.setNextDocId(0);
127-
assertEquals(2, scriptValues.size());
128-
assertEquals(bytesList1.get(0), scriptValues.get(0));
129-
assertEquals(bytesList1.get(1), scriptValues.get(1));
130-
131-
scriptValues.setNextDocId(1);
132-
assertEquals(1, scriptValues.size());
133-
assertEquals(bytes1, scriptValues.get(0));
134-
135-
scriptValues.setNextDocId(2);
136-
assertEquals(0, scriptValues.size());
137-
138-
scriptValues.setNextDocId(3);
139-
assertEquals(2, scriptValues.size());
140-
assertEquals(bytesList2.get(0), scriptValues.get(0));
141-
assertEquals(bytesList2.get(1), scriptValues.get(1));
125+
Object[][] retValues = new BytesRef[4][0];
126+
for (int i = 0; i < 4; i++) {
127+
scriptValues.setNextDocId(i);
128+
retValues[i] = new BytesRef[scriptValues.size()];
129+
for (int j = 0; j < retValues[i].length; j++) {
130+
retValues[i][j] = scriptValues.get(j);
131+
}
132+
}
133+
assertEquals(2, retValues[0].length);
134+
assertEquals(bytesList1.get(0), retValues[0][0]);
135+
assertEquals(bytesList1.get(1), retValues[0][1]);
136+
137+
assertEquals(1, retValues[1].length);
138+
assertEquals(bytes1, retValues[1][0]);
139+
140+
assertEquals(0, retValues[2].length);
141+
142+
assertEquals(2, retValues[3].length);
143+
assertEquals(bytesList2.get(0), retValues[3][0]);
144+
assertEquals(bytesList2.get(1), retValues[3][1]);
142145
}
143146

144147
private static BytesRef randomBytes() {

0 commit comments

Comments
 (0)