Remove BytesRef usage from XContentParser and its subclasses #28792
Conversation
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
This removes all the BytesRef usage from XContentParser in favor of directly returning a CharBuffer (this was originally what was returned, it was just immediately wraped in a BytesRef). Relates to elastic#28504
Member
Author
|
This is the first one (and definitely not going to be the last one) that's been particularly tricky. |
rjernst
approved these changes
Feb 22, 2018
| * @param obj the input object | ||
| * @return the same input object or a {@link BytesRef} representation if input was of type string | ||
| */ | ||
| static Object convertToBytesRefIfString(Object obj) { |
Member
There was a problem hiding this comment.
I would go with a simpler name since you are renaming anwyays: convertToBytesRef. I don't think the "if" really adds anything that can't/isn't explained in the javadoc.
Member
Author
There was a problem hiding this comment.
How about maybeConvertToBytesRef? Because it doesn't always convert the object to a BytesRef
Member
There was a problem hiding this comment.
Well, I think methods like this are trappy to begin with. We should have types, not be converting object to object. But that's outside the scope of this PR. Using maybe is fine I guess.
Member
Author
There was a problem hiding this comment.
Yeah, I definitely don't like this method at all, using Object is definitely not something I want to do, but I agree it's out of scope for this PR.
nik9000
approved these changes
Feb 26, 2018
| * @param obj the input object | ||
| * @return the same input object or a {@link BytesRef} representation if input was of type string | ||
| */ | ||
| static Object convertToBytesRefIfString(Object obj) { |
Member
Author
|
@elasticmachine test this again please but don't hang the build. |
Member
Author
|
@elasticmachine test this please |
dakrone
added a commit
that referenced
this pull request
Mar 7, 2018
* Remove BytesRef usage from XContentParser and its subclasses This removes all the BytesRef usage from XContentParser in favor of directly returning a CharBuffer (this was originally what was returned, it was just immediately wraped in a BytesRef). Relates to #28504 * Rename method after Ryan's feedback
jasontedor
added a commit
to jasontedor/elasticsearch
that referenced
this pull request
Mar 7, 2018
* master: [TEST] AwaitsFix QueryRescorerIT.testRescoreAfterCollapse Decouple XContentType from StreamInput/Output (elastic#28927) Remove BytesRef usage from XContentParser and its subclasses (elastic#28792) [DOCS] Correct typo in configuration (elastic#28903) Fix incorrect datemath example (elastic#28904) Add a usage example of the JLH score (elastic#28905) Wrap stream passed to createParser in try-with-resources (elastic#28897) Rescore collapsed documents (elastic#28521) Fix (simple)_query_string to ignore removed terms (elastic#28871) [Docs] Fix typo in composite aggregation (elastic#28891) Try if tombstone is eligable for pruning before locking on it's key (elastic#28767)
jasontedor
added a commit
to jasontedor/elasticsearch
that referenced
this pull request
Mar 7, 2018
* master: [TEST] AwaitsFix QueryRescorerIT.testRescoreAfterCollapse Decouple XContentType from StreamInput/Output (elastic#28927) Remove BytesRef usage from XContentParser and its subclasses (elastic#28792)
martijnvg
added a commit
that referenced
this pull request
Mar 8, 2018
* es/master: (48 commits) Update bucket-sort-aggregation.asciidoc (#28937) [Docs] REST high-level client: Fix code for most basic search request (#28916) Improved percolator's random candidate query duel test and fixed bugs that were exposed by this: Revert "Rescore collapsed documents (#28521)" Build: Fix test logger NPE when no tests are run (#28929) [TEST] AwaitsFix QueryRescorerIT.testRescoreAfterCollapse Decouple XContentType from StreamInput/Output (#28927) Remove BytesRef usage from XContentParser and its subclasses (#28792) [DOCS] Correct typo in configuration (#28903) Fix incorrect datemath example (#28904) Add a usage example of the JLH score (#28905) Wrap stream passed to createParser in try-with-resources (#28897) Rescore collapsed documents (#28521) Fix (simple)_query_string to ignore removed terms (#28871) [Docs] Fix typo in composite aggregation (#28891) Try if tombstone is eligable for pruning before locking on it's key (#28767) Limit analyzed text for highlighting (improvements) (#28808) Missing `timeout` parameter from the REST API spec JSON files (#28328) Clarifies how query_string splits textual part (#28798) Update outdated java version reference (#28870) ...
martijnvg
added a commit
that referenced
this pull request
Mar 8, 2018
* es/6.x: (48 commits) Update bucket-sort-aggregation.asciidoc (#28937) [Docs] REST high-level client: Fix code for most basic search request (#28916) Improved percolator's random candidate query duel test and fixed bugs that were exposed by this: Revert "Rescore collapsed documents (#28521)" Build: Fix test logger NPE when no tests are run (#28929) [TEST] AwaitsFix QueryRescorerIT.testRescoreAfterCollapse Decouple XContentType from StreamInput/Output (#28927) Remove BytesRef usage from XContentParser and its subclasses (#28792) Add doc note for -server flag on Windows service [DOCS] Correct typo in configuration (#28903) Fix incorrect datemath example (#28904) Add a usage example of the JLH score (#28905) Limit analyzed text for highlighting (improvements) (#28907) Wrap stream passed to createParser in try-with-resources (#28897) [Docs] Fix typo in composite aggregation (#28891) Rescore collapsed documents (#28521) Fix (simple)_query_string to ignore removed terms (#28871) Missing `timeout` parameter from the REST API spec JSON files (#28328) Clarifies how query_string splits textual part (#28798) Update outdated java version reference (#28870) ...
sebasjm
pushed a commit
to sebasjm/elasticsearch
that referenced
this pull request
Mar 10, 2018
…#28792) * Remove BytesRef usage from XContentParser and its subclasses This removes all the BytesRef usage from XContentParser in favor of directly returning a CharBuffer (this was originally what was returned, it was just immediately wraped in a BytesRef). Relates to elastic#28504 * Rename method after Ryan's feedback
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
This removes all the BytesRef usage from XContentParser in favor of directly
returning a CharBuffer (this was originally what was returned, it was just
immediately wraped in a BytesRef).
Relates to #28504