Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct how meta-field is defined for pre 7.8 hits #57951

Merged
merged 4 commits into from
Jun 12, 2020
Merged

Correct how meta-field is defined for pre 7.8 hits #57951

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8cd058b
Correct how meta-field is defined for pre 7.8 hits
mayya-sharipova Jun 10, 2020
44add12
Address Chrishoph's feedback
mayya-sharipova Jun 11, 2020
7eadf78
Ensure that META_FIELDS_BEFORE_7DOT8 is used to create test hit insta…
mayya-sharipova Jun 11, 2020
d3c839f
Remove pre 6.6 tests
mayya-sharipova Jun 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions rest-api-spec/src/main/resources/rest-api-spec/test/search.inner_hits/10_basic.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ setup:
"Nested doc version and seqIDs":

- skip:
version: "all" #" - 6.99.99"
reason: "AwaitsFix https://github.com/elastic/elasticsearch/issues/57831"#"Triggers warnings before 7.0"
version: " - 6.99.99"
reason: "Triggers warnings before 7.0"

- do:
index:
Expand Down
11 changes: 6 additions & 5 deletions server/src/main/java/org/elasticsearch/index/mapper/MapperService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import java.io.Closeable;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -119,6 +120,10 @@ public enum MergeReason {
public static final Setting<Boolean> INDEX_MAPPER_DYNAMIC_SETTING =
Setting.boolSetting("index.mapper.dynamic", INDEX_MAPPER_DYNAMIC_DEFAULT,
Property.Dynamic, Property.IndexScope, Property.Deprecated);
@Deprecated
private static final Set<String> META_FIELDS_BEFORE_7_8 =
Collections.unmodifiableSet(new HashSet<>(Arrays.asList(
"_id", IgnoredFieldMapper.NAME, "_index", "_routing", "_size", "_timestamp", "_ttl", "_type")));

private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(MapperService.class));
static final String DEFAULT_MAPPING_ERROR_MESSAGE = "[_default_] mappings are not allowed on new indices and should no " +
Expand Down Expand Up @@ -813,11 +818,7 @@ public void close() throws IOException {
*/
@Deprecated
public static boolean isMetadataFieldStatic(String fieldName) {
if (IndicesModule.getBuiltInMetadataFields().contains(fieldName)) {
return true;
}
// if a node had Size Plugin installed, _size field should also be considered a meta-field
return fieldName.equals("_size");
return META_FIELDS_BEFORE_7_8.contains(fieldName);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember correctly we only use isMetadataFieldStatic in a few places now where we read from older nodes. Should we simply make the collection public and remove the static method? Or rename method to something like "isMetaFieldBefore7dot8" or something, just to improve readability in the locations where this it is actually used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cbuescher Thanks for the feedback! I have removed isMetadataFieldStatic method, and made the collection public.

}

/**
Expand Down