Skip to content

Add searchAfter interfaces to NativeSearchQueryBuilder #2106

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

Merged
merged 2 commits into from
Mar 12, 2022

Conversation

owen-q
Copy link
Contributor

@owen-q owen-q commented Mar 4, 2022

  • support searchAfter for nativeSearchQuery

closes #2105

- support searchAfter for nativeSearchQuery
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 4, 2022
Copy link
Collaborator

@sothawo sothawo left a comment

Choose a reason for hiding this comment

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

We should not use the SearchAfterIntegrationTests to test this change. The change is on the NativeSearchQueryBuilder and should be tested there. I know there is no test for this builder, but this might be a good opportunity to create it and just test that the searchAfter values are set on the Query after building it.


import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

import org.elasticsearch.search.sort.SortBuilders;
import org.elasticsearch.search.sort.SortOrder;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This pulls in dependencies to Elasticsearch classes in this test, this test is independent of the used query type.

@owen-q owen-q force-pushed the support-searchafter branch from 5b82f64 to 8f395c9 Compare March 12, 2022 00:16
- add validate conditions in searchAfter()
  - cause SearchAfterBuilder.setSortValues() assert not allow empty values
- delete not appropriate tests for searchAfter in SearchAfterIntegrationTests
@owen-q owen-q force-pushed the support-searchafter branch from 8f395c9 to a7129e9 Compare March 12, 2022 00:17
@owen-q
Copy link
Contributor Author

owen-q commented Mar 12, 2022

We should not use the SearchAfterIntegrationTests to test this change. The change is on the NativeSearchQueryBuilder and should be tested there. I know there is no test for this builder, but this might be a good opportunity to create it and just test that the searchAfter values are set on the Query after building it.

@sothawo
reflect your PR review.
please review again on changes, thank you

@@ -342,6 +344,15 @@ public NativeSearchQueryBuilder withSuggestBuilder(SuggestBuilder suggestBuilder
return this;
}

public NativeSearchQueryBuilder withSearchAfter(List<Object> searchAfter) {
if (searchAfter != null && searchAfter.isEmpty()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

since SearchAfterBuilder not allow empty collections (it throws illegalArgument exception)
i add this conditions for ignore empty collections

@sothawo sothawo merged commit 33c4bb2 into spring-projects:main Mar 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support searchAfter in NativeSearchQueryBuilder
4 participants