From c273298ad40087f06a653484ff4318fb3d0d660f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Xavier=20L=C3=A9aut=C3=A9?= <xavier@metamarkets.com>
Date: Tue, 27 May 2014 09:16:10 -0700
Subject: [PATCH] don't discard current maxOverflow if offset is high enough

---
 .../hyperloglog/HyperLogLogCollector.java     |  6 +++++-
 .../hyperloglog/HyperLogLogCollectorTest.java | 21 ++++---------------
 2 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/processing/src/main/java/io/druid/query/aggregation/hyperloglog/HyperLogLogCollector.java b/processing/src/main/java/io/druid/query/aggregation/hyperloglog/HyperLogLogCollector.java
index dbd766c391..fb64956dcc 100644
--- a/processing/src/main/java/io/druid/query/aggregation/hyperloglog/HyperLogLogCollector.java
+++ b/processing/src/main/java/io/druid/query/aggregation/hyperloglog/HyperLogLogCollector.java
@@ -299,8 +299,12 @@ public abstract class HyperLogLogCollector implements Comparable<HyperLogLogColl
     if (positionOf1 <= registerOffset) {
       return;
     } else if (positionOf1 > (registerOffset + range)) {
-      byte currMax = getMaxOverflowValue();
+      final byte currMax = getMaxOverflowValue();
       if (positionOf1 > currMax) {
+        if(currMax <= (registerOffset + range)) {
+          // this could be optimized by having an add without sanity checks
+          add(getMaxOverflowRegister(), currMax);
+        }
         setMaxOverflowValue(positionOf1);
         setMaxOverflowRegister(bucket);
       }
diff --git a/processing/src/test/java/io/druid/query/aggregation/hyperloglog/HyperLogLogCollectorTest.java b/processing/src/test/java/io/druid/query/aggregation/hyperloglog/HyperLogLogCollectorTest.java
index 2425613995..1a1780fc1e 100644
--- a/processing/src/test/java/io/druid/query/aggregation/hyperloglog/HyperLogLogCollectorTest.java
+++ b/processing/src/test/java/io/druid/query/aggregation/hyperloglog/HyperLogLogCollectorTest.java
@@ -844,29 +844,16 @@ public class HyperLogLogCollectorTest
     );
 
     Collection<List<HyperLogLogCollector>> permutations = Collections2.permutations(collectors);
-    Collection<List<HyperLogLogCollector>> selectPermutations = Lists.<List<HyperLogLogCollector>>newArrayList(
-        Iterables.get(
-            permutations,
-            3
-        )
-    );
-
-    selectPermutations = permutations;
-
-//    ArrayList<List<HyperLogLogCollector>> selectPermutations = Lists.<List<HyperLogLogCollector>>newArrayList(
-//        Iterables.get(
-//            permutations,
-//            1
-//        )
-//    );
 
-    for(List<HyperLogLogCollector> permutation : selectPermutations) {
+    for(List<HyperLogLogCollector> permutation : permutations) {
       HyperLogLogCollector collector = HyperLogLogCollector.makeLatestCollector();
 
       for (HyperLogLogCollector foldee : permutation) {
         collector.fold(foldee);
       }
-      System.err.println(collector.estimateCardinality());
+      Assert.assertEquals(29, collector.getMaxOverflowValue());
+      Assert.assertEquals(366, collector.getMaxOverflowRegister());
+      Assert.assertEquals(1.0429189446653817E7, collector.estimateCardinality(), 1);
     }
   }
 
-- 
GitLab