From 019291a66cbb62012c9aad0cf8f39ed1bfec123e Mon Sep 17 00:00:00 2001 From: pgomulka Date: Tue, 22 Jun 2021 16:20:18 +0200 Subject: [PATCH 1/3] [Rest Api Compatibility] Ignore use_field_mapping option for docvalue previously removed in #55622 use_field_mapping option can be used on doc value format under rest api compatibility. The value itself is ignored (replaced with null) as it is a default behaviour to use field mapping format. --- rest-api-spec/build.gradle | 1 - .../search/fetch/subphase/FieldAndFormat.java | 39 +++++++++++++++++-- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/rest-api-spec/build.gradle b/rest-api-spec/build.gradle index 59e4360fc2339..2fc6407ed6570 100644 --- a/rest-api-spec/build.gradle +++ b/rest-api-spec/build.gradle @@ -104,7 +104,6 @@ tasks.named("yamlRestCompatTest").configure { 'mtermvectors/11_basic_with_types/Basic tests for multi termvector get', 'mtermvectors/21_deprecated_with_types/Deprecated camel case and _ parameters should fail in Term Vectors query', 'mtermvectors/30_mix_typeless_typeful/mtermvectors without types on an index that has types', - 'search/10_source_filtering/docvalue_fields with default format', //use_field_mapping change 'search/40_indices_boost/Indices boost using object', //indices_boost 'search/150_rewrite_on_coordinator/Ensure that we fetch the document only once', //terms_lookup 'search/171_terms_query_with_types/Terms Query with No.of terms exceeding index.max_terms_count should FAIL', //bulk diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldAndFormat.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldAndFormat.java index d9f2489ef7815..f8d6dfc112097 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldAndFormat.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldAndFormat.java @@ -9,25 +9,35 @@ package org.elasticsearch.search.fetch.subphase; import org.elasticsearch.Version; -import org.elasticsearch.core.Nullable; -import org.elasticsearch.common.xcontent.ParseField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.ObjectParser; +import org.elasticsearch.common.xcontent.ParseField; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.core.CheckedFunction; +import org.elasticsearch.core.Nullable; +import org.elasticsearch.core.RestApiVersion; import java.io.IOException; import java.util.Objects; +import static org.elasticsearch.core.RestApiVersion.equalTo; +import static org.elasticsearch.core.RestApiVersion.onOrAfter; + /** * Wrapper around a field name and the format that should be used to * display values of this field. */ public final class FieldAndFormat implements Writeable, ToXContentObject { + private static final String USE_DEFAULT_FORMAT = "use_field_mapping"; + private static final DeprecationLogger DEPRECATION_LOGGER = DeprecationLogger.getLogger(FetchDocValuesPhase.class); + private static final ParseField FIELD_FIELD = new ParseField("field"); private static final ParseField FORMAT_FIELD = new ParseField("format"); private static final ParseField INCLUDE_UNMAPPED_FIELD = new ParseField("include_unmapped"); @@ -38,10 +48,33 @@ public final class FieldAndFormat implements Writeable, ToXContentObject { static { PARSER.declareString(ConstructingObjectParser.constructorArg(), FIELD_FIELD); - PARSER.declareStringOrNull(ConstructingObjectParser.optionalConstructorArg(), FORMAT_FIELD); + PARSER.declareStringOrNull(ConstructingObjectParser.optionalConstructorArg(), + FORMAT_FIELD.forRestApiVersion(onOrAfter(RestApiVersion.V_8))); + PARSER.declareField(ConstructingObjectParser.optionalConstructorArg(), + ignoreUseFieldMappingStringParser(), + FORMAT_FIELD.forRestApiVersion(equalTo(RestApiVersion.V_7)), + ObjectParser.ValueType.STRING_OR_NULL); PARSER.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), INCLUDE_UNMAPPED_FIELD); } + private static CheckedFunction ignoreUseFieldMappingStringParser() { + return (p) -> { + if (p.currentToken() == XContentParser.Token.VALUE_NULL) { + return null; + } else { + String text = p.text(); + if (text.equals("use_field_mapping")) { + DEPRECATION_LOGGER.compatibleApiWarning("explicit_default_format", + "[" + USE_DEFAULT_FORMAT + "] is a special format that was only used to " + + "ease the transition to 7.x. It has become the default and shouldn't be set explicitly anymore."); + return null; + } else { + return text; + } + } + }; + } + /** * Parse a {@link FieldAndFormat} from some {@link XContent}. */ From 32698c3699e29fcbe5a0dc31a546294f0f4b7c91 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Tue, 22 Jun 2021 22:21:29 +0200 Subject: [PATCH 2/3] use constant in custom field format parser --- rest-api-spec/build.gradle | 14 +++++++------- .../search/fetch/subphase/FieldAndFormat.java | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/rest-api-spec/build.gradle b/rest-api-spec/build.gradle index 2fc6407ed6570..d5ae9de4c77ab 100644 --- a/rest-api-spec/build.gradle +++ b/rest-api-spec/build.gradle @@ -111,13 +111,13 @@ tasks.named("yamlRestCompatTest").configure { 'search/310_match_bool_prefix/multi_match multiple fields with cutoff_frequency throws exception', //cutoff_frequency 'search/340_type_query/type query', // type_query - probably should behave like match_all 'search_shards/10_basic/Search shards aliases with and without filters', - 'snapshot.get/10_basic/Get missing snapshot info succeeds when ignore_unavailable is true', - 'snapshot.get/10_basic/Get missing snapshot info throws an exception', - 'snapshot.get/10_basic/Get snapshot info', - 'snapshot.get/10_basic/Get snapshot info contains include_global_state', - 'snapshot.get/10_basic/Get snapshot info when verbose is false', - 'snapshot.get/10_basic/Get snapshot info with metadata', - 'snapshot.get/10_basic/Get snapshot info with index details', +// 'snapshot.get/10_basic/Get missing snapshot info succeeds when ignore_unavailable is true', +// 'snapshot.get/10_basic/Get missing snapshot info throws an exception', +// 'snapshot.get/10_basic/Get snapshot info', +// 'snapshot.get/10_basic/Get snapshot info contains include_global_state', +// 'snapshot.get/10_basic/Get snapshot info when verbose is false', +// 'snapshot.get/10_basic/Get snapshot info with metadata', +// 'snapshot.get/10_basic/Get snapshot info with index details', 'suggest/20_completion/Suggestions with source should work' ] + v7compatiblityNotSupportedTests()) .join(',') diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldAndFormat.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldAndFormat.java index f8d6dfc112097..6f5550c010096 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldAndFormat.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldAndFormat.java @@ -63,7 +63,7 @@ private static CheckedFunction ignoreUseFie return null; } else { String text = p.text(); - if (text.equals("use_field_mapping")) { + if (text.equals(USE_DEFAULT_FORMAT)) { DEPRECATION_LOGGER.compatibleApiWarning("explicit_default_format", "[" + USE_DEFAULT_FORMAT + "] is a special format that was only used to " + "ease the transition to 7.x. It has become the default and shouldn't be set explicitly anymore."); From 837acb022dbacdaccaac0c754816a514bd8c49f6 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Tue, 22 Jun 2021 22:45:24 +0200 Subject: [PATCH 3/3] revert accidental test unmute --- rest-api-spec/build.gradle | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/rest-api-spec/build.gradle b/rest-api-spec/build.gradle index d5ae9de4c77ab..2fc6407ed6570 100644 --- a/rest-api-spec/build.gradle +++ b/rest-api-spec/build.gradle @@ -111,13 +111,13 @@ tasks.named("yamlRestCompatTest").configure { 'search/310_match_bool_prefix/multi_match multiple fields with cutoff_frequency throws exception', //cutoff_frequency 'search/340_type_query/type query', // type_query - probably should behave like match_all 'search_shards/10_basic/Search shards aliases with and without filters', -// 'snapshot.get/10_basic/Get missing snapshot info succeeds when ignore_unavailable is true', -// 'snapshot.get/10_basic/Get missing snapshot info throws an exception', -// 'snapshot.get/10_basic/Get snapshot info', -// 'snapshot.get/10_basic/Get snapshot info contains include_global_state', -// 'snapshot.get/10_basic/Get snapshot info when verbose is false', -// 'snapshot.get/10_basic/Get snapshot info with metadata', -// 'snapshot.get/10_basic/Get snapshot info with index details', + 'snapshot.get/10_basic/Get missing snapshot info succeeds when ignore_unavailable is true', + 'snapshot.get/10_basic/Get missing snapshot info throws an exception', + 'snapshot.get/10_basic/Get snapshot info', + 'snapshot.get/10_basic/Get snapshot info contains include_global_state', + 'snapshot.get/10_basic/Get snapshot info when verbose is false', + 'snapshot.get/10_basic/Get snapshot info with metadata', + 'snapshot.get/10_basic/Get snapshot info with index details', 'suggest/20_completion/Suggestions with source should work' ] + v7compatiblityNotSupportedTests()) .join(',')