Ensure that field collapsing works with field aliases. (#50766)

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.
This commit is contained in:
Julie Tibshirani 2020-01-08 14:51:15 -08:00 committed by GitHub
parent b1b4282273
commit a299aba2f8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 82 additions and 45 deletions

View File

@ -372,32 +372,47 @@ setup:
---
"field collapsing on a field alias":
- skip:
version: " - 6.3.99"
reason: Field aliases were introduced in 6.4.0.
version: " - 7.5.99"
reason: the bug fix is not yet backported to 7.5
- do:
indices.put_mapping:
include_type_name: false
index: test
indices.create:
index: alias-test
body:
mappings:
properties:
group_alias: { type: alias, path: numeric_group }
numeric_group: { type: alias, path: other_numeric_group }
other_numeric_group: { type: integer }
- do:
index:
index: alias-test
id: 1
body: { other_numeric_group: 1, sort: 6 }
- do:
index:
index: alias-test
id: 2
body: { other_numeric_group: 25, sort: 10 }
- do:
indices.refresh:
index: alias-test
- do:
search:
rest_total_hits_as_int: true
index: test
index: [alias-test, test]
body:
collapse: { field: group_alias, inner_hits: { name: sub_hits } }
collapse: { field: numeric_group, inner_hits: { name: sub_hits } }
sort: [{ sort: desc }]
- match: { hits.total: 6 }
- match: { hits.total: 8 }
- length: { hits.hits: 3 }
- match: { hits.hits.0.fields.group_alias: [3] }
- match: { hits.hits.0.fields.numeric_group: [3] }
- match: { hits.hits.0.inner_hits.sub_hits.hits.total: 1}
- match: { hits.hits.1.fields.group_alias: [1] }
- match: { hits.hits.1.inner_hits.sub_hits.hits.total: 3}
- match: { hits.hits.2.fields.group_alias: [25] }
- match: { hits.hits.2.inner_hits.sub_hits.hits.total: 2}
- match: { hits.hits.1.fields.numeric_group: [1] }
- match: { hits.hits.1.inner_hits.sub_hits.hits.total: 4}
- match: { hits.hits.2.fields.numeric_group: [25] }
- match: { hits.hits.2.inner_hits.sub_hits.hits.total: 3}
---
"field collapsing, inner_hits and seq_no":

View File

@ -30,6 +30,7 @@ import org.apache.lucene.index.SortedSetDocValues;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.index.fielddata.AbstractNumericDocValues;
import org.elasticsearch.index.fielddata.AbstractSortedDocValues;
import org.elasticsearch.index.mapper.MappedFieldType;
import java.io.IOException;
import java.util.Collection;
@ -58,8 +59,8 @@ abstract class CollapsingDocValuesSource<T> extends GroupSelector<T> {
private long value;
private boolean hasValue;
Numeric(String field) {
super(field);
Numeric(MappedFieldType fieldType) {
super(fieldType.name());
}
@Override
@ -148,8 +149,8 @@ abstract class CollapsingDocValuesSource<T> extends GroupSelector<T> {
private SortedDocValues values;
private int ord;
Keyword(String field) {
super(field);
Keyword(MappedFieldType fieldType) {
super(fieldType.name());
}
@Override

View File

@ -25,6 +25,7 @@ import org.apache.lucene.search.ScoreMode;
import org.apache.lucene.search.Sort;
import org.apache.lucene.search.SortField;
import org.apache.lucene.search.TotalHits;
import org.elasticsearch.index.mapper.MappedFieldType;
import java.io.IOException;
import java.util.Collection;
@ -119,17 +120,19 @@ public final class CollapsingTopDocsCollector<T> extends FirstPassGroupingCollec
* the collect will fail with an {@link IllegalStateException} if a document contains more than one value for the
* field.
*
* @param collapseField The sort field used to group
* documents.
* @param collapseField The sort field used to group documents.
* @param collapseFieldType The {@link MappedFieldType} for this sort field.
* @param sort The {@link Sort} used to sort the collapsed hits.
* The collapsing keeps only the top sorted document per collapsed key.
* This must be non-null, ie, if you want to groupSort by relevance
* use Sort.RELEVANCE.
* @param topN How many top groups to keep.
*/
public static CollapsingTopDocsCollector<?> createNumeric(String collapseField, Sort sort,
public static CollapsingTopDocsCollector<?> createNumeric(String collapseField,
MappedFieldType collapseFieldType,
Sort sort,
int topN) {
return new CollapsingTopDocsCollector<>(new CollapsingDocValuesSource.Numeric(collapseField),
return new CollapsingTopDocsCollector<>(new CollapsingDocValuesSource.Numeric(collapseFieldType),
collapseField, sort, topN);
}
@ -139,16 +142,18 @@ public final class CollapsingTopDocsCollector<T> extends FirstPassGroupingCollec
* the collect will fail with an {@link IllegalStateException} if a document contains more than one value for the
* field.
*
* @param collapseField The sort field used to group
* documents.
* @param collapseField The sort field used to group documents.
* @param collapseFieldType The {@link MappedFieldType} for this sort field.
* @param sort The {@link Sort} used to sort the collapsed hits. The collapsing keeps only the top sorted
* document per collapsed key.
* This must be non-null, ie, if you want to groupSort by relevance use Sort.RELEVANCE.
* @param topN How many top groups to keep.
*/
public static CollapsingTopDocsCollector<?> createKeyword(String collapseField, Sort sort,
public static CollapsingTopDocsCollector<?> createKeyword(String collapseField,
MappedFieldType collapseFieldType,
Sort sort,
int topN) {
return new CollapsingTopDocsCollector<>(new CollapsingDocValuesSource.Keyword(collapseField),
return new CollapsingTopDocsCollector<>(new CollapsingDocValuesSource.Keyword(collapseFieldType),
collapseField, sort, topN);
}
}

View File

@ -62,11 +62,11 @@ public class CollapseContext {
public CollapsingTopDocsCollector<?> createTopDocs(Sort sort, int topN) {
if (fieldType instanceof KeywordFieldMapper.KeywordFieldType) {
return CollapsingTopDocsCollector.createKeyword(fieldType.name(), sort, topN);
return CollapsingTopDocsCollector.createKeyword(fieldName, fieldType, sort, topN);
} else if (fieldType instanceof NumberFieldMapper.NumberFieldType) {
return CollapsingTopDocsCollector.createNumeric(fieldType.name(), sort, topN);
return CollapsingTopDocsCollector.createNumeric(fieldName, fieldType, sort, topN);
} else {
throw new IllegalStateException("unknown type for collapse field " + fieldType.name() +
throw new IllegalStateException("unknown type for collapse field " + fieldName +
", only keywords and numbers are accepted");
}
}

View File

@ -48,6 +48,8 @@ import org.apache.lucene.search.grouping.CollapsingTopDocsCollector;
import org.apache.lucene.store.Directory;
import org.apache.lucene.util.BytesRef;
import org.apache.lucene.util.NumericUtils;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.MockFieldMapper;
import org.elasticsearch.test.ESTestCase;
import java.io.IOException;
@ -108,6 +110,7 @@ public class CollapsingTopDocsCollectorTests extends ESTestCase {
w.addDocument(doc);
totalHits++;
}
List<T> valueList = new ArrayList<>(values);
Collections.sort(valueList);
final IndexReader reader = w.getReader();
@ -117,15 +120,18 @@ public class CollapsingTopDocsCollectorTests extends ESTestCase {
final SortField sort2 = new SortField("sort2", SortField.Type.LONG);
Sort sort = new Sort(sort1, sort2, collapseField);
MappedFieldType fieldType = new MockFieldMapper.FakeFieldType();
fieldType.setName(collapseField.getField());
int expectedNumGroups = values.size();
final CollapsingTopDocsCollector<?> collapsingCollector;
if (numeric) {
collapsingCollector =
CollapsingTopDocsCollector.createNumeric(collapseField.getField(), sort, expectedNumGroups);
CollapsingTopDocsCollector.createNumeric(collapseField.getField(), fieldType, sort, expectedNumGroups);
} else {
collapsingCollector =
CollapsingTopDocsCollector.createKeyword(collapseField.getField(), sort, expectedNumGroups);
CollapsingTopDocsCollector.createKeyword(collapseField.getField(), fieldType, sort, expectedNumGroups);
}
TopFieldCollector topFieldCollector =
@ -195,9 +201,9 @@ public class CollapsingTopDocsCollectorTests extends ESTestCase {
final SegmentSearcher subSearcher = subSearchers[shardIDX];
final CollapsingTopDocsCollector<?> c;
if (numeric) {
c = CollapsingTopDocsCollector.createNumeric(collapseField.getField(), sort, expectedNumGroups);
c = CollapsingTopDocsCollector.createNumeric(collapseField.getField(), fieldType, sort, expectedNumGroups);
} else {
c = CollapsingTopDocsCollector.createKeyword(collapseField.getField(), sort, expectedNumGroups);
c = CollapsingTopDocsCollector.createKeyword(collapseField.getField(), fieldType, sort, expectedNumGroups);
}
subSearcher.search(weight, c);
shardHits[shardIDX] = c.getTopDocs();
@ -374,11 +380,16 @@ public class CollapsingTopDocsCollectorTests extends ESTestCase {
w.commit();
final IndexReader reader = w.getReader();
final IndexSearcher searcher = newSearcher(reader);
MappedFieldType fieldType = new MockFieldMapper.FakeFieldType();
fieldType.setName("group");
SortField sortField = new SortField("group", SortField.Type.LONG);
sortField.setMissingValue(Long.MAX_VALUE);
Sort sort = new Sort(sortField);
final CollapsingTopDocsCollector<?> collapsingCollector =
CollapsingTopDocsCollector.createNumeric("group", sort, 10);
CollapsingTopDocsCollector.createNumeric("group", fieldType, sort, 10);
searcher.search(new MatchAllDocsQuery(), collapsingCollector);
CollapseTopFieldDocs collapseTopFieldDocs = collapsingCollector.getTopDocs();
assertEquals(4, collapseTopFieldDocs.scoreDocs.length);
@ -412,9 +423,14 @@ public class CollapsingTopDocsCollectorTests extends ESTestCase {
w.commit();
final IndexReader reader = w.getReader();
final IndexSearcher searcher = newSearcher(reader);
MappedFieldType fieldType = new MockFieldMapper.FakeFieldType();
fieldType.setName("group");
Sort sort = new Sort(new SortField("group", SortField.Type.STRING_VAL));
final CollapsingTopDocsCollector<?> collapsingCollector =
CollapsingTopDocsCollector.createKeyword("group", sort, 10);
CollapsingTopDocsCollector.createKeyword("group", fieldType, sort, 10);
searcher.search(new MatchAllDocsQuery(), collapsingCollector);
CollapseTopFieldDocs collapseTopFieldDocs = collapsingCollector.getTopDocs();
assertEquals(4, collapseTopFieldDocs.scoreDocs.length);