Skip to content

Commit 2dbaa65

Browse files
authored
Ensure that field collapsing works with field aliases. (#50722)
Previously, the following situation would throw an error: * A search contains a `collapse` on a particular field. * The search spans multiple indices, and in one index the field is mapped as a concrete field, but in another it is a field alias. The error occurs when we attempt to merge `CollapseTopFieldDocs` across shards. When merging, we validate that the name of the collapse field is the same across shards. But the name has already been resolved to the concrete field name, so it will be different on shards where the field was mapped as an alias vs. shards where it was a concrete field. This PR updates the collapse field name in `CollapseTopFieldDocs` to the original requested field, so that it will always be consistent across shards. Note that in #32648, we already made a fix around collapsing on field aliases. However, we didn't test this specific scenario where the field was mapped as an alias in only one of the indices being searched.
1 parent cdc0dbd commit 2dbaa65

File tree

5 files changed

+83
-43
lines changed

5 files changed

+83
-43
lines changed

rest-api-spec/src/main/resources/rest-api-spec/test/search/110_field_collapsing.yml

+31-13
Original file line numberDiff line numberDiff line change
@@ -347,30 +347,48 @@ setup:
347347

348348
---
349349
"field collapsing on a field alias":
350-
350+
- skip:
351+
version: " - 7.99.99"
352+
reason: the bug fix is not yet backported to 7.x
351353
- do:
352-
indices.put_mapping:
353-
index: test
354+
indices.create:
355+
index: alias-test
354356
body:
355-
properties:
356-
group_alias: { type: alias, path: numeric_group }
357+
mappings:
358+
properties:
359+
numeric_group: { type: alias, path: other_numeric_group }
360+
other_numeric_group: { type: integer }
361+
- do:
362+
index:
363+
index: alias-test
364+
id: 1
365+
body: { other_numeric_group: 1, sort: 6 }
366+
- do:
367+
index:
368+
index: alias-test
369+
id: 2
370+
body: { other_numeric_group: 25, sort: 10 }
371+
- do:
372+
indices.refresh:
373+
index: alias-test
374+
357375
- do:
358376
search:
359377
rest_total_hits_as_int: true
360-
index: test
378+
index: [alias-test, test]
361379
body:
362-
collapse: { field: group_alias, inner_hits: { name: sub_hits } }
380+
collapse: { field: numeric_group, inner_hits: { name: sub_hits } }
363381
sort: [{ sort: desc }]
364382

365-
- match: { hits.total: 6 }
383+
- match: { hits.total: 8 }
366384
- length: { hits.hits: 3 }
367385

368-
- match: { hits.hits.0.fields.group_alias: [3] }
386+
- match: { hits.hits.0.fields.numeric_group: [3] }
369387
- match: { hits.hits.0.inner_hits.sub_hits.hits.total: 1}
370-
- match: { hits.hits.1.fields.group_alias: [1] }
371-
- match: { hits.hits.1.inner_hits.sub_hits.hits.total: 3}
372-
- match: { hits.hits.2.fields.group_alias: [25] }
373-
- match: { hits.hits.2.inner_hits.sub_hits.hits.total: 2}
388+
- match: { hits.hits.1.fields.numeric_group: [1] }
389+
- match: { hits.hits.1.inner_hits.sub_hits.hits.total: 4}
390+
- match: { hits.hits.2.fields.numeric_group: [25] }
391+
- match: { hits.hits.2.inner_hits.sub_hits.hits.total: 3}
374392

375393
---
376394
"field collapsing, inner_hits and seq_no":

server/src/main/java/org/apache/lucene/search/grouping/CollapsingDocValuesSource.java

+5-4
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.apache.lucene.util.BytesRef;
3131
import org.elasticsearch.index.fielddata.AbstractNumericDocValues;
3232
import org.elasticsearch.index.fielddata.AbstractSortedDocValues;
33+
import org.elasticsearch.index.mapper.MappedFieldType;
3334

3435
import java.io.IOException;
3536
import java.util.Collection;
@@ -58,8 +59,8 @@ static class Numeric extends CollapsingDocValuesSource<Long> {
5859
private long value;
5960
private boolean hasValue;
6061

61-
Numeric(String field) {
62-
super(field);
62+
Numeric(MappedFieldType fieldType) {
63+
super(fieldType.name());
6364
}
6465

6566
@Override
@@ -148,8 +149,8 @@ static class Keyword extends CollapsingDocValuesSource<BytesRef> {
148149
private SortedDocValues values;
149150
private int ord;
150151

151-
Keyword(String field) {
152-
super(field);
152+
Keyword(MappedFieldType fieldType) {
153+
super(fieldType.name());
153154
}
154155

155156
@Override

server/src/main/java/org/apache/lucene/search/grouping/CollapsingTopDocsCollector.java

+22-17
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.apache.lucene.search.Sort;
2626
import org.apache.lucene.search.SortField;
2727
import org.apache.lucene.search.TotalHits;
28+
import org.elasticsearch.index.mapper.MappedFieldType;
2829

2930
import java.io.IOException;
3031
import java.util.Collection;
@@ -119,17 +120,19 @@ public void collect(int doc) throws IOException {
119120
* the collect will fail with an {@link IllegalStateException} if a document contains more than one value for the
120121
* field.
121122
*
122-
* @param collapseField The sort field used to group
123-
* documents.
124-
* @param sort The {@link Sort} used to sort the collapsed hits.
125-
* The collapsing keeps only the top sorted document per collapsed key.
126-
* This must be non-null, ie, if you want to groupSort by relevance
127-
* use Sort.RELEVANCE.
128-
* @param topN How many top groups to keep.
123+
* @param collapseField The sort field used to group documents.
124+
* @param collapseFieldType The {@link MappedFieldType} for this sort field.
125+
* @param sort The {@link Sort} used to sort the collapsed hits.
126+
* The collapsing keeps only the top sorted document per collapsed key.
127+
* This must be non-null, ie, if you want to groupSort by relevance
128+
* use Sort.RELEVANCE.
129+
* @param topN How many top groups to keep.
129130
*/
130-
public static CollapsingTopDocsCollector<?> createNumeric(String collapseField, Sort sort,
131+
public static CollapsingTopDocsCollector<?> createNumeric(String collapseField,
132+
MappedFieldType collapseFieldType,
133+
Sort sort,
131134
int topN) {
132-
return new CollapsingTopDocsCollector<>(new CollapsingDocValuesSource.Numeric(collapseField),
135+
return new CollapsingTopDocsCollector<>(new CollapsingDocValuesSource.Numeric(collapseFieldType),
133136
collapseField, sort, topN);
134137
}
135138

@@ -139,16 +142,18 @@ public static CollapsingTopDocsCollector<?> createNumeric(String collapseField,
139142
* the collect will fail with an {@link IllegalStateException} if a document contains more than one value for the
140143
* field.
141144
*
142-
* @param collapseField The sort field used to group
143-
* documents.
144-
* @param sort The {@link Sort} used to sort the collapsed hits. The collapsing keeps only the top sorted
145-
* document per collapsed key.
146-
* This must be non-null, ie, if you want to groupSort by relevance use Sort.RELEVANCE.
147-
* @param topN How many top groups to keep.
145+
* @param collapseField The sort field used to group documents.
146+
* @param collapseFieldType The {@link MappedFieldType} for this sort field.
147+
* @param sort The {@link Sort} used to sort the collapsed hits. The collapsing keeps only the top sorted
148+
* document per collapsed key.
149+
* This must be non-null, ie, if you want to groupSort by relevance use Sort.RELEVANCE.
150+
* @param topN How many top groups to keep.
148151
*/
149-
public static CollapsingTopDocsCollector<?> createKeyword(String collapseField, Sort sort,
152+
public static CollapsingTopDocsCollector<?> createKeyword(String collapseField,
153+
MappedFieldType collapseFieldType,
154+
Sort sort,
150155
int topN) {
151-
return new CollapsingTopDocsCollector<>(new CollapsingDocValuesSource.Keyword(collapseField),
156+
return new CollapsingTopDocsCollector<>(new CollapsingDocValuesSource.Keyword(collapseFieldType),
152157
collapseField, sort, topN);
153158
}
154159
}

server/src/main/java/org/elasticsearch/search/collapse/CollapseContext.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,11 @@ public List<InnerHitBuilder> getInnerHit() {
6262

6363
public CollapsingTopDocsCollector<?> createTopDocs(Sort sort, int topN) {
6464
if (fieldType instanceof KeywordFieldMapper.KeywordFieldType) {
65-
return CollapsingTopDocsCollector.createKeyword(fieldType.name(), sort, topN);
65+
return CollapsingTopDocsCollector.createKeyword(fieldName, fieldType, sort, topN);
6666
} else if (fieldType instanceof NumberFieldMapper.NumberFieldType) {
67-
return CollapsingTopDocsCollector.createNumeric(fieldType.name(), sort, topN);
67+
return CollapsingTopDocsCollector.createNumeric(fieldName, fieldType, sort, topN);
6868
} else {
69-
throw new IllegalStateException("unknown type for collapse field " + fieldType.name() +
69+
throw new IllegalStateException("unknown type for collapse field " + fieldName +
7070
", only keywords and numbers are accepted");
7171
}
7272
}

server/src/test/java/org/apache/lucene/grouping/CollapsingTopDocsCollectorTests.java

+22-6
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@
4848
import org.apache.lucene.store.Directory;
4949
import org.apache.lucene.util.BytesRef;
5050
import org.apache.lucene.util.NumericUtils;
51+
import org.elasticsearch.index.mapper.MappedFieldType;
52+
import org.elasticsearch.index.mapper.MockFieldMapper;
5153
import org.elasticsearch.test.ESTestCase;
5254

5355
import java.io.IOException;
@@ -108,6 +110,7 @@ private <T extends Comparable<T>> void assertSearchCollapse(CollapsingDocValuesP
108110
w.addDocument(doc);
109111
totalHits++;
110112
}
113+
111114
List<T> valueList = new ArrayList<>(values);
112115
Collections.sort(valueList);
113116
final IndexReader reader = w.getReader();
@@ -117,15 +120,18 @@ private <T extends Comparable<T>> void assertSearchCollapse(CollapsingDocValuesP
117120
final SortField sort2 = new SortField("sort2", SortField.Type.LONG);
118121
Sort sort = new Sort(sort1, sort2, collapseField);
119122

123+
MappedFieldType fieldType = new MockFieldMapper.FakeFieldType();
124+
fieldType.setName(collapseField.getField());
125+
120126
int expectedNumGroups = values.size();
121127

122128
final CollapsingTopDocsCollector<?> collapsingCollector;
123129
if (numeric) {
124130
collapsingCollector =
125-
CollapsingTopDocsCollector.createNumeric(collapseField.getField(), sort, expectedNumGroups);
131+
CollapsingTopDocsCollector.createNumeric(collapseField.getField(), fieldType, sort, expectedNumGroups);
126132
} else {
127133
collapsingCollector =
128-
CollapsingTopDocsCollector.createKeyword(collapseField.getField(), sort, expectedNumGroups);
134+
CollapsingTopDocsCollector.createKeyword(collapseField.getField(), fieldType, sort, expectedNumGroups);
129135
}
130136

131137
TopFieldCollector topFieldCollector =
@@ -195,9 +201,9 @@ private <T extends Comparable<T>> void assertSearchCollapse(CollapsingDocValuesP
195201
final SegmentSearcher subSearcher = subSearchers[shardIDX];
196202
final CollapsingTopDocsCollector<?> c;
197203
if (numeric) {
198-
c = CollapsingTopDocsCollector.createNumeric(collapseField.getField(), sort, expectedNumGroups);
204+
c = CollapsingTopDocsCollector.createNumeric(collapseField.getField(), fieldType, sort, expectedNumGroups);
199205
} else {
200-
c = CollapsingTopDocsCollector.createKeyword(collapseField.getField(), sort, expectedNumGroups);
206+
c = CollapsingTopDocsCollector.createKeyword(collapseField.getField(), fieldType, sort, expectedNumGroups);
201207
}
202208
subSearcher.search(weight, c);
203209
shardHits[shardIDX] = c.getTopDocs();
@@ -374,11 +380,16 @@ public void testEmptyNumericSegment() throws Exception {
374380
w.commit();
375381
final IndexReader reader = w.getReader();
376382
final IndexSearcher searcher = newSearcher(reader);
383+
384+
MappedFieldType fieldType = new MockFieldMapper.FakeFieldType();
385+
fieldType.setName("group");
386+
377387
SortField sortField = new SortField("group", SortField.Type.LONG);
378388
sortField.setMissingValue(Long.MAX_VALUE);
379389
Sort sort = new Sort(sortField);
390+
380391
final CollapsingTopDocsCollector<?> collapsingCollector =
381-
CollapsingTopDocsCollector.createNumeric("group", sort, 10);
392+
CollapsingTopDocsCollector.createNumeric("group", fieldType, sort, 10);
382393
searcher.search(new MatchAllDocsQuery(), collapsingCollector);
383394
CollapseTopFieldDocs collapseTopFieldDocs = collapsingCollector.getTopDocs();
384395
assertEquals(4, collapseTopFieldDocs.scoreDocs.length);
@@ -412,9 +423,14 @@ public void testEmptySortedSegment() throws Exception {
412423
w.commit();
413424
final IndexReader reader = w.getReader();
414425
final IndexSearcher searcher = newSearcher(reader);
426+
427+
MappedFieldType fieldType = new MockFieldMapper.FakeFieldType();
428+
fieldType.setName("group");
429+
415430
Sort sort = new Sort(new SortField("group", SortField.Type.STRING_VAL));
431+
416432
final CollapsingTopDocsCollector<?> collapsingCollector =
417-
CollapsingTopDocsCollector.createKeyword("group", sort, 10);
433+
CollapsingTopDocsCollector.createKeyword("group", fieldType, sort, 10);
418434
searcher.search(new MatchAllDocsQuery(), collapsingCollector);
419435
CollapseTopFieldDocs collapseTopFieldDocs = collapsingCollector.getTopDocs();
420436
assertEquals(4, collapseTopFieldDocs.scoreDocs.length);

0 commit comments

Comments
 (0)