Add null checks for HllSketchHolder (#15502)

Fixes a potential NPE which could occur while folding the HllSketchAggregator. If the sketch is null, druid could return a null HllSketchHolder object. Adding a null check here could help here

Resolves a null pointer exception in HllSketchAggregatorFactory
This commit is contained in:
Adarsh Sanjeev 2023-12-08 11:43:04 +05:30 committed by GitHub
parent 935aa187a0
commit 254a8eb7e0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 25 additions and 2 deletions

View File

@ -172,8 +172,12 @@ public abstract class HllSketchAggregatorFactory extends AggregatorFactory
@Override @Override
public void fold(final ColumnValueSelector selector) public void fold(final ColumnValueSelector selector)
{ {
final HllSketchHolder sketch = (HllSketchHolder) selector.getObject(); final HllSketchHolder sketchHolder = (HllSketchHolder) selector.getObject();
union.update(sketch.getSketch()); // sketchHolder can be null here, if the sketch is empty. This is an optimisation done by
// HllSketchHolderObjectStrategy. If the holder is null, this should be a no-op.
if (sketchHolder != null) {
union.update(sketchHolder.getSketch());
}
} }
@Nullable @Nullable

View File

@ -45,6 +45,7 @@ public class HllSketchHolderObjectStrategy implements ObjectStrategy<HllSketchHo
return HllSketchAggregatorFactory.COMPARATOR.compare(sketch1, sketch2); return HllSketchAggregatorFactory.COMPARATOR.compare(sketch1, sketch2);
} }
@Nullable
@Override @Override
public HllSketchHolder fromByteBuffer(final ByteBuffer buf, final int size) public HllSketchHolder fromByteBuffer(final ByteBuffer buf, final int size)
{ {

View File

@ -24,10 +24,12 @@ import org.apache.datasketches.hll.TgtHllType;
import org.apache.druid.java.util.common.StringEncoding; import org.apache.druid.java.util.common.StringEncoding;
import org.apache.druid.java.util.common.granularity.Granularities; import org.apache.druid.java.util.common.granularity.Granularities;
import org.apache.druid.query.Druids; import org.apache.druid.query.Druids;
import org.apache.druid.query.aggregation.AggregateCombiner;
import org.apache.druid.query.aggregation.Aggregator; import org.apache.druid.query.aggregation.Aggregator;
import org.apache.druid.query.aggregation.AggregatorFactory; import org.apache.druid.query.aggregation.AggregatorFactory;
import org.apache.druid.query.aggregation.BufferAggregator; import org.apache.druid.query.aggregation.BufferAggregator;
import org.apache.druid.query.aggregation.CountAggregatorFactory; import org.apache.druid.query.aggregation.CountAggregatorFactory;
import org.apache.druid.query.aggregation.TestObjectColumnSelector;
import org.apache.druid.query.aggregation.post.FieldAccessPostAggregator; import org.apache.druid.query.aggregation.post.FieldAccessPostAggregator;
import org.apache.druid.query.aggregation.post.FinalizingFieldAccessPostAggregator; import org.apache.druid.query.aggregation.post.FinalizingFieldAccessPostAggregator;
import org.apache.druid.query.timeseries.TimeseriesQuery; import org.apache.druid.query.timeseries.TimeseriesQuery;
@ -348,6 +350,22 @@ public class HllSketchAggregatorFactoryTest
); );
} }
@Test
public void testFoldWithNullObject()
{
TestHllSketchAggregatorFactory factory = new TestHllSketchAggregatorFactory(
NAME,
FIELD_NAME,
LG_K,
TGT_HLL_TYPE,
STRING_ENCODING,
!ROUND
);
AggregateCombiner aggregateCombiner = factory.makeAggregateCombiner();
TestObjectColumnSelector objectColumnSelector = new TestObjectColumnSelector(new Object[] {null});
aggregateCombiner.fold(objectColumnSelector);
}
private static boolean isToStringField(Field field) private static boolean isToStringField(Field field)
{ {
int modfiers = field.getModifiers(); int modfiers = field.getModifiers();