From c10fdc4e9f5265cd83ef0fe90bbb714bf5afa6bb Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Mon, 10 Sep 2018 18:45:26 +0100 Subject: [PATCH 1/5] 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 --- .../fetch/subphase/DocValueFieldsFetchSubPhase.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java index 3ef3064697a72..fa37290f312a8 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java @@ -76,6 +76,7 @@ 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 noFormatFields = new ArrayList<>(); for (FieldAndFormat fieldAndFormat : context.docValueFieldsContext().fields()) { String field = fieldAndFormat.field; @@ -84,10 +85,7 @@ public void hitsExecute(SearchContext context, SearchHit[] hits) throws IOExcept 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); + noFormatFields.add(fieldAndFormat.field); format = null; } else { String formatDesc = fieldAndFormat.format; @@ -158,5 +156,12 @@ public void hitsExecute(SearchContext context, SearchHit[] hits) throws IOExcept } } } + + if (noFormatFields.isEmpty() == false) { + DEPRECATION_LOGGER.deprecated("There are doc-value field 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); + } } } From 92033311b924e781b6999259b2e436bb7104661e Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Mon, 10 Sep 2018 20:58:39 +0100 Subject: [PATCH 2/5] Updates YAML test with new deprecation message Also adds a test to ensure multiple deprecation warnings are collated into one message --- .../test/search/10_source_filtering.yml | 18 ++++++++++++++++-- .../subphase/DocValueFieldsFetchSubPhase.java | 2 +- 2 files changed, 17 insertions(+), 3 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 index 59692873cc456..190b176fdeaba 100644 --- 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 field 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 field 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]' + search: + body: + docvalue_fields: [ "count", "include.field1" ] + - 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 field 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/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java index fa37290f312a8..57ed1e9ecc36a 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java @@ -161,7 +161,7 @@ public void hitsExecute(SearchContext context, SearchHit[] hits) throws IOExcept DEPRECATION_LOGGER.deprecated("There are doc-value field 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); + + "7.0: {}", DocValueFieldsContext.USE_DEFAULT_FORMAT, noFormatFields); } } } From d59899a6cd6cb47c62ec5d3aeec3f3f2ebac5349 Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Mon, 10 Sep 2018 21:06:33 +0100 Subject: [PATCH 3/5] 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 --- .../subphase/DocValueFieldsFetchSubPhase.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java index 57ed1e9ecc36a..1ee43076cc630 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java +++ b/server/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 @@ -76,7 +77,15 @@ 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 noFormatFields = new ArrayList<>(); + + List 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 field 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; @@ -85,7 +94,6 @@ public void hitsExecute(SearchContext context, SearchHit[] hits) throws IOExcept final IndexFieldData indexFieldData = context.getForField(fieldType); final DocValueFormat format; if (fieldAndFormat.format == null) { - noFormatFields.add(fieldAndFormat.field); format = null; } else { String formatDesc = fieldAndFormat.format; @@ -156,12 +164,5 @@ public void hitsExecute(SearchContext context, SearchHit[] hits) throws IOExcept } } } - - if (noFormatFields.isEmpty() == false) { - DEPRECATION_LOGGER.deprecated("There are doc-value field 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); - } } } From 56ea52eb686baa6155118c5a772e83ba9c9ceed4 Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Mon, 10 Sep 2018 21:38:01 +0100 Subject: [PATCH 4/5] fixes typo --- .../rest-api-spec/test/search/10_source_filtering.yml | 6 +++--- .../search/fetch/subphase/DocValueFieldsFetchSubPhase.java | 2 +- 2 files changed, 4 insertions(+), 4 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 index 190b176fdeaba..a56f8a3f3f9d4 100644 --- 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,7 +139,7 @@ setup: features: warnings - do: warnings: - - 'There are doc-value field 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]' + - '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" ] @@ -153,7 +153,7 @@ setup: features: warnings - do: warnings: - - 'There are doc-value field 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]' + - '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]' search: body: docvalue_fields: [ "count", "include.field1" ] @@ -167,7 +167,7 @@ setup: features: warnings - do: warnings: - - 'There are doc-value field 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]' + - '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/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java index 1ee43076cc630..97e5b70f9da51 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java @@ -81,7 +81,7 @@ public void hitsExecute(SearchContext context, SearchHit[] hits) throws IOExcept List 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 field which are not using a format. The output will " + 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); From 3fd12906498302e44707e8a287573ae3df481c4b Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Tue, 11 Sep 2018 09:15:29 +0100 Subject: [PATCH 5/5] Fixes test --- .../rest-api-spec/test/search/10_source_filtering.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 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 index a56f8a3f3f9d4..2725580d9e8c1 100644 --- 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 @@ -153,10 +153,10 @@ setup: 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]' + - '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" ] + docvalue_fields: [ "count", "include.field1.keyword" ] - match: { hits.hits.0.fields.count: [1] } ---