Skip to content

Commit 290e8bb

Browse files
committed
Allow unmapped fields in composite aggregations (#35331)
Today the `composite` aggregation throws an error if a source targets an unmapped field and `missing_bucket` is set to false. Documents without a value for a source cannot produce any bucket if `missing_bucket` is not activated so the error is a shortcut to say that the response will be empty. However this is not consistent with the `terms` aggregation which accepts unmapped field by default even if the response is also guaranteed to be empty. This commit removes this restriction, if a source contains an unmapped field we now return an empty response (no buckets). Closes #35317
1 parent 43aff62 commit 290e8bb

File tree

2 files changed

+75
-16
lines changed

2 files changed

+75
-16
lines changed

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -330,12 +330,6 @@ public final CompositeValuesSourceConfig build(SearchContext context) throws IOE
330330
ValuesSourceConfig<?> config = ValuesSourceConfig.resolve(context.getQueryShardContext(),
331331
valueType, field, script, missing, null, format);
332332

333-
if (config.unmapped() && field != null && missing == null && missingBucket == false) {
334-
// this source cannot produce any values so we refuse to build
335-
// since composite buckets are not created on null values by default.
336-
throw new QueryShardException(context.getQueryShardContext(),
337-
"failed to find field [" + field + "] and [missing_bucket] is not set");
338-
}
339333
if (missingBucket && missing != null) {
340334
throw new QueryShardException(context.getQueryShardContext(),
341335
"cannot use [missing] option in conjunction with [missing_bucket]");

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

Lines changed: 75 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -130,16 +130,81 @@ public void tearDown() throws Exception {
130130
}
131131

132132
public void testUnmappedField() throws Exception {
133-
TermsValuesSourceBuilder terms = new TermsValuesSourceBuilder(randomAlphaOfLengthBetween(5, 10))
134-
.field("unknown");
135-
CompositeAggregationBuilder builder = new CompositeAggregationBuilder("test", Collections.singletonList(terms));
136-
IndexSearcher searcher = new IndexSearcher(new MultiReader());
137-
QueryShardException exc =
138-
expectThrows(QueryShardException.class, () -> createAggregatorFactory(builder, searcher));
139-
assertThat(exc.getMessage(), containsString("failed to find field [unknown] and [missing_bucket] is not set"));
140-
// should work when missing_bucket is set
141-
terms.missingBucket(true);
142-
createAggregatorFactory(builder, searcher);
133+
final List<Map<String, List<Object>>> dataset = new ArrayList<>();
134+
dataset.addAll(
135+
Arrays.asList(
136+
createDocument("keyword", "a"),
137+
createDocument("keyword", "c"),
138+
createDocument("keyword", "a"),
139+
createDocument("keyword", "d"),
140+
createDocument("keyword", "c")
141+
)
142+
);
143+
testSearchCase(Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("keyword")), dataset,
144+
() -> new CompositeAggregationBuilder("name",
145+
Arrays.asList(
146+
new TermsValuesSourceBuilder("unmapped").field("unmapped")
147+
)
148+
),
149+
(result) -> {
150+
assertEquals(0, result.getBuckets().size());
151+
}
152+
);
153+
154+
testSearchCase(Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("keyword")), dataset,
155+
() -> new CompositeAggregationBuilder("name",
156+
Arrays.asList(
157+
new TermsValuesSourceBuilder("unmapped").field("unmapped").missingBucket(true)
158+
)
159+
),
160+
(result) -> {
161+
assertEquals(1, result.getBuckets().size());
162+
assertEquals("{unmapped=null}", result.afterKey().toString());
163+
assertEquals("{unmapped=null}", result.getBuckets().get(0).getKeyAsString());
164+
assertEquals(5L, result.getBuckets().get(0).getDocCount());
165+
}
166+
);
167+
168+
testSearchCase(Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("keyword")), dataset,
169+
() -> new CompositeAggregationBuilder("name",
170+
Arrays.asList(
171+
new TermsValuesSourceBuilder("unmapped").field("unmapped").missingBucket(true)
172+
)).aggregateAfter(Collections.singletonMap("unmapped", null)),
173+
(result) -> {
174+
assertEquals(0, result.getBuckets().size());
175+
}
176+
);
177+
178+
testSearchCase(Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("keyword")), dataset,
179+
() -> new CompositeAggregationBuilder("name",
180+
Arrays.asList(
181+
new TermsValuesSourceBuilder("keyword").field("keyword"),
182+
new TermsValuesSourceBuilder("unmapped").field("unmapped")
183+
)
184+
),
185+
(result) -> {
186+
assertEquals(0, result.getBuckets().size());
187+
}
188+
);
189+
190+
testSearchCase(Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("keyword")), dataset,
191+
() -> new CompositeAggregationBuilder("name",
192+
Arrays.asList(
193+
new TermsValuesSourceBuilder("keyword").field("keyword"),
194+
new TermsValuesSourceBuilder("unmapped").field("unmapped").missingBucket(true)
195+
)
196+
),
197+
(result) -> {
198+
assertEquals(3, result.getBuckets().size());
199+
assertEquals("{keyword=d, unmapped=null}", result.afterKey().toString());
200+
assertEquals("{keyword=a, unmapped=null}", result.getBuckets().get(0).getKeyAsString());
201+
assertEquals(2L, result.getBuckets().get(0).getDocCount());
202+
assertEquals("{keyword=c, unmapped=null}", result.getBuckets().get(1).getKeyAsString());
203+
assertEquals(2L, result.getBuckets().get(1).getDocCount());
204+
assertEquals("{keyword=d, unmapped=null}", result.getBuckets().get(2).getKeyAsString());
205+
assertEquals(1L, result.getBuckets().get(2).getDocCount());
206+
}
207+
);
143208
}
144209

145210
public void testMissingBucket() throws Exception {

0 commit comments

Comments
 (0)