-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Field collapsing supports search_after #19261
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
Conversation
Signed-off-by: Binlong Gao <[email protected]>
Signed-off-by: Binlong Gao <[email protected]>
|
❌ Gradle check result for 9dc634d: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Binlong Gao <[email protected]>
Signed-off-by: Binlong Gao <[email protected]>
Signed-off-by: Binlong Gao <[email protected]>
|
❌ Gradle check result for a876128: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Apologies for the delay, @gaobinlong , I will take a look today / tomorrow, thank you |
server/src/main/java/org/apache/lucene/search/grouping/CollapsingTopDocsCollector.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/search/collapse/CollapseContext.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/search/query/TopDocsCollectorContext.java
Outdated
Show resolved
Hide resolved
|
Thanks a lot @gaobinlong , a few minor comments but LGTM otherwise, apologies for the delay |
Signed-off-by: Binlong Gao <[email protected]>
|
❌ Gradle check result for ea9fd6b: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Binlong Gao <[email protected]>
|
❌ Gradle check result for fbf7a77: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for fbf7a77: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Binlong Gao <[email protected]>
|
❌ Gradle check result for 62767e6: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for 62767e6: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #19261 +/- ##
============================================
- Coverage 72.89% 72.88% -0.01%
- Complexity 69763 69765 +2
============================================
Files 5667 5667
Lines 320547 320590 +43
Branches 46348 46357 +9
============================================
+ Hits 233648 233654 +6
- Misses 68005 68038 +33
- Partials 18894 18898 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
* Field collapsing supports search_after Signed-off-by: Binlong Gao <[email protected]> * Modify change log Signed-off-by: Binlong Gao <[email protected]> * Avoid changing public APIs Signed-off-by: Binlong Gao <[email protected]> * Revert change for the existing method Signed-off-by: Binlong Gao <[email protected]> * Format change log Signed-off-by: Binlong Gao <[email protected]> * Fix some issues Signed-off-by: Binlong Gao <[email protected]> * Remove duplicate code Signed-off-by: Binlong Gao <[email protected]> * Fix format issue Signed-off-by: Binlong Gao <[email protected]> --------- Signed-off-by: Binlong Gao <[email protected]>
* Field collapsing supports search_after Signed-off-by: Binlong Gao <[email protected]> * Modify change log Signed-off-by: Binlong Gao <[email protected]> * Avoid changing public APIs Signed-off-by: Binlong Gao <[email protected]> * Revert change for the existing method Signed-off-by: Binlong Gao <[email protected]> * Format change log Signed-off-by: Binlong Gao <[email protected]> * Fix some issues Signed-off-by: Binlong Gao <[email protected]> * Remove duplicate code Signed-off-by: Binlong Gao <[email protected]> * Fix format issue Signed-off-by: Binlong Gao <[email protected]> --------- Signed-off-by: Binlong Gao <[email protected]> Signed-off-by: Ankit Jain <[email protected]>
* Field collapsing supports search_after Signed-off-by: Binlong Gao <[email protected]> * Modify change log Signed-off-by: Binlong Gao <[email protected]> * Avoid changing public APIs Signed-off-by: Binlong Gao <[email protected]> * Revert change for the existing method Signed-off-by: Binlong Gao <[email protected]> * Format change log Signed-off-by: Binlong Gao <[email protected]> * Fix some issues Signed-off-by: Binlong Gao <[email protected]> * Remove duplicate code Signed-off-by: Binlong Gao <[email protected]> * Fix format issue Signed-off-by: Binlong Gao <[email protected]> --------- Signed-off-by: Binlong Gao <[email protected]> Signed-off-by: Ankit Jain <[email protected]>
* Field collapsing supports search_after Signed-off-by: Binlong Gao <[email protected]> * Modify change log Signed-off-by: Binlong Gao <[email protected]> * Avoid changing public APIs Signed-off-by: Binlong Gao <[email protected]> * Revert change for the existing method Signed-off-by: Binlong Gao <[email protected]> * Format change log Signed-off-by: Binlong Gao <[email protected]> * Fix some issues Signed-off-by: Binlong Gao <[email protected]> * Remove duplicate code Signed-off-by: Binlong Gao <[email protected]> * Fix format issue Signed-off-by: Binlong Gao <[email protected]> --------- Signed-off-by: Binlong Gao <[email protected]>
* Field collapsing supports search_after Signed-off-by: Binlong Gao <[email protected]> * Modify change log Signed-off-by: Binlong Gao <[email protected]> * Avoid changing public APIs Signed-off-by: Binlong Gao <[email protected]> * Revert change for the existing method Signed-off-by: Binlong Gao <[email protected]> * Format change log Signed-off-by: Binlong Gao <[email protected]> * Fix some issues Signed-off-by: Binlong Gao <[email protected]> * Remove duplicate code Signed-off-by: Binlong Gao <[email protected]> * Fix format issue Signed-off-by: Binlong Gao <[email protected]> --------- Signed-off-by: Binlong Gao <[email protected]>
* Field collapsing supports search_after Signed-off-by: Binlong Gao <[email protected]> * Modify change log Signed-off-by: Binlong Gao <[email protected]> * Avoid changing public APIs Signed-off-by: Binlong Gao <[email protected]> * Revert change for the existing method Signed-off-by: Binlong Gao <[email protected]> * Format change log Signed-off-by: Binlong Gao <[email protected]> * Fix some issues Signed-off-by: Binlong Gao <[email protected]> * Remove duplicate code Signed-off-by: Binlong Gao <[email protected]> * Fix format issue Signed-off-by: Binlong Gao <[email protected]> --------- Signed-off-by: Binlong Gao <[email protected]>
Description
Why field collapsing doesn't support pagination by search_after? The reason is that if we collapse and sort on different field, and use search_after to fetch next page, the same collapsed key may show again in the next page, see the example blow:

Now we collapse on session_id and sort by timestamp ascending, the first page would be:
, then paging with search_after, the second page would be:
, the above behavior would break the field collapsing, but we could imagine that if we collapse and sort on same filed, search_after will work.
So this PR adds the search_after support for field collapsing, but limit the usage that collapse field and sort field must be the same and secondary sort is not supported.
Related Issues
Resolves #3725
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.