nested sorting: If sorting by nested field then the `nested_path` should always be specified.

Closes #13420

core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortParser.java

@@ -168,17 +168,6 @@ public class GeoDistanceSortParser implements SortParser {
             distances[i] = geoDistance.fixedSourceDistance(geoPoints.get(i).lat(), geoPoints.get(i).lon(), unit);
         }
 
-        // TODO: remove this in master, we should be explicit when we want to sort on nested fields and don't do anything automatically
-        if (nestedHelper == null || nestedHelper.getNestedObjectMapper() == null) {
-            ObjectMapper objectMapper = context.mapperService().resolveClosestNestedObjectMapper(fieldName);
-            if (objectMapper != null && objectMapper.nested().isNested()) {
-                if (nestedHelper == null) {
-                    nestedHelper = new NestedInnerQueryParseSupport(context.queryParserService().getParseContext());
-                }
-                nestedHelper.setPath(objectMapper.fullPath());
-            }
-        }
-
         final Nested nested;
         if (nestedHelper != null && nestedHelper.getPath() != null) {
             

core/src/main/java/org/elasticsearch/search/sort/SortParseElement.java

@@ -237,19 +237,6 @@ public class SortParseElement implements SearchParseElement {
                 sortMode = resolveDefaultSortMode(reverse);
             }
 
-            // TODO: remove this in master, we should be explicit when we want to sort on nested fields and don't do anything automatically
-            if (!(context instanceof SubSearchContext)) {
-                // Only automatically resolve nested path when sort isn't defined for top_hits
-                if (nestedHelper == null || nestedHelper.getNestedObjectMapper() == null) {
-                    ObjectMapper objectMapper = context.mapperService().resolveClosestNestedObjectMapper(fieldName);
-                    if (objectMapper != null && objectMapper.nested().isNested()) {
-                        if (nestedHelper == null) {
-                            nestedHelper = new NestedInnerQueryParseSupport(context.queryParserService().getParseContext());
-                        }
-                        nestedHelper.setPath(objectMapper.fullPath());
-                    }
-                }
-            }
             final Nested nested;
             if (nestedHelper != null && nestedHelper.getPath() != null) {
                 BitSetProducer rootDocumentsFilter = context.bitsetFilterCache().getBitSetProducer(Queries.newNonNestedFilter());

core/src/test/java/org/elasticsearch/nested/SimpleNestedIT.java

@@ -384,7 +384,7 @@ public class SimpleNestedIT extends ESIntegTestCase {
         SearchResponse searchResponse = client().prepareSearch("test")
                 .setTypes("type1")
                 .setQuery(QueryBuilders.matchAllQuery())
-                .addSort(SortBuilders.fieldSort("nested1.field1").order(SortOrder.ASC))
+                .addSort(SortBuilders.fieldSort("nested1.field1").order(SortOrder.ASC).setNestedPath("nested1"))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 3);
@@ -398,7 +398,7 @@ public class SimpleNestedIT extends ESIntegTestCase {
         searchResponse = client().prepareSearch("test")
                 .setTypes("type1")
                 .setQuery(QueryBuilders.matchAllQuery())
-                .addSort(SortBuilders.fieldSort("nested1.field1").order(SortOrder.DESC))
+                .addSort(SortBuilders.fieldSort("nested1.field1").order(SortOrder.DESC).setNestedPath("nested1"))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 3);
@@ -570,7 +570,7 @@ public class SimpleNestedIT extends ESIntegTestCase {
 
         SearchRequestBuilder searchRequestBuilder = client().prepareSearch("test").setTypes("type1")
                 .setQuery(QueryBuilders.matchAllQuery())
-                .addSort(SortBuilders.fieldSort("nested1.field1").setNestedFilter(termQuery("nested1.field2", true)).missing(10).order(SortOrder.ASC));
+                .addSort(SortBuilders.fieldSort("nested1.field1").setNestedPath("nested1").setNestedFilter(termQuery("nested1.field2", true)).missing(10).order(SortOrder.ASC));
 
         if (randomBoolean()) {
             searchRequestBuilder.setScroll("10m");
@@ -587,7 +587,7 @@ public class SimpleNestedIT extends ESIntegTestCase {
         assertThat(searchResponse.getHits().hits()[2].sortValues()[0].toString(), equalTo("10"));
 
         searchRequestBuilder = client().prepareSearch("test").setTypes("type1").setQuery(QueryBuilders.matchAllQuery())
-                .addSort(SortBuilders.fieldSort("nested1.field1").setNestedFilter(termQuery("nested1.field2", true)).missing(10).order(SortOrder.DESC));
+                .addSort(SortBuilders.fieldSort("nested1.field1").setNestedPath("nested1").setNestedFilter(termQuery("nested1.field2", true)).missing(10).order(SortOrder.DESC));
 
         if (randomBoolean()) {
             searchRequestBuilder.setScroll("10m");
@@ -766,6 +766,7 @@ public class SimpleNestedIT extends ESIntegTestCase {
                 .setQuery(matchAllQuery())
                 .addSort(
                         SortBuilders.fieldSort("parent.child.child_values")
+                                .setNestedPath("parent.child")
                                 .setNestedFilter(QueryBuilders.termQuery("parent.child.filter", true))
                                 .order(SortOrder.ASC)
                 )
@@ -824,6 +825,7 @@ public class SimpleNestedIT extends ESIntegTestCase {
                 .setQuery(matchAllQuery())
                 .addSort(
                         SortBuilders.fieldSort("parent.child.child_obj.value")
+                                .setNestedPath("parent.child")
                                 .setNestedFilter(QueryBuilders.termQuery("parent.child.filter", true))
                                 .order(SortOrder.ASC)
                 )
@@ -1080,6 +1082,7 @@ public class SimpleNestedIT extends ESIntegTestCase {
 
         SearchResponse searchResponse = client().prepareSearch("test")
                 .addSort(SortBuilders.fieldSort("users.first")
+                        .setNestedPath("users")
                         .order(SortOrder.ASC))
                 .addSort(SortBuilders.fieldSort("users.first")
                         .order(SortOrder.ASC)

core/src/test/java/org/elasticsearch/search/geo/GeoDistanceIT.java

@@ -551,7 +551,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
 
         // Order: Asc
         SearchResponse searchResponse = client().prepareSearch("companies").setQuery(matchAllQuery())
-                .addSort(SortBuilders.geoDistanceSort("branches.location").point(40.7143528, -74.0059731).order(SortOrder.ASC))
+                .addSort(SortBuilders.geoDistanceSort("branches.location").point(40.7143528, -74.0059731).order(SortOrder.ASC).setNestedPath("branches"))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 4);
@@ -563,7 +563,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
 
         // Order: Asc, Mode: max
         searchResponse = client().prepareSearch("companies").setQuery(matchAllQuery())
-                .addSort(SortBuilders.geoDistanceSort("branches.location").point(40.7143528, -74.0059731).order(SortOrder.ASC).sortMode("max"))
+                .addSort(SortBuilders.geoDistanceSort("branches.location").point(40.7143528, -74.0059731).order(SortOrder.ASC).sortMode("max").setNestedPath("branches"))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 4);
@@ -575,7 +575,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
 
         // Order: Desc
         searchResponse = client().prepareSearch("companies").setQuery(matchAllQuery())
-                .addSort(SortBuilders.geoDistanceSort("branches.location").point(40.7143528, -74.0059731).order(SortOrder.DESC))
+                .addSort(SortBuilders.geoDistanceSort("branches.location").point(40.7143528, -74.0059731).order(SortOrder.DESC).setNestedPath("branches"))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 4);
@@ -587,7 +587,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
 
         // Order: Desc, Mode: min
         searchResponse = client().prepareSearch("companies").setQuery(matchAllQuery())
-                .addSort(SortBuilders.geoDistanceSort("branches.location").point(40.7143528, -74.0059731).order(SortOrder.DESC).sortMode("min"))
+                .addSort(SortBuilders.geoDistanceSort("branches.location").point(40.7143528, -74.0059731).order(SortOrder.DESC).sortMode("min").setNestedPath("branches"))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 4);
@@ -598,7 +598,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
         assertThat(((Number) searchResponse.getHits().getAt(3).sortValues()[0]).doubleValue(), closeTo(0d, 10d));
 
         searchResponse = client().prepareSearch("companies").setQuery(matchAllQuery())
-                .addSort(SortBuilders.geoDistanceSort("branches.location").point(40.7143528, -74.0059731).sortMode("avg").order(SortOrder.ASC))
+                .addSort(SortBuilders.geoDistanceSort("branches.location").point(40.7143528, -74.0059731).sortMode("avg").order(SortOrder.ASC).setNestedPath("branches"))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 4);
@@ -611,7 +611,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
         searchResponse = client().prepareSearch("companies").setQuery(matchAllQuery())
                 .addSort(
                     SortBuilders.geoDistanceSort("branches.location").setNestedPath("branches")
-                            .point(40.7143528, -74.0059731).sortMode("avg").order(SortOrder.DESC)
+                            .point(40.7143528, -74.0059731).sortMode("avg").order(SortOrder.DESC).setNestedPath("branches")
                 )
                 .execute().actionGet();
 
@@ -625,7 +625,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
         searchResponse = client().prepareSearch("companies").setQuery(matchAllQuery())
                 .addSort(
                         SortBuilders.geoDistanceSort("branches.location").setNestedFilter(termQuery("branches.name", "brooklyn"))
-                                .point(40.7143528, -74.0059731).sortMode("avg").order(SortOrder.ASC)
+                                .point(40.7143528, -74.0059731).sortMode("avg").order(SortOrder.ASC).setNestedPath("branches")
                 )
                 .execute().actionGet();
         assertHitCount(searchResponse, 4);
@@ -637,7 +637,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
         assertThat(((Number) searchResponse.getHits().getAt(3).sortValues()[0]).doubleValue(), equalTo(Double.MAX_VALUE));
 
         assertFailures(client().prepareSearch("companies").setQuery(matchAllQuery())
-                    .addSort(SortBuilders.geoDistanceSort("branches.location").point(40.7143528, -74.0059731).sortMode("sum")),
+                    .addSort(SortBuilders.geoDistanceSort("branches.location").point(40.7143528, -74.0059731).sortMode("sum").setNestedPath("branches")),
                 RestStatus.BAD_REQUEST,
                 containsString("sort_mode [sum] isn't supported for sorting by geo distance"));
     }

core/src/test/java/org/elasticsearch/search/sort/SimpleSortIT.java

@@ -1661,7 +1661,7 @@ public class SimpleSortIT extends ESIntegTestCase {
         // We sort on nested field
         SearchResponse searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
-                .addSort("nested.foo", SortOrder.DESC)
+                .addSort(SortBuilders.fieldSort("nested.foo").setNestedPath("nested").order(SortOrder.DESC))
                 .execute().actionGet();
         assertNoFailures(searchResponse);
         SearchHit[] hits = searchResponse.getHits().hits();
@@ -1678,7 +1678,7 @@ public class SimpleSortIT extends ESIntegTestCase {
         // We sort on nested sub field
         searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
-                .addSort("nested.foo.sub", SortOrder.DESC)
+                .addSort(SortBuilders.fieldSort("nested.foo.sub").setNestedPath("nested").order(SortOrder.DESC))
                 .execute().actionGet();
         assertNoFailures(searchResponse);
         hits = searchResponse.getHits().hits();

docs/reference/migration/migrate_2_1.asciidoc

@@ -50,3 +50,9 @@ to using the unlike methods.
 
 MoreLikeThisBuilder#addItem has been deprecated in favor to using
 MoreLikeThisBuilder#addLikeItem.
+
+=== Nested sorting
+
+If sorting on field inside a nested object then the `nested_path` should be specified.
+Before there was an attempt to resolve the nested path automatically, but that was sometimes incorrect.
+To avoid confusion the `nested_path` should always be specified.

docs/reference/search/request/sort.asciidoc

@@ -97,10 +97,8 @@ existing sort options:
 
 ===== Nested sorting example
 
-In the below example `offer` is a field of type `nested`. Because
+In the below example `offer` is a field of type `nested`.
-`offer` is the closest inherited nested field, it is picked as
+The `nested_path` needs to be specified other elasticsearch doesn't on what nested level sort values need to be captured.
-`nested_path`. Only the inner objects that have color blue will
-participate in sorting.
 
 [source,js]
 --------------------------------------------------
@@ -113,6 +111,7 @@ curl -XPOST 'localhost:9200/_search' -d '{
           "offer.price" : {
              "mode" :  "avg",
              "order" : "asc",
+             "nested_path" : "offer",
              "nested_filter" : {
                 "term" : { "offer.color" : "blue" }
              }