Skip to content

Commit 199fff8

Browse files
erayarslanjimczi
authored andcommitted
Allow max_children only in top level nested sort (#46731)
This commit restricts the usage of max_children to the top level nested sort since it is ignored on the other levels.
1 parent a815f8b commit 199fff8

File tree

4 files changed

+48
-3
lines changed

4 files changed

+48
-3
lines changed

server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -409,16 +409,15 @@ public SortFieldAndFormat build(QueryShardContext context) throws IOException {
409409
throw new QueryShardException(context,
410410
"max_children is only supported on last level of nested sort");
411411
}
412-
// new nested sorts takes priority
412+
validateMaxChildrenExistOnlyInTopLevelNestedSort(context, nestedSort);
413413
nested = resolveNested(context, nestedSort);
414414
} else {
415415
nested = resolveNested(context, nestedPath, nestedFilter);
416416
}
417417
}
418-
419418
IndexFieldData<?> fieldData = context.getForField(fieldType);
420419
if (fieldData instanceof IndexNumericFieldData == false
421-
&& (sortMode == SortMode.SUM || sortMode == SortMode.AVG || sortMode == SortMode.MEDIAN)) {
420+
&& (sortMode == SortMode.SUM || sortMode == SortMode.AVG || sortMode == SortMode.MEDIAN)) {
422421
throw new QueryShardException(context, "we only support AVG, MEDIAN and SUM on number based fields");
423422
}
424423
final SortField field;
@@ -437,6 +436,18 @@ public SortFieldAndFormat build(QueryShardContext context) throws IOException {
437436
}
438437
}
439438

439+
/**
440+
* Throws an exception if max children is not located at top level nested sort.
441+
*/
442+
static void validateMaxChildrenExistOnlyInTopLevelNestedSort(QueryShardContext context, NestedSortBuilder nestedSort) {
443+
for (NestedSortBuilder child = nestedSort.getNestedSort(); child != null; child = child.getNestedSort()) {
444+
if (child.getMaxChildren() != Integer.MAX_VALUE) {
445+
throw new QueryShardException(context,
446+
"max_children is only supported on top level of nested sort");
447+
}
448+
}
449+
}
450+
440451
@Override
441452
public boolean equals(Object other) {
442453
if (this == other) {

server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
import java.util.Objects;
6767

6868
import static org.elasticsearch.index.query.AbstractQueryBuilder.parseInnerQueryBuilder;
69+
import static org.elasticsearch.search.sort.FieldSortBuilder.validateMaxChildrenExistOnlyInTopLevelNestedSort;
6970
import static org.elasticsearch.search.sort.NestedSortBuilder.NESTED_FIELD;
7071

7172
/**
@@ -630,6 +631,7 @@ public SortFieldAndFormat build(QueryShardContext context) throws IOException {
630631
"max_children is only supported on last level of nested sort");
631632
}
632633
// new nested sorts takes priority
634+
validateMaxChildrenExistOnlyInTopLevelNestedSort(context, nestedSort);
633635
nested = resolveNested(context, nestedSort);
634636
} else {
635637
nested = resolveNested(context, nestedPath, nestedFilter);

server/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
import java.util.Objects;
6161

6262
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
63+
import static org.elasticsearch.search.sort.FieldSortBuilder.validateMaxChildrenExistOnlyInTopLevelNestedSort;
6364
import static org.elasticsearch.search.sort.NestedSortBuilder.NESTED_FIELD;
6465

6566
/**
@@ -325,6 +326,7 @@ public SortFieldAndFormat build(QueryShardContext context) throws IOException {
325326
"max_children is only supported on last level of nested sort");
326327
}
327328
// new nested sorts takes priority
329+
validateMaxChildrenExistOnlyInTopLevelNestedSort(context, nestedSort);
328330
nested = resolveNested(context, nestedSort);
329331
} else {
330332
nested = resolveNested(context, nestedPath, nestedFilter);

server/src/test/java/org/elasticsearch/search/sort/FieldSortIT.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1421,6 +1421,20 @@ public void testNestedSort() throws IOException, InterruptedException, Execution
14211421
.endObject()
14221422
.endObject()
14231423
.endObject()
1424+
.startObject("bar")
1425+
.field("type", "nested")
1426+
.startObject("properties")
1427+
.startObject("foo")
1428+
.field("type", "text")
1429+
.field("fielddata", true)
1430+
.startObject("fields")
1431+
.startObject("sub")
1432+
.field("type", "keyword")
1433+
.endObject()
1434+
.endObject()
1435+
.endObject()
1436+
.endObject()
1437+
.endObject()
14241438
.endObject()
14251439
.endObject()
14261440
.endObject()
@@ -1471,6 +1485,22 @@ public void testNestedSort() throws IOException, InterruptedException, Execution
14711485
assertThat(hits[0].getSortValues()[0], is("bar"));
14721486
assertThat(hits[1].getSortValues()[0], is("abc"));
14731487

1488+
{
1489+
SearchPhaseExecutionException exc = expectThrows(SearchPhaseExecutionException.class,
1490+
() -> client().prepareSearch()
1491+
.setQuery(matchAllQuery())
1492+
.addSort(SortBuilders
1493+
.fieldSort("nested.bar.foo")
1494+
.setNestedSort(new NestedSortBuilder("nested")
1495+
.setNestedSort(new NestedSortBuilder("nested.bar")
1496+
.setMaxChildren(1)))
1497+
.order(SortOrder.DESC))
1498+
.get()
1499+
);
1500+
assertThat(exc.toString(),
1501+
containsString("max_children is only supported on top level of nested sort"));
1502+
}
1503+
14741504
// We sort on nested sub field
14751505
searchResponse = client().prepareSearch()
14761506
.setQuery(matchAllQuery())

0 commit comments

Comments
 (0)