Handle canMatchSearchAfter for frozen context scenario

Signed-off-by: Chaitanya Gohel <gashutos@amazon.com>
opensearch-project · Nov 17, 2023 · 5236b2f · 5236b2f
1 parent 37ec313
commit 5236b2f
Show file tree

Hide file tree

Showing 5 changed files with 121 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -90,6 +90,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Fix 'org.apache.hc.core5.http.ParseException: Invalid protocol version' under JDK 16+ ([#4827](https://github.com/opensearch-project/OpenSearch/pull/4827))
 - Fix compression support for h2c protocol ([#4944](https://github.com/opensearch-project/OpenSearch/pull/4944))
 - Don't over-allocate in HeapBufferedAsyncEntityConsumer in order to consume the response ([#9993](https://github.com/opensearch-project/OpenSearch/pull/9993))
+- Handle canMatchSearchAfter for frozen context scenario ([#11249](https://github.com/opensearch-project/OpenSearch/pull/11249))
 
 ### Security
 

diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/240_date_nanos.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/240_date_nanos.yml
@@ -164,3 +164,99 @@ setup:
   - match: { aggregations.date.buckets.1.key: 1540857600000 }
   - match: { aggregations.date.buckets.1.key_as_string: "2018-10-30T00:00:00.000Z" }
   - match: { aggregations.date.buckets.1.doc_count: 2 }
+
+
+---
+"date with nested sort now":
+  - skip:
+      version: " - 2.12.99"
+      reason: fixed in 3.0.0
+
+  # This tests cover scenario where nested sort have now() in date field type.
+  # For this test, we have date field as nested field and we trigger asc/desc sort
+  # on that nested field. `filter` clause is needed when we sort any nested field,
+  # like in this case, "gte": "now/h" says sort nested field date_field only where
+  # document is having value greater than current time now().
+  # Nested field sort query doesn't sort documents if it is not qualified through
+  # `filter` clause.
+  # Only adding tests for `gte` as `lte` would be same behaviour
+
+  - do:
+      indices.create:
+        index: test
+        body:
+          mappings:
+            properties:
+              nested_field:
+                type: nested
+                properties:
+                  date_field:
+                    type: date
+                    format: date_optional_time
+  - do:
+      bulk:
+        refresh: true
+        index: test
+        body: |
+          {"index":{}}
+          {"nested_field": [{"date_field": "3023-10-26T12:00:00+09:00"}]}
+          {"index":{}}
+          {"nested_field": [{"date_field": "3024-10-26T12:00:00+09:00"}]}
+          {"index":{}}
+          {"nested_field": [{"date_field": "3025-10-26T12:00:00+09:00"}]}
+          {"index":{}}
+          {"nested_field": [{"date_field": "3026-10-26T12:00:00+09:00"}]}
+          {"index":{}}
+          {"nested_field": [{"date_field": "3027-10-26T12:00:00+09:00"}]}
+          {"index":{}}
+          {"nested_field": [{"date_field": "2022-10-26T12:00:00+09:00"}]}
+          {"index":{}}
+          {"nested_field": [{"date_field": "2023-10-26T12:00:00+09:00"}]}
+          {"index":{}}
+          {"nested_field": [{"date_field": "2021-10-26T12:00:00+09:00"}]}
+          {"index":{}}
+          {"nested_field": [{"date_field": "2020-10-26T12:00:00+09:00"}]}
+          {"index":{}}
+          {"nested_field": [{"date_field": "2019-10-26T12:00:00+09:00"}]}
+
+  # gte: now/h with the desc sort
+  - do:
+      search:
+        index: test
+        body:
+          size: 5
+          sort: [{ nested_field.date_field: { mode: max, order: desc, nested: { path: nested_field, filter: { bool: { filter : [{ range : { nested_field.date_field: { gte: now/h, time_zone: +09:00} } }] } } } } } ]
+  - match: {hits.total.value: 10 }
+  - length: {hits.hits: 5 }
+  - match: { hits.hits.0._index: test }
+  - match: { hits.hits.0._source.nested_field.0.date_field: "3027-10-26T12:00:00+09:00" }
+  - match: { hits.hits.0.sort: [33381428400000] }
+  - match: { hits.hits.1._source.nested_field.0.date_field: "3026-10-26T12:00:00+09:00" }
+  - match: { hits.hits.1.sort: [ 33349892400000 ] }
+  - match: { hits.hits.2._source.nested_field.0.date_field: "3025-10-26T12:00:00+09:00" }
+  - match: { hits.hits.2.sort: [ 33318356400000 ] }
+  - match: { hits.hits.3._source.nested_field.0.date_field: "3024-10-26T12:00:00+09:00" }
+  - match: { hits.hits.3.sort: [ 33286820400000 ] }
+  - match: { hits.hits.4._source.nested_field.0.date_field: "3023-10-26T12:00:00+09:00" }
+  - match: { hits.hits.4.sort: [ 33255198000000 ] }
+
+  # gte: now/h with the asc sort
+  - do:
+      search:
+        index: test
+        body:
+          size: 5
+          sort: [ { nested_field.date_field: { mode: max, order: asc, nested: { path: nested_field, filter: { bool: { filter: [ { range: { nested_field.date_field: { gte: now/h, time_zone: +09:00 } } } ] } } } } } ]
+  - match: { hits.total.value: 10 }
+  - length: { hits.hits: 5 }
+  - match: { hits.hits.0._index: test }
+  - match: { hits.hits.0._source.nested_field.0.date_field: "3023-10-26T12:00:00+09:00" }
+  - match: { hits.hits.0.sort: [ 33255198000000 ] }
+  - match: { hits.hits.1._source.nested_field.0.date_field: "3024-10-26T12:00:00+09:00" }
+  - match: { hits.hits.1.sort: [ 33286820400000 ] }
+  - match: { hits.hits.2._source.nested_field.0.date_field: "3025-10-26T12:00:00+09:00" }
+  - match: { hits.hits.2.sort: [ 33318356400000 ] }
+  - match: { hits.hits.3._source.nested_field.0.date_field: "3026-10-26T12:00:00+09:00" }
+  - match: { hits.hits.3.sort: [ 33349892400000 ] }
+  - match: { hits.hits.4._source.nested_field.0.date_field: "3027-10-26T12:00:00+09:00" }
+  - match: { hits.hits.4.sort: [ 33381428400000 ] }
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml
@@ -219,7 +219,7 @@
   - match: {hits.hits.0._source.timestamp: "2019-10-21 08:30:04.828733" }
   - match: {hits.hits.0.sort: [1571646604828733000] }
 
-  # search_after with the sort
+  # search_after with the asc sort
   - do:
       search:
         index: test

diff --git a/server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java b/server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java
@@ -507,14 +507,18 @@ private boolean canMatch(LeafReaderContext ctx) throws IOException {
     }
 
     private boolean canMatchSearchAfter(LeafReaderContext ctx) throws IOException {
-        if (searchContext.request() != null && searchContext.request().source() != null) {
+        if (searchContext.searchAfter() != null
+            && searchContext.sort() != null
+            && searchContext.request() != null
+            && searchContext.request().source() != null) {
             // Only applied on primary sort field and primary search_after.
             FieldSortBuilder primarySortField = FieldSortBuilder.getPrimaryFieldSortOrNull(searchContext.request().source());
             if (primarySortField != null) {
                 MinAndMax<?> minMax = FieldSortBuilder.getMinMaxOrNullForSegment(
                     this.searchContext.getQueryShardContext(),
                     ctx,
-                    primarySortField
+                    primarySortField,
+                    searchContext.sort()
                 );
                 return SearchService.canMatchSearchAfter(
                     searchContext.searchAfter(),

diff --git a/server/src/main/java/org/opensearch/search/sort/FieldSortBuilder.java b/server/src/main/java/org/opensearch/search/sort/FieldSortBuilder.java
@@ -611,22 +611,32 @@ public static FieldSortBuilder getPrimaryFieldSortOrNull(SearchSourceBuilder sou
      * and configurations return <code>null</code>.
      */
     public static MinAndMax<?> getMinMaxOrNull(QueryShardContext context, FieldSortBuilder sortBuilder) throws IOException {
-        return getMinMaxOrNullInternal(context.getIndexReader(), context, sortBuilder);
+        return getMinMaxOrNullInternal(context.getIndexReader(), context, sortBuilder, null);
     }
 
     /**
      * Return the {@link MinAndMax} indexed value for segment from the provided {@link FieldSortBuilder} or <code>null</code> if unknown.
      * The value can be extracted on non-nested indexed mapped fields of type keyword, numeric or date, other fields
      * and configurations return <code>null</code>.
      */
-    public static MinAndMax<?> getMinMaxOrNullForSegment(QueryShardContext context, LeafReaderContext ctx, FieldSortBuilder sortBuilder)
-        throws IOException {
-        return getMinMaxOrNullInternal(ctx.reader(), context, sortBuilder);
+    public static MinAndMax<?> getMinMaxOrNullForSegment(
+        QueryShardContext context,
+        LeafReaderContext ctx,
+        FieldSortBuilder sortBuilder,
+        SortAndFormats sort
+    ) throws IOException {
+        return getMinMaxOrNullInternal(ctx.reader(), context, sortBuilder, sort);
     }
 
-    private static MinAndMax<?> getMinMaxOrNullInternal(IndexReader reader, QueryShardContext context, FieldSortBuilder sortBuilder)
-        throws IOException {
-        SortAndFormats sort = SortBuilder.buildSort(Collections.singletonList(sortBuilder), context).get();
+    private static MinAndMax<?> getMinMaxOrNullInternal(
+        IndexReader reader,
+        QueryShardContext context,
+        FieldSortBuilder sortBuilder,
+        SortAndFormats sort
+    ) throws IOException {
+        if (sort == null) {
+            sort = SortBuilder.buildSort(Collections.singletonList(sortBuilder), context).get();
+        }
         SortField sortField = sort.sort.getSort()[0];
         if (sortField.getField() == null) {
             return null;