minor changes to address code review

This commit is contained in:
Xavier Léauté 2014-04-25 14:50:22 -07:00
parent 42c4e955fa
commit de5b1749bc
2 changed files with 6 additions and 9 deletions

View File

@ -19,7 +19,6 @@
package io.druid.query.aggregation.cardinality; package io.druid.query.aggregation.cardinality;
import com.google.common.collect.Lists;
import com.google.common.hash.HashFunction; import com.google.common.hash.HashFunction;
import com.google.common.hash.Hasher; import com.google.common.hash.Hasher;
import com.google.common.hash.Hashing; import com.google.common.hash.Hashing;
@ -52,24 +51,23 @@ public class CardinalityAggregator implements Aggregator
final DimensionSelector selector = selectorList.get(k); final DimensionSelector selector = selectorList.get(k);
final IndexedInts row = selector.getRow(); final IndexedInts row = selector.getRow();
final int size = row.size(); final int size = row.size();
// nothing to add to hasher if size == 0, only handle size == 1 and size != 0 cases.
if (size == 1) { if (size == 1) {
final String value = selector.lookupName(row.get(0)); final String value = selector.lookupName(row.get(0));
hasher.putString(value != null ? value : NULL_STRING); hasher.putString(value != null ? value : NULL_STRING);
} else if (size == 0) { } else if (size != 0) {
// nothing to add to hasher
} else {
final String[] values = new String[size]; final String[] values = new String[size];
for (int i = 0; i < size; ++i) { for (int i = 0; i < size; ++i) {
final String value = selector.lookupName(row.get(i)); final String value = selector.lookupName(row.get(i));
values[i] = value != null ? value : NULL_STRING; values[i] = value != null ? value : NULL_STRING;
} }
// Values need to be sorted to ensure consistent multi-value ordering across different segments
Arrays.sort(values); Arrays.sort(values);
for (int i = 0; i < size; ++i) { for (int i = 0; i < size; ++i) {
if(i != 0) { if(i != 0) {
hasher.putChar(SEPARATOR); hasher.putChar(SEPARATOR);
} }
final String value = values[i]; hasher.putString(values[i]);
hasher.putString(value);
} }
} }
} }
@ -95,7 +93,7 @@ public class CardinalityAggregator implements Aggregator
) )
{ {
this.name = name; this.name = name;
this.selectorList = Lists.newArrayList(selectorList); this.selectorList = selectorList;
this.collector = HyperLogLogCollector.makeLatestCollector(); this.collector = HyperLogLogCollector.makeLatestCollector();
this.byRow = byRow; this.byRow = byRow;
} }

View File

@ -19,7 +19,6 @@
package io.druid.query.aggregation.cardinality; package io.druid.query.aggregation.cardinality;
import com.google.common.collect.Lists;
import io.druid.query.aggregation.BufferAggregator; import io.druid.query.aggregation.BufferAggregator;
import io.druid.query.aggregation.hyperloglog.HyperLogLogCollector; import io.druid.query.aggregation.hyperloglog.HyperLogLogCollector;
import io.druid.segment.DimensionSelector; import io.druid.segment.DimensionSelector;
@ -39,7 +38,7 @@ public class CardinalityBufferAggregator implements BufferAggregator
boolean byRow boolean byRow
) )
{ {
this.selectorList = Lists.newArrayList(selectorList); this.selectorList = selectorList;
this.byRow = byRow; this.byRow = byRow;
} }