Closed
Conversation
Contributor
|
LGTM |
nik9000
approved these changes
Dec 30, 2016
Member
nik9000
left a comment
There was a problem hiding this comment.
Same comment and Jim. It looks like you'd documented use_field_mapping in the breaking changes doc which didn't survive the backport proposal.
4e9b1ab to
c20df6a
Compare
Currently `docvalues_fields` return the values of the fields as they are stored in doc values. I don't like that it exposes implementation details, but there are also user-facing issues like the fact it cannot work with binary fields. This change will also make it easier for users to reindex if they do not store the source, since `docvalues_fields` will return data is such a format that it can be put in an indexing request with the same mappings. The hard part of the change is backward compatibility, since it is breaking. The approach taken here is that 5.x will keep exposing the internal representation, with a special format name called `use_field_format` which will format the field depending on how it is mapped. This will become the default in 6.0, and this hardcoded format name will be removed in 7.0 to ease the transition from 5.x to 6.x.
c20df6a to
43c14e2
Compare
Contributor
|
what happened to this PR? seems like it stayed opened for months despite it had LGTMs, and then it was closed without further commenting. |
Contributor
|
I think it got closed because it was opened against 5.x, which was deleted. |
Contributor
Author
|
Right. This PR requires sync with the Kibana team since it is going to be a breaking change for them and I never took the time to do it. I'll try to do it during the 6.x series. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #22146
Note to reviewers: the diff between this change and #22146 is contained in the 2nd commit.