Browse Source

Remove deprecated sort options: nested_path and nested_filter (#42809)

This commit removes the nested_path and nested_filter options deprecated in 6x.
This change also checks that the sort field has a [nested] option if it is under a nested
object and throws an exception if it's not the case.

Closes #27098
Jim Ferenczi 6 years ago
parent
commit
a614415838

+ 5 - 0
docs/reference/migration/migrate_8_0/search.asciidoc

@@ -20,3 +20,8 @@ The same functionality can be achieved by the `match` query if the total number
 
 The `cutoff_frequency` parameter was deprecated in 7.x and has been removed in 8.0 from `match` and `multi_match` queries.
 The same functionality can be achieved without any configuration provided that the total number of hits is not tracked.
+
+[float]
+===== Removal of sort parameters
+
+The `nested_filter` and `nested_path` options, deprecated in 6.x, have been removed in favor of the `nested` context.

+ 5 - 11
docs/reference/search/request/sort.asciidoc

@@ -252,7 +252,7 @@ field support has a `nested` sort option with the following properties:
     A filter that the inner objects inside the nested path
     should match with in order for its field values to be taken into account
     by sorting. Common case is to repeat the query / filter inside the
-    nested filter or query. By default no `nested_filter` is active.
+    nested filter or query. By default no `filter` is active.
 `max_children`::
     The maximum number of children to consider per root document
     when picking the sort value. Defaults to unlimited.
@@ -260,14 +260,8 @@ field support has a `nested` sort option with the following properties:
     Same as top-level `nested` but applies to another nested path within the
     current nested object.
 
-[WARNING]
-.Nested sort options before Elasticsearch 6.1
-============================================
-
-The `nested_path` and `nested_filter` options have been deprecated in
-favor of the options documented above.
-
-============================================
+NOTE: Elasticsearch will throw an error if a nested field is defined in a sort without
+a `nested` context.
 
 ===== Nested sorting examples
 
@@ -300,7 +294,7 @@ POST /_search
 // CONSOLE
 
 In the below example `parent` and `child` fields are of type `nested`.
-The `nested_path` needs to be specified at each level; otherwise, Elasticsearch doesn't know on what nested level sort values need to be captured.
+The `nested.path` needs to be specified at each level; otherwise, Elasticsearch doesn't know on what nested level sort values need to be captured.
 
 [source,js]
 --------------------------------------------------
@@ -374,7 +368,7 @@ GET /_search
 // CONSOLE
 
 NOTE: If a nested inner object doesn't match with
-the `nested_filter` then a missing value is used.
+the `nested.filter` then a missing value is used.
 
 ==== Ignoring Unmapped Fields
 

+ 6 - 3
rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/200_top_hits_metric.yml

@@ -92,16 +92,19 @@ setup:
               aggs:
                 users:
                   top_hits:
-                    sort: "users.last.keyword"
+                    sort:
+                      users.last.keyword:
+                        nested:
+                          path: users
                     seq_no_primary_term: true
 
   - match: { hits.total: 2 }
   - length: { aggregations.groups.buckets.0.users.hits.hits: 2 }
-  - match: { aggregations.groups.buckets.0.users.hits.hits.0._id: "1" }
+  - match: { aggregations.groups.buckets.0.users.hits.hits.0._id: "2" }
   - match: { aggregations.groups.buckets.0.users.hits.hits.0._index: my-index }
   - gte: { aggregations.groups.buckets.0.users.hits.hits.0._seq_no: 0 }
   - gte: { aggregations.groups.buckets.0.users.hits.hits.0._primary_term: 1 }
-  - match: { aggregations.groups.buckets.0.users.hits.hits.1._id: "2" }
+  - match: { aggregations.groups.buckets.0.users.hits.hits.1._id: "1" }
   - match: { aggregations.groups.buckets.0.users.hits.hits.1._index: my-index }
   - gte: { aggregations.groups.buckets.0.users.hits.hits.1._seq_no: 0 }
   - gte: { aggregations.groups.buckets.0.users.hits.hits.1._primary_term: 1 }

+ 1 - 1
server/src/main/java/org/elasticsearch/index/search/NestedHelper.java

@@ -192,7 +192,7 @@ public final class NestedHelper {
         return true; // the field is not a sub field of the nested path
     }
 
-    private static String parentObject(String field) {
+    public static String parentObject(String field) {
         int lastDot = field.lastIndexOf('.');
         if (lastDot == -1) {
             return null;

+ 86 - 145
server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java

@@ -19,13 +19,11 @@
 
 package org.elasticsearch.search.sort;
 
-import org.apache.logging.log4j.LogManager;
 import org.apache.lucene.search.SortField;
 import org.elasticsearch.Version;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
-import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.xcontent.ObjectParser;
 import org.elasticsearch.common.xcontent.ObjectParser.ValueType;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -36,6 +34,7 @@ import org.elasticsearch.index.fielddata.IndexNumericFieldData;
 import org.elasticsearch.index.fielddata.IndexNumericFieldData.NumericType;
 import org.elasticsearch.index.fielddata.plain.SortedNumericDVIndexFieldData;
 import org.elasticsearch.index.mapper.MappedFieldType;
+import org.elasticsearch.index.mapper.ObjectMapper;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.index.query.QueryRewriteContext;
 import org.elasticsearch.index.query.QueryShardContext;
@@ -47,13 +46,13 @@ import java.io.IOException;
 import java.util.Locale;
 import java.util.Objects;
 
+import static org.elasticsearch.index.search.NestedHelper.parentObject;
 import static org.elasticsearch.search.sort.NestedSortBuilder.NESTED_FIELD;
 
 /**
  * A sort builder to sort based on a document field.
  */
 public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
-    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(FieldSortBuilder.class));
 
     public static final String NAME = "field_sort";
     public static final ParseField MISSING = new ParseField("missing");
@@ -80,10 +79,6 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
 
     private SortMode sortMode;
 
-    private QueryBuilder nestedFilter;
-
-    private String nestedPath;
-
     private NestedSortBuilder nestedSort;
 
     /** Copy constructor. */
@@ -95,8 +90,6 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
         if (template.sortMode != null) {
             this.sortMode(template.sortMode());
         }
-        this.setNestedFilter(template.getNestedFilter());
-        this.setNestedPath(template.getNestedPath());
         if (template.getNestedSort() != null) {
             this.setNestedSort(template.getNestedSort());
         }
@@ -121,8 +114,12 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
      */
     public FieldSortBuilder(StreamInput in) throws IOException {
         fieldName = in.readString();
-        nestedFilter = in.readOptionalNamedWriteable(QueryBuilder.class);
-        nestedPath = in.readOptionalString();
+        if (in.getVersion().before(Version.V_8_0_0)) {
+            if (in.readOptionalNamedWriteable(QueryBuilder.class) != null || in.readOptionalString() != null) {
+                throw new IOException("the [sort] options [nested_path] and [nested_filter] are removed in 8.x, " +
+                    "please use [nested] instead");
+            }
+        }
         missing = in.readGenericValue();
         order = in.readOptionalWriteable(SortOrder::readFromStream);
         sortMode = in.readOptionalWriteable(SortMode::readFromStream);
@@ -136,8 +133,10 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         out.writeString(fieldName);
-        out.writeOptionalNamedWriteable(nestedFilter);
-        out.writeOptionalString(nestedPath);
+        if (out.getVersion().before(Version.V_8_0_0)) {
+            out.writeOptionalNamedWriteable(null);
+            out.writeOptionalString(null);
+        }
         out.writeGenericValue(missing);
         out.writeOptionalWriteable(order);
         out.writeOptionalWriteable(sortMode);
@@ -210,58 +209,6 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
         return this.sortMode;
     }
 
-    /**
-     * Sets the nested filter that the nested objects should match with in order
-     * to be taken into account for sorting.
-     *
-     * @deprecated set nested sort with {@link #setNestedSort(NestedSortBuilder)} and retrieve with {@link #getNestedSort()}
-     */
-    @Deprecated
-    public FieldSortBuilder setNestedFilter(QueryBuilder nestedFilter) {
-        if (this.nestedSort != null) {
-            throw new IllegalArgumentException("Setting both nested_path/nested_filter and nested not allowed");
-        }
-        this.nestedFilter = nestedFilter;
-        return this;
-    }
-
-    /**
-     * Returns the nested filter that the nested objects should match with in
-     * order to be taken into account for sorting.
-     *
-     * @deprecated set nested sort with {@link #setNestedSort(NestedSortBuilder)} and retrieve with {@link #getNestedSort()}
-     */
-    @Deprecated
-    public QueryBuilder getNestedFilter() {
-        return this.nestedFilter;
-    }
-
-    /**
-     * Sets the nested path if sorting occurs on a field that is inside a nested
-     * object. By default when sorting on a field inside a nested object, the
-     * nearest upper nested object is selected as nested path.
-     *
-     * @deprecated set nested sort with {@link #setNestedSort(NestedSortBuilder)} and retrieve with {@link #getNestedSort()}
-     */
-    @Deprecated
-    public FieldSortBuilder setNestedPath(String nestedPath) {
-        if (this.nestedSort != null) {
-            throw new IllegalArgumentException("Setting both nested_path/nested_filter and nested not allowed");
-        }
-        this.nestedPath = nestedPath;
-        return this;
-    }
-
-    /**
-     * Returns the nested path if sorting occurs in a field that is inside a
-     * nested object.
-     * @deprecated set nested sort with {@link #setNestedSort(NestedSortBuilder)} and retrieve with {@link #getNestedSort()}
-     */
-    @Deprecated
-    public String getNestedPath() {
-        return this.nestedPath;
-    }
-
     /**
      * Returns the {@link NestedSortBuilder}
      */
@@ -276,9 +223,6 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
      * order to be taken into account for sorting.
      */
     public FieldSortBuilder setNestedSort(final NestedSortBuilder nestedSort) {
-        if (this.nestedFilter != null || this.nestedPath != null) {
-            throw new IllegalArgumentException("Setting both nested_path/nested_filter and nested not allowed");
-        }
         this.nestedSort = nestedSort;
         return this;
     }
@@ -330,12 +274,6 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
         if (sortMode != null) {
             builder.field(SORT_MODE.getPreferredName(), sortMode);
         }
-        if (nestedFilter != null) {
-            builder.field(NESTED_FILTER_FIELD.getPreferredName(), nestedFilter, params);
-        }
-        if (nestedPath != null) {
-            builder.field(NESTED_PATH_FIELD.getPreferredName(), nestedPath);
-        }
         if (nestedSort != null) {
             builder.field(NESTED_FIELD.getPreferredName(), nestedSort);
         }
@@ -367,65 +305,85 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
     @Override
     public SortFieldAndFormat build(QueryShardContext context) throws IOException {
         if (DOC_FIELD_NAME.equals(fieldName)) {
-            if (order == SortOrder.DESC) {
-                return SORT_DOC_REVERSE;
+            return order == SortOrder.DESC ? SORT_DOC_REVERSE : SORT_DOC;
+        }
+
+        boolean isUnmapped = false;
+        MappedFieldType fieldType = context.fieldMapper(fieldName);
+        if (fieldType == null) {
+            isUnmapped = true;
+            if (unmappedType != null) {
+                fieldType = context.getMapperService().unmappedFieldType(unmappedType);
             } else {
-                return SORT_DOC;
-            }
-        } else {
-            boolean isUnmapped = false;
-            MappedFieldType fieldType = context.fieldMapper(fieldName);
-            if (fieldType == null) {
-                isUnmapped = true;
-                if (unmappedType != null) {
-                    fieldType = context.getMapperService().unmappedFieldType(unmappedType);
-                } else {
-                    throw new QueryShardException(context, "No mapping found for [" + fieldName + "] in order to sort on");
-                }
+                throw new QueryShardException(context, "No mapping found for [" + fieldName + "] in order to sort on");
             }
+        }
 
-            MultiValueMode localSortMode = null;
-            if (sortMode != null) {
-                localSortMode = MultiValueMode.fromString(sortMode.toString());
-            }
+        MultiValueMode localSortMode = null;
+        if (sortMode != null) {
+            localSortMode = MultiValueMode.fromString(sortMode.toString());
+        }
 
-            boolean reverse = (order == SortOrder.DESC);
-            if (localSortMode == null) {
-                localSortMode = reverse ? MultiValueMode.MAX : MultiValueMode.MIN;
-            }
+        boolean reverse = (order == SortOrder.DESC);
+        if (localSortMode == null) {
+            localSortMode = reverse ? MultiValueMode.MAX : MultiValueMode.MIN;
+        }
 
-            Nested nested = null;
-            if (isUnmapped == false) {
-                if (nestedSort != null) {
-                    if (nestedSort.getNestedSort() != null && nestedSort.getMaxChildren() != Integer.MAX_VALUE) {
-                        throw new QueryShardException(context,
-                            "max_children is only supported on last level of nested sort");
-                    }
-                    // new nested sorts takes priority
-                    nested = resolveNested(context, nestedSort);
-                } else {
-                    nested = resolveNested(context, nestedPath, nestedFilter);
+        Nested nested = null;
+        if (isUnmapped == false) {
+            if (nestedSort != null) {
+                if (nestedSort.getNestedSort() != null && nestedSort.getMaxChildren() != Integer.MAX_VALUE) {
+                    throw new QueryShardException(context,
+                        "max_children is only supported on last level of nested sort");
                 }
+                nested = resolveNested(context, nestedSort);
+            } else {
+                validateMissingNestedPath(context, fieldName);
             }
+        }
 
-            IndexFieldData<?> fieldData = context.getForField(fieldType);
-            if (fieldData instanceof IndexNumericFieldData == false
-                    && (sortMode == SortMode.SUM || sortMode == SortMode.AVG || sortMode == SortMode.MEDIAN)) {
-                throw new QueryShardException(context, "we only support AVG, MEDIAN and SUM on number based fields");
+        IndexFieldData<?> fieldData = context.getForField(fieldType);
+        if (fieldData instanceof IndexNumericFieldData == false
+                && (sortMode == SortMode.SUM || sortMode == SortMode.AVG || sortMode == SortMode.MEDIAN)) {
+            throw new QueryShardException(context, "we only support AVG, MEDIAN and SUM on number based fields");
+        }
+        final SortField field;
+        if (numericType != null) {
+            if (fieldData instanceof IndexNumericFieldData == false) {
+                throw new QueryShardException(context,
+                    "[numeric_type] option cannot be set on a non-numeric field, got " + fieldType.typeName());
             }
-            final SortField field;
-            if (numericType != null) {
-                if (fieldData instanceof IndexNumericFieldData == false) {
+            SortedNumericDVIndexFieldData numericFieldData = (SortedNumericDVIndexFieldData) fieldData;
+            NumericType resolvedType = resolveNumericType(numericType);
+            field = numericFieldData.sortField(resolvedType, missing, localSortMode, nested, reverse);
+        } else {
+            field = fieldData.sortField(missing, localSortMode, nested, reverse);
+        }
+        return new SortFieldAndFormat(field, fieldType.docValueFormat(null, null));
+    }
+
+    /**
+     * Throws an exception if the provided <code>field</code> requires a nested context.
+     */
+    static void validateMissingNestedPath(QueryShardContext context, String field) {
+        ObjectMapper contextMapper = context.nestedScope().getObjectMapper();
+        if (contextMapper != null && contextMapper.nested().isNested() == false) {
+            // already in nested context
+            return;
+        }
+        for (String parent = parentObject(field); parent != null; parent = parentObject(parent)) {
+            ObjectMapper parentMapper = context.getObjectMapper(parent);
+            if (parentMapper != null && parentMapper.nested().isNested()) {
+                if (contextMapper != null && contextMapper.fullPath().equals(parentMapper.fullPath())) {
+                    // we are in a nested context that matches the path of the provided field so the nested path
+                    // is not required
+                    return ;
+                }
+                if (parentMapper.nested().isIncludeInRoot() == false) {
                     throw new QueryShardException(context,
-                        "[numeric_type] option cannot be set on a non-numeric field, got " + fieldType.typeName());
+                        "it is mandatory to set the [nested] context on the nested sort field: [" + field + "].");
                 }
-                SortedNumericDVIndexFieldData numericFieldData = (SortedNumericDVIndexFieldData) fieldData;
-                NumericType resolvedType = resolveNumericType(numericType);
-                field = numericFieldData.sortField(resolvedType, missing, localSortMode, nested, reverse);
-            } else {
-                field = fieldData.sortField(missing, localSortMode, nested, reverse);
             }
-            return new SortFieldAndFormat(field, fieldType.docValueFormat(null, null));
         }
     }
 
@@ -440,8 +398,7 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
         }
 
         FieldSortBuilder builder = (FieldSortBuilder) other;
-        return (Objects.equals(this.fieldName, builder.fieldName) && Objects.equals(this.nestedFilter, builder.nestedFilter)
-                && Objects.equals(this.nestedPath, builder.nestedPath) && Objects.equals(this.missing, builder.missing)
+        return (Objects.equals(this.fieldName, builder.fieldName) && Objects.equals(this.missing, builder.missing)
                 && Objects.equals(this.order, builder.order) && Objects.equals(this.sortMode, builder.sortMode)
                 && Objects.equals(this.unmappedType, builder.unmappedType) && Objects.equals(this.nestedSort, builder.nestedSort))
                 && Objects.equals(this.numericType, builder.numericType);
@@ -449,7 +406,7 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
 
     @Override
     public int hashCode() {
-        return Objects.hash(this.fieldName, this.nestedFilter, this.nestedPath, this.nestedSort, this.missing, this.order, this.sortMode,
+        return Objects.hash(this.fieldName, this.nestedSort, this.missing, this.order, this.sortMode,
             this.unmappedType, this.numericType);
     }
 
@@ -475,38 +432,22 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
 
     static {
         PARSER.declareField(FieldSortBuilder::missing, p -> p.objectText(),  MISSING, ValueType.VALUE);
-        PARSER.declareString((fieldSortBuilder, nestedPath) -> {
-            deprecationLogger.deprecated("[nested_path] has been deprecated in favor of the [nested] parameter");
-            fieldSortBuilder.setNestedPath(nestedPath);
-        }, NESTED_PATH_FIELD);
         PARSER.declareString(FieldSortBuilder::unmappedType , UNMAPPED_TYPE);
         PARSER.declareString((b, v) -> b.order(SortOrder.fromString(v)) , ORDER_FIELD);
         PARSER.declareString((b, v) -> b.sortMode(SortMode.fromString(v)), SORT_MODE);
-        PARSER.declareObject(FieldSortBuilder::setNestedFilter, (p, c) -> {
-            deprecationLogger.deprecated("[nested_filter] has been deprecated in favour for the [nested] parameter");
-            return SortBuilder.parseNestedFilter(p);
-        }, NESTED_FILTER_FIELD);
         PARSER.declareObject(FieldSortBuilder::setNestedSort, (p, c) -> NestedSortBuilder.fromXContent(p), NESTED_FIELD);
         PARSER.declareString((b, v) -> b.setNumericType(v), NUMERIC_TYPE);
     }
 
     @Override
     public FieldSortBuilder rewrite(QueryRewriteContext ctx) throws IOException {
-        if (nestedFilter == null && nestedSort == null) {
+        if (nestedSort == null) {
             return this;
         }
-        if (nestedFilter != null) {
-            QueryBuilder rewrite = nestedFilter.rewrite(ctx);
-            if (nestedFilter == rewrite) {
-                return this;
-            }
-            return new FieldSortBuilder(this).setNestedFilter(rewrite);
-        } else {
-            NestedSortBuilder rewrite = nestedSort.rewrite(ctx);
-            if (nestedSort == rewrite) {
-                return this;
-            }
-            return new FieldSortBuilder(this).setNestedSort(rewrite);
+        NestedSortBuilder rewrite = nestedSort.rewrite(ctx);
+        if (nestedSort == rewrite) {
+            return this;
         }
+        return new FieldSortBuilder(this).setNestedSort(rewrite);
     }
 }

+ 22 - 107
server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java

@@ -19,7 +19,6 @@
 
 package org.elasticsearch.search.sort;
 
-import org.apache.logging.log4j.LogManager;
 import org.apache.lucene.document.LatLonDocValuesField;
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.NumericDocValues;
@@ -28,6 +27,7 @@ import org.apache.lucene.search.FieldComparator;
 import org.apache.lucene.search.SortField;
 import org.apache.lucene.util.BitSet;
 import org.elasticsearch.ElasticsearchParseException;
+import org.elasticsearch.Version;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.geo.GeoDistance;
@@ -35,7 +35,6 @@ import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.geo.GeoUtils;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
-import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.unit.DistanceUnit;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
@@ -64,14 +63,13 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Objects;
 
-import static org.elasticsearch.index.query.AbstractQueryBuilder.parseInnerQueryBuilder;
+import static org.elasticsearch.search.sort.FieldSortBuilder.validateMissingNestedPath;
 import static org.elasticsearch.search.sort.NestedSortBuilder.NESTED_FIELD;
 
 /**
  * A geo distance based sorting on a geo point like field.
  */
 public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder> {
-    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(GeoDistanceSortBuilder.class));
 
     public static final String NAME = "_geo_distance";
     public static final String ALTERNATIVE_NAME = "_geoDistance";
@@ -90,8 +88,6 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
     private DistanceUnit unit = DistanceUnit.DEFAULT;
 
     private SortMode sortMode = null;
-    private QueryBuilder nestedFilter;
-    private String nestedPath;
 
     private NestedSortBuilder nestedSort;
 
@@ -150,8 +146,6 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
         this.unit = original.unit;
         this.order = original.order;
         this.sortMode = original.sortMode;
-        this.nestedFilter = original.nestedFilter;
-        this.nestedPath = original.nestedPath;
         this.validation = original.validation;
         this.nestedSort = original.nestedSort;
         this.ignoreUnmapped = original.ignoreUnmapped;
@@ -168,8 +162,13 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
         unit = DistanceUnit.readFromStream(in);
         order = SortOrder.readFromStream(in);
         sortMode = in.readOptionalWriteable(SortMode::readFromStream);
-        nestedFilter = in.readOptionalNamedWriteable(QueryBuilder.class);
-        nestedPath = in.readOptionalString();
+        if (in.getVersion().before(Version.V_8_0_0)) {
+            if (in.readOptionalNamedWriteable(QueryBuilder.class) != null || in.readOptionalString() != null) {
+                throw new IOException("the [sort] options [nested_path] and [nested_filter] are removed in 8.x, " +
+                    "please use [nested] instead");
+            }
+
+        }
         nestedSort = in.readOptionalWriteable(NestedSortBuilder::new);
         validation = GeoValidationMethod.readFromStream(in);
         ignoreUnmapped = in.readBoolean();
@@ -183,8 +182,10 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
         unit.writeTo(out);
         order.writeTo(out);
         out.writeOptionalWriteable(sortMode);
-        out.writeOptionalNamedWriteable(nestedFilter);
-        out.writeOptionalString(nestedPath);
+        if (out.getVersion().before(Version.V_8_0_0)) {
+            out.writeOptionalNamedWriteable(null);
+            out.writeOptionalString(null);
+        }
         out.writeOptionalWriteable(nestedSort);
         validation.writeTo(out);
         out.writeBoolean(ignoreUnmapped);
@@ -288,59 +289,6 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
         return this.sortMode;
     }
 
-    /**
-     * Sets the nested filter that the nested objects should match with in order to
-     * be taken into account for sorting.
-     *
-     * @deprecated set nested sort with {@link #setNestedSort(NestedSortBuilder)}
-     *             and retrieve with {@link #getNestedSort()}
-     **/
-   @Deprecated
-    public GeoDistanceSortBuilder setNestedFilter(QueryBuilder nestedFilter) {
-       if (this.nestedSort != null) {
-           throw new IllegalArgumentException("Setting both nested_path/nested_filter and nested not allowed");
-       }
-        this.nestedFilter = nestedFilter;
-        return this;
-    }
-
-    /**
-     * Returns the nested filter that the nested objects should match with in order to be taken into account
-     * for sorting.
-     * @deprecated set nested sort with {@link #setNestedSort(NestedSortBuilder)}
-     *             and retrieve with {@link #getNestedSort()}
-     **/
-    @Deprecated
-    public QueryBuilder getNestedFilter() {
-        return this.nestedFilter;
-    }
-
-    /**
-     * Sets the nested path if sorting occurs on a field that is inside a nested object. By default when sorting on a
-     * field inside a nested object, the nearest upper nested object is selected as nested path.
-     * @deprecated set nested sort with {@link #setNestedSort(NestedSortBuilder)}
-     *             and retrieve with {@link #getNestedSort()}
-     **/
-    @Deprecated
-    public GeoDistanceSortBuilder setNestedPath(String nestedPath) {
-        if (this.nestedSort != null) {
-            throw new IllegalArgumentException("Setting both nested_path/nested_filter and nested not allowed");
-        }
-        this.nestedPath = nestedPath;
-        return this;
-    }
-
-    /**
-     * Returns the nested path if sorting occurs on a field that is inside a nested object. By default when sorting on a
-     * field inside a nested object, the nearest upper nested object is selected as nested path.
-     * @deprecated set nested sort with {@link #setNestedSort(NestedSortBuilder)}
-     *             and retrieve with {@link #getNestedSort()}
-     **/
-    @Deprecated
-    public String getNestedPath() {
-        return this.nestedPath;
-    }
-
     /**
      * Returns the {@link NestedSortBuilder}
      */
@@ -355,9 +303,6 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
      * order to be taken into account for sorting.
      */
     public GeoDistanceSortBuilder setNestedSort(final NestedSortBuilder nestedSort) {
-        if (this.nestedFilter != null || this.nestedPath != null) {
-            throw new IllegalArgumentException("Setting both nested_path/nested_filter and nested not allowed");
-        }
         this.nestedSort = nestedSort;
         return this;
     }
@@ -393,12 +338,6 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
             builder.field(SORTMODE_FIELD.getPreferredName(), sortMode);
         }
 
-        if (nestedPath != null) {
-            builder.field(NESTED_PATH_FIELD.getPreferredName(), nestedPath);
-        }
-        if (nestedFilter != null) {
-            builder.field(NESTED_FILTER_FIELD.getPreferredName(), nestedFilter, params);
-        }
         if (nestedSort != null) {
             builder.field(NESTED_FIELD.getPreferredName(), nestedSort);
         }
@@ -432,8 +371,6 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
                 Objects.equals(unit, other.unit) &&
                 Objects.equals(sortMode, other.sortMode) &&
                 Objects.equals(order, other.order) &&
-                Objects.equals(nestedFilter, other.nestedFilter) &&
-                Objects.equals(nestedPath, other.nestedPath) &&
                 Objects.equals(validation, other.validation) &&
                 Objects.equals(nestedSort, other.nestedSort) &&
                 ignoreUnmapped == other.ignoreUnmapped;
@@ -442,8 +379,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
     @Override
     public int hashCode() {
         return Objects.hash(this.fieldName, this.points, this.geoDistance,
-                this.unit, this.sortMode, this.order, this.nestedFilter,
-                this.nestedPath, this.validation, this.nestedSort, this.ignoreUnmapped);
+                this.unit, this.sortMode, this.order, this.validation, this.nestedSort, this.ignoreUnmapped);
     }
 
     /**
@@ -463,8 +399,6 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
         GeoDistance geoDistance = GeoDistance.ARC;
         SortOrder order = SortOrder.ASC;
         SortMode sortMode = null;
-        QueryBuilder nestedFilter = null;
-        String nestedPath = null;
         NestedSortBuilder nestedSort = null;
         GeoValidationMethod validation = null;
         boolean ignoreUnmapped = false;
@@ -479,10 +413,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
 
                 fieldName = currentName;
             } else if (token == XContentParser.Token.START_OBJECT) {
-                if (NESTED_FILTER_FIELD.match(currentName, parser.getDeprecationHandler())) {
-                    deprecationLogger.deprecated("[nested_filter] has been deprecated in favour of the [nested] parameter");
-                    nestedFilter = parseInnerQueryBuilder(parser);
-                } else if (NESTED_FIELD.match(currentName, parser.getDeprecationHandler())) {
+                if (NESTED_FIELD.match(currentName, parser.getDeprecationHandler())) {
                     nestedSort = NestedSortBuilder.fromXContent(parser);
                 } else {
                     // the json in the format of -> field : { lat : 30, lon : 12 }
@@ -509,9 +440,6 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
                     validation = GeoValidationMethod.fromString(parser.text());
                 } else if (SORTMODE_FIELD.match(currentName, parser.getDeprecationHandler())) {
                     sortMode = SortMode.fromString(parser.text());
-                } else if (NESTED_PATH_FIELD.match(currentName, parser.getDeprecationHandler())) {
-                    deprecationLogger.deprecated("[nested_path] has been deprecated in favour of the [nested] parameter");
-                    nestedPath = parser.text();
                 } else if (IGNORE_UNMAPPED.match(currentName, parser.getDeprecationHandler())) {
                     ignoreUnmapped = parser.booleanValue();
                 } else if (token == Token.VALUE_STRING){
@@ -549,10 +477,6 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
         if (sortMode != null) {
             result.sortMode(sortMode);
         }
-        if (nestedFilter != null) {
-            result.setNestedFilter(nestedFilter);
-        }
-        result.setNestedPath(nestedPath);
         if (nestedSort != null) {
             result.setNestedSort(nestedSort);
         }
@@ -610,16 +534,15 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
         }
         final IndexGeoPointFieldData geoIndexFieldData = context.getForField(fieldType);
 
-        final Nested nested;
+        Nested nested = null;
         if (nestedSort != null) {
             if (nestedSort.getNestedSort() != null && nestedSort.getMaxChildren() != Integer.MAX_VALUE)  {
                 throw new QueryShardException(context,
                     "max_children is only supported on last level of nested sort");
             }
-            // new nested sorts takes priority
             nested = resolveNested(context, nestedSort);
         } else {
-            nested = resolveNested(context, nestedPath, nestedFilter);
+            validateMissingNestedPath(context, fieldName);
         }
 
         if (geoIndexFieldData.getClass() == LatLonPointDVIndexFieldData.class // only works with 5.x geo_point
@@ -698,21 +621,13 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
 
     @Override
     public GeoDistanceSortBuilder rewrite(QueryRewriteContext ctx) throws IOException {
-        if (nestedFilter == null && nestedSort == null) {
+        if (nestedSort == null) {
             return this;
         }
-        if (nestedFilter != null) {
-            QueryBuilder rewrite = nestedFilter.rewrite(ctx);
-            if (nestedFilter == rewrite) {
-                return this;
-            }
-            return new GeoDistanceSortBuilder(this).setNestedFilter(rewrite);
-        } else {
-            NestedSortBuilder rewrite = nestedSort.rewrite(ctx);
-            if (nestedSort == rewrite) {
-                return this;
-            }
-            return new GeoDistanceSortBuilder(this).setNestedSort(rewrite);
+        NestedSortBuilder rewrite = nestedSort.rewrite(ctx);
+        if (nestedSort == rewrite) {
+            return this;
         }
+        return new GeoDistanceSortBuilder(this).setNestedSort(rewrite);
     }
 }

+ 18 - 100
server/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java

@@ -19,18 +19,17 @@
 
 package org.elasticsearch.search.sort;
 
-import org.apache.logging.log4j.LogManager;
 import org.apache.lucene.index.BinaryDocValues;
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.search.Scorable;
 import org.apache.lucene.search.SortField;
 import org.apache.lucene.util.BytesRef;
 import org.apache.lucene.util.BytesRefBuilder;
+import org.elasticsearch.Version;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
-import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.xcontent.ConstructingObjectParser;
 import org.elasticsearch.common.xcontent.ObjectParser.ValueType;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -65,7 +64,6 @@ import static org.elasticsearch.search.sort.NestedSortBuilder.NESTED_FIELD;
  * Script sort builder allows to sort based on a custom script expression.
  */
 public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> {
-    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(ScriptSortBuilder.class));
 
     public static final String NAME = "_script";
     public static final ParseField TYPE_FIELD = new ParseField("type");
@@ -78,10 +76,6 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> {
 
     private SortMode sortMode;
 
-    private QueryBuilder nestedFilter;
-
-    private String nestedPath;
-
     private NestedSortBuilder nestedSort;
 
     /**
@@ -105,8 +99,6 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> {
         this.type = original.type;
         this.order = original.order;
         this.sortMode = original.sortMode;
-        this.nestedFilter = original.nestedFilter;
-        this.nestedPath = original.nestedPath;
         this.nestedSort = original.nestedSort;
     }
 
@@ -118,8 +110,12 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> {
         type = ScriptSortType.readFromStream(in);
         order = SortOrder.readFromStream(in);
         sortMode = in.readOptionalWriteable(SortMode::readFromStream);
-        nestedPath = in.readOptionalString();
-        nestedFilter = in.readOptionalNamedWriteable(QueryBuilder.class);
+        if (in.getVersion().before(Version.V_8_0_0)) {
+            if (in.readOptionalNamedWriteable(QueryBuilder.class) != null || in.readOptionalString() != null) {
+                throw new IOException("the [sort] options [nested_path] and [nested_filter] are removed in 8.x, " +
+                    "please use [nested] instead");
+            }
+        }
         nestedSort = in.readOptionalWriteable(NestedSortBuilder::new);
     }
 
@@ -129,8 +125,10 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> {
         type.writeTo(out);
         order.writeTo(out);
         out.writeOptionalWriteable(sortMode);
-        out.writeOptionalString(nestedPath);
-        out.writeOptionalNamedWriteable(nestedFilter);
+        if (out.getVersion().before(Version.V_8_0_0)) {
+            out.writeOptionalString(null);
+            out.writeOptionalNamedWriteable(null);
+        }
         out.writeOptionalWriteable(nestedSort);
     }
 
@@ -169,56 +167,6 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> {
         return this.sortMode;
     }
 
-    /**
-     * Sets the nested filter that the nested objects should match with in order to be taken into account
-     * for sorting.
-     *
-     * @deprecated set nested sort with {@link #setNestedSort(NestedSortBuilder)} and retrieve with {@link #getNestedSort()}
-     */
-    @Deprecated
-    public ScriptSortBuilder setNestedFilter(QueryBuilder nestedFilter) {
-        if (this.nestedSort != null) {
-            throw new IllegalArgumentException("Setting both nested_path/nested_filter and nested not allowed");
-        }
-        this.nestedFilter = nestedFilter;
-        return this;
-    }
-
-    /**
-     * Gets the nested filter.
-     *
-     * @deprecated set nested sort with {@link #setNestedSort(NestedSortBuilder)} and retrieve with {@link #getNestedSort()}
-     */
-    @Deprecated
-    public QueryBuilder getNestedFilter() {
-        return this.nestedFilter;
-    }
-
-    /**
-     * Sets the nested path if sorting occurs on a field that is inside a nested object. For sorting by script this
-     * needs to be specified.
-     *
-     * @deprecated set nested sort with {@link #setNestedSort(NestedSortBuilder)} and retrieve with {@link #getNestedSort()}
-     */
-    @Deprecated
-    public ScriptSortBuilder setNestedPath(String nestedPath) {
-        if (this.nestedSort != null) {
-            throw new IllegalArgumentException("Setting both nested_path/nested_filter and nested not allowed");
-        }
-        this.nestedPath = nestedPath;
-        return this;
-    }
-
-    /**
-     * Gets the nested path.
-     *
-     * @deprecated set nested sort with {@link #setNestedSort(NestedSortBuilder)} and retrieve with {@link #getNestedSort()}
-     */
-    @Deprecated
-    public String getNestedPath() {
-        return this.nestedPath;
-    }
-
     /**
      * Returns the {@link NestedSortBuilder}
      */
@@ -233,9 +181,6 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> {
      * order to be taken into account for sorting.
      */
     public ScriptSortBuilder setNestedSort(final NestedSortBuilder nestedSort) {
-        if (this.nestedFilter != null || this.nestedPath != null) {
-            throw new IllegalArgumentException("Setting both nested_path/nested_filter and nested not allowed");
-        }
         this.nestedSort = nestedSort;
         return this;
     }
@@ -250,12 +195,6 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> {
         if (sortMode != null) {
             builder.field(SORTMODE_FIELD.getPreferredName(), sortMode);
         }
-        if (nestedPath != null) {
-            builder.field(NESTED_PATH_FIELD.getPreferredName(), nestedPath);
-        }
-        if (nestedFilter != null) {
-            builder.field(NESTED_FILTER_FIELD.getPreferredName(), nestedFilter, builderParams);
-        }
         if (nestedSort != null) {
             builder.field(NESTED_FIELD.getPreferredName(), nestedSort);
         }
@@ -273,14 +212,6 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> {
         PARSER.declareField(constructorArg(), p -> ScriptSortType.fromString(p.text()), TYPE_FIELD, ValueType.STRING);
         PARSER.declareString((b, v) -> b.order(SortOrder.fromString(v)), ORDER_FIELD);
         PARSER.declareString((b, v) -> b.sortMode(SortMode.fromString(v)), SORTMODE_FIELD);
-        PARSER.declareString((fieldSortBuilder, nestedPath) -> {
-            deprecationLogger.deprecated("[nested_path] has been deprecated in favor of the [nested] parameter");
-            fieldSortBuilder.setNestedPath(nestedPath);
-        }, NESTED_PATH_FIELD);
-        PARSER.declareObject(ScriptSortBuilder::setNestedFilter, (p, c) -> {
-            deprecationLogger.deprecated("[nested_filter] has been deprecated in favour for the [nested] parameter");
-            return SortBuilder.parseNestedFilter(p);
-        }, NESTED_FILTER_FIELD);
         PARSER.declareObject(ScriptSortBuilder::setNestedSort, (p, c) -> NestedSortBuilder.fromXContent(p), NESTED_FIELD);
     }
 
@@ -309,16 +240,13 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> {
             valueMode = reverse ? MultiValueMode.MAX : MultiValueMode.MIN;
         }
 
-        final Nested nested;
+        Nested nested = null;
         if (nestedSort != null) {
             if (nestedSort.getNestedSort() != null && nestedSort.getMaxChildren() != Integer.MAX_VALUE)  {
                 throw new QueryShardException(context,
                     "max_children is only supported on last level of nested sort");
             }
-            // new nested sorts takes priority
             nested = resolveNested(context, nestedSort);
-        } else {
-            nested = resolveNested(context, nestedPath, nestedFilter);
         }
 
         final IndexFieldData.XFieldComparatorSource fieldComparatorSource;
@@ -399,14 +327,12 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> {
                 Objects.equals(type, other.type) &&
                 Objects.equals(order, other.order) &&
                 Objects.equals(sortMode, other.sortMode) &&
-                Objects.equals(nestedFilter, other.nestedFilter) &&
-                Objects.equals(nestedPath, other.nestedPath) &&
                 Objects.equals(nestedSort, other.nestedSort);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(script, type, order, sortMode, nestedFilter, nestedPath, nestedSort);
+        return Objects.hash(script, type, order, sortMode, nestedSort);
     }
 
     @Override
@@ -452,21 +378,13 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> {
 
     @Override
     public ScriptSortBuilder rewrite(QueryRewriteContext ctx) throws IOException {
-        if (nestedFilter == null && nestedSort == null) {
+        if (nestedSort == null) {
             return this;
         }
-        if (nestedFilter != null) {
-            QueryBuilder rewrite = nestedFilter.rewrite(ctx);
-            if (nestedFilter == rewrite) {
-                return this;
-            }
-            return new ScriptSortBuilder(this).setNestedFilter(rewrite);
-        } else {
-            NestedSortBuilder rewrite = nestedSort.rewrite(ctx);
-            if (nestedSort == rewrite) {
-                return this;
-            }
-            return new ScriptSortBuilder(this).setNestedSort(rewrite);
+        NestedSortBuilder rewrite = nestedSort.rewrite(ctx);
+        if (nestedSort == rewrite) {
+            return this;
         }
+        return new ScriptSortBuilder(this).setNestedSort(rewrite);
     }
 }

+ 3 - 10
server/src/main/java/org/elasticsearch/search/sort/SortBuilder.java

@@ -46,6 +46,7 @@ import java.util.Objects;
 import java.util.Optional;
 
 import static org.elasticsearch.index.query.AbstractQueryBuilder.parseInnerQueryBuilder;
+import static org.elasticsearch.search.sort.NestedSortBuilder.FILTER_FIELD;
 
 public abstract class SortBuilder<T extends SortBuilder<T>> implements NamedWriteable, ToXContentObject, Rewriteable<SortBuilder<?>> {
 
@@ -53,8 +54,6 @@ public abstract class SortBuilder<T extends SortBuilder<T>> implements NamedWrit
 
     // parse fields common to more than one SortBuilder
     public static final ParseField ORDER_FIELD = new ParseField("order");
-    public static final ParseField NESTED_FILTER_FIELD = new ParseField("nested_filter");
-    public static final ParseField NESTED_PATH_FIELD = new ParseField("nested_path");
 
     private static final Map<String, Parser<?>> PARSERS = Map.of(
             ScriptSortBuilder.NAME, ScriptSortBuilder::fromXContent,
@@ -171,12 +170,6 @@ public abstract class SortBuilder<T extends SortBuilder<T>> implements NamedWrit
         return Optional.empty();
     }
 
-    protected static Nested resolveNested(QueryShardContext context, String nestedPath, QueryBuilder nestedFilter) throws IOException {
-        NestedSortBuilder nestedSortBuilder = new NestedSortBuilder(nestedPath);
-        nestedSortBuilder.setFilter(nestedFilter);
-        return resolveNested(context, nestedSortBuilder);
-    }
-
     protected static Nested resolveNested(QueryShardContext context, NestedSortBuilder nestedSort) throws IOException {
         final Query childQuery = resolveNestedQuery(context, nestedSort, null);
         if (childQuery == null) {
@@ -207,7 +200,7 @@ public abstract class SortBuilder<T extends SortBuilder<T>> implements NamedWrit
         if (nestedObjectMapper == null) {
             throw new QueryShardException(context, "[nested] failed to find nested object under path [" + nestedPath + "]");
         }
-        if (!nestedObjectMapper.nested().isNested()) {
+        if (nestedObjectMapper.nested().isNested() == false) {
             throw new QueryShardException(context, "[nested] nested object under path [" + nestedPath + "] is not of nested type");
         }
         ObjectMapper objectMapper = context.nestedScope().getObjectMapper();
@@ -256,7 +249,7 @@ public abstract class SortBuilder<T extends SortBuilder<T>> implements NamedWrit
         try {
             return parseInnerQueryBuilder(parser);
         } catch (Exception e) {
-            throw new ParsingException(parser.getTokenLocation(), "Expected " + NESTED_FILTER_FIELD.getPreferredName() + " element.", e);
+            throw new ParsingException(parser.getTokenLocation(), "Expected " + FILTER_FIELD.getPreferredName() + " element.", e);
         }
     }
 

+ 37 - 27
server/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java

@@ -385,7 +385,8 @@ public class SimpleNestedIT extends ESIntegTestCase {
         SearchResponse searchResponse = client().prepareSearch("test")
 
                 .setQuery(QueryBuilders.matchAllQuery())
-                .addSort(SortBuilders.fieldSort("nested1.field1").order(SortOrder.ASC).setNestedPath("nested1"))
+                .addSort(SortBuilders.fieldSort("nested1.field1").order(SortOrder.ASC)
+                    .setNestedSort(new NestedSortBuilder("nested1")))
                 .get();
 
         assertHitCount(searchResponse, 3);
@@ -399,7 +400,8 @@ public class SimpleNestedIT extends ESIntegTestCase {
         searchResponse = client().prepareSearch("test")
 
                 .setQuery(QueryBuilders.matchAllQuery())
-                .addSort(SortBuilders.fieldSort("nested1.field1").order(SortOrder.DESC).setNestedPath("nested1"))
+                .addSort(SortBuilders.fieldSort("nested1.field1").order(SortOrder.DESC)
+                    .setNestedSort(new NestedSortBuilder("nested1")))
                 .get();
 
         assertHitCount(searchResponse, 3);
@@ -476,8 +478,10 @@ public class SimpleNestedIT extends ESIntegTestCase {
 
         SearchRequestBuilder searchRequestBuilder = client().prepareSearch("test")
                 .setQuery(QueryBuilders.matchAllQuery())
-                .addSort(SortBuilders.fieldSort("nested1.field1").setNestedPath("nested1")
-                        .setNestedFilter(termQuery("nested1.field2", true)).missing(10).order(SortOrder.ASC));
+                .addSort(SortBuilders.fieldSort("nested1.field1")
+                    .setNestedSort(new NestedSortBuilder("nested1")
+                        .setFilter(termQuery("nested1.field2", true)))
+                    .missing(10).order(SortOrder.ASC));
 
         if (randomBoolean()) {
             searchRequestBuilder.setScroll("10m");
@@ -494,8 +498,10 @@ public class SimpleNestedIT extends ESIntegTestCase {
         assertThat(searchResponse.getHits().getHits()[2].getSortValues()[0].toString(), equalTo("10"));
 
         searchRequestBuilder = client().prepareSearch("test").setQuery(QueryBuilders.matchAllQuery())
-                .addSort(SortBuilders.fieldSort("nested1.field1").setNestedPath("nested1")
-                        .setNestedFilter(termQuery("nested1.field2", true)).missing(10).order(SortOrder.DESC));
+                .addSort(SortBuilders.fieldSort("nested1.field1")
+                    .setNestedSort(new NestedSortBuilder("nested1")
+                        .setFilter(termQuery("nested1.field2", true)))
+                    .missing(10).order(SortOrder.DESC));
 
         if (randomBoolean()) {
             searchRequestBuilder.setScroll("10m");
@@ -953,7 +959,7 @@ public class SimpleNestedIT extends ESIntegTestCase {
                 .setQuery(matchAllQuery())
                 .addSort(
                         SortBuilders.fieldSort("parent.child.child_values")
-                                .setNestedPath("parent.child")
+                                .setNestedSort(new NestedSortBuilder("parent.child"))
                                 .order(SortOrder.ASC)
                 )
                 .get();
@@ -967,12 +973,13 @@ public class SimpleNestedIT extends ESIntegTestCase {
         assertThat(searchResponse.getHits().getHits()[2].getSortValues()[0].toString(), equalTo("-1"));
 
         // With nested filter
+        NestedSortBuilder nestedSort = new NestedSortBuilder("parent.child");
+        nestedSort.setFilter(QueryBuilders.termQuery("parent.child.filter", true));
         searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
                 .addSort(
                         SortBuilders.fieldSort("parent.child.child_values")
-                                .setNestedPath("parent.child")
-                                .setNestedFilter(QueryBuilders.termQuery("parent.child.filter", true))
+                                .setNestedSort(nestedSort)
                                 .order(SortOrder.ASC)
                 )
                 .get();
@@ -990,8 +997,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))
+                                .setNestedSort(nestedSort)
                                 .order(SortOrder.ASC)
                 )
                 .get();
@@ -1005,12 +1011,12 @@ public class SimpleNestedIT extends ESIntegTestCase {
         assertThat(searchResponse.getHits().getHits()[2].getId(), equalTo("3"));
         assertThat(searchResponse.getHits().getHits()[2].getSortValues()[0].toString(), equalTo("3"));
 
+        nestedSort.setFilter(QueryBuilders.termQuery("parent.filter", false));
         searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
                 .addSort(
                         SortBuilders.fieldSort("parent.parent_values")
-                                .setNestedPath("parent.child")
-                                .setNestedFilter(QueryBuilders.termQuery("parent.filter", false))
+                                .setNestedSort(nestedSort)
                                 .order(SortOrder.ASC)
                 )
                 .get();
@@ -1051,8 +1057,8 @@ 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))
+                                .setNestedSort(new NestedSortBuilder("parent.child")
+                                    .setFilter(QueryBuilders.termQuery("parent.child.filter", true)))
                                 .order(SortOrder.ASC)
                 )
                 .get();
@@ -1071,7 +1077,7 @@ public class SimpleNestedIT extends ESIntegTestCase {
                 .setQuery(matchAllQuery())
                 .addSort(
                         SortBuilders.fieldSort("parent.child.child_values")
-                                .setNestedPath("parent.child")
+                                .setNestedSort(new NestedSortBuilder("parent.child"))
                                 .sortMode(SortMode.SUM)
                                 .order(SortOrder.ASC)
                 )
@@ -1091,7 +1097,7 @@ public class SimpleNestedIT extends ESIntegTestCase {
                 .setQuery(matchAllQuery())
                 .addSort(
                         SortBuilders.fieldSort("parent.child.child_values")
-                                .setNestedPath("parent.child")
+                                .setNestedSort(new NestedSortBuilder("parent.child"))
                                 .sortMode(SortMode.SUM)
                                 .order(SortOrder.DESC)
                 )
@@ -1111,8 +1117,9 @@ public class SimpleNestedIT extends ESIntegTestCase {
                 .setQuery(matchAllQuery())
                 .addSort(
                         SortBuilders.fieldSort("parent.child.child_values")
-                                .setNestedPath("parent.child")
-                                .setNestedFilter(QueryBuilders.termQuery("parent.child.filter", true))
+                                .setNestedSort(new NestedSortBuilder("parent.child")
+                                    .setFilter(QueryBuilders.termQuery("parent.child.filter", true))
+                                )
                                 .sortMode(SortMode.SUM)
                                 .order(SortOrder.ASC)
                 )
@@ -1132,7 +1139,7 @@ public class SimpleNestedIT extends ESIntegTestCase {
                 .setQuery(matchAllQuery())
                 .addSort(
                         SortBuilders.fieldSort("parent.child.child_values")
-                                .setNestedPath("parent.child")
+                                .setNestedSort(new NestedSortBuilder("parent.child"))
                                 .sortMode(SortMode.AVG)
                                 .order(SortOrder.ASC)
                 )
@@ -1151,7 +1158,7 @@ public class SimpleNestedIT extends ESIntegTestCase {
                 .setQuery(matchAllQuery())
                 .addSort(
                         SortBuilders.fieldSort("parent.child.child_values")
-                                .setNestedPath("parent.child")
+                                .setNestedSort(new NestedSortBuilder("parent.child"))
                                 .sortMode(SortMode.AVG)
                                 .order(SortOrder.DESC)
                 )
@@ -1171,8 +1178,9 @@ public class SimpleNestedIT extends ESIntegTestCase {
                 .setQuery(matchAllQuery())
                 .addSort(
                         SortBuilders.fieldSort("parent.child.child_values")
-                                .setNestedPath("parent.child")
-                                .setNestedFilter(QueryBuilders.termQuery("parent.child.filter", true))
+                                .setNestedSort(new NestedSortBuilder("parent.child")
+                                    .setFilter(QueryBuilders.termQuery("parent.child.filter", true))
+                                )
                                 .sortMode(SortMode.AVG)
                                 .order(SortOrder.ASC)
                 )
@@ -1307,13 +1315,15 @@ public class SimpleNestedIT extends ESIntegTestCase {
 
         SearchResponse searchResponse = client().prepareSearch("test")
                 .addSort(SortBuilders.fieldSort("users.first")
-                        .setNestedPath("users")
+                        .setNestedSort(new NestedSortBuilder("users"))
                         .order(SortOrder.ASC))
                 .addSort(SortBuilders.fieldSort("users.first")
                         .order(SortOrder.ASC)
-                        .setNestedPath("users")
-                        .setNestedFilter(nestedQuery("users.workstations", termQuery("users.workstations.stationid", "s5"),
-                                ScoreMode.Avg)))
+                        .setNestedSort(new NestedSortBuilder("users")
+                            .setFilter(nestedQuery("users.workstations", termQuery("users.workstations.stationid", "s5"),
+                                    ScoreMode.Avg))
+                        )
+                )
                 .get();
         assertNoFailures(searchResponse);
         assertHitCount(searchResponse, 2);

+ 7 - 2
server/src/test/java/org/elasticsearch/search/scroll/DuelScrollIT.java

@@ -30,6 +30,7 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.search.SearchHits;
+import org.elasticsearch.search.sort.NestedSortBuilder;
 import org.elasticsearch.search.sort.SortBuilder;
 import org.elasticsearch.search.sort.SortBuilders;
 import org.elasticsearch.search.sort.SortOrder;
@@ -163,9 +164,13 @@ public class DuelScrollIT extends ESIntegTestCase {
             }
         } else {
             if (randomBoolean()) {
-                sort = SortBuilders.fieldSort("nested.field3").missing(1);
+                sort = SortBuilders.fieldSort("nested.field3")
+                    .setNestedSort(new NestedSortBuilder("nested"))
+                    .missing(1);
             } else {
-                sort = SortBuilders.fieldSort("nested.field4").missing("1");
+                sort = SortBuilders.fieldSort("nested.field4")
+                    .setNestedSort(new NestedSortBuilder("nested"))
+                    .missing("1");
             }
         }
         sort.order(randomBoolean() ? SortOrder.ASC : SortOrder.DESC);

+ 11 - 62
server/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java

@@ -45,7 +45,6 @@ import org.elasticsearch.search.DocValueFormat;
 import org.elasticsearch.search.MultiValueMode;
 
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
@@ -90,17 +89,7 @@ public class FieldSortBuilderTests extends AbstractSortTestCase<FieldSortBuilder
             builder.sortMode(randomFrom(SortMode.values()));
         }
         if (randomBoolean()) {
-            if (randomBoolean()) {
-                builder.setNestedSort(createRandomNestedSort(3));
-            } else {
-                // the following are alternative ways to setNestedSort for nested sorting
-                if (randomBoolean()) {
-                    builder.setNestedFilter(randomNestedFilter());
-                }
-                if (randomBoolean()) {
-                    builder.setNestedPath(randomAlphaOfLengthBetween(1, 10));
-                }
-            }
+            builder.setNestedSort(createRandomNestedSort(3));
         }
         if (randomBoolean()) {
             builder.setNumericType(randomFrom(random(), "long", "double", "date", "date_nanos"));
@@ -114,16 +103,8 @@ public class FieldSortBuilderTests extends AbstractSortTestCase<FieldSortBuilder
         int parameter = randomIntBetween(0, 5);
         switch (parameter) {
         case 0:
-            if (original.getNestedPath() == null && original.getNestedFilter() == null) {
-                mutated.setNestedSort(
-                        randomValueOtherThan(original.getNestedSort(), () -> NestedSortBuilderTests.createRandomNestedSort(3)));
-            } else {
-                if (randomBoolean()) {
-                    mutated.setNestedPath(randomValueOtherThan(original.getNestedPath(), () -> randomAlphaOfLengthBetween(1, 10)));
-                } else {
-                    mutated.setNestedFilter(randomValueOtherThan(original.getNestedFilter(), () -> randomNestedFilter()));
-                }
-            }
+            mutated.setNestedSort(
+                randomValueOtherThan(original.getNestedSort(), () -> NestedSortBuilderTests.createRandomNestedSort(3)));
             break;
         case 1:
             mutated.sortMode(randomValueOtherThan(original.sortMode(), () -> randomFrom(SortMode.values())));
@@ -285,7 +266,8 @@ public class FieldSortBuilderTests extends AbstractSortTestCase<FieldSortBuilder
         assertNotNull(nested);
         assertEquals(new TermQuery(new Term(MAPPED_STRING_FIELDNAME, "value")), nested.getInnerQuery());
 
-        sortBuilder = new FieldSortBuilder("fieldName").setNestedPath("path");
+        NestedSortBuilder nestedSort = new NestedSortBuilder("path");
+        sortBuilder = new FieldSortBuilder("fieldName").setNestedSort(nestedSort);
         sortField = sortBuilder.build(shardContextMock).field;
         assertThat(sortField.getComparatorSource(), instanceOf(XFieldComparatorSource.class));
         comparatorSource = (XFieldComparatorSource) sortField.getComparatorSource();
@@ -293,19 +275,14 @@ public class FieldSortBuilderTests extends AbstractSortTestCase<FieldSortBuilder
         assertNotNull(nested);
         assertEquals(new TermQuery(new Term(TypeFieldMapper.NAME, "__path")), nested.getInnerQuery());
 
-        sortBuilder = new FieldSortBuilder("fieldName").setNestedPath("path")
-                .setNestedFilter(QueryBuilders.termQuery(MAPPED_STRING_FIELDNAME, "value"));
+        nestedSort.setFilter(QueryBuilders.termQuery(MAPPED_STRING_FIELDNAME, "value"));
+        sortBuilder = new FieldSortBuilder("fieldName").setNestedSort(nestedSort);
         sortField = sortBuilder.build(shardContextMock).field;
         assertThat(sortField.getComparatorSource(), instanceOf(XFieldComparatorSource.class));
         comparatorSource = (XFieldComparatorSource) sortField.getComparatorSource();
         nested = comparatorSource.nested();
         assertNotNull(nested);
         assertEquals(new TermQuery(new Term(MAPPED_STRING_FIELDNAME, "value")), nested.getInnerQuery());
-
-        // if nested path is missing, we omit any filter and return a SortedNumericSortField
-        sortBuilder = new FieldSortBuilder("fieldName").setNestedFilter(QueryBuilders.termQuery(MAPPED_STRING_FIELDNAME, "value"));
-        sortField = sortBuilder.build(shardContextMock).field;
-        assertThat(sortField, instanceOf(SortedNumericSortField.class));
     }
 
     public void testUnknownOptionFails() throws IOException {
@@ -364,22 +341,6 @@ public class FieldSortBuilderTests extends AbstractSortTestCase<FieldSortBuilder
         assertEquals(expectedError, e.getMessage());
     }
 
-    /**
-     * Test we can either set nested sort via path/filter or via nested sort builder, not both
-     */
-    public void testNestedSortBothThrows() throws IOException {
-        FieldSortBuilder sortBuilder = new FieldSortBuilder(MAPPED_STRING_FIELDNAME);
-        IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
-                () -> sortBuilder.setNestedPath("nestedPath").setNestedSort(new NestedSortBuilder("otherPath")));
-        assertEquals("Setting both nested_path/nested_filter and nested not allowed", iae.getMessage());
-        iae = expectThrows(IllegalArgumentException.class,
-                () -> sortBuilder.setNestedSort(new NestedSortBuilder("otherPath")).setNestedPath("nestedPath"));
-        assertEquals("Setting both nested_path/nested_filter and nested not allowed", iae.getMessage());
-        iae = expectThrows(IllegalArgumentException.class,
-                () -> sortBuilder.setNestedSort(new NestedSortBuilder("otherPath")).setNestedFilter(QueryBuilders.matchAllQuery()));
-        assertEquals("Setting both nested_path/nested_filter and nested not allowed", iae.getMessage());
-    }
-
     /**
      * Test the nested Filter gets rewritten
      */
@@ -391,10 +352,12 @@ public class FieldSortBuilderTests extends AbstractSortTestCase<FieldSortBuilder
                 return new MatchNoneQueryBuilder();
             }
         };
-        sortBuilder.setNestedPath("path").setNestedFilter(rangeQuery);
+        NestedSortBuilder nestedSort = new NestedSortBuilder("path");
+        nestedSort.setFilter(rangeQuery);
+        sortBuilder.setNestedSort(nestedSort);
         FieldSortBuilder rewritten = sortBuilder
                 .rewrite(createMockShardContext());
-        assertNotSame(rangeQuery, rewritten.getNestedFilter());
+        assertNotSame(rangeQuery, rewritten.getNestedSort().getFilter());
     }
 
     /**
@@ -414,20 +377,6 @@ public class FieldSortBuilderTests extends AbstractSortTestCase<FieldSortBuilder
         assertNotSame(rangeQuery, rewritten.getNestedSort().getFilter());
     }
 
-    @Override
-    protected void assertWarnings(FieldSortBuilder testItem) {
-        List<String> expectedWarnings = new ArrayList<>();
-        if (testItem.getNestedFilter() != null) {
-            expectedWarnings.add("[nested_filter] has been deprecated in favour for the [nested] parameter");
-        }
-        if (testItem.getNestedPath() != null) {
-            expectedWarnings.add("[nested_path] has been deprecated in favor of the [nested] parameter");
-        }
-        if (expectedWarnings.isEmpty() == false) {
-            assertWarnings(expectedWarnings.toArray(new String[expectedWarnings.size()]));
-        }
-    }
-
     @Override
     protected FieldSortBuilder fromXContent(XContentParser parser, String fieldName) throws IOException {
         return FieldSortBuilder.fromXContent(parser, fieldName);

+ 13 - 2
server/src/test/java/org/elasticsearch/search/sort/FieldSortIT.java

@@ -1443,9 +1443,11 @@ public class FieldSortIT extends ESIntegTestCase {
         refresh();
 
         // We sort on nested field
+
         SearchResponse searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
-                .addSort(SortBuilders.fieldSort("nested.foo").setNestedPath("nested").order(SortOrder.DESC))
+                .addSort(SortBuilders.fieldSort("nested.foo")
+                    .setNestedSort(new NestedSortBuilder("nested")).order(SortOrder.DESC))
                 .get();
         assertNoFailures(searchResponse);
         SearchHit[] hits = searchResponse.getHits().getHits();
@@ -1474,7 +1476,8 @@ public class FieldSortIT extends ESIntegTestCase {
         // We sort on nested sub field
         searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
-                .addSort(SortBuilders.fieldSort("nested.foo.sub").setNestedPath("nested").order(SortOrder.DESC))
+                .addSort(SortBuilders.fieldSort("nested.foo.sub")
+                    .setNestedSort(new NestedSortBuilder("nested")).order(SortOrder.DESC))
                 .get();
         assertNoFailures(searchResponse);
         hits = searchResponse.getHits().getHits();
@@ -1483,6 +1486,14 @@ public class FieldSortIT extends ESIntegTestCase {
         assertThat(hits[1].getSortValues().length, is(1));
         assertThat(hits[0].getSortValues()[0], is("cba bca"));
         assertThat(hits[1].getSortValues()[0], is("bar bar"));
+
+        // missing nested path
+        SearchPhaseExecutionException exc = expectThrows(SearchPhaseExecutionException.class,
+            () -> client().prepareSearch()
+                    .setQuery(matchAllQuery()).addSort(SortBuilders.fieldSort("nested.foo"))
+                    .get()
+        );
+        assertThat(exc.toString(), containsString("it is mandatory to set the [nested] context"));
     }
 
     public void testSortDuelBetweenSingleShardAndMultiShardIndex() throws Exception {

+ 17 - 9
server/src/test/java/org/elasticsearch/search/sort/GeoDistanceIT.java

@@ -272,7 +272,8 @@ public class GeoDistanceIT extends ESIntegTestCase {
 
         // Order: Asc
         SearchResponse searchResponse = client().prepareSearch("companies").setQuery(matchAllQuery()).addSort(SortBuilders
-                .geoDistanceSort("branches.location", 40.7143528, -74.0059731).order(SortOrder.ASC).setNestedPath("branches"))
+                .geoDistanceSort("branches.location", 40.7143528, -74.0059731).order(SortOrder.ASC)
+                .setNestedSort(new NestedSortBuilder("branches")))
                 .get();
 
         assertHitCount(searchResponse, 4);
@@ -285,7 +286,8 @@ public class GeoDistanceIT extends ESIntegTestCase {
         // Order: Asc, Mode: max
         searchResponse = client()
                 .prepareSearch("companies").setQuery(matchAllQuery()).addSort(SortBuilders.geoDistanceSort("branches.location",
-                        40.7143528, -74.0059731).order(SortOrder.ASC).sortMode(SortMode.MAX).setNestedPath("branches"))
+                        40.7143528, -74.0059731).order(SortOrder.ASC).sortMode(SortMode.MAX)
+                .setNestedSort(new NestedSortBuilder("branches")))
                 .get();
 
         assertHitCount(searchResponse, 4);
@@ -297,7 +299,8 @@ public class GeoDistanceIT extends ESIntegTestCase {
 
         // Order: Desc
         searchResponse = client().prepareSearch("companies").setQuery(matchAllQuery()).addSort(SortBuilders
-                .geoDistanceSort("branches.location", 40.7143528, -74.0059731).order(SortOrder.DESC).setNestedPath("branches"))
+                .geoDistanceSort("branches.location", 40.7143528, -74.0059731).order(SortOrder.DESC)
+                .setNestedSort(new NestedSortBuilder("branches")))
                 .get();
 
         assertHitCount(searchResponse, 4);
@@ -310,7 +313,8 @@ public class GeoDistanceIT extends ESIntegTestCase {
         // Order: Desc, Mode: min
         searchResponse = client()
                 .prepareSearch("companies").setQuery(matchAllQuery()).addSort(SortBuilders.geoDistanceSort("branches.location",
-                        40.7143528, -74.0059731).order(SortOrder.DESC).sortMode(SortMode.MIN).setNestedPath("branches"))
+                        40.7143528, -74.0059731).order(SortOrder.DESC).sortMode(SortMode.MIN)
+                .setNestedSort(new NestedSortBuilder("branches")))
                 .get();
 
         assertHitCount(searchResponse, 4);
@@ -322,7 +326,8 @@ public class GeoDistanceIT extends ESIntegTestCase {
 
         searchResponse = client()
                 .prepareSearch("companies").setQuery(matchAllQuery()).addSort(SortBuilders.geoDistanceSort("branches.location",
-                        40.7143528, -74.0059731).sortMode(SortMode.AVG).order(SortOrder.ASC).setNestedPath("branches"))
+                        40.7143528, -74.0059731).sortMode(SortMode.AVG).order(SortOrder.ASC)
+                .setNestedSort(new NestedSortBuilder("branches")))
                 .get();
 
         assertHitCount(searchResponse, 4);
@@ -334,7 +339,8 @@ public class GeoDistanceIT extends ESIntegTestCase {
 
         searchResponse = client().prepareSearch("companies")
                 .setQuery(matchAllQuery()).addSort(SortBuilders.geoDistanceSort("branches.location", 40.7143528, -74.0059731)
-                        .setNestedPath("branches").sortMode(SortMode.AVG).order(SortOrder.DESC).setNestedPath("branches"))
+                .setNestedSort(new NestedSortBuilder("branches"))
+                .sortMode(SortMode.AVG).order(SortOrder.DESC))
                 .get();
 
         assertHitCount(searchResponse, 4);
@@ -346,8 +352,10 @@ public class GeoDistanceIT extends ESIntegTestCase {
 
         searchResponse = client().prepareSearch("companies").setQuery(matchAllQuery())
                 .addSort(SortBuilders.geoDistanceSort("branches.location", 40.7143528, -74.0059731)
-                        .setNestedFilter(termQuery("branches.name", "brooklyn"))
-                        .sortMode(SortMode.AVG).order(SortOrder.ASC).setNestedPath("branches"))
+                        .setNestedSort(new NestedSortBuilder("branches")
+                            .setFilter(termQuery("branches.name", "brooklyn"))
+                        )
+                        .sortMode(SortMode.AVG).order(SortOrder.ASC))
                 .get();
         assertHitCount(searchResponse, 4);
         assertFirstHit(searchResponse, hasId("4"));
@@ -360,7 +368,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
         try {
                 client().prepareSearch("companies").setQuery(matchAllQuery())
                         .addSort(SortBuilders.geoDistanceSort("branches.location", 40.7143528, -74.0059731).sortMode(SortMode.SUM)
-                                .setNestedPath("branches"));
+                                .setNestedSort(new NestedSortBuilder("branches")));
                 fail("Sum should not be allowed as sort mode");
         } catch (IllegalArgumentException e) {
             //expected

+ 19 - 71
server/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java

@@ -50,9 +50,7 @@ import org.elasticsearch.search.MultiValueMode;
 import org.elasticsearch.test.geo.RandomGeoGenerator;
 
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.List;
 
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.hamcrest.Matchers.instanceOf;
@@ -106,20 +104,10 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
             result.validation(randomValueOtherThan(result.validation(), () -> randomFrom(GeoValidationMethod.values())));
         }
         if (randomBoolean()) {
-            if (randomBoolean()) {
-                // don't fully randomize here, GeoDistanceSort is picky about the filters that are allowed
-                NestedSortBuilder nestedSort = new NestedSortBuilder(randomAlphaOfLengthBetween(3, 10));
-                nestedSort.setFilter(new MatchAllQueryBuilder());
-                result.setNestedSort(nestedSort);
-            } else {
-                // the following are alternative ways to setNestedSort for nested sorting
-                if (randomBoolean()) {
-                    result.setNestedFilter(new MatchAllQueryBuilder());
-                }
-                if (randomBoolean()) {
-                    result.setNestedPath(randomAlphaOfLengthBetween(1, 10));
-                }
-            }
+            // don't fully randomize here, GeoDistanceSort is picky about the filters that are allowed
+            NestedSortBuilder nestedSort = new NestedSortBuilder(randomAlphaOfLengthBetween(3, 10));
+            nestedSort.setFilter(new MatchAllQueryBuilder());
+            result.setNestedSort(nestedSort);
         }
         if (randomBoolean()) {
             result.ignoreUnmapped(result.ignoreUnmapped() == false);
@@ -183,16 +171,8 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
                     () -> randomFrom(SortMode.values())));
             break;
         case 6:
-            if (original.getNestedPath() == null && original.getNestedFilter() == null) {
-                result.setNestedSort(
-                        randomValueOtherThan(original.getNestedSort(), () -> NestedSortBuilderTests.createRandomNestedSort(3)));
-            } else {
-                if (randomBoolean()) {
-                    result.setNestedPath(randomValueOtherThan(original.getNestedPath(), () -> randomAlphaOfLengthBetween(1, 10)));
-                } else {
-                    result.setNestedFilter(randomValueOtherThan(original.getNestedFilter(), () -> randomNestedFilter()));
-                }
-            }
+            result.setNestedSort(
+                randomValueOtherThan(original.getNestedSort(), () -> NestedSortBuilderTests.createRandomNestedSort(3)));
             break;
         case 7:
             result.validation(randomValueOtherThan(result.validation(), () -> randomFrom(GeoValidationMethod.values())));
@@ -387,21 +367,6 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
         }
     }
 
-    @Override
-    protected void assertWarnings(GeoDistanceSortBuilder testItem) {
-        List<String> expectedWarnings = new ArrayList<>();
-        if (testItem.getNestedFilter() != null) {
-            expectedWarnings.add("[nested_filter] has been deprecated in favour of the [nested] parameter");
-        }
-        if (testItem.getNestedPath() != null) {
-            expectedWarnings.add("[nested_path] has been deprecated in favour of the [nested] parameter");
-        }
-        if (expectedWarnings.isEmpty() == false) {
-            assertWarnings(expectedWarnings.toArray(new String[expectedWarnings.size()]));
-        }
-    }
-
-
     @Override
     protected GeoDistanceSortBuilder fromXContent(XContentParser parser, String fieldName) throws IOException {
         return GeoDistanceSortBuilder.fromXContent(parser, fieldName);
@@ -433,7 +398,7 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
         assertEquals(SortField.class, sort.field.getClass()); // descending means the max value should be considered rather than min
 
         builder = new GeoDistanceSortBuilder("random_field_name", new GeoPoint(3.5, 2.1));
-        builder.setNestedPath("some_nested_path");
+        builder.setNestedSort(new NestedSortBuilder("some_nested_path"));
         sort = builder.build(context);
         assertEquals(SortField.class, sort.field.getClass()); // can't use LatLon optimized sorting with nested fields
 
@@ -523,7 +488,8 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
         assertNotNull(nested);
         assertEquals(new MatchAllDocsQuery(), nested.getInnerQuery());
 
-        sortBuilder = new GeoDistanceSortBuilder("fieldName", 1.0, 1.0).setNestedPath("path");
+        sortBuilder = new GeoDistanceSortBuilder("fieldName", 1.0, 1.0)
+            .setNestedSort(new NestedSortBuilder("path"));
         sortField = sortBuilder.build(shardContextMock).field;
         assertThat(sortField.getComparatorSource(), instanceOf(XFieldComparatorSource.class));
         comparatorSource = (XFieldComparatorSource) sortField.getComparatorSource();
@@ -531,20 +497,16 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
         assertNotNull(nested);
         assertEquals(new TermQuery(new Term(TypeFieldMapper.NAME, "__path")), nested.getInnerQuery());
 
-        sortBuilder = new GeoDistanceSortBuilder("fieldName", 1.0, 1.0).setNestedPath("path")
-                .setNestedFilter(QueryBuilders.matchAllQuery());
+        sortBuilder = new GeoDistanceSortBuilder("fieldName", 1.0, 1.0)
+            .setNestedSort(new NestedSortBuilder("path")
+                .setFilter(QueryBuilders.matchAllQuery())
+            );
         sortField = sortBuilder.build(shardContextMock).field;
         assertThat(sortField.getComparatorSource(), instanceOf(XFieldComparatorSource.class));
         comparatorSource = (XFieldComparatorSource) sortField.getComparatorSource();
         nested = comparatorSource.nested();
         assertNotNull(nested);
         assertEquals(new MatchAllDocsQuery(), nested.getInnerQuery());
-
-        // if nested path is missing, we omit any filter and return a regular SortField
-        // (LatLonSortField)
-        sortBuilder = new GeoDistanceSortBuilder("fieldName", 1.0, 1.0).setNestedFilter(QueryBuilders.termQuery("fieldName", "value"));
-        sortField = sortBuilder.build(shardContextMock).field;
-        assertThat(sortField, instanceOf(SortField.class));
     }
 
     /**
@@ -579,22 +541,6 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
         }
     }
 
-    /**
-     * Test we can either set nested sort via path/filter or via nested sort builder, not both
-     */
-    public void testNestedSortBothThrows() throws IOException {
-        GeoDistanceSortBuilder sortBuilder = new GeoDistanceSortBuilder("fieldName", 0.0, 0.0);
-        IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
-                () -> sortBuilder.setNestedPath("nestedPath").setNestedSort(new NestedSortBuilder("otherPath")));
-        assertEquals("Setting both nested_path/nested_filter and nested not allowed", iae.getMessage());
-        iae = expectThrows(IllegalArgumentException.class,
-                () -> sortBuilder.setNestedSort(new NestedSortBuilder("otherPath")).setNestedPath("nestedPath"));
-        assertEquals("Setting both nested_path/nested_filter and nested not allowed", iae.getMessage());
-        iae = expectThrows(IllegalArgumentException.class,
-                () -> sortBuilder.setNestedSort(new NestedSortBuilder("otherPath")).setNestedFilter(QueryBuilders.matchAllQuery()));
-        assertEquals("Setting both nested_path/nested_filter and nested not allowed", iae.getMessage());
-    }
-
     /**
      * Test the nested Filter gets rewritten
      */
@@ -606,10 +552,12 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
                 return new MatchNoneQueryBuilder();
             }
         };
-        sortBuilder.setNestedPath("path").setNestedFilter(rangeQuery);
-        GeoDistanceSortBuilder rewritten = (GeoDistanceSortBuilder) sortBuilder
-                .rewrite(createMockShardContext());
-        assertNotSame(rangeQuery, rewritten.getNestedFilter());
+        sortBuilder.setNestedSort(
+            new NestedSortBuilder("path")
+                .setFilter(rangeQuery)
+        );
+        GeoDistanceSortBuilder rewritten = sortBuilder.rewrite(createMockShardContext());
+        assertNotSame(rangeQuery, rewritten.getNestedSort().getFilter());
     }
 
     /**

+ 7 - 29
server/src/test/java/org/elasticsearch/search/sort/ScriptSortBuilderTests.java

@@ -324,7 +324,8 @@ public class ScriptSortBuilderTests extends AbstractSortTestCase<ScriptSortBuild
         assertNotNull(nested);
         assertEquals(new MatchAllDocsQuery(), nested.getInnerQuery());
 
-        sortBuilder = new ScriptSortBuilder(mockScript(MOCK_SCRIPT_NAME), ScriptSortType.NUMBER).setNestedPath("path");
+        sortBuilder = new ScriptSortBuilder(mockScript(MOCK_SCRIPT_NAME), ScriptSortType.NUMBER)
+            .setNestedSort(new NestedSortBuilder("path"));
         sortField = sortBuilder.build(shardContextMock).field;
         assertThat(sortField.getComparatorSource(), instanceOf(XFieldComparatorSource.class));
         comparatorSource = (XFieldComparatorSource) sortField.getComparatorSource();
@@ -332,40 +333,17 @@ public class ScriptSortBuilderTests extends AbstractSortTestCase<ScriptSortBuild
         assertNotNull(nested);
         assertEquals(new TermQuery(new Term(TypeFieldMapper.NAME, "__path")), nested.getInnerQuery());
 
-        sortBuilder = new ScriptSortBuilder(mockScript(MOCK_SCRIPT_NAME), ScriptSortType.NUMBER).setNestedPath("path")
-                .setNestedFilter(QueryBuilders.matchAllQuery());
+        sortBuilder = new ScriptSortBuilder(mockScript(MOCK_SCRIPT_NAME), ScriptSortType.NUMBER)
+            .setNestedSort(new NestedSortBuilder("path")
+                .setFilter(QueryBuilders.matchAllQuery()));
         sortField = sortBuilder.build(shardContextMock).field;
         assertThat(sortField.getComparatorSource(), instanceOf(XFieldComparatorSource.class));
         comparatorSource = (XFieldComparatorSource) sortField.getComparatorSource();
         nested = comparatorSource.nested();
         assertNotNull(nested);
         assertEquals(new MatchAllDocsQuery(), nested.getInnerQuery());
-
-        // if nested path is missing, we omit nested element in the comparator
-        sortBuilder = new ScriptSortBuilder(mockScript(MOCK_SCRIPT_NAME), ScriptSortType.NUMBER)
-                .setNestedFilter(QueryBuilders.matchAllQuery());
-        sortField = sortBuilder.build(shardContextMock).field;
-        assertThat(sortField.getComparatorSource(), instanceOf(XFieldComparatorSource.class));
-        comparatorSource = (XFieldComparatorSource) sortField.getComparatorSource();
-        assertNull(comparatorSource.nested());
     }
 
-    /**
-     * Test we can either set nested sort via path/filter or via nested sort builder, not both
-     */
-    public void testNestedSortBothThrows() throws IOException {
-        ScriptSortBuilder sortBuilder = new ScriptSortBuilder(mockScript(MOCK_SCRIPT_NAME), ScriptSortType.NUMBER);
-        IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
-                () -> sortBuilder.setNestedPath("nestedPath").setNestedSort(new NestedSortBuilder("otherPath")));
-        assertEquals("Setting both nested_path/nested_filter and nested not allowed", iae.getMessage());
-        iae = expectThrows(IllegalArgumentException.class,
-                () -> sortBuilder.setNestedSort(new NestedSortBuilder("otherPath")).setNestedPath("nestedPath"));
-        assertEquals("Setting both nested_path/nested_filter and nested not allowed", iae.getMessage());
-        iae = expectThrows(IllegalArgumentException.class,
-                () -> sortBuilder.setNestedSort(new NestedSortBuilder("otherPath")).setNestedFilter(QueryBuilders.matchAllQuery()));
-        assertEquals("Setting both nested_path/nested_filter and nested not allowed", iae.getMessage());
-     }
-
     /**
      * Test the nested Filter gets rewritten
      */
@@ -377,10 +355,10 @@ public class ScriptSortBuilderTests extends AbstractSortTestCase<ScriptSortBuild
                 return new MatchNoneQueryBuilder();
             }
         };
-        sortBuilder.setNestedPath("path").setNestedFilter(rangeQuery);
+        sortBuilder.setNestedSort(new NestedSortBuilder("path").setFilter(rangeQuery));
         ScriptSortBuilder rewritten = sortBuilder
                 .rewrite(createMockShardContext());
-        assertNotSame(rangeQuery, rewritten.getNestedFilter());
+        assertNotSame(rangeQuery, rewritten.getNestedSort().getFilter());
     }
 
     /**

+ 0 - 10
server/src/test/java/org/elasticsearch/search/sort/SortBuilderTests.java

@@ -143,16 +143,6 @@ public class SortBuilderTests extends ESTestCase {
                 xContentBuilder.field("sort");
             }
             for (SortBuilder<?> builder : testBuilders) {
-                if (builder instanceof GeoDistanceSortBuilder) {
-                    GeoDistanceSortBuilder gdsb = (GeoDistanceSortBuilder) builder;
-                    if (gdsb.getNestedFilter() != null) {
-                        expectedWarningHeaders.add("[nested_filter] has been deprecated in favour of the [nested] parameter");
-                    }
-                    if (gdsb.getNestedPath() != null) {
-                        expectedWarningHeaders.add("[nested_path] has been deprecated in favour of the [nested] parameter");
-                    }
-                }
-
                 if (builder instanceof ScoreSortBuilder || builder instanceof FieldSortBuilder) {
                     switch (randomIntBetween(0, 2)) {
                     case 0: