gianm commented on a change in pull request #7496: refactor druid-bloom-filter
aggregators
URL: https://github.com/apache/incubator-druid/pull/7496#discussion_r276409315
##########
File path:
extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/aggregation/bloom/BaseBloomFilterAggregator.java
##########
@@ -20,20 +20,116 @@
package org.apache.druid.query.aggregation.bloom;
import org.apache.druid.query.aggregation.Aggregator;
+import org.apache.druid.query.aggregation.BufferAggregator;
import org.apache.druid.query.filter.BloomKFilter;
+import org.apache.druid.query.monomorphicprocessing.RuntimeShapeInspector;
import org.apache.druid.segment.BaseNullableColumnValueSelector;
import javax.annotation.Nullable;
+import java.nio.ByteBuffer;
-public abstract class BaseBloomFilterAggregator<TSelector extends
BaseNullableColumnValueSelector> implements Aggregator
+/**
+ * All bloom filter aggregations are done using {@link ByteBuffer}, so this
base class implements both {@link Aggregator}
+ * and {@link BufferAggregator}.
+ *
+ * If used as an {@link Aggregator} the caller MUST specify the 'onHeap'
parameter in the
+ * constructor as "true", or else the "collector" will not be allocated and
null pointer exceptions will make things sad.
+ *
+ * If used as a {@link BufferAggregator}, the "collector" buffer is not
necessary, and should be called with "false",
+ * but at least nothing dramatic will happen like incorrect use in the {@link
Aggregator} case.
+ *
+ * {@link BloomFilterAggregatorFactory} and {@link
BloomFilterMergeAggregatorFactory}, which should be the creators of
+ * all implementations of {@link BaseBloomFilterAggregator} outside of tests,
should be sure to set the 'onHeap' value
+ * to "true" and "false" respectively for
+ * {@link org.apache.druid.query.aggregation.AggregatorFactory#factorize} and
+ * {@link
org.apache.druid.query.aggregation.AggregatorFactory#factorizeBuffered}
+ *
+ * @param <TSelector> type of {@link BaseNullableColumnValueSelector} that
feeds this aggregator, likely either values
+ * to add to a bloom filter, or other bloom filters to merge
into this bloom filter.
+ */
+public abstract class BaseBloomFilterAggregator<TSelector extends
BaseNullableColumnValueSelector>
+ implements BufferAggregator, Aggregator
{
- final BloomKFilter collector;
+
+ protected final ByteBuffer collector;
Review comment:
Please mark this `@Nullable`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
