Add a format option to docvalue_fields. (#29639)

This commit adds the ability to configure how a docvalue field should be
formatted, so that it would be possible eg. to return a date field
formatted as the number of milliseconds since Epoch.

Closes #27740

Author: jpountz

docs/reference/search/request/docvalue-fields.asciidoc

@@ -11,13 +11,38 @@ GET /_search
     "query" : {
         "match_all": {}
     },
-    "docvalue_fields" : ["test1", "test2"]
+    "docvalue_fields" : [
+        {
+            "field": "my_ip_field", <1>
+            "format": "use_field_mapping" <2>
+        },
+        {
+            "field": "my_date_field",
+            "format": "epoch_millis" <3>
+        }
+    ]
 }
 --------------------------------------------------
 // CONSOLE
+<1> the name of the field
+<2> the special `use_field_mapping` format tells Elasticsearch to use the format from the mapping
+<3> date fields may use a custom format
 
 Doc value fields can work on fields that are not stored.
 
 Note that if the fields parameter specifies fields without docvalues it will try to load the value from the fielddata cache
 causing the terms for that field to be loaded to memory (cached), which will result in more memory consumption.
 
+[float]
+==== Custom formats
+
+While most fields do not support custom formats, some of them do:
+ - <<date,Date>> fields can take any <<mapping-date-format,date format>>.
+ - <<number,Numeric>> fields accept a https://docs.oracle.com/javase/8/docs/api/java/text/DecimalFormat.html[DecimalFormat pattern].
+
+All fields support the special `use_field_mapping` format, which tells
+Elasticsearch to use the mappings to figure out a default format.
+
+NOTE: The default is currently to return the same output as
+<<search-request-script-fields,script fields>>. However it will change in 7.0
+to behave as if the `use_field_mapping` format was provided.

docs/reference/search/request/inner-hits.asciidoc

@@ -242,7 +242,12 @@ POST test/_search
       },
       "inner_hits": {
         "_source" : false,
-        "docvalue_fields" : ["comments.text.keyword"]
+        "docvalue_fields" : [
+          {
+            "field": "comments.text.keyword",
+            "format": "use_field_mapping"
+          }
+        ]
       }
     }
   }

rest-api-spec/src/main/resources/rest-api-spec/test/search.inner_hits/10_basic.yml

@@ -45,9 +45,8 @@ setup:
 "Nested doc version and seqIDs":
 
     - skip:
-        # fixed in 6.0.1
-        version: " - 6.0.0"
-        reason:  "version and seq IDs where not accurate in previous versions"
+        version: " - 6.99.99" # TODO change to 6.3.99 on backport
+        reason:  "object notation for docvalue_fields was introduced in 6.4"
 
     - do:
         index:
@@ -61,7 +60,7 @@ setup:
 
     - do:
         search:
-          body: { "query" : { "nested" : { "path" : "nested_field", "query" : { "match_all" : {} }, "inner_hits" : { version: true, "docvalue_fields": ["_seq_no"]} }}, "version": true, "docvalue_fields" : ["_seq_no"] }
+          body: { "query" : { "nested" : { "path" : "nested_field", "query" : { "match_all" : {} }, "inner_hits" : { version: true, "docvalue_fields": [ { "field": "_seq_no", "format": "use_field_mapping" } ]} }}, "version": true, "docvalue_fields" : [ { "field": "_seq_no", "format": "use_field_mapping" } ] }
 
     - match: { hits.total: 1 }
     - match: { hits.hits.0._index: "test" }
@@ -84,7 +83,7 @@ setup:
 
     - do:
         search:
-          body: { "query" : { "nested" : { "path" : "nested_field", "query" : { "match_all" : {} }, "inner_hits" : { version: true, "docvalue_fields": ["_seq_no"]} }}, "version": true, "docvalue_fields" : ["_seq_no"] }
+          body: { "query" : { "nested" : { "path" : "nested_field", "query" : { "match_all" : {} }, "inner_hits" : { version: true, "docvalue_fields": [ { "field": "_seq_no", "format": "use_field_mapping" } ]} }}, "version": true, "docvalue_fields" : [ { "field": "_seq_no", "format": "use_field_mapping" } ] }
 
     - match: { hits.total: 1 }
     - match: { hits.hits.0._index: "test" }

rest-api-spec/src/main/resources/rest-api-spec/test/search/10_source_filtering.yml

@@ -133,7 +133,53 @@ setup:
 
 ---
 "docvalue_fields":
+  - skip:
+      version: " - 6.99.99" # TODO: change version on backport
+      reason: format option was added in 6.4
+      features: warnings
   - do:
+      warnings:
+        - 'Doc-value field [count] is not using a format. The output will change in 7.0 when doc value fields get formatted based on mappings by default. It is recommended to pass [format=use_field_mapping] with the doc value field in order to opt in for the future behaviour and ease the migration to 7.0.'
+      search:
+        body:
+          docvalue_fields: [ "count" ]
+  - match: { hits.hits.0.fields.count: [1] }
+
+---
+"docvalue_fields as url param":
+  - skip:
+      version: " - 6.99.99" # TODO: change version on backport
+      reason: format option was added in 6.4
+      features: warnings
+  - do:
+      warnings:
+        - 'Doc-value field [count] is not using a format. The output will change in 7.0 when doc value fields get formatted based on mappings by default. It is recommended to pass [format=use_field_mapping] with the doc value field in order to opt in for the future behaviour and ease the migration to 7.0.'
       search:
         docvalue_fields: [ "count" ]
   - match: { hits.hits.0.fields.count: [1] }
+
+---
+"docvalue_fields with default format":
+  - skip:
+      version: " - 6.99.99" # TODO: change version on backport
+      reason: format option was added in 6.4
+  - do:
+      search:
+        body:
+          docvalue_fields:
+            - field: "count"
+              format: "use_field_mapping"
+  - match: { hits.hits.0.fields.count: [1] }
+
+---
+"docvalue_fields with explicit format":
+  - skip:
+      version: " - 6.99.99" # TODO: change version on backport
+      reason: format option was added in 6.4
+  - do:
+      search:
+        body:
+          docvalue_fields:
+            - field: "count"
+              format: "#.0"
+  - match: { hits.hits.0.fields.count: ["1.0"] }

rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yml

@@ -64,14 +64,23 @@ setup:
             settings:
               index.max_docvalue_fields_search: 2
 
+  - skip:
+      version: " - 6.99.99" # TODO: change to 6.3.99 on backport
+      reason: "The object notation for docvalue_fields is only supported on 6.4+"
   - do:
       catch:      /Trying to retrieve too many docvalue_fields\. Must be less than or equal to[:] \[2\] but was \[3\]\. This limit can be set by changing the \[index.max_docvalue_fields_search\] index level setting\./
       search:
         index: test_2
         body:
           query:
             match_all: {}
-          docvalue_fields: ["one", "two", "three"]
+          docvalue_fields:
+            - field: "one"
+              format: "use_field_mapping"
+            - field: "two"
+              format: "use_field_mapping"
+            - field: "three"
+              format: "use_field_mapping"
 
 ---
 "Script_fields size limit":

server/src/main/java/org/elasticsearch/action/search/ExpandSearchPhase.java

@@ -153,7 +153,7 @@ private SearchSourceBuilder buildExpandSearchSourceBuilder(InnerHitBuilder optio
             }
         }
         if (options.getDocValueFields() != null) {
-            options.getDocValueFields().forEach(groupSource::docValueField);
+            options.getDocValueFields().forEach(ff -> groupSource.docValueField(ff.field, ff.format));
         }
         if (options.getStoredFieldsContext() != null && options.getStoredFieldsContext().fieldNames() != null) {
             options.getStoredFieldsContext().fieldNames().forEach(groupSource::storedField);

server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java

@@ -288,11 +288,21 @@ public SearchRequestBuilder setFetchSource(@Nullable String[] includes, @Nullabl
      *
      * @param name The field to get from the docvalue
      */
-    public SearchRequestBuilder addDocValueField(String name) {
-        sourceBuilder().docValueField(name);
+    public SearchRequestBuilder addDocValueField(String name, String format) {
+        sourceBuilder().docValueField(name, format);
         return this;
     }
 
+    /**
+     * Adds a docvalue based field to load and return. The field does not have to be stored,
+     * but its recommended to use non analyzed or numeric fields.
+     *
+     * @param name The field to get from the docvalue
+     */
+    public SearchRequestBuilder addDocValueField(String name) {
+        return addDocValueField(name, null);
+    }
+
     /**
      * Adds a stored field to load and return (note, it must be stored) as part of the search request.
      */

server/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java

@@ -33,6 +33,7 @@
 import org.elasticsearch.search.builder.SearchSourceBuilder;
 import org.elasticsearch.search.builder.SearchSourceBuilder.ScriptField;
 import org.elasticsearch.search.fetch.StoredFieldsContext;
+import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext.FieldAndFormat;
 import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
 import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilder;
 import org.elasticsearch.search.sort.SortBuilder;
@@ -45,6 +46,7 @@
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import static org.elasticsearch.common.xcontent.XContentParser.Token.END_OBJECT;
 
@@ -65,7 +67,8 @@ public final class InnerHitBuilder implements Writeable, ToXContentObject {
         PARSER.declareBoolean(InnerHitBuilder::setVersion, SearchSourceBuilder.VERSION_FIELD);
         PARSER.declareBoolean(InnerHitBuilder::setTrackScores, SearchSourceBuilder.TRACK_SCORES_FIELD);
         PARSER.declareStringArray(InnerHitBuilder::setStoredFieldNames, SearchSourceBuilder.STORED_FIELDS_FIELD);
-        PARSER.declareStringArray(InnerHitBuilder::setDocValueFields, SearchSourceBuilder.DOCVALUE_FIELDS_FIELD);
+        PARSER.declareObjectArray(InnerHitBuilder::setDocValueFields,
+                (p,c) -> FieldAndFormat.fromXContent(p), SearchSourceBuilder.DOCVALUE_FIELDS_FIELD);
         PARSER.declareField((p, i, c) -> {
             try {
                 Set<ScriptField> scriptFields = new HashSet<>();
@@ -102,7 +105,7 @@ public final class InnerHitBuilder implements Writeable, ToXContentObject {
     private StoredFieldsContext storedFieldsContext;
     private QueryBuilder query = DEFAULT_INNER_HIT_QUERY;
     private List<SortBuilder<?>> sorts;
-    private List<String> docValueFields;
+    private List<FieldAndFormat> docValueFields;
     private Set<ScriptField> scriptFields;
     private HighlightBuilder highlightBuilder;
     private FetchSourceContext fetchSourceContext;
@@ -134,7 +137,18 @@ public InnerHitBuilder(StreamInput in) throws IOException {
         version = in.readBoolean();
         trackScores = in.readBoolean();
         storedFieldsContext = in.readOptionalWriteable(StoredFieldsContext::new);
-        docValueFields = (List<String>) in.readGenericValue();
+        if (in.getVersion().before(Version.V_7_0_0_alpha1)) { // TODO: change to 6.4.0 after backport
+            List<String> fieldList = (List<String>) in.readGenericValue();
+            if (fieldList == null) {
+                docValueFields = null;
+            } else {
+                docValueFields = fieldList.stream()
+                        .map(field -> new FieldAndFormat(field, null))
+                        .collect(Collectors.toList());
+            }
+        } else {
+            docValueFields = in.readBoolean() ? in.readList(FieldAndFormat::new) : null;
+        }
         if (in.readBoolean()) {
             int size = in.readVInt();
             scriptFields = new HashSet<>(size);
@@ -174,7 +188,16 @@ public void writeTo(StreamOutput out) throws IOException {
         out.writeBoolean(version);
         out.writeBoolean(trackScores);
         out.writeOptionalWriteable(storedFieldsContext);
-        out.writeGenericValue(docValueFields);
+        if (out.getVersion().before(Version.V_7_0_0_alpha1)) { // TODO: change to 6.4.0 after backport
+            out.writeGenericValue(docValueFields == null
+                    ? null
+                    : docValueFields.stream().map(ff -> ff.field).collect(Collectors.toList()));
+        } else {
+            out.writeBoolean(docValueFields != null);
+            if (docValueFields != null) {
+                out.writeList(docValueFields);
+            }
+        }
         boolean hasScriptFields = scriptFields != null;
         out.writeBoolean(hasScriptFields);
         if (hasScriptFields) {
@@ -248,7 +271,9 @@ private void writeToBWC(StreamOutput out,
         out.writeBoolean(version);
         out.writeBoolean(trackScores);
         out.writeOptionalWriteable(storedFieldsContext);
-        out.writeGenericValue(docValueFields);
+        out.writeGenericValue(docValueFields == null
+                ? null
+                : docValueFields.stream().map(ff -> ff.field).collect(Collectors.toList()));
         boolean hasScriptFields = scriptFields != null;
         out.writeBoolean(hasScriptFields);
         if (hasScriptFields) {
@@ -390,29 +415,36 @@ public InnerHitBuilder setStoredFieldNames(List<String> fieldNames) {
     /**
      * Gets the docvalue fields.
      */
-    public List<String> getDocValueFields() {
+    public List<FieldAndFormat> getDocValueFields() {
         return docValueFields;
     }
 
     /**
      * Sets the stored fields to load from the docvalue and return.
      */
-    public InnerHitBuilder setDocValueFields(List<String> docValueFields) {
+    public InnerHitBuilder setDocValueFields(List<FieldAndFormat> docValueFields) {
         this.docValueFields = docValueFields;
         return this;
     }
 
     /**
      * Adds a field to load from the docvalue and return.
      */
-    public InnerHitBuilder addDocValueField(String field) {
+    public InnerHitBuilder addDocValueField(String field, String format) {
         if (docValueFields == null) {
             docValueFields = new ArrayList<>();
         }
-        docValueFields.add(field);
+        docValueFields.add(new FieldAndFormat(field, null));
         return this;
     }
 
+    /**
+     * Adds a field to load from doc values and return.
+     */
+    public InnerHitBuilder addDocValueField(String field) {
+        return addDocValueField(field, null);
+    }
+
     public Set<ScriptField> getScriptFields() {
         return scriptFields;
     }
@@ -489,8 +521,15 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         }
         if (docValueFields != null) {
             builder.startArray(SearchSourceBuilder.DOCVALUE_FIELDS_FIELD.getPreferredName());
-            for (String docValueField : docValueFields) {
-                builder.value(docValueField);
+            for (FieldAndFormat docValueField : docValueFields) {
+                if (docValueField.format == null) {
+                    builder.value(docValueField.field);
+                } else {
+                    builder.startObject()
+                        .field("field", docValueField.field)
+                        .field("format", docValueField.format)
+                        .endObject();
+                }
             }
             builder.endArray();
         }

server/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java

@@ -202,7 +202,7 @@ private static void parseSearchSource(final SearchSourceBuilder searchSourceBuil
             if (Strings.hasText(sDocValueFields)) {
                 String[] sFields = Strings.splitStringByCommaToArray(sDocValueFields);
                 for (String field : sFields) {
-                    searchSourceBuilder.docValueField(field);
+                    searchSourceBuilder.docValueField(field, null);
                 }
             }
         }