Skip to content

Commit

Permalink
Fixed naming inconsistency for fields/stored_fields in the APIs (#20166)
Browse files Browse the repository at this point in the history
This change replaces the fields parameter with stored_fields when it makes sense.
This is dictated by the renaming we made in #18943 for the search API.

The following list of endpoint has been changed to use `stored_fields` instead of `fields`:
* get
* mget
* explain

The documentation and the rest API spec has been updated to cope with the changes for the following APIs:
* delete_by_query
* get
* mget
* explain

The `fields` parameter has been deprecated for the following APIs (it is replaced by _source filtering):
* update: the fields are extracted from the _source directly.
* bulk: the fields parameter is used but fields are extracted from the source directly so it is allowed to have non-stored fields.

Some APIs still have the `fields` parameter for various reasons:
* cat.fielddata: the fields paramaters relates to the fielddata fields that should be printed.
* indices.clear_cache: used to indicate which fielddata fields should be cleared.
* indices.get_field_mapping: used to filter fields in the mapping.
* indices.stats: get stats on fields (stored or not stored).
* termvectors: fields are retrieved from the stored fields if possible and extracted from the _source otherwise.
* mtermvectors:
* nodes.stats: the fields parameter is used to concatenate completion_fields and fielddata_fields so it's not related to stored_fields at all.

Fixes #20155
  • Loading branch information
jimczi authored Sep 13, 2016
1 parent fbe2766 commit 1764ec5
Show file tree
Hide file tree
Showing 80 changed files with 918 additions and 496 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public void handleRequest(final RestRequest request, final RestChannel channel,
}
bulkRequest.timeout(request.paramAsTime("timeout", BulkShardRequest.DEFAULT_TIMEOUT));
bulkRequest.setRefreshPolicy(request.param("refresh"));
bulkRequest.add(request.content(), defaultIndex, defaultType, defaultRouting, defaultFields, defaultPipeline, null, true);
bulkRequest.add(request.content(), defaultIndex, defaultType, defaultRouting, defaultFields, null, defaultPipeline, null, true);

// short circuit the call to the transport layer
BulkRestBuilderListener listener = new BulkRestBuilderListener(channel, request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ public BulkProcessor add(BytesReference data, @Nullable String defaultIndex, @Nu
}

public synchronized BulkProcessor add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType, @Nullable String defaultPipeline, @Nullable Object payload) throws Exception {
bulkRequest.add(data, defaultIndex, defaultType, null, null, defaultPipeline, payload, true);
bulkRequest.add(data, defaultIndex, defaultType, null, null, null, defaultPipeline, payload, true);
executeIfNeeded();
return this;
}
Expand Down
22 changes: 18 additions & 4 deletions core/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,15 @@
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.lucene.uid.Versions;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.XContent;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.VersionType;
import org.elasticsearch.search.fetch.subphase.FetchSourceContext;

import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -57,6 +60,8 @@
* @see org.elasticsearch.client.Client#bulk(BulkRequest)
*/
public class BulkRequest extends ActionRequest<BulkRequest> implements CompositeIndicesRequest, WriteRequest<BulkRequest> {
private static final DeprecationLogger DEPRECATION_LOGGER =
new DeprecationLogger(Loggers.getLogger(BulkRequest.class));

private static final int REQUEST_OVERHEAD = 50;

Expand Down Expand Up @@ -257,17 +262,17 @@ public BulkRequest add(byte[] data, int from, int length, @Nullable String defau
* Adds a framed data in binary format
*/
public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType) throws Exception {
return add(data, defaultIndex, defaultType, null, null, null, null, true);
return add(data, defaultIndex, defaultType, null, null, null, null, null, true);
}

/**
* Adds a framed data in binary format
*/
public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType, boolean allowExplicitIndex) throws Exception {
return add(data, defaultIndex, defaultType, null, null, null, null, allowExplicitIndex);
return add(data, defaultIndex, defaultType, null, null, null, null, null, allowExplicitIndex);
}

public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType, @Nullable String defaultRouting, @Nullable String[] defaultFields, @Nullable String defaultPipeline, @Nullable Object payload, boolean allowExplicitIndex) throws Exception {
public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType, @Nullable String defaultRouting, @Nullable String[] defaultFields, @Nullable FetchSourceContext defaultFetchSourceContext, @Nullable String defaultPipeline, @Nullable Object payload, boolean allowExplicitIndex) throws Exception {
XContent xContent = XContentFactory.xContent(data);
int line = 0;
int from = 0;
Expand Down Expand Up @@ -301,6 +306,7 @@ public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Null
String id = null;
String routing = defaultRouting;
String parent = null;
FetchSourceContext fetchSourceContext = defaultFetchSourceContext;
String[] fields = defaultFields;
String timestamp = null;
TimeValue ttl = null;
Expand Down Expand Up @@ -353,16 +359,21 @@ public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Null
pipeline = parser.text();
} else if ("fields".equals(currentFieldName)) {
throw new IllegalArgumentException("Action/metadata line [" + line + "] contains a simple value for parameter [fields] while a list is expected");
} else if ("_source".equals(currentFieldName)) {
fetchSourceContext = FetchSourceContext.parse(parser);
} else {
throw new IllegalArgumentException("Action/metadata line [" + line + "] contains an unknown parameter [" + currentFieldName + "]");
}
} else if (token == XContentParser.Token.START_ARRAY) {
if ("fields".equals(currentFieldName)) {
DEPRECATION_LOGGER.deprecated("Deprecated field [fields] used, expected [_source] instead");
List<Object> values = parser.list();
fields = values.toArray(new String[values.size()]);
} else {
throw new IllegalArgumentException("Malformed action/metadata line [" + line + "], expected a simple value for field [" + currentFieldName + "] but found [" + token + "]");
}
} else if (token == XContentParser.Token.START_OBJECT && "_source".equals(currentFieldName)) {
fetchSourceContext = FetchSourceContext.parse(parser);
} else if (token != XContentParser.Token.VALUE_NULL) {
throw new IllegalArgumentException("Malformed action/metadata line [" + line + "], expected a simple value for field [" + currentFieldName + "] but found [" + token + "]");
}
Expand Down Expand Up @@ -402,7 +413,10 @@ public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Null
.version(version).versionType(versionType)
.routing(routing)
.parent(parent)
.source(data.slice(from, nextMarker - from));
.fromXContent(data.slice(from, nextMarker - from));
if (fetchSourceContext != null) {
updateRequest.fetchSource(fetchSourceContext);
}
if (fields != null) {
updateRequest.fields(fields);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,8 @@ private Tuple<Translog.Location, BulkItemRequest> update(IndexMetaData metaData,
// add the response
IndexResponse indexResponse = result.getResponse();
UpdateResponse updateResponse = new UpdateResponse(indexResponse.getShardInfo(), indexResponse.getShardId(), indexResponse.getType(), indexResponse.getId(), indexResponse.getVersion(), indexResponse.getResult());
if (updateRequest.fields() != null && updateRequest.fields().length > 0) {
if ((updateRequest.fetchSource() != null && updateRequest.fetchSource().fetchSource()) ||
(updateRequest.fields() != null && updateRequest.fields().length > 0)) {
Tuple<XContentType, Map<String, Object>> sourceAndContent = XContentHelper.convertToMap(indexSourceAsBytes, true);
updateResponse.setGetResult(updateHelper.extractGetResult(updateRequest, request.index(), indexResponse.getVersion(), sourceAndContent.v2(), sourceAndContent.v1(), indexSourceAsBytes));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class ExplainRequest extends SingleShardRequest<ExplainRequest> {
private String routing;
private String preference;
private QueryBuilder query;
private String[] fields;
private String[] storedFields;
private FetchSourceContext fetchSourceContext;

private String[] filteringAlias = Strings.EMPTY_ARRAY;
Expand Down Expand Up @@ -122,12 +122,12 @@ public FetchSourceContext fetchSourceContext() {
}


public String[] fields() {
return fields;
public String[] storedFields() {
return storedFields;
}

public ExplainRequest fields(String[] fields) {
this.fields = fields;
public ExplainRequest storedFields(String[] fields) {
this.storedFields = fields;
return this;
}

Expand Down Expand Up @@ -167,8 +167,8 @@ public void readFrom(StreamInput in) throws IOException {
preference = in.readOptionalString();
query = in.readNamedWriteable(QueryBuilder.class);
filteringAlias = in.readStringArray();
fields = in.readOptionalStringArray();
fetchSourceContext = in.readOptionalStreamable(FetchSourceContext::new);
storedFields = in.readOptionalStringArray();
fetchSourceContext = in.readOptionalWriteable(FetchSourceContext::new);
nowInMillis = in.readVLong();
}

Expand All @@ -181,8 +181,8 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeOptionalString(preference);
out.writeNamedWriteable(query);
out.writeStringArray(filteringAlias);
out.writeOptionalStringArray(fields);
out.writeOptionalStreamable(fetchSourceContext);
out.writeOptionalStringArray(storedFields);
out.writeOptionalWriteable(fetchSourceContext);
out.writeVLong(nowInMillis);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,10 @@ public ExplainRequestBuilder setQuery(QueryBuilder query) {
}

/**
* Explicitly specify the fields that will be returned for the explained document. By default, nothing is returned.
* Explicitly specify the stored fields that will be returned for the explained document. By default, nothing is returned.
*/
public ExplainRequestBuilder setFields(String... fields) {
request.fields(fields);
public ExplainRequestBuilder setStoredFields(String... fields) {
request.storedFields(fields);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,11 @@ protected ExplainResponse shardOperation(ExplainRequest request, ShardId shardId
Rescorer rescorer = ctx.rescorer();
explanation = rescorer.explain(topLevelDocId, context, ctx, explanation);
}
if (request.fields() != null || (request.fetchSourceContext() != null && request.fetchSourceContext().fetchSource())) {
if (request.storedFields() != null || (request.fetchSourceContext() != null && request.fetchSourceContext().fetchSource())) {
// Advantage is that we're not opening a second searcher to retrieve the _source. Also
// because we are working in the same searcher in engineGetResult we can be sure that a
// doc isn't deleted between the initial get and this call.
GetResult getResult = context.indexShard().getService().get(result, request.id(), request.type(), request.fields(),
request.fetchSourceContext());
GetResult getResult = context.indexShard().getService().get(result, request.id(), request.type(), request.storedFields(), request.fetchSourceContext());
return new ExplainResponse(shardId.getIndexName(), request.type(), request.id(), true, explanation, getResult);
} else {
return new ExplainResponse(shardId.getIndexName(), request.type(), request.id(), true, explanation);
Expand Down
35 changes: 11 additions & 24 deletions core/src/main/java/org/elasticsearch/action/get/GetRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public class GetRequest extends SingleShardRequest<GetRequest> implements Realti
private String parent;
private String preference;

private String[] fields;
private String[] storedFields;

private FetchSourceContext fetchSourceContext;

Expand Down Expand Up @@ -186,20 +186,20 @@ public FetchSourceContext fetchSourceContext() {
}

/**
* Explicitly specify the fields that will be returned. By default, the <tt>_source</tt>
* Explicitly specify the stored fields that will be returned. By default, the <tt>_source</tt>
* field will be returned.
*/
public GetRequest fields(String... fields) {
this.fields = fields;
public GetRequest storedFields(String... fields) {
this.storedFields = fields;
return this;
}

/**
* Explicitly specify the fields that will be returned. By default, the <tt>_source</tt>
* Explicitly specify the stored fields that will be returned. By default, the <tt>_source</tt>
* field will be returned.
*/
public String[] fields() {
return this.fields;
public String[] storedFields() {
return this.storedFields;
}

/**
Expand Down Expand Up @@ -260,18 +260,12 @@ public void readFrom(StreamInput in) throws IOException {
parent = in.readOptionalString();
preference = in.readOptionalString();
refresh = in.readBoolean();
int size = in.readInt();
if (size >= 0) {
fields = new String[size];
for (int i = 0; i < size; i++) {
fields[i] = in.readString();
}
}
storedFields = in.readOptionalStringArray();
realtime = in.readBoolean();

this.versionType = VersionType.fromValue(in.readByte());
this.version = in.readLong();
fetchSourceContext = in.readOptionalStreamable(FetchSourceContext::new);
fetchSourceContext = in.readOptionalWriteable(FetchSourceContext::new);
}

@Override
Expand All @@ -284,18 +278,11 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeOptionalString(preference);

out.writeBoolean(refresh);
if (fields == null) {
out.writeInt(-1);
} else {
out.writeInt(fields.length);
for (String field : fields) {
out.writeString(field);
}
}
out.writeOptionalStringArray(storedFields);
out.writeBoolean(realtime);
out.writeByte(versionType.getValue());
out.writeLong(version);
out.writeOptionalStreamable(fetchSourceContext);
out.writeOptionalWriteable(fetchSourceContext);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ public GetRequestBuilder setPreference(String preference) {
* Explicitly specify the fields that will be returned. By default, the <tt>_source</tt>
* field will be returned.
*/
public GetRequestBuilder setFields(String... fields) {
request.fields(fields);
public GetRequestBuilder setStoredFields(String... fields) {
request.storedFields(fields);
return this;
}

Expand Down
12 changes: 12 additions & 0 deletions core/src/main/java/org/elasticsearch/action/get/GetResponse.java
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,26 @@ public Map<String, Object> getSource() {
return getResult.getSource();
}

/**
* @deprecated Use {@link GetResponse#getSource()} instead
*/
@Deprecated
public Map<String, GetField> getFields() {
return getResult.getFields();
}

/**
* @deprecated Use {@link GetResponse#getSource()} instead
*/
@Deprecated
public GetField getField(String name) {
return getResult.field(name);
}

/**
* @deprecated Use {@link GetResponse#getSource()} instead
*/
@Deprecated
@Override
public Iterator<GetField> iterator() {
return getResult.iterator();
Expand Down
Loading

0 comments on commit 1764ec5

Please sign in to comment.