honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-02-17 02:14:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix composite aggregation when after term is missing in the shard (#27936)

Browse Source 
This change fixes a bug when a keyword term in the `after` key is not present in the shard.
In this case the global ord of the document values are compared with the insertion point of the
`after` keyword and values that are equal to the insertion point should be considered "after" the top value.
This commit is contained in:
Jim Ferenczi 2017-12-26 09:58:49 +01:00 committed by GitHub
parent f6b9d3fd8f
commit 0b2c8c835e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 111 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
core/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSource.java
View File

@ -129,7 +129,8 @@ abstract class CompositeValuesSource<VS extends ValuesSource, T extends Comparab
private static class GlobalOrdinalValuesSource extends CompositeValuesSource<ValuesSource.Bytes.WithOrdinals, BytesRef> {
private final long[] values;
private SortedSetDocValues lookup;
private Long topValueLong;
private Long topValueGlobalOrd;
private boolean isTopValueInsertionPoint;
GlobalOrdinalValuesSource(ValuesSource.Bytes.WithOrdinals vs, int size, int reverseMul) {
super(vs, size, reverseMul);
@ -153,7 +154,14 @@ abstract class CompositeValuesSource<VS extends ValuesSource, T extends Comparab
@Override
int compareTop(int slot) {
return Long.compare(values[slot], topValueLong) * reverseMul;
int cmp = Long.compare(values[slot], topValueGlobalOrd);
if (cmp == 0 && isTopValueInsertionPoint) {
// the top value is missing in this shard, the comparison is against
// the insertion point of the top value so equality means that the value
// is "after" the insertion point.
return reverseMul;
}
return cmp * reverseMul;
}
@Override
@ -177,11 +185,12 @@ abstract class CompositeValuesSource<VS extends ValuesSource, T extends Comparab
final SortedSetDocValues dvs = vs.globalOrdinalsValues(context);
if (lookup == null) {
lookup = dvs;
if (topValue != null && topValueLong == null) {
topValueLong = lookup.lookupTerm(topValue);
if (topValueLong < 0) {
if (topValue != null && topValueGlobalOrd == null) {
topValueGlobalOrd = lookup.lookupTerm(topValue);
if (topValueGlobalOrd < 0) {
// convert negative insert position
topValueLong = -topValueLong - 2;
topValueGlobalOrd = -topValueGlobalOrd - 1;
isTopValueInsertionPoint = true;
}
}
}
@ -202,7 +211,6 @@ abstract class CompositeValuesSource<VS extends ValuesSource, T extends Comparab
*/
private static class BinaryValuesSource extends CompositeValuesSource<ValuesSource.Bytes, BytesRef> {
private final BytesRef[] values;
private BytesRef topValue;
BinaryValuesSource(ValuesSource.Bytes vs, int size, int reverseMul) {
super(vs, size, reverseMul);
@ -265,7 +273,6 @@ abstract class CompositeValuesSource<VS extends ValuesSource, T extends Comparab
*/
private static class LongValuesSource extends CompositeValuesSource<ValuesSource.Numeric, Long> {
private final long[] values;
private long topValue;
LongValuesSource(ValuesSource.Numeric vs, int size, int reverseMul) {
super(vs, size, reverseMul);
@ -326,7 +333,6 @@ abstract class CompositeValuesSource<VS extends ValuesSource, T extends Comparab
*/
private static class DoubleValuesSource extends CompositeValuesSource<ValuesSource.Numeric, Double> {
private final double[] values;
private double topValue;
DoubleValuesSource(ValuesSource.Numeric vs, int size, int reverseMul) {
super(vs, size, reverseMul);

66
core/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java
View File

@ -148,6 +148,72 @@ public class CompositeAggregatorTests extends AggregatorTestCase {
);
}
public void testWithKeywordMissingAfter() throws Exception {
final List<Map<String, List<Object>>> dataset = new ArrayList<>();
dataset.addAll(
Arrays.asList(
createDocument("keyword", "foo"),
createDocument("keyword", "bar"),
createDocument("keyword", "foo"),
createDocument("keyword", "zoo"),
createDocument("keyword", "bar"),
createDocument("keyword", "delta")
)
);
final Sort sort = new Sort(new SortedSetSortField("keyword", false));
testSearchCase(new MatchAllDocsQuery(), sort, dataset,
() -> {
TermsValuesSourceBuilder terms = new TermsValuesSourceBuilder("keyword")
.field("keyword");
return new CompositeAggregationBuilder("name", Collections.singletonList(terms));
}, (result) -> {
assertEquals(4, result.getBuckets().size());
assertEquals("{keyword=bar}", result.getBuckets().get(0).getKeyAsString());
assertEquals(2L, result.getBuckets().get(0).getDocCount());
assertEquals("{keyword=delta}", result.getBuckets().get(1).getKeyAsString());
assertEquals(1L, result.getBuckets().get(1).getDocCount());
assertEquals("{keyword=foo}", result.getBuckets().get(2).getKeyAsString());
assertEquals(2L, result.getBuckets().get(2).getDocCount());
assertEquals("{keyword=zoo}", result.getBuckets().get(3).getKeyAsString());
assertEquals(1L, result.getBuckets().get(3).getDocCount());
}
);
testSearchCase(new MatchAllDocsQuery(), sort, dataset,
() -> {
TermsValuesSourceBuilder terms = new TermsValuesSourceBuilder("keyword")
.field("keyword");
return new CompositeAggregationBuilder("name", Collections.singletonList(terms))
.aggregateAfter(Collections.singletonMap("keyword", "car"));
}, (result) -> {
assertEquals(3, result.getBuckets().size());
assertEquals("{keyword=delta}", result.getBuckets().get(0).getKeyAsString());
assertEquals(1L, result.getBuckets().get(0).getDocCount());
assertEquals("{keyword=foo}", result.getBuckets().get(1).getKeyAsString());
assertEquals(2L, result.getBuckets().get(1).getDocCount());
assertEquals("{keyword=zoo}", result.getBuckets().get(2).getKeyAsString());
assertEquals(1L, result.getBuckets().get(2).getDocCount());
}
);
testSearchCase(new MatchAllDocsQuery(), null, dataset,
() -> {
TermsValuesSourceBuilder terms = new TermsValuesSourceBuilder("keyword")
.field("keyword").order(SortOrder.DESC);
return new CompositeAggregationBuilder("name", Collections.singletonList(terms))
.aggregateAfter(Collections.singletonMap("keyword", "mar"));
}, (result) -> {
assertEquals(3, result.getBuckets().size());
assertEquals("{keyword=foo}", result.getBuckets().get(0).getKeyAsString());
assertEquals(2L, result.getBuckets().get(0).getDocCount());
assertEquals("{keyword=delta}", result.getBuckets().get(1).getKeyAsString());
assertEquals(1L, result.getBuckets().get(1).getDocCount());
assertEquals("{keyword=bar}", result.getBuckets().get(2).getKeyAsString());
assertEquals(2L, result.getBuckets().get(2).getDocCount());
}
);
}
public void testWithKeywordDesc() throws Exception {
final List<Map<String, List<Object>>> dataset = new ArrayList<>();
dataset.addAll(

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

@ -163,6 +163,36 @@ setup:
- match: { aggregations.test.buckets.1.key.kw: "bar" }
- match: { aggregations.test.buckets.1.doc_count: 1 }
---
"Aggregate After Missing":
- skip:
version: " - 6.99.99"
reason: bug fixed in 7.0.0
- do:
search:
index: test
body:
aggregations:
test:
composite:
sources: [
{
"kw": {
"terms": {
"field": "keyword"
}
}
}
]
after: { "kw": "delta" }
- match: {hits.total: 4}
- length: { aggregations.test.buckets: 1 }
- match: { aggregations.test.buckets.0.key.kw: "foo" }
- match: { aggregations.test.buckets.0.doc_count: 2 }
---
"Invalid Composite aggregation":
- skip: