Add configurable sort options to opensearch source search_options#6761
Conversation
Signed-off-by: Srikanth Padakanti <srikanth_padakanti@apple.com>
dlvenable
left a comment
There was a problem hiding this comment.
Thanks @srikanthpadakanti ! This looks good, and I only have a few small comments.
| private String name; | ||
|
|
||
| @JsonProperty("order") | ||
| private String order = "ascending"; |
There was a problem hiding this comment.
Using enums helps us with our JSON schema generation and provides a list of valid values. You should be able to search the repo for enums that serve similar purposes and follow the same patterns. WhitespaceOption in the key-value processor is a nice simple example.
You can also add a method to get the OpenSearch name for each.
e.g.
ASCENDING("ascending", "asc"),
DESCENDING("decending", desc");
There was a problem hiding this comment.
Good call, updated to use a SortOrder enum following the WhitespaceOption pattern. Also added getSortOrderValue() for the asc/desc mapping.
| private static SortingOptions fromSortConfig(final SortConfig sortConfig) { | ||
| final SortingOptions sortingOptions = new SortingOptions(); | ||
| sortingOptions.fieldName = sortConfig.getName(); | ||
| sortingOptions.order = "descending".equalsIgnoreCase(sortConfig.getOrder()) ? "desc" : "asc"; |
There was a problem hiding this comment.
See the enum comment above for a way to handle this in the enum itself.
There was a problem hiding this comment.
Addressed.
| .collect(Collectors.toList()); | ||
| } | ||
|
|
||
| private static SortingOptions fromSortConfig(final SortConfig sortConfig) { |
There was a problem hiding this comment.
It may be beneficial to detect if _id is present. If it isn't, there is the possibility that we won't be able to break ties. Probably just log a warning for this.
There was a problem hiding this comment.
Added a warning log in SortingOptions.fromSortConfigs() when _id is not present in the custom sort configuration.
Signed-off-by: Srikanth Padakanti <srikanth_padakanti@apple.com>
Signed-off-by: Srikanth Padakanti <srikanth_padakanti@apple.com>
Thank you for the review, @dlvenable. I’ve addressed your comments. Please take a look when you have a moment. |
…into opensearch-source-sort-options
dlvenable
left a comment
There was a problem hiding this comment.
Thank you @srikanthpadakanti !
Description
Adds configurable sort option to the opensearch source's search_options, allowing pipeline authors to specify custom sort fields for pagination instead of the hardcoded [_doc asc, _id asc] default.
Example configuration:
When no sort is configured, behavior is unchanged, defaults to _doc ascending + _id ascending for PIT/no-context searches, and _doc ascending for scroll.
Changes include:
SortingOptionsmodel andSearchPointInTimeRequest.sortingOptionsfieldIssues Resolved
Resolves #6332
#6332
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.