Fix lenient merging of conflicting aggregators. (#3113)

This should have marked the conflicting aggregator as null, but instead it
threw an NPE for the entire query.
This commit is contained in:
Gian Merlino 2016-06-08 15:56:48 -07:00 committed by Charles Allen
parent 37c8a8f186
commit 5998de7d5b
2 changed files with 14 additions and 3 deletions

View File

@ -295,8 +295,10 @@ public class SegmentMetadataQueryQueryToolChest extends QueryToolChest<SegmentAn
// Merge each aggregator individually, ignoring nulls
for (SegmentAnalysis analysis : ImmutableList.of(arg1, arg2)) {
if (analysis.getAggregators() != null) {
for (AggregatorFactory aggregator : analysis.getAggregators().values()) {
AggregatorFactory merged = aggregators.get(aggregator.getName());
for (Map.Entry<String, AggregatorFactory> entry : analysis.getAggregators().entrySet()) {
final String aggregatorName = entry.getKey();
final AggregatorFactory aggregator = entry.getValue();
AggregatorFactory merged = aggregators.get(aggregatorName);
if (merged != null) {
try {
merged = merged.getMergingFactory(aggregator);
@ -307,7 +309,7 @@ public class SegmentMetadataQueryQueryToolChest extends QueryToolChest<SegmentAn
} else {
merged = aggregator;
}
aggregators.put(aggregator.getName(), merged);
aggregators.put(aggregatorName, merged);
}
}
}

View File

@ -244,6 +244,15 @@ public class SegmentMetadataQueryQueryToolChestTest
expectedLenient.put("baz", new LongMaxAggregatorFactory("baz", "baz"));
Assert.assertNull(mergeStrict(analysis1, analysis2).getAggregators());
Assert.assertEquals(expectedLenient, mergeLenient(analysis1, analysis2).getAggregators());
// Simulate multi-level merge
Assert.assertEquals(
expectedLenient,
mergeLenient(
mergeLenient(analysis1, analysis2),
mergeLenient(analysis1, analysis2)
).getAggregators()
);
}
private static SegmentAnalysis mergeStrict(SegmentAnalysis analysis1, SegmentAnalysis analysis2)