Skip to content

Commit 6cc059d

Browse files
authored
Improve handling of incompatible after key types in composite aggs (#70839) (#70939)
Composite aggs on a field that have the same name but different types in different indices can produce confusing errors. This commit add more information to the error message making debugging of such issues easier. Fixes #70480
1 parent 5398a9c commit 6cc059d

File tree

4 files changed

+45
-10
lines changed

4 files changed

+45
-10
lines changed

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.apache.lucene.search.comparators.LongComparator;
3333
import org.apache.lucene.util.Bits;
3434
import org.apache.lucene.util.RoaringDocIdSet;
35+
import org.elasticsearch.ElasticsearchParseException;
3536
import org.elasticsearch.common.lease.Releasables;
3637
import org.elasticsearch.index.IndexSortConfig;
3738
import org.elasticsearch.search.DocValueFormat;
@@ -103,7 +104,15 @@ final class CompositeAggregator extends BucketsAggregator {
103104
this::addRequestCircuitBreakerBytes
104105
);
105106
}
106-
this.queue = new CompositeValuesCollectorQueue(context.bigArrays(), sources, size, rawAfterKey);
107+
this.queue = new CompositeValuesCollectorQueue(context.bigArrays(), sources, size);
108+
if (rawAfterKey != null) {
109+
try {
110+
this.queue.setAfterKey(rawAfterKey);
111+
} catch (IllegalArgumentException ex) {
112+
throw new ElasticsearchParseException("Cannot set after key in the composite aggregation [" + name + "] - " +
113+
ex.getMessage(), ex);
114+
}
115+
}
107116
this.rawAfterKey = rawAfterKey;
108117
}
109118

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

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,22 +62,30 @@ public int hashCode() {
6262
*
6363
* @param sources The list of {@link CompositeValuesSourceConfig} to build the composite buckets.
6464
* @param size The number of composite buckets to keep.
65-
* @param afterKey composite key
6665
*/
67-
CompositeValuesCollectorQueue(BigArrays bigArrays, SingleDimensionValuesSource<?>[] sources, int size, CompositeKey afterKey) {
66+
CompositeValuesCollectorQueue(BigArrays bigArrays, SingleDimensionValuesSource<?>[] sources, int size) {
6867
super(size);
6968
this.bigArrays = bigArrays;
7069
this.maxSize = size;
7170
this.arrays = sources;
7271
this.map = new HashMap<>(size);
73-
if (afterKey != null) {
74-
assert afterKey.size() == sources.length;
75-
afterKeyIsSet = true;
76-
for (int i = 0; i < afterKey.size(); i++) {
77-
sources[i].setAfter(afterKey.get(i));
72+
this.docCounts = bigArrays.newLongArray(1, false);
73+
}
74+
75+
/**
76+
* Sets after key
77+
* @param afterKey composite key
78+
*/
79+
public void setAfterKey(CompositeKey afterKey) {
80+
assert afterKey.size() == arrays.length;
81+
afterKeyIsSet = true;
82+
for (int i = 0; i < afterKey.size(); i++) {
83+
try {
84+
arrays[i].setAfter(afterKey.get(i));
85+
} catch (IllegalArgumentException ex) {
86+
throw new IllegalArgumentException("incompatible value in the position " + i + ": " + ex.getMessage(), ex);
7887
}
7988
}
80-
this.docCounts = bigArrays.newLongArray(1, false);
8189
}
8290

8391
@Override

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -938,6 +938,21 @@ public void testWithKeywordAndLong() throws Exception {
938938
assertEquals(1L, result.getBuckets().get(1).getDocCount());
939939
}
940940
);
941+
942+
Exception exc = expectThrows(ElasticsearchParseException.class,
943+
() -> testSearchCase(Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("date")), Collections.emptyList(),
944+
() -> new CompositeAggregationBuilder("test",
945+
Arrays.asList(
946+
new TermsValuesSourceBuilder("keyword").field("keyword"),
947+
new TermsValuesSourceBuilder("long").field("long")
948+
)
949+
).aggregateAfter(createAfterKey("keyword", 0L, "long", 100L)
950+
),
951+
(result) -> {
952+
}
953+
));
954+
assertThat(exc.getMessage(), containsString("Cannot set after key in the composite aggregation [test] - incompatible value in " +
955+
"the position 0: invalid value, expected string, got Long"));
941956
}
942957

943958
public void testWithKeywordAndLongDesc() throws Exception {

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,10 @@ private void testRandomCase(boolean forceMerge,
288288
CompositeKey last = null;
289289
while (pos < size) {
290290
final CompositeValuesCollectorQueue queue =
291-
new CompositeValuesCollectorQueue(BigArrays.NON_RECYCLING_INSTANCE, sources, size, last);
291+
new CompositeValuesCollectorQueue(BigArrays.NON_RECYCLING_INSTANCE, sources, size);
292+
if (last != null) {
293+
queue.setAfterKey(last);
294+
}
292295
final SortedDocsProducer docsProducer = sources[0].createSortedDocsProducerOrNull(reader, new MatchAllDocsQuery());
293296
for (LeafReaderContext leafReaderContext : reader.leaves()) {
294297
if (docsProducer != null && withProducer) {

0 commit comments

Comments
 (0)