[GitHub] [incubator-druid] clintropolis commented on a change in pull request #8834: Fixes, adjustments to numeric null handling and string first/last aggregators.

[GitBox <gi...@apache.org>]

clintropolis commented on a change in pull request #8834: Fixes, adjustments to numeric null handling and string first/last aggregators.
URL: https://github.com/apache/incubator-druid/pull/8834#discussion_r343937286
 
 

 ##########
 File path: extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/aggregation/bloom/ObjectBloomFilterAggregator.java
 ##########
 @@ -48,16 +46,14 @@ void bufferAdd(ByteBuffer buf)
       final ByteBuffer other = (ByteBuffer) object;
       BloomKFilter.mergeBloomFilterByteBuffers(buf, buf.position(), other, other.position());
     } else {
-      if (NullHandling.replaceWithDefault() || !selector.isNull()) {
-        if (object instanceof Long) {
-          BloomKFilter.addLong(buf, selector.getLong());
-        } else if (object instanceof Double) {
-          BloomKFilter.addDouble(buf, selector.getDouble());
-        } else if (object instanceof Float) {
-          BloomKFilter.addFloat(buf, selector.getFloat());
-        } else {
-          StringBloomFilterAggregator.stringBufferAdd(buf, (DimensionSelector) selector);
-        }
+      if (object instanceof Long) {
+        BloomKFilter.addLong(buf, (long) object);
+      } else if (object instanceof Double) {
+        BloomKFilter.addDouble(buf, (double) object);
+      } else if (object instanceof Float) {
+        BloomKFilter.addFloat(buf, (float) object);
+      } else if (object instanceof String) {
+        BloomKFilter.addString(buf, (String) object);
 
 Review comment:
   :+1: this is a good change I think, moving the DimensionSelector out of here. This doesn't need to handle `List` since it doesn't work at ingestion time i think, and i can't think of anywhere else they would come from without a column capabilities.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org