Skip to content

Commit 5f244e6

Browse files
Kubik42Kubik42
committed
Addressed feedback
1 parent ffc2498 commit 5f244e6

File tree

5 files changed

+43
-75
lines changed

5 files changed

+43
-75
lines changed

‎server/src/main/java/org/elasticsearch/index/mapper/BinaryDocValuesSyntheticFieldLoaderLayer.java‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,24 +24,21 @@ public final class BinaryDocValuesSyntheticFieldLoaderLayer implements Composite
2424
// the binary doc values for a document are all encoded in a single binary array, which this stream knows how to read
2525
// the doc values in the array take the form of [doc value count][length of value 1][value 1][length of value 2][value 2]...
2626
private final ByteArrayStreamInput stream = new ByteArrayStreamInput();
27-
private BytesRef docValuesBytes;
2827
private int valueCount;
2928

3029
public BinaryDocValuesSyntheticFieldLoaderLayer(String fieldName) {
3130
this.fieldName = fieldName;
3231
}
3332

34-
@Override
35-
public long valueCount() {
36-
return valueCount;
37-
}
38-
3933
@Override
4034
public DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf) throws IOException {
4135
BinaryDocValues docValues = leafReader.getBinaryDocValues(fieldName);
4236

4337
// there are no values associated with this field
44-
if (docValues == null) return null;
38+
if (docValues == null) {
39+
valueCount = 0;
40+
return null;
41+
}
4542

4643
return docId -> {
4744
// there are no more documents to process
@@ -51,19 +48,14 @@ public DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf
5148
}
5249

5350
// otherwise, extract the doc values into a stream to later read from
54-
docValuesBytes = docValues.binaryValue();
51+
BytesRef docValuesBytes = docValues.binaryValue();
5552
stream.reset(docValuesBytes.bytes, docValuesBytes.offset, docValuesBytes.length);
5653
valueCount = stream.readVInt();
5754

5855
return hasValue();
5956
};
6057
}
6158

62-
@Override
63-
public boolean hasValue() {
64-
return valueCount > 0;
65-
}
66-
6759
@Override
6860
public void write(XContentBuilder b) throws IOException {
6961
for (int i = 0; i < valueCount; i++) {
@@ -73,6 +65,16 @@ public void write(XContentBuilder b) throws IOException {
7365
}
7466
}
7567

68+
@Override
69+
public boolean hasValue() {
70+
return valueCount > 0;
71+
}
72+
73+
@Override
74+
public long valueCount() {
75+
return valueCount;
76+
}
77+
7678
@Override
7779
public String fieldName() {
7880
return fieldName;

‎server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1448,9 +1448,11 @@ private static final class MultiValuedBinaryDocValuesField extends CustomDocValu
14481448
}
14491449

14501450
public void add(final BytesRef value) {
1451-
uniqueValues.add(value);
1452-
// might as well track these on the go as opposed to having to loop through all entries later
1453-
docValuesByteCount += value.length;
1451+
if (uniqueValues.add(value)) {
1452+
// might as well track these on the go as opposed to having to loop through all entries later
1453+
docValuesByteCount += value.length;
1454+
}
1455+
;
14541456
}
14551457

14561458
/**

‎server/src/test/java/org/elasticsearch/index/mapper/KeywordSyntheticSourceNativeArrayIntegrationTests.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ public void testSynthesizeArrayIgnoreAbove() throws Exception {
5252
.endObject()
5353
.endObject()
5454
.endObject();
55+
5556
// Note values that would be ignored are added at the end of arrays,
5657
// this makes testing easier as ignored values are always synthesized after regular values:
5758
var arrayValues = new Object[][] {
@@ -60,7 +61,16 @@ public void testSynthesizeArrayIgnoreAbove() throws Exception {
6061
new Object[] { "123", "1234", "12345" },
6162
new Object[] { null, null, null, "blabla" },
6263
new Object[] { "1", "2", "3", "blabla" } };
63-
verifySyntheticArray(arrayValues, mapping, "_id", "field._original");
64+
65+
// values in the original array should be deduplicated
66+
var expectedArrayValues = new Object[][] {
67+
new Object[] { null, "a", "ab", "abc", "abcd", null, "abcde" },
68+
new Object[] { "12345" },
69+
new Object[] { "123", "1234", "12345" },
70+
new Object[] { null, null, null, "blabla" },
71+
new Object[] { "1", "2", "3", "blabla" } };
72+
73+
verifySyntheticArray(arrayValues, expectedArrayValues, mapping, "_id");
6474
}
6575

6676
public void testSynthesizeObjectArray() throws Exception {

‎test/framework/src/main/java/org/elasticsearch/index/mapper/NativeArrayIntegrationTestCase.java‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -259,11 +259,17 @@ protected void verifySyntheticArray(Object[][] arrays, XContentBuilder mapping,
259259
private XContentBuilder arrayToSource(Object[] array) throws IOException {
260260
var source = jsonBuilder().startObject();
261261
if (array != null) {
262-
source.startArray("field");
263-
for (Object arrayValue : array) {
264-
source.value(arrayValue);
262+
// collapse array if it only consists of one element
263+
// if the only element is null, then we'll skip synthesizing source for that field
264+
if (array.length == 1 && array[0] != null) {
265+
source.field("field", array[0]);
266+
} else {
267+
source.startArray("field");
268+
for (Object arrayValue : array) {
269+
source.value(arrayValue);
270+
}
271+
source.endArray();
265272
}
266-
source.endArray();
267273
} else {
268274
source.field("field").nullValue();
269275
}

‎x-pack/plugin/wildcard/src/main/java/org/elasticsearch/xpack/wildcard/mapper/WildcardFieldMapper.java‎

Lines changed: 2 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,8 @@
1717
import org.apache.lucene.document.Field;
1818
import org.apache.lucene.document.FieldType;
1919
import org.apache.lucene.document.StoredField;
20-
import org.apache.lucene.index.BinaryDocValues;
2120
import org.apache.lucene.index.IndexOptions;
2221
import org.apache.lucene.index.IndexableField;
23-
import org.apache.lucene.index.LeafReader;
2422
import org.apache.lucene.index.Term;
2523
import org.apache.lucene.search.BooleanClause;
2624
import org.apache.lucene.search.BooleanClause.Occur;
@@ -43,7 +41,6 @@
4341
import org.apache.lucene.util.automaton.RegExp;
4442
import org.elasticsearch.ElasticsearchParseException;
4543
import org.elasticsearch.common.geo.ShapeRelation;
46-
import org.elasticsearch.common.io.stream.ByteArrayStreamInput;
4744
import org.elasticsearch.common.lucene.BytesRefs;
4845
import org.elasticsearch.common.lucene.Lucene;
4946
import org.elasticsearch.common.time.DateMathParser;
@@ -58,6 +55,7 @@
5855
import org.elasticsearch.index.fielddata.FieldDataContext;
5956
import org.elasticsearch.index.fielddata.IndexFieldData;
6057
import org.elasticsearch.index.fielddata.plain.StringBinaryIndexFieldData;
58+
import org.elasticsearch.index.mapper.BinaryDocValuesSyntheticFieldLoaderLayer;
6159
import org.elasticsearch.index.mapper.BinaryFieldMapper.CustomBinaryDocValuesField;
6260
import org.elasticsearch.index.mapper.BlockLoader;
6361
import org.elasticsearch.index.mapper.CompositeSyntheticFieldLoader;
@@ -1106,7 +1104,7 @@ public FieldMapper.Builder getMergeBuilder() {
11061104
protected SyntheticSourceSupport syntheticSourceSupport() {
11071105
return new SyntheticSourceSupport.Native(() -> {
11081106
var layers = new ArrayList<CompositeSyntheticFieldLoader.Layer>();
1109-
layers.add(new WildcardSyntheticFieldLoader());
1107+
layers.add(new BinaryDocValuesSyntheticFieldLoaderLayer(fullPath()));
11101108
if (ignoreAbove.valuesPotentiallyIgnored()) {
11111109
layers.add(new CompositeSyntheticFieldLoader.StoredFieldLayer(originalName()) {
11121110
@Override
@@ -1120,54 +1118,4 @@ protected void writeValue(Object value, XContentBuilder b) throws IOException {
11201118
});
11211119
}
11221120

1123-
private class WildcardSyntheticFieldLoader implements CompositeSyntheticFieldLoader.DocValuesLayer {
1124-
private final ByteArrayStreamInput docValuesStream = new ByteArrayStreamInput();
1125-
private int docValueCount;
1126-
private BytesRef docValueBytes;
1127-
1128-
@Override
1129-
public DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf) throws IOException {
1130-
BinaryDocValues values = leafReader.getBinaryDocValues(fullPath());
1131-
if (values == null) {
1132-
docValueCount = 0;
1133-
return null;
1134-
}
1135-
1136-
return docId -> {
1137-
if (values.advanceExact(docId) == false) {
1138-
docValueCount = 0;
1139-
return hasValue();
1140-
}
1141-
docValueBytes = values.binaryValue();
1142-
docValuesStream.reset(docValueBytes.bytes);
1143-
docValuesStream.setPosition(docValueBytes.offset);
1144-
docValueCount = docValuesStream.readVInt();
1145-
return hasValue();
1146-
};
1147-
}
1148-
1149-
@Override
1150-
public boolean hasValue() {
1151-
return docValueCount > 0;
1152-
}
1153-
1154-
@Override
1155-
public long valueCount() {
1156-
return docValueCount;
1157-
}
1158-
1159-
@Override
1160-
public void write(XContentBuilder b) throws IOException {
1161-
for (int i = 0; i < docValueCount; i++) {
1162-
int length = docValuesStream.readVInt();
1163-
b.utf8Value(docValueBytes.bytes, docValuesStream.getPosition(), length);
1164-
docValuesStream.skipBytes(length);
1165-
}
1166-
}
1167-
1168-
@Override
1169-
public String fieldName() {
1170-
return fullPath();
1171-
}
1172-
}
11731121
}

0 commit comments

Comments
 (0)