honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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 (https://github.com/elastic/elasticsearch/issues/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
This commit is contained in:
Jim Ferenczi 2018-04-18 13:01:06 +02:00 committed by GitHub
parent 8b34066d8b
commit a7c9857976
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 30 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java
View File

@ -633,7 +633,12 @@ public abstract class ScriptDocValues<T> extends AbstractList<T> {
public BytesRef getBytesValue() {
if (size() > 0) {
return values[0].get();
/**
* We need to make a copy here because {@link BinaryScriptDocValues} might reuse the
* returned value and the same instance might be used to
* return values from multiple documents.
**/
return values[0].toBytesRef();
} else {
return null;
}
@ -658,14 +663,19 @@ public abstract class ScriptDocValues<T> extends AbstractList<T> {
@Override
public BytesRef get(int index) {
return values[index].get();
/**
* We need to make a copy here because {@link BinaryScriptDocValues} might reuse the
* returned value and the same instance might be used to
* return values from multiple documents.
**/
return values[index].toBytesRef();
}
public BytesRef getValue() {
if (count == 0) {
return new BytesRef();
}
return values[0].get();
return values[0].toBytesRef();
}
}

31
server/src/test/java/org/elasticsearch/index/fielddata/BinaryDVFieldDataTests.java
View File

@ -52,7 +52,6 @@ public class BinaryDVFieldDataTests extends AbstractFieldDataTestCase {
final DocumentMapper mapper = mapperService.documentMapperParser().parse("test", new CompressedXContent(mapping));
List<BytesRef> bytesList1 = new ArrayList<>(2);
bytesList1.add(randomBytes());
bytesList1.add(randomBytes());
@ -123,22 +122,26 @@ public class BinaryDVFieldDataTests extends AbstractFieldDataTestCase {
// Test whether ScriptDocValues.BytesRefs makes a deepcopy
fieldData = indexFieldData.load(reader);
ScriptDocValues<?> scriptValues = fieldData.getScriptValues();
scriptValues.setNextDocId(0);
assertEquals(2, scriptValues.size());
assertEquals(bytesList1.get(0), scriptValues.get(0));
assertEquals(bytesList1.get(1), scriptValues.get(1));
Object[][] retValues = new BytesRef[4][0];
for (int i = 0; i < 4; i++) {
scriptValues.setNextDocId(i);
retValues[i] = new BytesRef[scriptValues.size()];
for (int j = 0; j < retValues[i].length; j++) {
retValues[i][j] = scriptValues.get(j);
}
}
assertEquals(2, retValues[0].length);
assertEquals(bytesList1.get(0), retValues[0][0]);
assertEquals(bytesList1.get(1), retValues[0][1]);
scriptValues.setNextDocId(1);
assertEquals(1, scriptValues.size());
assertEquals(bytes1, scriptValues.get(0));
assertEquals(1, retValues[1].length);
assertEquals(bytes1, retValues[1][0]);
scriptValues.setNextDocId(2);
assertEquals(0, scriptValues.size());
assertEquals(0, retValues[2].length);
scriptValues.setNextDocId(3);
assertEquals(2, scriptValues.size());
assertEquals(bytesList2.get(0), scriptValues.get(0));
assertEquals(bytesList2.get(1), scriptValues.get(1));
assertEquals(2, retValues[3].length);
assertEquals(bytesList2.get(0), retValues[3][0]);
assertEquals(bytesList2.get(1), retValues[3][1]);
}
private static BytesRef randomBytes() {