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.

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

Fix MapperService.allEnabled(). #22227

Merged
merged 2 commits into from Dec 19, 2016
Merged

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Dec 16, 2016

It returns whether the last merged mapping has _all enabled rather than
whether any of the types has _all enabled.

@jpountz jpountz added :Search/Mapping Index mappings, including merging and defining field types >bug labels Dec 16, 2016
Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -377,7 +377,7 @@ private synchronized DocumentMapper merge(DocumentMapper mapper, MergeReason rea
this.hasNested = hasNested;
this.fullPathObjectMappers = fullPathObjectMappers;
this.parentTypes = parentTypes;
this.allEnabled = mapper.allFieldMapper().enabled();
this.allEnabled |= mapper.allFieldMapper().enabled();
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add an assertion / comment here why this is safe to do? (i.e. we never go from enabled to disabled as the all_field enabledness of a type cannot be changed and types cannot be removed)

It returns whether the last merged mapping has `_all` enabled rather than
whether any of the types has `_all` enabled.
@jpountz jpountz merged commit 1ed2e18 into elastic:master Dec 19, 2016
@jpountz jpountz deleted the fix/_all_enabled branch December 19, 2016 08:55
jpountz added a commit that referenced this pull request Dec 19, 2016
It returns whether the last merged mapping has `_all` enabled rather than
whether any of the types has `_all` enabled.
jpountz added a commit that referenced this pull request Dec 19, 2016
It returns whether the last merged mapping has `_all` enabled rather than
whether any of the types has `_all` enabled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Mapping Index mappings, including merging and defining field types v5.1.2 v5.2.0 v6.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants