From c94c434cd1774919da8a153c95a22338577c86ad Mon Sep 17 00:00:00 2001 From: Anthony Leong Date: Sat, 26 Jul 2025 00:28:40 -0700 Subject: [PATCH 01/11] null catch Signed-off-by: Anthony Leong --- .../opensearch/search/internal/ExitableDirectoryReader.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/src/main/java/org/opensearch/search/internal/ExitableDirectoryReader.java b/server/src/main/java/org/opensearch/search/internal/ExitableDirectoryReader.java index 83ecd11a493ef..24a1072d12f65 100644 --- a/server/src/main/java/org/opensearch/search/internal/ExitableDirectoryReader.java +++ b/server/src/main/java/org/opensearch/search/internal/ExitableDirectoryReader.java @@ -108,6 +108,9 @@ private ExitableLeafReader(LeafReader leafReader, QueryCancellation queryCancell @Override public PointValues getPointValues(String field) throws IOException { + if (field == null) { + return null; + } final PointValues pointValues = in.getPointValues(field); if (pointValues == null) { return null; From de82425ec12e596dd4a9755db9d090f4bc9fe81d Mon Sep 17 00:00:00 2001 From: Anthony Leong Date: Sat, 26 Jul 2025 19:46:57 -0700 Subject: [PATCH 02/11] added tests for code coverage Signed-off-by: Anthony Leong --- .../search/sort/GeoDistanceSortBuilderIT.java | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/server/src/internalClusterTest/java/org/opensearch/search/sort/GeoDistanceSortBuilderIT.java b/server/src/internalClusterTest/java/org/opensearch/search/sort/GeoDistanceSortBuilderIT.java index b6f53936d5939..c5a5cdc8effc3 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/sort/GeoDistanceSortBuilderIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/sort/GeoDistanceSortBuilderIT.java @@ -42,6 +42,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.DistanceUnit; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.index.query.BoolQueryBuilder; import org.opensearch.index.query.GeoValidationMethod; import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.test.ParameterizedStaticSettingsOpenSearchIntegTestCase; @@ -56,6 +57,8 @@ import java.util.concurrent.ExecutionException; import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.opensearch.index.query.QueryBuilders.boolQuery; +import static org.opensearch.index.query.QueryBuilders.existsQuery; import static org.opensearch.index.query.QueryBuilders.matchAllQuery; import static org.opensearch.search.SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING; import static org.opensearch.search.sort.SortBuilders.fieldSort; @@ -430,4 +433,38 @@ public void testCrossIndexIgnoreUnmapped() throws Exception { new Object[] { Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY } ); } + + public void testGeoDistanceQueryThenSort() throws Exception { + assertAcked(prepareCreate("index").setMapping("admin", "type=keyword", LOCATION_FIELD, "type=geo_point")); + + indexRandom( + true, + client().prepareIndex("index") + .setId("d1") + .setSource( + jsonBuilder().startObject() + .startObject(LOCATION_FIELD) + .field("lat", 48.8331) + .field("lon", 2.3264) + .endObject() + .field("admin", "11") + .endObject() + ) + ); + + GeoDistanceSortBuilder geoDistanceSortBuilder = new GeoDistanceSortBuilder(LOCATION_FIELD, new GeoPoint(40.7128, -74.0060)); + + BoolQueryBuilder bool = boolQuery().filter(existsQuery(LOCATION_FIELD)); + + SearchResponse searchResponse = client().prepareSearch() + .setQuery(bool) + .addSort(geoDistanceSortBuilder.unit(DistanceUnit.KILOMETERS).ignoreUnmapped(true).order(SortOrder.DESC)) + .setSize(4) + .get(); + assertOrderedSearchHits(searchResponse, "d1"); + assertThat( + (Double) searchResponse.getHits().getAt(0).getSortValues()[0], + closeTo(GeoDistance.ARC.calculate(40.7128, -74.0060, 48.8331, 2.3264, DistanceUnit.KILOMETERS), 1.e-1) + ); + } } From 70cf1f66d833da71cbe5c8c7c02f4a0263ba64de Mon Sep 17 00:00:00 2001 From: Anthony Leong Date: Mon, 28 Jul 2025 15:58:29 -0700 Subject: [PATCH 03/11] Update server/src/main/java/org/opensearch/search/internal/ExitableDirectoryReader.java Co-authored-by: Owais Kazi Signed-off-by: Anthony Leong --- .../org/opensearch/search/internal/ExitableDirectoryReader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/search/internal/ExitableDirectoryReader.java b/server/src/main/java/org/opensearch/search/internal/ExitableDirectoryReader.java index 24a1072d12f65..3749518735c15 100644 --- a/server/src/main/java/org/opensearch/search/internal/ExitableDirectoryReader.java +++ b/server/src/main/java/org/opensearch/search/internal/ExitableDirectoryReader.java @@ -108,7 +108,7 @@ private ExitableLeafReader(LeafReader leafReader, QueryCancellation queryCancell @Override public PointValues getPointValues(String field) throws IOException { - if (field == null) { + if (Strings.isEmpty(field)) { return null; } final PointValues pointValues = in.getPointValues(field); From 97b94f5e61d16e3705a5a719151d394c5a6678c5 Mon Sep 17 00:00:00 2001 From: Anthony Leong Date: Mon, 28 Jul 2025 17:01:27 -0700 Subject: [PATCH 04/11] empty string check Signed-off-by: Anthony Leong --- .../org/opensearch/search/internal/ExitableDirectoryReader.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/java/org/opensearch/search/internal/ExitableDirectoryReader.java b/server/src/main/java/org/opensearch/search/internal/ExitableDirectoryReader.java index 3749518735c15..5622b0709bedf 100644 --- a/server/src/main/java/org/opensearch/search/internal/ExitableDirectoryReader.java +++ b/server/src/main/java/org/opensearch/search/internal/ExitableDirectoryReader.java @@ -45,6 +45,7 @@ import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.automaton.CompiledAutomaton; import org.opensearch.common.lucene.index.SequentialStoredFieldsLeafReader; +import org.opensearch.core.common.Strings; import java.io.IOException; From 22ab054269530492c514de3102200938361ae048 Mon Sep 17 00:00:00 2001 From: Anthony Leong Date: Tue, 29 Jul 2025 11:30:26 -0700 Subject: [PATCH 05/11] add yaml test Signed-off-by: Anthony Leong --- .../test/search/260_sort_geopoint.yml | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_geopoint.yml diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_geopoint.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_geopoint.yml new file mode 100644 index 0000000000000..acb07b67afd6d --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_geopoint.yml @@ -0,0 +1,30 @@ +setup: + - do: + indices.create: + index: index + body: + mappings: + properties: + admin: + type: keyword + location: + type: geo_point + - do: + index: + index: index + refresh: true + body: {admin: "11", location: {lat: 48.8331, lon: 2.3264}} + +--- +"test query then sort with geo_point distance": + + - do: + search: + index: index + body: + query: + exists: + field: location + sort: [{_geo_distance: {location: {lat: 40.7128, lon: -74.0060}, ignore_unmapped: true, order: "desc", unit: "km"}}] + size: 4 + - match: { hits.total.value: 1 } From 64983fd979057d6eec2538b8b2046e0ad3d3c33b Mon Sep 17 00:00:00 2001 From: Anthony Leong Date: Tue, 29 Jul 2025 12:21:51 -0700 Subject: [PATCH 06/11] add extra test Signed-off-by: Anthony Leong --- .../test/search/260_sort_geopoint.yml | 9 +++++++++ .../search/sort/GeoDistanceSortBuilderIT.java | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_geopoint.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_geopoint.yml index acb07b67afd6d..ff40669542178 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_geopoint.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/260_sort_geopoint.yml @@ -28,3 +28,12 @@ setup: sort: [{_geo_distance: {location: {lat: 40.7128, lon: -74.0060}, ignore_unmapped: true, order: "desc", unit: "km"}}] size: 4 - match: { hits.total.value: 1 } + + - do: + search: + index: index + body: + query: { match_all: {} } + sort: [{_geo_distance: {location: [9.227400, 49.189800], order: "desc", unit: "km", distance_type: "arc", mode: "min", ignore_unmapped: true } } ] + size: 10 + - match: { hits.total.value: 1 } diff --git a/server/src/internalClusterTest/java/org/opensearch/search/sort/GeoDistanceSortBuilderIT.java b/server/src/internalClusterTest/java/org/opensearch/search/sort/GeoDistanceSortBuilderIT.java index c5a5cdc8effc3..0b3501cdb6d73 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/sort/GeoDistanceSortBuilderIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/sort/GeoDistanceSortBuilderIT.java @@ -44,6 +44,7 @@ import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.index.query.BoolQueryBuilder; import org.opensearch.index.query.GeoValidationMethod; +import org.opensearch.index.query.MatchAllQueryBuilder; import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.test.ParameterizedStaticSettingsOpenSearchIntegTestCase; import org.opensearch.test.VersionUtils; @@ -466,5 +467,22 @@ public void testGeoDistanceQueryThenSort() throws Exception { (Double) searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.ARC.calculate(40.7128, -74.0060, 48.8331, 2.3264, DistanceUnit.KILOMETERS), 1.e-1) ); + + geoDistanceSortBuilder = new GeoDistanceSortBuilder(LOCATION_FIELD, new GeoPoint(9.227400, 49.189800)); + searchResponse = client().prepareSearch() + .setQuery(new MatchAllQueryBuilder()) + .addSort(geoDistanceSortBuilder + .unit(DistanceUnit.KILOMETERS) + .ignoreUnmapped(true) + .order(SortOrder.DESC) + .geoDistance(GeoDistance.ARC) + .sortMode(SortMode.MIN)) + .setSize(10) + .get(); + assertOrderedSearchHits(searchResponse, "d1"); + assertThat( + (Double) searchResponse.getHits().getAt(0).getSortValues()[0], + closeTo(GeoDistance.ARC.calculate(9.227400, 49.189800, 48.8331, 2.3264, DistanceUnit.KILOMETERS), 1.e-1) + ); } } From a8d94d9e69e6615bf535ddbc736fe0598ebc9779 Mon Sep 17 00:00:00 2001 From: Anthony Leong Date: Tue, 29 Jul 2025 15:33:48 -0700 Subject: [PATCH 07/11] spotless apply Signed-off-by: Anthony Leong --- .../search/sort/GeoDistanceSortBuilderIT.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/search/sort/GeoDistanceSortBuilderIT.java b/server/src/internalClusterTest/java/org/opensearch/search/sort/GeoDistanceSortBuilderIT.java index 0b3501cdb6d73..d27f7d0d24da4 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/sort/GeoDistanceSortBuilderIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/sort/GeoDistanceSortBuilderIT.java @@ -471,12 +471,13 @@ public void testGeoDistanceQueryThenSort() throws Exception { geoDistanceSortBuilder = new GeoDistanceSortBuilder(LOCATION_FIELD, new GeoPoint(9.227400, 49.189800)); searchResponse = client().prepareSearch() .setQuery(new MatchAllQueryBuilder()) - .addSort(geoDistanceSortBuilder - .unit(DistanceUnit.KILOMETERS) - .ignoreUnmapped(true) - .order(SortOrder.DESC) - .geoDistance(GeoDistance.ARC) - .sortMode(SortMode.MIN)) + .addSort( + geoDistanceSortBuilder.unit(DistanceUnit.KILOMETERS) + .ignoreUnmapped(true) + .order(SortOrder.DESC) + .geoDistance(GeoDistance.ARC) + .sortMode(SortMode.MIN) + ) .setSize(10) .get(); assertOrderedSearchHits(searchResponse, "d1"); From 02e9f80afb9e93087b317086ed0ff16d7dea7834 Mon Sep 17 00:00:00 2001 From: Anthony Leong Date: Wed, 30 Jul 2025 09:47:04 -0700 Subject: [PATCH 08/11] random change to rerun workflow Signed-off-by: Anthony Leong --- ADMINS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ADMINS.md b/ADMINS.md index 3f75d8ac1a7ce..a4fe5cd8ac01b 100644 --- a/ADMINS.md +++ b/ADMINS.md @@ -1,3 +1,3 @@ ## Admins -Kindly refer to [ADMINS.md](https://github.com/opensearch-project/.github/blob/main/ADMINS.md) for opensearch-project administrators, their responsibilities in this repository, and their operational procedures. If you're interested in becoming a maintainer, see [MAINTAINERS](MAINTAINERS.md). If you're interested in contributing, see [CONTRIBUTING](CONTRIBUTING.md). +indly refer to [ADMINS.md](https://github.com/opensearch-project/.github/blob/main/ADMINS.md) for opensearch-project administrators, their responsibilities in this repository, and their operational procedures. If you're interested in becoming a maintainer, see [MAINTAINERS](MAINTAINERS.md). If you're interested in contributing, see [CONTRIBUTING](CONTRIBUTING.md). From 02e7de5e1242b22e78b8e42148478c59d01a33b8 Mon Sep 17 00:00:00 2001 From: Anthony Leong Date: Wed, 30 Jul 2025 09:47:17 -0700 Subject: [PATCH 09/11] rerun workflow Signed-off-by: Anthony Leong --- ADMINS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ADMINS.md b/ADMINS.md index a4fe5cd8ac01b..3f75d8ac1a7ce 100644 --- a/ADMINS.md +++ b/ADMINS.md @@ -1,3 +1,3 @@ ## Admins -indly refer to [ADMINS.md](https://github.com/opensearch-project/.github/blob/main/ADMINS.md) for opensearch-project administrators, their responsibilities in this repository, and their operational procedures. If you're interested in becoming a maintainer, see [MAINTAINERS](MAINTAINERS.md). If you're interested in contributing, see [CONTRIBUTING](CONTRIBUTING.md). +Kindly refer to [ADMINS.md](https://github.com/opensearch-project/.github/blob/main/ADMINS.md) for opensearch-project administrators, their responsibilities in this repository, and their operational procedures. If you're interested in becoming a maintainer, see [MAINTAINERS](MAINTAINERS.md). If you're interested in contributing, see [CONTRIBUTING](CONTRIBUTING.md). From 3651294785e3790984d90bba5a914a61d54e4390 Mon Sep 17 00:00:00 2001 From: Anthony Leong Date: Wed, 30 Jul 2025 16:05:05 -0700 Subject: [PATCH 10/11] added getPointValues unit test Signed-off-by: Anthony Leong --- .../test/java/org/opensearch/search/SearchCancellationTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/test/java/org/opensearch/search/SearchCancellationTests.java b/server/src/test/java/org/opensearch/search/SearchCancellationTests.java index 266052444da46..a7a2a9ed11b19 100644 --- a/server/src/test/java/org/opensearch/search/SearchCancellationTests.java +++ b/server/src/test/java/org/opensearch/search/SearchCancellationTests.java @@ -201,6 +201,7 @@ public void testExitableDirectoryReader() throws IOException { ); cancelled.set(false); // Avoid exception during construction of the wrapper objects + assertNull(searcher.getIndexReader().leaves().get(0).reader().getPointValues(null)); Terms terms = searcher.getIndexReader().leaves().get(0).reader().terms(STRING_FIELD_NAME); TermsEnum termsIterator = terms.iterator(); TermsEnum termsIntersect = terms.intersect(automaton, null); From 42b6129c44257e94367c9f386cedf1ea99f5169c Mon Sep 17 00:00:00 2001 From: Anthony Leong Date: Mon, 4 Aug 2025 21:40:50 -0700 Subject: [PATCH 11/11] non-deterministic tests Signed-off-by: Anthony Leong