Improves doc values format deprecation message (#33576)

* Improves doc values format deprecation message This changes the deprecation message when doc values fields do not supply a format form logging a deprecation warning for each offending field individually to logging a single message which lists all offending fields Closes #33572 * Updates YAML test with new deprecation message Also adds a test to ensure multiple deprecation warnings are collated into one message * Condenses collection of fields without format check Moves the collection of fields that don't have a format to a separate loop and moves the logging of the deprecation warning to be next to it at the expesnse of looping through the field list twice * fixes typo * Fixes test
elastic · Sep 11, 2018 · 624b84f · 624b84f
1 parent 73c75be
commit 624b84f
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 6 deletions.
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/10_source_filtering.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/10_source_filtering.yml
@@ -139,12 +139,26 @@ setup:
       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.'
+        - 'There are doc-value fields which are 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 a doc value field in order to opt in for the future behaviour and ease the migration to 7.0: [count]'
       search:
         body:
           docvalue_fields: [ "count" ]
   - match: { hits.hits.0.fields.count: [1] }
 
+---
+"multiple docvalue_fields":
+  - skip:
+      version: " - 6.3.99"
+      reason: format option was added in 6.4
+      features: warnings
+  - do:
+      warnings:
+        - 'There are doc-value fields which are 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 a doc value field in order to opt in for the future behaviour and ease the migration to 7.0: [count, include.field1.keyword]'
+      search:
+        body:
+          docvalue_fields: [ "count", "include.field1.keyword" ]
+  - match: { hits.hits.0.fields.count: [1] }
+
 ---
 "docvalue_fields as url param":
   - skip:
@@ -153,7 +167,7 @@ setup:
       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.'
+        - 'There are doc-value fields which are 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 a doc value field in order to opt in for the future behaviour and ease the migration to 7.0: [count]'
       search:
         docvalue_fields: [ "count" ]
   - match: { hits.hits.0.fields.count: [1] }

diff --git a/...er/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java b/...er/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java
@@ -46,6 +46,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Objects;
+import java.util.stream.Collectors;
 
 /**
  * Query sub phase which pulls data from doc values
@@ -77,17 +78,22 @@ public void hitsExecute(SearchContext context, SearchHit[] hits) throws IOExcept
         hits = hits.clone(); // don't modify the incoming hits
         Arrays.sort(hits, Comparator.comparingInt(SearchHit::docId));
 
+        List<String> noFormatFields = context.docValueFieldsContext().fields().stream().filter(f -> f.format == null).map(f -> f.field)
+                .collect(Collectors.toList());
+        if (noFormatFields.isEmpty() == false) {
+            DEPRECATION_LOGGER.deprecated("There are doc-value fields which are 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={}] with a doc value field in order to opt in for the future behaviour and ease the migration to "
+                    + "7.0: {}", DocValueFieldsContext.USE_DEFAULT_FORMAT, noFormatFields);
+        }
+
         for (FieldAndFormat fieldAndFormat : context.docValueFieldsContext().fields()) {
             String field = fieldAndFormat.field;
             MappedFieldType fieldType = context.mapperService().fullName(field);
             if (fieldType != null) {
                 final IndexFieldData<?> indexFieldData = context.getForField(fieldType);
                 final DocValueFormat format;
                 if (fieldAndFormat.format == null) {
-                    DEPRECATION_LOGGER.deprecated("Doc-value field [" + fieldAndFormat.field + "] 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={}] with the doc value field in order to opt in for the future behaviour and ease the migration to " +
-                            "7.0.", DocValueFieldsContext.USE_DEFAULT_FORMAT);
                     format = null;
                 } else {
                     String formatDesc = fieldAndFormat.format;