fix AggregatorFactory.finalizeComputation implementations to be ok with null inputs (#7731)

* AggregatorFactory finalizeComputation is nullable with nullable input, make implementations honor this

* fixes
This commit is contained in:
Clint Wylie 2019-05-22 21:13:09 -07:00 committed by Fangjin Yang
parent 1b284ca847
commit ffc2397bcd
21 changed files with 69 additions and 31 deletions

View File

@ -35,6 +35,7 @@ import org.apache.druid.query.dimension.DefaultDimensionSpec;
import org.apache.druid.segment.ColumnSelectorFactory; import org.apache.druid.segment.ColumnSelectorFactory;
import org.apache.druid.segment.DimensionSelector; import org.apache.druid.segment.DimensionSelector;
import javax.annotation.Nullable;
import java.nio.ByteBuffer; import java.nio.ByteBuffer;
import java.util.Collections; import java.util.Collections;
import java.util.Comparator; import java.util.Comparator;
@ -147,8 +148,9 @@ public class DistinctCountAggregatorFactory extends AggregatorFactory
return object; return object;
} }
@Nullable
@Override @Override
public Object finalizeComputation(Object object) public Object finalizeComputation(@Nullable Object object)
{ {
return object; return object;
} }

View File

@ -206,8 +206,9 @@ public class MomentSketchAggregatorFactory extends AggregatorFactory
); );
} }
@Nullable
@Override @Override
public Object finalizeComputation(Object object) public Object finalizeComputation(@Nullable Object object)
{ {
return object; return object;
} }

View File

@ -25,6 +25,7 @@ import org.apache.druid.query.aggregation.BufferAggregator;
import org.apache.druid.query.movingaverage.averagers.AveragerFactory; import org.apache.druid.query.movingaverage.averagers.AveragerFactory;
import org.apache.druid.segment.ColumnSelectorFactory; import org.apache.druid.segment.ColumnSelectorFactory;
import javax.annotation.Nullable;
import java.util.Comparator; import java.util.Comparator;
import java.util.List; import java.util.List;
@ -125,8 +126,9 @@ public class AveragerFactoryWrapper<T, R> extends AggregatorFactory
* Not implemented. Throws UnsupportedOperationException. * Not implemented. Throws UnsupportedOperationException.
*/ */
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
@Nullable
@Override @Override
public Object finalizeComputation(Object object) public Object finalizeComputation(@Nullable Object object)
{ {
return af.finalizeComputation((T) object); return af.finalizeComputation((T) object);
} }

View File

@ -190,8 +190,9 @@ public class TDigestBuildSketchAggregatorFactory extends AggregatorFactory
return TDigestSketchUtils.deserialize(serializedSketch); return TDigestSketchUtils.deserialize(serializedSketch);
} }
@Nullable
@Override @Override
public Object finalizeComputation(Object object) public Object finalizeComputation(@Nullable Object object)
{ {
return object; return object;
} }

View File

@ -27,6 +27,7 @@ import org.apache.druid.segment.ColumnSelectorFactory;
import org.apache.druid.segment.ColumnValueSelector; import org.apache.druid.segment.ColumnValueSelector;
import org.joda.time.DateTime; import org.joda.time.DateTime;
import javax.annotation.Nullable;
import java.nio.ByteBuffer; import java.nio.ByteBuffer;
import java.sql.Timestamp; import java.sql.Timestamp;
import java.util.Collections; import java.util.Collections;
@ -157,10 +158,11 @@ public class TimestampAggregatorFactory extends AggregatorFactory
return object; return object;
} }
@Nullable
@Override @Override
public Object finalizeComputation(Object object) public Object finalizeComputation(@Nullable Object object)
{ {
return DateTimes.utc((long) object); return object == null ? null : DateTimes.utc((long) object);
} }
@Override @Override

View File

@ -157,9 +157,13 @@ public abstract class HllSketchAggregatorFactory extends AggregatorFactory
}; };
} }
@Nullable
@Override @Override
public Double finalizeComputation(final Object object) public Double finalizeComputation(@Nullable final Object object)
{ {
if (object == null) {
return null;
}
final HllSketch sketch = (HllSketch) object; final HllSketch sketch = (HllSketch) object;
return sketch.getEstimate(); return sketch.getEstimate();
} }

View File

@ -240,10 +240,11 @@ public class DoublesSketchAggregatorFactory extends AggregatorFactory
} }
} }
@Nullable
@Override @Override
public Object finalizeComputation(final Object object) public Object finalizeComputation(@Nullable final Object object)
{ {
return ((DoublesSketch) object).getN(); return object == null ? null : ((DoublesSketch) object).getN();
} }
@Override @Override

View File

@ -117,9 +117,14 @@ public class SketchMergeAggregatorFactory extends SketchAggregatorFactory
* *
* @return sketch object * @return sketch object
*/ */
@Nullable
@Override @Override
public Object finalizeComputation(Object object) public Object finalizeComputation(@Nullable Object object)
{ {
if (object == null) {
return null;
}
if (shouldFinalize) { if (shouldFinalize) {
SketchHolder holder = (SketchHolder) object; SketchHolder holder = (SketchHolder) object;
if (errorBoundsStdDev != null) { if (errorBoundsStdDev != null) {

View File

@ -285,10 +285,11 @@ public class ArrayOfDoublesSketchAggregatorFactory extends AggregatorFactory
return new ArrayOfDoublesSketchAggregatorFactory(name, name, nominalEntries, null, numberOfValues); return new ArrayOfDoublesSketchAggregatorFactory(name, name, nominalEntries, null, numberOfValues);
} }
@Nullable
@Override @Override
public Object finalizeComputation(final Object object) public Object finalizeComputation(@Nullable final Object object)
{ {
return ((ArrayOfDoublesSketch) object).getEstimate(); return object == null ? null : ((ArrayOfDoublesSketch) object).getEstimate();
} }
@Override @Override

View File

@ -227,8 +227,9 @@ public class BloomFilterAggregatorFactory extends AggregatorFactory
} }
} }
@Nullable
@Override @Override
public Object finalizeComputation(Object object) public Object finalizeComputation(@Nullable Object object)
{ {
return object; return object;
} }

View File

@ -214,10 +214,11 @@ public class ApproximateHistogramAggregatorFactory extends AggregatorFactory
} }
} }
@Nullable
@Override @Override
public Object finalizeComputation(Object object) public Object finalizeComputation(@Nullable Object object)
{ {
return ((ApproximateHistogram) object).toHistogram(numBuckets); return object == null ? null : ((ApproximateHistogram) object).toHistogram(numBuckets);
} }
@JsonProperty @JsonProperty

View File

@ -39,6 +39,7 @@ import org.apache.druid.segment.ColumnSelectorFactory;
import org.apache.druid.segment.ColumnValueSelector; import org.apache.druid.segment.ColumnValueSelector;
import org.apache.druid.segment.NilColumnValueSelector; import org.apache.druid.segment.NilColumnValueSelector;
import javax.annotation.Nullable;
import java.nio.ByteBuffer; import java.nio.ByteBuffer;
import java.util.Collections; import java.util.Collections;
import java.util.Comparator; import java.util.Comparator;
@ -202,10 +203,11 @@ public class VarianceAggregatorFactory extends AggregatorFactory
return VarianceAggregatorCollector.COMPARATOR; return VarianceAggregatorCollector.COMPARATOR;
} }
@Nullable
@Override @Override
public Object finalizeComputation(Object object) public Object finalizeComputation(@Nullable Object object)
{ {
return ((VarianceAggregatorCollector) object).getVariance(isVariancePop); return object == null ? null : ((VarianceAggregatorCollector) object).getVariance(isVariancePop);
} }
@Override @Override

View File

@ -25,6 +25,7 @@ import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import org.apache.druid.segment.ColumnSelectorFactory; import org.apache.druid.segment.ColumnSelectorFactory;
import javax.annotation.Nullable;
import java.util.Collections; import java.util.Collections;
import java.util.Comparator; import java.util.Comparator;
import java.util.List; import java.util.List;
@ -93,8 +94,9 @@ public class CountAggregatorFactory extends AggregatorFactory
return object; return object;
} }
@Nullable
@Override @Override
public Object finalizeComputation(Object object) public Object finalizeComputation(@Nullable Object object)
{ {
return object; return object;
} }

View File

@ -32,6 +32,7 @@ import org.apache.druid.segment.column.ColumnHolder;
import org.apache.druid.segment.filter.Filters; import org.apache.druid.segment.filter.Filters;
import org.joda.time.Interval; import org.joda.time.Interval;
import javax.annotation.Nullable;
import java.nio.ByteBuffer; import java.nio.ByteBuffer;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Comparator; import java.util.Comparator;
@ -118,8 +119,9 @@ public class FilteredAggregatorFactory extends AggregatorFactory
return delegate.deserialize(object); return delegate.deserialize(object);
} }
@Nullable
@Override @Override
public Object finalizeComputation(Object object) public Object finalizeComputation(@Nullable Object object)
{ {
return delegate.finalizeComputation(object); return delegate.finalizeComputation(object);
} }

View File

@ -154,10 +154,11 @@ public class HistogramAggregatorFactory extends AggregatorFactory
return object; return object;
} }
@Nullable
@Override @Override
public Object finalizeComputation(Object object) public Object finalizeComputation(@Nullable Object object)
{ {
return ((Histogram) object).asVisual(); return object == null ? null : ((Histogram) object).asVisual();
} }
@Override @Override

View File

@ -35,6 +35,7 @@ import org.apache.druid.segment.ColumnSelectorFactory;
import org.apache.druid.segment.ColumnValueSelector; import org.apache.druid.segment.ColumnValueSelector;
import org.checkerframework.checker.nullness.qual.EnsuresNonNull; import org.checkerframework.checker.nullness.qual.EnsuresNonNull;
import org.checkerframework.checker.nullness.qual.MonotonicNonNull; import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.mozilla.javascript.Context; import org.mozilla.javascript.Context;
import org.mozilla.javascript.ContextAction; import org.mozilla.javascript.ContextAction;
import org.mozilla.javascript.ContextFactory; import org.mozilla.javascript.ContextFactory;
@ -202,8 +203,9 @@ public class JavaScriptAggregatorFactory extends AggregatorFactory
return object; return object;
} }
@Nullable
@Override @Override
public Object finalizeComputation(Object object) public Object finalizeComputation(@Nullable Object object)
{ {
return object; return object;
} }

View File

@ -104,8 +104,9 @@ public class SuppressedAggregatorFactory extends AggregatorFactory
return delegate.deserialize(object); return delegate.deserialize(object);
} }
@Nullable
@Override @Override
public Object finalizeComputation(Object object) public Object finalizeComputation(@Nullable Object object)
{ {
return delegate.finalizeComputation(object); return delegate.finalizeComputation(object);
} }

View File

@ -44,6 +44,7 @@ import org.apache.druid.query.dimension.DimensionSpec;
import org.apache.druid.segment.ColumnSelectorFactory; import org.apache.druid.segment.ColumnSelectorFactory;
import org.apache.druid.segment.DimensionHandlerUtils; import org.apache.druid.segment.DimensionHandlerUtils;
import javax.annotation.Nullable;
import java.nio.ByteBuffer; import java.nio.ByteBuffer;
import java.util.Collections; import java.util.Collections;
import java.util.Comparator; import java.util.Comparator;
@ -234,9 +235,9 @@ public class CardinalityAggregatorFactory extends AggregatorFactory
return HyperLogLogCollector.makeCollector(buffer); return HyperLogLogCollector.makeCollector(buffer);
} }
@Nullable
@Override @Override
public Object finalizeComputation(@Nullable Object object)
public Object finalizeComputation(Object object)
{ {
return HyperUniquesAggregatorFactory.estimateCardinality(object, round); return HyperUniquesAggregatorFactory.estimateCardinality(object, round);
} }

View File

@ -36,6 +36,7 @@ import org.apache.druid.segment.BaseObjectColumnValueSelector;
import org.apache.druid.segment.ColumnSelectorFactory; import org.apache.druid.segment.ColumnSelectorFactory;
import org.apache.druid.segment.column.ColumnHolder; import org.apache.druid.segment.column.ColumnHolder;
import javax.annotation.Nullable;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import java.util.Comparator; import java.util.Comparator;
@ -171,10 +172,11 @@ public class StringFirstAggregatorFactory extends NullableAggregatorFactory<Base
return new SerializablePairLongString(((Number) map.get("lhs")).longValue(), ((String) map.get("rhs"))); return new SerializablePairLongString(((Number) map.get("lhs")).longValue(), ((String) map.get("rhs")));
} }
@Nullable
@Override @Override
public Object finalizeComputation(Object object) public Object finalizeComputation(@Nullable Object object)
{ {
return ((SerializablePairLongString) object).rhs; return object == null ? null : ((SerializablePairLongString) object).rhs;
} }
@Override @Override

View File

@ -39,6 +39,7 @@ import org.apache.druid.segment.BaseObjectColumnValueSelector;
import org.apache.druid.segment.ColumnSelectorFactory; import org.apache.druid.segment.ColumnSelectorFactory;
import org.apache.druid.segment.NilColumnValueSelector; import org.apache.druid.segment.NilColumnValueSelector;
import javax.annotation.Nullable;
import java.nio.ByteBuffer; import java.nio.ByteBuffer;
import java.util.Collections; import java.util.Collections;
import java.util.Comparator; import java.util.Comparator;
@ -49,7 +50,7 @@ import java.util.Objects;
*/ */
public class HyperUniquesAggregatorFactory extends AggregatorFactory public class HyperUniquesAggregatorFactory extends AggregatorFactory
{ {
public static Object estimateCardinality(Object object, boolean round) public static Object estimateCardinality(@Nullable Object object, boolean round)
{ {
if (object == null) { if (object == null) {
return 0; return 0;
@ -192,8 +193,9 @@ public class HyperUniquesAggregatorFactory extends AggregatorFactory
return HyperLogLogCollector.makeCollector(buffer); return HyperLogLogCollector.makeCollector(buffer);
} }
@Nullable
@Override @Override
public Object finalizeComputation(Object object) public Object finalizeComputation(@Nullable Object object)
{ {
return estimateCardinality(object, round); return estimateCardinality(object, round);
} }

View File

@ -36,6 +36,7 @@ import org.apache.druid.segment.BaseObjectColumnValueSelector;
import org.apache.druid.segment.ColumnSelectorFactory; import org.apache.druid.segment.ColumnSelectorFactory;
import org.apache.druid.segment.column.ColumnHolder; import org.apache.druid.segment.column.ColumnHolder;
import javax.annotation.Nullable;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import java.util.Comparator; import java.util.Comparator;
@ -129,10 +130,11 @@ public class StringLastAggregatorFactory extends NullableAggregatorFactory<BaseO
return new SerializablePairLongString(((Number) map.get("lhs")).longValue(), ((String) map.get("rhs"))); return new SerializablePairLongString(((Number) map.get("lhs")).longValue(), ((String) map.get("rhs")));
} }
@Nullable
@Override @Override
public Object finalizeComputation(Object object) public Object finalizeComputation(@Nullable Object object)
{ {
return ((SerializablePairLongString) object).rhs; return object == null ? null : ((SerializablePairLongString) object).rhs;
} }
@Override @Override