[druid] branch master updated: Fix bug in Variance Buffer Aggregator resulting in intermittent NaN when druid.generic.useDefaultValueForNull=false (#11617)

[su...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

suneet pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git


The following commit(s) were added to refs/heads/master by this push:
     new b36242b  Fix bug in Variance Buffer Aggregator resulting in intermittent NaN when druid.generic.useDefaultValueForNull=false (#11617)
b36242b is described below

commit b36242b404c77453b3464b17da32be076e57d39e
Author: Maytas Monsereenusorn <ma...@apache.org>
AuthorDate: Fri Aug 20 23:13:51 2021 +0700

    Fix bug in Variance Buffer Aggregator resulting in intermittent NaN when druid.generic.useDefaultValueForNull=false (#11617)
    
    * Fix bug in Variance Aggregator resulting in intermittent NaN when druid.generic.useDefaultValueForNull=false
    
    * fix checkstyle
    
    * address comments
---
 .../variance/VarianceBufferAggregator.java         |  4 ++-
 .../variance/VarianceAggregatorTest.java           | 42 ++++++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/extensions-core/stats/src/main/java/org/apache/druid/query/aggregation/variance/VarianceBufferAggregator.java b/extensions-core/stats/src/main/java/org/apache/druid/query/aggregation/variance/VarianceBufferAggregator.java
index 065ad2a..d8c4d5d 100644
--- a/extensions-core/stats/src/main/java/org/apache/druid/query/aggregation/variance/VarianceBufferAggregator.java
+++ b/extensions-core/stats/src/main/java/org/apache/druid/query/aggregation/variance/VarianceBufferAggregator.java
@@ -231,6 +231,9 @@ public abstract class VarianceBufferAggregator implements BufferAggregator
     {
       VarianceAggregatorCollector holder2 = (VarianceAggregatorCollector) selector.getObject();
       Preconditions.checkState(holder2 != null);
+      if (holder2.count == 0) {
+        return;
+      }
       long count = getCount(buf, position);
       if (count == 0) {
         buf.putLong(position, holder2.count);
@@ -238,7 +241,6 @@ public abstract class VarianceBufferAggregator implements BufferAggregator
         buf.putDouble(position + NVARIANCE_OFFSET, holder2.nvariance);
         return;
       }
-
       double sum = getSum(buf, position);
       double nvariance = buf.getDouble(position + NVARIANCE_OFFSET);
 
diff --git a/extensions-core/stats/src/test/java/org/apache/druid/query/aggregation/variance/VarianceAggregatorTest.java b/extensions-core/stats/src/test/java/org/apache/druid/query/aggregation/variance/VarianceAggregatorTest.java
index f3b9453..c6c8989 100644
--- a/extensions-core/stats/src/test/java/org/apache/druid/query/aggregation/variance/VarianceAggregatorTest.java
+++ b/extensions-core/stats/src/test/java/org/apache/druid/query/aggregation/variance/VarianceAggregatorTest.java
@@ -22,7 +22,10 @@ package org.apache.druid.query.aggregation.variance;
 import org.apache.druid.common.config.NullHandling;
 import org.apache.druid.jackson.DefaultObjectMapper;
 import org.apache.druid.query.aggregation.TestFloatColumnSelector;
+import org.apache.druid.query.aggregation.TestObjectColumnSelector;
 import org.apache.druid.segment.ColumnSelectorFactory;
+import org.apache.druid.segment.column.ColumnCapabilitiesImpl;
+import org.apache.druid.segment.column.ValueType;
 import org.apache.druid.testing.InitializedNullHandlingTest;
 import org.easymock.EasyMock;
 import org.junit.Assert;
@@ -123,6 +126,34 @@ public class VarianceAggregatorTest extends InitializedNullHandlingTest
   }
 
   @Test
+  public void testObjectVarianceBufferAggregatorWithZeroCount()
+  {
+    VarianceAggregatorCollector holder1 = new VarianceAggregatorCollector().add(1.1f);
+    VarianceAggregatorCollector holder2 = new VarianceAggregatorCollector().add(2.7f);
+    VarianceAggregatorCollector holder3 = new VarianceAggregatorCollector();
+    VarianceAggregatorCollector[] values = {holder1, holder2, holder3};
+    TestObjectColumnSelector<VarianceAggregatorCollector> selector = new TestObjectColumnSelector(values);
+    colSelectorFactory = EasyMock.createMock(ColumnSelectorFactory.class);
+    EasyMock.expect(colSelectorFactory.makeColumnValueSelector("nilly")).andReturn(selector);
+    EasyMock.expect(colSelectorFactory.getColumnCapabilities("nilly")).andReturn(new ColumnCapabilitiesImpl().setType(ValueType.COMPLEX));
+    EasyMock.replay(colSelectorFactory);
+
+    VarianceBufferAggregator agg = (VarianceBufferAggregator) aggFactory.factorizeBuffered(
+        colSelectorFactory
+    );
+
+    ByteBuffer buffer = ByteBuffer.wrap(new byte[aggFactory.getMaxIntermediateSizeWithNulls()]);
+    agg.init(buffer, 0);
+    assertValues((VarianceAggregatorCollector) agg.get(buffer, 0), 0, 0d, 0d);
+    aggregate(selector, agg, buffer, 0);
+    assertValues((VarianceAggregatorCollector) agg.get(buffer, 0), 1, 1.1d, 0d);
+    aggregate(selector, agg, buffer, 0);
+    assertValues((VarianceAggregatorCollector) agg.get(buffer, 0), 2, 3.8d, 1.28d);
+    aggregate(selector, agg, buffer, 0);
+    assertValues((VarianceAggregatorCollector) agg.get(buffer, 0), 2, 3.8d, 1.28d);
+  }
+
+  @Test
   public void testCombine()
   {
     VarianceAggregatorCollector holder1 = new VarianceAggregatorCollector().add(1.1f).add(2.7f);
@@ -160,4 +191,15 @@ public class VarianceAggregatorTest extends InitializedNullHandlingTest
     agg.aggregate(buff, position);
     selector.increment();
   }
+
+  private void aggregate(
+      TestObjectColumnSelector<VarianceAggregatorCollector> selector,
+      VarianceBufferAggregator agg,
+      ByteBuffer buff,
+      int position
+  )
+  {
+    agg.aggregate(buff, position);
+    selector.increment();
+  }
 }

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