Skip to content

Commit 366d343

Browse files
martijnvgmartijnvg
committed
The fields option should always return an array for json document fields and single valued field for metadata fields.
The `fields` option can only be used to fetch leaf fields, trying to do fetch object fields will return in a client error. Closes to elastic#4542
1 parent aa548f5 commit 366d343

File tree

12 files changed

+211
-30
lines changed

12 files changed

+211
-30
lines changed

‎docs/reference/docs/get.asciidoc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,12 @@ For backward compatibility, if the requested fields are not stored, they will be
115115
from the `_source` (parsed and extracted). This functionality has been replaced by the
116116
<<get-source-filtering,source filtering>> parameter.
117117

118+
Field values fetched from the document it self are always returned as an array. Metadata fields like `_routing` and
119+
`_parent` fields are never returned as an array.
120+
121+
Also only leaf fields can be returned via the `field` option. So object fields can't be returned and such requests
122+
will fail.
123+
118124
[float]
119125
[[_source]]
120126
=== Getting the _source directly

‎docs/reference/search/request/fields.asciidoc

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,15 @@ For backwards compatibility, if the fields parameter specifies fields which are
3434
`false`), it will load the `_source` and extract it from it. This functionality has been replaced by the
3535
<<search-request-source-filtering,source filtering>> parameter.
3636

37+
Field values fetched from the document it self are always returned as an array. Metadata fields like `_routing` and
38+
`_parent` fields are never returned as an array.
39+
40+
Also only leaf fields can be returned via the `field` option. So object fields can't be returned and such requests
41+
will fail.
42+
3743
Script fields can also be automatically detected and used as fields, so
38-
things like `_source.obj1.obj2` can be used, though not recommended, as
39-
`obj1.obj2` will work as well.
44+
things like `_source.obj1.field1` can be used, though not recommended, as
45+
`obj1.field1` will work as well.
4046

4147
[[partial]]
4248
==== Partial

‎src/main/java/org/elasticsearch/index/get/GetField.java

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,20 @@
3434
public class GetField implements Streamable, Iterable<Object> {
3535

3636
private String name;
37-
37+
private boolean metadataField;
3838
private List<Object> values;
3939

4040
private GetField() {
41-
4241
}
4342

4443
public GetField(String name, List<Object> values) {
44+
this(name, values, false);
45+
}
46+
47+
public GetField(String name, List<Object> values, boolean metadataField) {
4548
this.name = name;
4649
this.values = values;
50+
this.metadataField = metadataField;
4751
}
4852

4953
public String getName() {
@@ -61,6 +65,10 @@ public List<Object> getValues() {
6165
return values;
6266
}
6367

68+
public boolean isMetadataField() {
69+
return metadataField;
70+
}
71+
6472
@Override
6573
public Iterator<Object> iterator() {
6674
return values.iterator();
@@ -75,6 +83,7 @@ public static GetField readGetField(StreamInput in) throws IOException {
7583
@Override
7684
public void readFrom(StreamInput in) throws IOException {
7785
name = in.readString();
86+
metadataField = in.readBoolean();
7887
int size = in.readVInt();
7988
values = new ArrayList<Object>(size);
8089
for (int i = 0; i < size; i++) {
@@ -85,6 +94,7 @@ public void readFrom(StreamInput in) throws IOException {
8594
@Override
8695
public void writeTo(StreamOutput out) throws IOException {
8796
out.writeString(name);
97+
out.writeBoolean(metadataField);
8898
out.writeVInt(values.size());
8999
for (Object obj : values) {
90100
out.writeGenericValue(obj);

‎src/main/java/org/elasticsearch/index/get/GetResult.java

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -219,13 +219,20 @@ public XContentBuilder toXContentEmbedded(XContentBuilder builder, Params params
219219
if (field.getValues().isEmpty()) {
220220
continue;
221221
}
222-
if (field.getValues().size() == 1) {
223-
builder.field(field.getName(), field.getValues().get(0));
222+
String fieldName = field.getName();
223+
if (field.isMetadataField()) {
224+
builder.field(fieldName, field.getValue());
224225
} else {
225-
builder.field(field.getName());
226-
builder.startArray();
226+
builder.startArray(field.getName());
227227
for (Object value : field.getValues()) {
228-
builder.value(value);
228+
if (value instanceof Iterable) {
229+
Iterable iterable = (Iterable) value;
230+
for (Object iterVal : iterable) {
231+
builder.value(iterVal);
232+
}
233+
} else {
234+
builder.value(value);
235+
}
229236
}
230237
builder.endArray();
231238
}

‎src/main/java/org/elasticsearch/index/get/ShardGetService.java

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.google.common.collect.Sets;
2424
import org.apache.lucene.index.Term;
2525
import org.elasticsearch.ElasticSearchException;
26+
import org.elasticsearch.ElasticSearchIllegalArgumentException;
2627
import org.elasticsearch.common.Nullable;
2728
import org.elasticsearch.common.bytes.BytesReference;
2829
import org.elasticsearch.common.collect.Tuple;
@@ -268,8 +269,13 @@ public GetResult innerGet(String type, String id, String[] gFields, boolean real
268269
}
269270

270271
FieldMapper<?> x = docMapper.mappers().smartNameFieldMapper(field);
271-
// only if the field is stored or source is enabled we should add it..
272-
if (docMapper.sourceMapper().enabled() || x == null || x.fieldType().stored()) {
272+
if (x == null) {
273+
if (docMapper.objectMappers().get(field) != null) {
274+
// Only fail if we know it is a object field, missing paths / fields shouldn't fail.
275+
throw new ElasticSearchIllegalArgumentException("field [" + field + "] isn't a leaf field");
276+
}
277+
} else if (docMapper.sourceMapper().enabled() || x.fieldType().stored()) {
278+
// only if the field is stored or source is enabled we should add it..
273279
value = searchLookup.source().extractValue(field);
274280
// normalize the data if needed (mainly for binary fields, to convert from base64 strings to bytes)
275281
if (value != null && x != null) {
@@ -292,7 +298,7 @@ public GetResult innerGet(String type, String id, String[] gFields, boolean real
292298
if (value instanceof List) {
293299
fields.put(field, new GetField(field, (List) value));
294300
} else {
295-
fields.put(field, new GetField(field, ImmutableList.of(value)));
301+
fields.put(field, new GetField(field, ImmutableList.of(value), mapperService.isMetadataField(field)));
296302
}
297303
}
298304
}
@@ -358,7 +364,8 @@ private GetResult innerGetLoadFromStoredFields(String type, String id, String[]
358364
fieldVisitor.postProcess(docMapper);
359365
fields = new HashMap<String, GetField>(fieldVisitor.fields().size());
360366
for (Map.Entry<String, List<Object>> entry : fieldVisitor.fields().entrySet()) {
361-
fields.put(entry.getKey(), new GetField(entry.getKey(), entry.getValue()));
367+
boolean isMetadataField = mapperService.isMetadataField(entry.getKey());
368+
fields.put(entry.getKey(), new GetField(entry.getKey(), entry.getValue(), isMetadataField));
362369
}
363370
}
364371
}
@@ -388,7 +395,12 @@ private GetResult innerGetLoadFromStoredFields(String type, String id, String[]
388395
}
389396
} else {
390397
FieldMappers x = docMapper.mappers().smartName(field);
391-
if (x == null || !x.mapper().fieldType().stored()) {
398+
if (x == null) {
399+
if (docMapper.objectMappers().get(field) != null) {
400+
// Only fail if we know it is a object field, missing paths / fields shouldn't fail.
401+
throw new ElasticSearchIllegalArgumentException("field [" + field + "] isn't a leaf field");
402+
}
403+
} else if (!x.mapper().fieldType().stored()) {
392404
if (searchLookup == null) {
393405
searchLookup = new SearchLookup(mapperService, fieldDataService, new String[]{type});
394406
searchLookup.setNextReader(docIdAndVersion.context);
@@ -397,7 +409,7 @@ private GetResult innerGetLoadFromStoredFields(String type, String id, String[]
397409
}
398410
value = searchLookup.source().extractValue(field);
399411
// normalize the data if needed (mainly for binary fields, to convert from base64 strings to bytes)
400-
if (value != null && x != null) {
412+
if (value != null) {
401413
if (value instanceof List) {
402414
List list = (List) value;
403415
for (int i = 0; i < list.size(); i++) {

‎src/main/java/org/elasticsearch/index/mapper/MapperService.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
package org.elasticsearch.index.mapper;
2121

22+
import com.carrotsearch.hppc.ObjectOpenHashSet;
2223
import com.google.common.base.Charsets;
2324
import com.google.common.collect.*;
2425
import org.apache.lucene.analysis.Analyzer;
@@ -75,6 +76,10 @@
7576
public class MapperService extends AbstractIndexComponent implements Iterable<DocumentMapper> {
7677

7778
public static final String DEFAULT_MAPPING = "_default_";
79+
private static ObjectOpenHashSet<String> META_FIELDS = ObjectOpenHashSet.from(
80+
"_uid", "_id", "_type", "_all", "_analyzer", "_boost", "_parent", "_routing", "_index",
81+
"_size", "_timestamp", "_ttl"
82+
);
7883

7984
private final AnalysisService analysisService;
8085
private final IndexFieldDataService fieldDataService;
@@ -841,6 +846,13 @@ public ObjectMapper resolveClosestNestedObjectMapper(String fieldName) {
841846
return null;
842847
}
843848

849+
/**
850+
* @return Whether a field is a metadata field.
851+
*/
852+
public boolean isMetadataField(String fieldName) {
853+
return META_FIELDS.contains(fieldName);
854+
}
855+
844856
public static class SmartNameObjectMapper {
845857
private final ObjectMapper mapper;
846858
private final DocumentMapper docMapper;

‎src/main/java/org/elasticsearch/search/SearchHitField.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,4 +59,9 @@ public interface SearchHitField extends Streamable, Iterable<Object> {
5959
* The field values.
6060
*/
6161
List<Object> getValues();
62+
63+
/**
64+
* @return The field is a metadata field
65+
*/
66+
boolean isMetadataField();
6267
}

‎src/main/java/org/elasticsearch/search/fetch/FetchPhase.java

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.google.common.collect.ImmutableMap;
2323
import org.apache.lucene.index.AtomicReaderContext;
2424
import org.apache.lucene.index.ReaderUtil;
25+
import org.elasticsearch.ElasticSearchIllegalArgumentException;
2526
import org.elasticsearch.common.inject.Inject;
2627
import org.elasticsearch.common.text.StringAndBytesText;
2728
import org.elasticsearch.common.text.Text;
@@ -117,7 +118,12 @@ public void execute(SearchContext context) {
117118
continue;
118119
}
119120
FieldMappers x = context.smartNameFieldMappers(fieldName);
120-
if (x != null && x.mapper().fieldType().stored()) {
121+
if (x == null) {
122+
// Only fail if we know it is a object field, missing paths / fields shouldn't fail.
123+
if (context.smartNameObjectMapper(fieldName) != null) {
124+
throw new ElasticSearchIllegalArgumentException("field [" + fieldName + "] isn't a leaf field");
125+
}
126+
} else if (x.mapper().fieldType().stored()) {
121127
if (fieldNames == null) {
122128
fieldNames = new HashSet<String>();
123129
}
@@ -153,7 +159,8 @@ public void execute(SearchContext context) {
153159
if (!fieldsVisitor.fields().isEmpty()) {
154160
searchFields = new HashMap<String, SearchHitField>(fieldsVisitor.fields().size());
155161
for (Map.Entry<String, List<Object>> entry : fieldsVisitor.fields().entrySet()) {
156-
searchFields.put(entry.getKey(), new InternalSearchHitField(entry.getKey(), entry.getValue()));
162+
boolean isMetadataField = context.mapperService().isMetadataField(entry.getKey());
163+
searchFields.put(entry.getKey(), new InternalSearchHitField(entry.getKey(), entry.getValue(), isMetadataField));
157164
}
158165
}
159166

@@ -191,7 +198,14 @@ public void execute(SearchContext context) {
191198
hitField = new InternalSearchHitField(extractFieldName, new ArrayList<Object>(2));
192199
searchHit.fields().put(extractFieldName, hitField);
193200
}
194-
hitField.values().add(value);
201+
if (value instanceof Iterable) {
202+
Iterable iterable = (Iterable) value;
203+
for (Object iterVal : iterable) {
204+
hitField.values().add(iterVal);
205+
}
206+
} else {
207+
hitField.values().add(value);
208+
}
195209
}
196210
}
197211
}

‎src/main/java/org/elasticsearch/search/internal/InternalSearchHit.java

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -415,13 +415,20 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
415415
if (field.values().isEmpty()) {
416416
continue;
417417
}
418-
if (field.values().size() == 1) {
419-
builder.field(field.name(), field.values().get(0));
418+
String fieldName = field.getName();
419+
if (field.isMetadataField()) {
420+
builder.field(fieldName, field.value());
420421
} else {
421-
builder.field(field.name());
422-
builder.startArray();
423-
for (Object value : field.values()) {
424-
builder.value(value);
422+
builder.startArray(fieldName);
423+
for (Object value : field.getValues()) {
424+
if (value instanceof Iterable) {
425+
Iterable iterable = (Iterable) value;
426+
for (Object iterVal : iterable) {
427+
builder.value(iterVal);
428+
}
429+
} else {
430+
builder.value(value);
431+
}
425432
}
426433
builder.endArray();
427434
}

‎src/main/java/org/elasticsearch/search/internal/InternalSearchHitField.java

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,19 @@
3434
public class InternalSearchHitField implements SearchHitField {
3535

3636
private String name;
37-
37+
private boolean metadataField;
3838
private List<Object> values;
3939

4040
private InternalSearchHitField() {
41-
4241
}
4342

4443
public InternalSearchHitField(String name, List<Object> values) {
44+
this(name, values, false);
45+
}
46+
47+
public InternalSearchHitField(String name, List<Object> values, boolean metadataField) {
4548
this.name = name;
49+
this.metadataField = metadataField;
4650
this.values = values;
4751
}
4852

@@ -77,6 +81,10 @@ public List<Object> getValues() {
7781
return values();
7882
}
7983

84+
@Override
85+
public boolean isMetadataField() {
86+
return metadataField;
87+
}
8088

8189
@Override
8290
public Iterator<Object> iterator() {
@@ -92,6 +100,7 @@ public static InternalSearchHitField readSearchHitField(StreamInput in) throws I
92100
@Override
93101
public void readFrom(StreamInput in) throws IOException {
94102
name = in.readSharedString();
103+
metadataField = in.readBoolean();
95104
int size = in.readVInt();
96105
values = new ArrayList<Object>(size);
97106
for (int i = 0; i < size; i++) {
@@ -102,6 +111,7 @@ public void readFrom(StreamInput in) throws IOException {
102111
@Override
103112
public void writeTo(StreamOutput out) throws IOException {
104113
out.writeSharedString(name);
114+
out.writeBoolean(metadataField);
105115
out.writeVInt(values.size());
106116
for (Object value : values) {
107117
out.writeGenericValue(value);

0 commit comments

Comments
 (0)