Switch to using refactored SortBuilder instead of using BytesReference in serialization#17205
Conversation
|
@MaineC this is still WIP since it relies on first adding the build() methods to the SortBuilders (see linked PR), but I went ahead to see how much is missing to really cut over to using the refactored SortBuilders in the SearchSourceBuilder and was able to get it working by moving over most of the current parsing logic into SortBuilder. Unfortunately this makes it a quiet large PR, but half of the changes come from #17146 and should go away once that is in. |
df70b8a to
1987f98
Compare
929ab51 to
6d127bb
Compare
…ewhere Switching from using list of BytesReference to real SortBuilder list in SearchSourceBuilder, TopHitsAggregatorBuilder and TopHitsAggregatorFactory. Removing SortParseElement and related sort parsers.
6d127bb to
0acb373
Compare
|
@MaineC after merging the two related PRs, this PR should only contain the changes that where necessary to make the switch from List to List in SearchSourceBuilder and related similar places where we used List for the sort. Some IT tests failures revealed problems (like storing "missing" String as BytesRef in FieldSortBuilder) that are also addressed here. I think this is ready to take a look. |
0acb373 to
f8e7462
Compare
Conflicts: core/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java
|
Just went over the code. Really love the amount of code that goes away after this is merged. Also like the added tests, even if checks are kept simple at the moment. So here's my LGTM Caveats: Didn't run the test suite here. Also we might want to add a ticket that describes where tests could be improved (like e.g. more assertions) so those don't get forgotten. |
|
@MaineC @colings86 I added the last commit to show that we can now again parse syntax mentioned in #17257 |
This is based on #17146 that needs to get in first. It moves the remaining parsing logic for the complete sort-element over to SortBuilder#fromXContent and moves further logic that used to be in SortParseElement concerned with building SortFields and populating the search context with it also to SortBuilder.
In a last step for the refactoring the current workaround of using opaque BytesReference lists in SearchSourceBuilder and similar places it removed and the refactored SortBuilders used instead.
Closes #17257