Skip to content

Commit

Permalink
Return NaN for non-present fields of FieldStatsResult
Browse files Browse the repository at this point in the history
Before this change, an ExtendedStatsAggregation could include an
arbitrary number of fields that are null. Assigning them a non-boxed
field type leads to an NPE and a 500 is being returned to the caller
when the result of a extended field stats widget is requested.

This change properly assigns a valid value for those fields, so a result
(albeit possibly containing NaN for one or more fields) is being
returned to the caller.

Fixes #4026

(cherry picked from commit 882727e)
  • Loading branch information
dennisoelkers committed Aug 2, 2017
1 parent 3f61180 commit e30b718
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 10 deletions.
Expand Up @@ -44,16 +44,16 @@ public FieldStatsResult(ValueCountAggregation valueCountAggregation,
long tookMs) {
super(query, source, tookMs);
this.count = getValueCount(valueCountAggregation, extendedStatsAggregation);
this.cardinality = cardinalityAggregation == null ? Long.MIN_VALUE : cardinalityAggregation.getCardinality();
this.cardinality = cardinalityAggregation == null || cardinalityAggregation.getCardinality() == null ? Long.MIN_VALUE : cardinalityAggregation.getCardinality();

if (extendedStatsAggregation != null) {
sum = extendedStatsAggregation.getSum();
sumOfSquares = extendedStatsAggregation.getSumOfSquares();
mean = extendedStatsAggregation.getAvg();
min = extendedStatsAggregation.getMin();
max = extendedStatsAggregation.getMax();
variance = extendedStatsAggregation.getVariance();
stdDeviation = extendedStatsAggregation.getStdDeviation();
sum = extendedStatsAggregation.getSum() != null ? extendedStatsAggregation.getSum() : Double.NaN;
sumOfSquares = extendedStatsAggregation.getSumOfSquares() != null ? extendedStatsAggregation.getSumOfSquares() : Double.NaN;
mean = extendedStatsAggregation.getAvg() != null ? extendedStatsAggregation.getAvg() : Double.NaN;
min = extendedStatsAggregation.getMin() != null ? extendedStatsAggregation.getMin() : Double.NaN;
max = extendedStatsAggregation.getMax() != null ? extendedStatsAggregation.getMax() : Double.NaN;
variance = extendedStatsAggregation.getVariance() != null ? extendedStatsAggregation.getVariance() : Double.NaN;
stdDeviation = extendedStatsAggregation.getStdDeviation() != null ? extendedStatsAggregation.getStdDeviation() : Double.NaN;
} else {
sum = Double.NaN;
sumOfSquares = Double.NaN;
Expand Down Expand Up @@ -82,9 +82,9 @@ private FieldStatsResult(String query, String bytesReference) {
}

private long getValueCount(ValueCountAggregation valueCountAggregation, ExtendedStatsAggregation extendedStatsAggregation) {
if (valueCountAggregation != null) {
if (valueCountAggregation != null && valueCountAggregation.getValueCount() != null) {
return valueCountAggregation.getValueCount();
} else if (extendedStatsAggregation != null) {
} else if (extendedStatsAggregation != null && extendedStatsAggregation.getCount() != null) {
return extendedStatsAggregation.getCount();
}
return Long.MIN_VALUE;
Expand Down
@@ -0,0 +1,62 @@
/**
* This file is part of Graylog.
*
* Graylog is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Graylog is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with Graylog. If not, see <http://www.gnu.org/licenses/>.
*/
package org.graylog2.indexer.results;

import io.searchbox.core.search.aggregation.ExtendedStatsAggregation;
import org.junit.Test;

import java.util.Collections;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class FieldStatsResultTest {
@Test
public void worksForNullFieldsInAggregationResults() throws Exception {
final ExtendedStatsAggregation extendedStatsAggregation = mock(ExtendedStatsAggregation.class);

when(extendedStatsAggregation.getCount()).thenReturn(null);
when(extendedStatsAggregation.getSum()).thenReturn(null);
when(extendedStatsAggregation.getSumOfSquares()).thenReturn(null);
when(extendedStatsAggregation.getAvg()).thenReturn(null);
when(extendedStatsAggregation.getMin()).thenReturn(null);
when(extendedStatsAggregation.getMax()).thenReturn(null);
when(extendedStatsAggregation.getVariance()).thenReturn(null);
when(extendedStatsAggregation.getStdDeviation()).thenReturn(null);

final FieldStatsResult result = new FieldStatsResult(null,
extendedStatsAggregation,
null,
Collections.emptyList(),
null,
null,
0);

assertThat(result).isNotNull();
assertThat(result.getSum()).isEqualTo(Double.NaN);
assertThat(result.getSumOfSquares()).isEqualTo(Double.NaN);
assertThat(result.getMean()).isEqualTo(Double.NaN);
assertThat(result.getMin()).isEqualTo(Double.NaN);
assertThat(result.getMax()).isEqualTo(Double.NaN);
assertThat(result.getVariance()).isEqualTo(Double.NaN);
assertThat(result.getStdDeviation()).isEqualTo(Double.NaN);

assertThat(result.getCount()).isEqualTo(Long.MIN_VALUE);
assertThat(result.getCardinality()).isEqualTo(Long.MIN_VALUE);
}
}

0 comments on commit e30b718

Please sign in to comment.