diff --git a/docs/reference/migration/migrate_8_0/search.asciidoc b/docs/reference/migration/migrate_8_0/search.asciidoc index 6fba2970f593e..97796a10fca22 100644 --- a/docs/reference/migration/migrate_8_0/search.asciidoc +++ b/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. diff --git a/docs/reference/search/request/sort.asciidoc b/docs/reference/search/request/sort.asciidoc index c12ec3a679a72..ccbc3da6e063b 100644 --- a/docs/reference/search/request/sort.asciidoc +++ b/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 diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/200_top_hits_metric.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/200_top_hits_metric.yml index cde56fa41e3d9..76274e9034d62 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/200_top_hits_metric.yml +++ b/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 } diff --git a/server/src/main/java/org/elasticsearch/index/search/NestedHelper.java b/server/src/main/java/org/elasticsearch/index/search/NestedHelper.java index 1c17fa0cb935f..ab2ae044bdbaf 100644 --- a/server/src/main/java/org/elasticsearch/index/search/NestedHelper.java +++ b/server/src/main/java/org/elasticsearch/index/search/NestedHelper.java @@ -192,7 +192,7 @@ boolean mightMatchNonNestedDocs(String field, String nestedPath) { 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; diff --git a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java index 6598d32bc2ca8..949a5a3ff441c 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java +++ b/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.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.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 { - 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 { private SortMode sortMode; - private QueryBuilder nestedFilter; - - private String nestedPath; - private NestedSortBuilder nestedSort; /** Copy constructor. */ @@ -95,8 +90,6 @@ public FieldSortBuilder(FieldSortBuilder template) { 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 FieldSortBuilder(String fieldName) { */ 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 FieldSortBuilder(StreamInput in) throws IOException { @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 SortMode sortMode() { 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 NestedSortBuilder getNestedSort() { * 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 XContentBuilder toXContent(XContentBuilder builder, Params params) throws 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 @@ private static NumericType resolveNumericType(String value) { @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 field 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 boolean equals(Object other) { } 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 boolean equals(Object other) { @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 static FieldSortBuilder fromXContent(XContentParser parser, String fieldN 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); } } diff --git a/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java index 73877b3faa309..630b93b4f34b5 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java +++ b/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.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.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.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 { - 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 private DistanceUnit unit = DistanceUnit.DEFAULT; private SortMode sortMode = null; - private QueryBuilder nestedFilter; - private String nestedPath; private NestedSortBuilder nestedSort; @@ -150,8 +146,6 @@ public GeoDistanceSortBuilder(String fieldName, String ... geohashes) { 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 GeoDistanceSortBuilder(StreamInput in) throws IOException { 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 void writeTo(StreamOutput out) throws IOException { 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 SortMode sortMode() { 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 NestedSortBuilder getNestedSort() { * 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 XContentBuilder toXContent(XContentBuilder builder, Params params) throws 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 boolean equals(Object object) { 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 boolean equals(Object object) { @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 static GeoDistanceSortBuilder fromXContent(XContentParser parser, String 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 static GeoDistanceSortBuilder fromXContent(XContentParser parser, String 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 static GeoDistanceSortBuilder fromXContent(XContentParser parser, String 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 static GeoDistanceSortBuilder fromXContent(XContentParser parser, String 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 SortFieldAndFormat build(QueryShardContext context) throws IOException { } 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 @@ static void parseGeoPoints(XContentParser parser, List geoPoints) thro @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); } } diff --git a/server/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java index 21de011e276b7..17fed4d9ac19c 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java +++ b/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 @@ * Script sort builder allows to sort based on a custom script expression. */ public class ScriptSortBuilder extends SortBuilder { - 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 { private SortMode sortMode; - private QueryBuilder nestedFilter; - - private String nestedPath; - private NestedSortBuilder nestedSort; /** @@ -105,8 +99,6 @@ public ScriptSortBuilder(Script script, ScriptSortType type) { 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 ScriptSortBuilder(StreamInput in) throws IOException { 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 void writeTo(StreamOutput out) throws IOException { 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 SortMode sortMode() { 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 NestedSortBuilder getNestedSort() { * 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 XContentBuilder toXContent(XContentBuilder builder, Params builderParams) 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 XContentBuilder toXContent(XContentBuilder builder, Params builderParams) 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 SortFieldAndFormat build(QueryShardContext context) throws IOException { 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 boolean equals(Object object) { 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 String toString() { @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); } } diff --git a/server/src/main/java/org/elasticsearch/search/sort/SortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/SortBuilder.java index 2060dde506907..881df666802d7 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/SortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/SortBuilder.java @@ -46,6 +46,7 @@ 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> implements NamedWriteable, ToXContentObject, Rewriteable> { @@ -53,8 +54,6 @@ public abstract class SortBuilder> 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> PARSERS = Map.of( ScriptSortBuilder.NAME, ScriptSortBuilder::fromXContent, @@ -171,12 +170,6 @@ public static Optional buildSort(List> sortBuilde 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 @@ private static Query resolveNestedQuery(QueryShardContext context, NestedSortBui 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 @@ protected static QueryBuilder parseNestedFilter(XContentParser parser) { 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); } } diff --git a/server/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java b/server/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java index 1937bb2a847fd..d80fa2c1e4cca 100644 --- a/server/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java +++ b/server/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java @@ -385,7 +385,8 @@ public void testSimpleNestedSorting() throws Exception { 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 void testSimpleNestedSorting() throws Exception { 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 void testSimpleNestedSortingWithNestedFilterMissing() throws Exception { 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 void testSimpleNestedSortingWithNestedFilterMissing() throws Exception { 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 void testSortNestedWithNestedFilter() throws Exception { .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 void testSortNestedWithNestedFilter() throws Exception { 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 void testSortNestedWithNestedFilter() throws Exception { .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 void testSortNestedWithNestedFilter() throws Exception { 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 void testSortNestedWithNestedFilter() throws Exception { .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 void testSortNestedWithNestedFilter() throws Exception { .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 void testSortNestedWithNestedFilter() throws Exception { .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 void testSortNestedWithNestedFilter() throws Exception { .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 void testSortNestedWithNestedFilter() throws Exception { .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 void testSortNestedWithNestedFilter() throws Exception { .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 void testSortNestedWithNestedFilter() throws Exception { .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 void testNestedSortingWithNestedFilterAsFilter() throws Exception { 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); diff --git a/server/src/test/java/org/elasticsearch/search/scroll/DuelScrollIT.java b/server/src/test/java/org/elasticsearch/search/scroll/DuelScrollIT.java index 04349e846c78f..fb42d244f78ff 100644 --- a/server/src/test/java/org/elasticsearch/search/scroll/DuelScrollIT.java +++ b/server/src/test/java/org/elasticsearch/search/scroll/DuelScrollIT.java @@ -30,6 +30,7 @@ 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 @@ private TestContext create(SearchType... searchTypes) throws Exception { } } 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); diff --git a/server/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java b/server/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java index ee9c8f8ed1105..1623bc2bb378d 100644 --- a/server/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java @@ -45,7 +45,6 @@ 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 FieldSortBuilder randomFieldSortBuilder() { 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 @@ protected FieldSortBuilder mutate(FieldSortBuilder original) throws IOException 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 void testBuildNested() throws IOException { 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 void testBuildNested() throws IOException { 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 void testModeNonNumericField() throws IOException { 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 QueryBuilder doRewrite(QueryRewriteContext queryShardContext) throws IOEx 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 QueryBuilder doRewrite(QueryRewriteContext queryShardContext) throws IOEx assertNotSame(rangeQuery, rewritten.getNestedSort().getFilter()); } - @Override - protected void assertWarnings(FieldSortBuilder testItem) { - List 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); diff --git a/server/src/test/java/org/elasticsearch/search/sort/FieldSortIT.java b/server/src/test/java/org/elasticsearch/search/sort/FieldSortIT.java index d3f21867ab1d1..400dc30afa11f 100644 --- a/server/src/test/java/org/elasticsearch/search/sort/FieldSortIT.java +++ b/server/src/test/java/org/elasticsearch/search/sort/FieldSortIT.java @@ -1443,9 +1443,11 @@ public void testNestedSort() throws IOException, InterruptedException, Execution 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 void testNestedSort() throws IOException, InterruptedException, Execution // 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 void testNestedSort() throws IOException, InterruptedException, Execution 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 { diff --git a/server/src/test/java/org/elasticsearch/search/sort/GeoDistanceIT.java b/server/src/test/java/org/elasticsearch/search/sort/GeoDistanceIT.java index 87d84484aca0a..f3899785c499c 100644 --- a/server/src/test/java/org/elasticsearch/search/sort/GeoDistanceIT.java +++ b/server/src/test/java/org/elasticsearch/search/sort/GeoDistanceIT.java @@ -272,7 +272,8 @@ public void testDistanceSortingNestedFields() throws Exception { // 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 void testDistanceSortingNestedFields() throws Exception { // 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 void testDistanceSortingNestedFields() throws Exception { // 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 void testDistanceSortingNestedFields() throws Exception { // 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 void testDistanceSortingNestedFields() throws Exception { 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 void testDistanceSortingNestedFields() throws Exception { 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 void testDistanceSortingNestedFields() throws Exception { 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 void testDistanceSortingNestedFields() throws Exception { 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 diff --git a/server/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java b/server/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java index c90e6ef0dde39..73be5837faa90 100644 --- a/server/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java @@ -50,9 +50,7 @@ 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 static GeoDistanceSortBuilder randomGeoDistanceSortBuilder() { 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 @@ protected GeoDistanceSortBuilder mutate(GeoDistanceSortBuilder original) throws () -> 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 @@ private GeoDistanceSortBuilder parse(XContentBuilder sortBuilder) throws Excepti } } - @Override - protected void assertWarnings(GeoDistanceSortBuilder testItem) { - List 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 void testCommonCaseIsOptimized() throws IOException { 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 void testBuildNested() throws IOException { 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 void testBuildNested() throws IOException { 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 void testBuildInvalidPoints() throws IOException { } } - /** - * 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 QueryBuilder doRewrite(QueryRewriteContext queryShardContext) throws IOEx 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()); } /** diff --git a/server/src/test/java/org/elasticsearch/search/sort/ScriptSortBuilderTests.java b/server/src/test/java/org/elasticsearch/search/sort/ScriptSortBuilderTests.java index 3017a2e0c067d..1d3b71bbf40db 100644 --- a/server/src/test/java/org/elasticsearch/search/sort/ScriptSortBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/sort/ScriptSortBuilderTests.java @@ -324,7 +324,8 @@ public void testBuildNested() throws IOException { 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 void testBuildNested() throws IOException { 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 QueryBuilder doRewrite(QueryRewriteContext queryShardContext) throws IOEx 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()); } /** diff --git a/server/src/test/java/org/elasticsearch/search/sort/SortBuilderTests.java b/server/src/test/java/org/elasticsearch/search/sort/SortBuilderTests.java index fab089ef2cfc2..7b1e56d5c69f2 100644 --- a/server/src/test/java/org/elasticsearch/search/sort/SortBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/sort/SortBuilderTests.java @@ -143,16 +143,6 @@ public void testRandomSortBuilders() throws IOException { 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: