Skip to content

Commit e454463

Browse files
committed
Fix composite agg sort bug (elastic#53296)
When an composite aggregation is run against an index with a sort that *starts* with the "source" fields from the composite but has additional fields it'd blow up in while trying to decide if it could use the sort. This changes it to decide that it *can* use the sort. Closes elastic#52480
1 parent d54d7f2 commit e454463

File tree

3 files changed

+129
-32
lines changed

3 files changed

+129
-32
lines changed

rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/230_composite.yml

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -885,3 +885,87 @@ setup:
885885
- length: { aggregations.test.buckets: 1 }
886886
- match: { aggregations.test.buckets.0.key.date: "2017-10-21T00:00:00.000-02:00" }
887887
- match: { aggregations.test.buckets.0.doc_count: 2 }
888+
889+
---
890+
"Terms source from sorted":
891+
- do:
892+
indices.create:
893+
index: sorted_test
894+
body:
895+
settings:
896+
sort.field: keyword
897+
mappings:
898+
properties:
899+
keyword:
900+
type: keyword
901+
long:
902+
type: long
903+
904+
905+
- do:
906+
index:
907+
index: sorted_test
908+
id: 2
909+
refresh: true
910+
body: { "keyword": "foo", "long": 1 }
911+
912+
- do:
913+
search:
914+
index: sorted_test
915+
body:
916+
aggregations:
917+
test:
918+
composite:
919+
sources:
920+
- keyword:
921+
terms:
922+
field: keyword
923+
924+
- match: {hits.total.value: 1}
925+
- length: { aggregations.test.buckets: 1 }
926+
- match: { aggregations.test.buckets.0.key.keyword: "foo" }
927+
- match: { aggregations.test.buckets.0.doc_count: 1 }
928+
929+
---
930+
"Terms source from part of sorted":
931+
- skip:
932+
version: " - 7.99.99"
933+
reason: fixed in 8.0.0. will be backported to 7.7.0.
934+
935+
- do:
936+
indices.create:
937+
index: sorted_test
938+
body:
939+
settings:
940+
sort.field: [keyword, long]
941+
mappings:
942+
properties:
943+
keyword:
944+
type: keyword
945+
long:
946+
type: long
947+
948+
949+
- do:
950+
index:
951+
index: sorted_test
952+
id: 2
953+
refresh: true
954+
body: { "keyword": "foo", "long": 1 }
955+
956+
- do:
957+
search:
958+
index: sorted_test
959+
body:
960+
aggregations:
961+
test:
962+
composite:
963+
sources:
964+
- keyword:
965+
terms:
966+
field: keyword
967+
968+
- match: {hits.total.value: 1}
969+
- length: { aggregations.test.buckets: 1 }
970+
- match: { aggregations.test.buckets.0.key.keyword: "foo" }
971+
- match: { aggregations.test.buckets.0.doc_count: 1 }

server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregator.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,8 @@ private Sort buildIndexSortPrefix(LeafReaderContext context) throws IOException
202202
return null;
203203
}
204204
List<SortField> sortFields = new ArrayList<>();
205-
for (int i = 0; i < indexSort.getSort().length; i++) {
205+
int end = Math.min(indexSort.getSort().length, sourceConfigs.length);
206+
for (int i = 0; i < end; i++) {
206207
CompositeValuesSourceConfig sourceConfig = sourceConfigs[i];
207208
SingleDimensionValuesSource<?> source = sources[i];
208209
SortField indexSortField = indexSort.getSort()[i];

server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java

Lines changed: 43 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2077,40 +2077,52 @@ private static long asLong(String dateTime) {
20772077

20782078
private static Sort buildIndexSort(List<CompositeValuesSourceBuilder<?>> sources, Map<String, MappedFieldType> fieldTypes) {
20792079
List<SortField> sortFields = new ArrayList<>();
2080+
Map<String, MappedFieldType> remainingFieldTypes = new HashMap<>(fieldTypes);
20802081
for (CompositeValuesSourceBuilder<?> source : sources) {
2081-
MappedFieldType type = fieldTypes.get(source.field());
2082-
if (type instanceof KeywordFieldMapper.KeywordFieldType) {
2083-
sortFields.add(new SortedSetSortField(type.name(), false));
2084-
} else if (type instanceof DateFieldMapper.DateFieldType) {
2085-
sortFields.add(new SortedNumericSortField(type.name(), SortField.Type.LONG, false));
2086-
} else if (type instanceof NumberFieldMapper.NumberFieldType) {
2087-
boolean comp = false;
2088-
switch (type.typeName()) {
2089-
case "byte":
2090-
case "short":
2091-
case "integer":
2092-
comp = true;
2093-
sortFields.add(new SortedNumericSortField(type.name(), SortField.Type.INT, false));
2094-
break;
2095-
2096-
case "long":
2097-
sortFields.add(new SortedNumericSortField(type.name(), SortField.Type.LONG, false));
2098-
break;
2099-
2100-
case "float":
2101-
case "double":
2102-
comp = true;
2103-
sortFields.add(new SortedNumericSortField(type.name(), SortField.Type.DOUBLE, false));
2104-
break;
2105-
2106-
default:
2107-
break;
2108-
}
2109-
if (comp == false) {
2110-
break;
2111-
}
2082+
MappedFieldType type = fieldTypes.remove(source.field());
2083+
remainingFieldTypes.remove(source.field());
2084+
SortField sortField = sortFieldFrom(type);
2085+
if (sortField == null) {
2086+
break;
2087+
}
2088+
sortFields.add(sortField);
2089+
if (sortField instanceof SortedNumericSortField && ((SortedNumericSortField) sortField).getType() == SortField.Type.LONG) {
2090+
break;
2091+
}
2092+
}
2093+
while (remainingFieldTypes.size() > 0 && randomBoolean()) {
2094+
// Add extra unused sorts
2095+
List<String> fields = new ArrayList<>(remainingFieldTypes.keySet());
2096+
Collections.sort(fields);
2097+
String field = fields.get(between(0, fields.size() - 1));
2098+
SortField sortField = sortFieldFrom(remainingFieldTypes.remove(field));
2099+
if (sortField != null) {
2100+
sortFields.add(sortField);
21122101
}
21132102
}
21142103
return sortFields.size() > 0 ? new Sort(sortFields.toArray(new SortField[0])) : null;
21152104
}
2105+
2106+
private static SortField sortFieldFrom(MappedFieldType type) {
2107+
if (type instanceof KeywordFieldMapper.KeywordFieldType) {
2108+
return new SortedSetSortField(type.name(), false);
2109+
} else if (type instanceof DateFieldMapper.DateFieldType) {
2110+
return new SortedNumericSortField(type.name(), SortField.Type.LONG, false);
2111+
} else if (type instanceof NumberFieldMapper.NumberFieldType) {
2112+
switch (type.typeName()) {
2113+
case "byte":
2114+
case "short":
2115+
case "integer":
2116+
return new SortedNumericSortField(type.name(), SortField.Type.INT, false);
2117+
case "long":
2118+
return new SortedNumericSortField(type.name(), SortField.Type.LONG, false);
2119+
case "float":
2120+
case "double":
2121+
return new SortedNumericSortField(type.name(), SortField.Type.DOUBLE, false);
2122+
default:
2123+
return null;
2124+
}
2125+
}
2126+
return null;
2127+
}
21162128
}

0 commit comments

Comments
 (0)