gianm closed pull request #5962: [Backport] Fix cache bug in stats module
URL: https://github.com/apache/incubator-druid/pull/5962
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/StandardDeviationPostAggregator.java
b/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/StandardDeviationPostAggregator.java
index 443d0126cb7..f7034844d81 100644
---
a/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/StandardDeviationPostAggregator.java
+++
b/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/StandardDeviationPostAggregator.java
@@ -107,7 +107,8 @@ public String toString()
return "StandardDeviationPostAggregator{" +
"name='" + name + '\'' +
", fieldName='" + fieldName + '\'' +
- ", isVariancePop='" + isVariancePop + '\'' +
+ ", estimator='" + estimator + '\'' +
+ ", isVariancePop=" + isVariancePop +
'}';
}
@@ -116,6 +117,7 @@ public String toString()
{
return new CacheKeyBuilder(PostAggregatorIds.VARIANCE_STANDARD_DEVIATION)
.appendString(fieldName)
+ .appendString(estimator)
.appendBoolean(isVariancePop)
.build();
}
diff --git
a/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/VarianceAggregatorCollector.java
b/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/VarianceAggregatorCollector.java
index a6c8aad4ee3..ee7e6c9d9cf 100644
---
a/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/VarianceAggregatorCollector.java
+++
b/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/VarianceAggregatorCollector.java
@@ -59,20 +59,15 @@ public static VarianceAggregatorCollector from(ByteBuffer
buffer)
return new VarianceAggregatorCollector(buffer.getLong(),
buffer.getDouble(), buffer.getDouble());
}
- public static final Comparator<VarianceAggregatorCollector> COMPARATOR = new
Comparator<VarianceAggregatorCollector>()
- {
- @Override
- public int compare(VarianceAggregatorCollector o1,
VarianceAggregatorCollector o2)
- {
- int compare = Longs.compare(o1.count, o2.count);
+ public static final Comparator<VarianceAggregatorCollector> COMPARATOR =
(o1, o2) -> {
+ int compare = Longs.compare(o1.count, o2.count);
+ if (compare == 0) {
+ compare = Doubles.compare(o1.sum, o2.sum);
if (compare == 0) {
- compare = Doubles.compare(o1.sum, o2.sum);
- if (compare == 0) {
- compare = Doubles.compare(o1.nvariance, o2.nvariance);
- }
+ compare = Doubles.compare(o1.nvariance, o2.nvariance);
}
- return compare;
}
+ return compare;
};
void fold(@Nullable VarianceAggregatorCollector other)
@@ -114,13 +109,6 @@ public VarianceAggregatorCollector()
this(0, 0, 0);
}
- public void reset()
- {
- count = 0;
- sum = 0;
- nvariance = 0;
- }
-
void copyFrom(VarianceAggregatorCollector other)
{
this.count = other.count;
diff --git
a/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/VarianceAggregatorFactory.java
b/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/VarianceAggregatorFactory.java
index aa868447835..86657977e82 100644
---
a/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/VarianceAggregatorFactory.java
+++
b/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/VarianceAggregatorFactory.java
@@ -34,13 +34,13 @@
import io.druid.query.aggregation.NoopAggregator;
import io.druid.query.aggregation.NoopBufferAggregator;
import io.druid.query.aggregation.ObjectAggregateCombiner;
+import io.druid.query.cache.CacheKeyBuilder;
import io.druid.segment.ColumnSelectorFactory;
import io.druid.segment.ColumnValueSelector;
import io.druid.segment.NilColumnValueSelector;
import org.apache.commons.codec.binary.Base64;
import java.nio.ByteBuffer;
-import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
@@ -184,7 +184,7 @@ public AggregatorFactory getCombiningFactory()
@Override
public List<AggregatorFactory> getRequiredColumns()
{
- return Arrays.<AggregatorFactory>asList(new
VarianceAggregatorFactory(fieldName, fieldName, estimator, inputType));
+ return Collections.singletonList(new VarianceAggregatorFactory(fieldName,
fieldName, estimator, inputType));
}
@Override
@@ -258,25 +258,23 @@ public String getInputType()
@Override
public byte[] getCacheKey()
{
- byte[] fieldNameBytes = StringUtils.toUtf8(fieldName);
- byte[] inputTypeBytes = StringUtils.toUtf8(inputType);
- return ByteBuffer.allocate(2 + fieldNameBytes.length + 1 +
inputTypeBytes.length)
- .put(AggregatorUtil.VARIANCE_CACHE_TYPE_ID)
- .put(isVariancePop ? (byte) 1 : 0)
- .put(fieldNameBytes)
- .put((byte) 0xFF)
- .put(inputTypeBytes)
- .array();
+ return new CacheKeyBuilder(AggregatorUtil.VARIANCE_CACHE_TYPE_ID)
+ .appendString(fieldName)
+ .appendString(inputType)
+ .appendBoolean(isVariancePop)
+ .appendString(estimator)
+ .build();
}
@Override
public String toString()
{
- return getClass().getSimpleName() + "{" +
+ return "VarianceAggregatorFactory{" +
"fieldName='" + fieldName + '\'' +
", name='" + name + '\'' +
- ", isVariancePop='" + isVariancePop + '\'' +
+ ", estimator='" + estimator + '\'' +
", inputType='" + inputType + '\'' +
+ ", isVariancePop=" + isVariancePop +
'}';
}
@@ -289,29 +287,18 @@ public boolean equals(Object o)
if (o == null || getClass() != o.getClass()) {
return false;
}
-
VarianceAggregatorFactory that = (VarianceAggregatorFactory) o;
-
- if (!Objects.equals(name, that.name)) {
- return false;
- }
- if (!Objects.equals(isVariancePop, that.isVariancePop)) {
- return false;
- }
- if (!Objects.equals(inputType, that.inputType)) {
- return false;
- }
-
- return true;
+ return isVariancePop == that.isVariancePop &&
+ Objects.equals(fieldName, that.fieldName) &&
+ Objects.equals(name, that.name) &&
+ Objects.equals(estimator, that.estimator) &&
+ Objects.equals(inputType, that.inputType);
}
@Override
public int hashCode()
{
- int result = fieldName.hashCode();
- result = 31 * result + Objects.hashCode(name);
- result = 31 * result + Objects.hashCode(isVariancePop);
- result = 31 * result + Objects.hashCode(inputType);
- return result;
+
+ return Objects.hash(fieldName, name, estimator, inputType, isVariancePop);
}
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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: dev-unsubscr...@druid.apache.org
For additional commands, e-mail: dev-h...@druid.apache.org
