[GitHub] [druid] maytasm3 commented on a change in pull request #9317: ANY Aggregator should not skip null values implementation

[GitBox]

maytasm3 commented on a change in pull request #9317: ANY Aggregator should not 
skip null values implementation
URL: https://github.com/apache/druid/pull/9317#discussion_r377977604
 
 

 ##########
 File path: 
processing/src/main/java/org/apache/druid/query/aggregation/any/DoubleAnyAggregatorFactory.java
 ##########
 @@ -19,103 +19,214 @@
 
 package org.apache.druid.query.aggregation.any;
 
-import com.fasterxml.jackson.annotation.JacksonInject;
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
-import org.apache.druid.math.expr.ExprMacroTable;
+import com.google.common.base.Preconditions;
+import org.apache.druid.java.util.common.UOE;
+import org.apache.druid.query.aggregation.AggregateCombiner;
 import org.apache.druid.query.aggregation.Aggregator;
 import org.apache.druid.query.aggregation.AggregatorFactory;
 import org.apache.druid.query.aggregation.AggregatorUtil;
 import org.apache.druid.query.aggregation.BufferAggregator;
-import org.apache.druid.query.aggregation.SimpleDoubleAggregatorFactory;
+import org.apache.druid.query.aggregation.DoubleSumAggregator;
 import org.apache.druid.query.cache.CacheKeyBuilder;
 import org.apache.druid.segment.BaseDoubleColumnValueSelector;
+import org.apache.druid.segment.ColumnSelectorFactory;
+import org.apache.druid.segment.NilColumnValueSelector;
+import org.apache.druid.segment.column.ColumnHolder;
 
 import javax.annotation.Nullable;
+import java.nio.ByteBuffer;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
+import java.util.Objects;
 
-public class DoubleAnyAggregatorFactory extends SimpleDoubleAggregatorFactory
+public class DoubleAnyAggregatorFactory extends AggregatorFactory
 {
+  private static final Comparator<Double> VALUE_COMPARATOR = 
Comparator.nullsFirst(Double::compare);
+
+  private static final Aggregator NIL_AGGREGATOR = new DoubleAnyAggregator(
+      NilColumnValueSelector.instance()
+  )
+  {
+    @Override
+    public void aggregate()
+    {
+      // no-op
+    }
+  };
+
+  private static final BufferAggregator NIL_BUFFER_AGGREGATOR = new 
DoubleAnyBufferAggregator(
+      NilColumnValueSelector.instance()
+  )
+  {
+    @Override
+    public void aggregate(ByteBuffer buf, int position)
+    {
+      // no-op
+    }
+  };
+
+  private final String fieldName;
+  private final String name;
+  private final boolean storeDoubleAsFloat;
+
   @JsonCreator
   public DoubleAnyAggregatorFactory(
       @JsonProperty("name") String name,
-      @JsonProperty("fieldName") final String fieldName,
-      @JsonProperty("expression") @Nullable String expression,
-      @JacksonInject ExprMacroTable macroTable
+      @JsonProperty("fieldName") final String fieldName
   )
   {
-    super(macroTable, name, fieldName, expression);
-  }
+    Preconditions.checkNotNull(name, "Must have a valid, non-null aggregator 
name");
+    Preconditions.checkNotNull(fieldName, "Must have a valid, non-null 
fieldName");
 
-  public DoubleAnyAggregatorFactory(String name, String fieldName)
-  {
-    this(name, fieldName, null, ExprMacroTable.nil());
+    this.name = name;
+    this.fieldName = fieldName;
+    this.storeDoubleAsFloat = ColumnHolder.storeDoubleAsFloat();
   }
 
   @Override
-  protected double nullValue()
+  public Aggregator factorize(ColumnSelectorFactory metricFactory)
   {
-    return Double.NaN;
+    final BaseDoubleColumnValueSelector valueSelector = 
metricFactory.makeColumnValueSelector(fieldName);
+    if (valueSelector instanceof NilColumnValueSelector) {
+      return NIL_AGGREGATOR;
+    } else {
+      return new DoubleAnyAggregator(
+          valueSelector
+      );
+    }
   }
 
   @Override
-  protected Aggregator buildAggregator(BaseDoubleColumnValueSelector selector)
+  public BufferAggregator factorizeBuffered(ColumnSelectorFactory 
metricFactory)
   {
-    return new DoubleAnyAggregator(selector);
+    final BaseDoubleColumnValueSelector valueSelector = 
metricFactory.makeColumnValueSelector(fieldName);
+    if (valueSelector instanceof NilColumnValueSelector) {
+      return NIL_BUFFER_AGGREGATOR;
+    } else {
+      return new DoubleAnyBufferAggregator(
+          valueSelector
+      );
+    }
   }
 
   @Override
-  protected BufferAggregator 
buildBufferAggregator(BaseDoubleColumnValueSelector selector)
+  public Comparator getComparator()
   {
-    return new DoubleAnyBufferAggregator(selector);
+    return VALUE_COMPARATOR;
   }
 
   @Override
   @Nullable
   public Object combine(@Nullable Object lhs, @Nullable Object rhs)
   {
-    if (lhs != null) {
-      return lhs;
-    } else {
-      return rhs;
-    }
+    return lhs;
 
 Review comment:
   to be consistent with the new policy of not discriminating null values 
#equality 
   Decided not to make assumption in preferring non-null value. 

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


With regards,
Apache Git Services

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