From 6ddd089f3a9d98321ccb4260b56988de2030b152 Mon Sep 17 00:00:00 2001 From: "Laguduwa Sankaran, Senthil Kumar" Date: Thu, 29 Mar 2018 15:01:03 +0530 Subject: [PATCH 1/4] Adding feature thetaSketchConstant to do some set operation in PostAggregator --- .../theta/SketchConstantPostAggregator.java | 133 ++++++++++++++++++ .../datasketches/theta/SketchModule.java | 5 +- .../theta/SketchAggregationTest.java | 20 +++ .../SketchAggregationWithSimpleDataTest.java | 45 ++++++ .../resources/topn_query_sketch_const.json | 104 ++++++++++++++ .../aggregation/post/PostAggregatorIds.java | 1 + 6 files changed, 307 insertions(+), 1 deletion(-) create mode 100644 extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchConstantPostAggregator.java create mode 100644 extensions-core/datasketches/src/test/resources/topn_query_sketch_const.json diff --git a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchConstantPostAggregator.java b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchConstantPostAggregator.java new file mode 100644 index 000000000000..0d2b48a87852 --- /dev/null +++ b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchConstantPostAggregator.java @@ -0,0 +1,133 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.druid.query.aggregation.datasketches.theta; + +import java.util.Comparator; +import java.util.Map; +import java.util.Set; + +import org.apache.commons.codec.binary.Base64; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Preconditions; +import com.google.common.collect.Sets; + +import io.druid.query.aggregation.AggregatorFactory; +import io.druid.query.aggregation.PostAggregator; +import io.druid.query.aggregation.post.PostAggregatorIds; +import io.druid.query.cache.CacheKeyBuilder; + +/** + */ +public class SketchConstantPostAggregator implements PostAggregator +{ + + private final String name; + private final SketchHolder sketchValue; + + @JsonCreator + public SketchConstantPostAggregator(@JsonProperty("name") String name, @JsonProperty("value") String sketchValue) + { + this.name = name; + Preconditions.checkNotNull(sketchValue); + this.sketchValue = SketchHolder.deserialize(sketchValue); + } + + @Override + public Set getDependentFields() + { + return Sets.newHashSet(); + } + + @Override + public Comparator getComparator() + { + return SketchHolder.COMPARATOR; + } + + @Override + public Object compute(Map combinedAggregators) + { + return sketchValue; + } + + @Override + @JsonProperty + public String getName() + { + return name; + } + + @Override + public SketchConstantPostAggregator decorate(Map aggregators) + { + return this; + } + + @JsonProperty("value") + public String getSketchValue() + { + return Base64.encodeBase64String(sketchValue.getSketch().toByteArray()); + } + + @Override + public String toString() + { + return "SketchConstantPostAggregator{" + "name='" + name + '\'' + ", value=" + getSketchValue() + '}'; + } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + SketchConstantPostAggregator that = (SketchConstantPostAggregator) o; + + if (!getSketchValue().equals(that.getSketchValue())) { + return false; + } + + if (name != null ? !name.equals(that.name) : that.name != null) { + return false; + } + + return true; + } + + @Override + public int hashCode() + { + int result = name != null ? name.hashCode() : 0; + result = 37 * result + getSketchValue().hashCode(); + return result; + } + + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.THETA_SKETCH_CONSTANT).appendInt(getSketchValue().hashCode()).build(); + } +} diff --git a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchModule.java b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchModule.java index 361dd4a2fbf9..18d699ec6c52 100644 --- a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchModule.java +++ b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchModule.java @@ -38,6 +38,8 @@ public class SketchModule implements DruidModule public static final String THETA_SKETCH_ESTIMATE_POST_AGG = "thetaSketchEstimate"; public static final String THETA_SKETCH_SET_OP_POST_AGG = "thetaSketchSetOp"; + + public static final String THETA_SKETCH_CONSTANT_POST_AGG = "thetaSketchConstant"; @Override public void configure(Binder binder) @@ -63,7 +65,8 @@ public List getJacksonModules() .registerSubtypes( new NamedType(SketchMergeAggregatorFactory.class, THETA_SKETCH), new NamedType(SketchEstimatePostAggregator.class, THETA_SKETCH_ESTIMATE_POST_AGG), - new NamedType(SketchSetPostAggregator.class, THETA_SKETCH_SET_OP_POST_AGG) + new NamedType(SketchSetPostAggregator.class, THETA_SKETCH_SET_OP_POST_AGG), + new NamedType(SketchConstantPostAggregator.class, THETA_SKETCH_CONSTANT_POST_AGG) ) .addSerializer( SketchHolder.class, new SketchHolderJsonSerializer() diff --git a/extensions-core/datasketches/src/test/java/io/druid/query/aggregation/datasketches/theta/SketchAggregationTest.java b/extensions-core/datasketches/src/test/java/io/druid/query/aggregation/datasketches/theta/SketchAggregationTest.java index 260dc12b3507..b5eb3cb0f6c7 100644 --- a/extensions-core/datasketches/src/test/java/io/druid/query/aggregation/datasketches/theta/SketchAggregationTest.java +++ b/extensions-core/datasketches/src/test/java/io/druid/query/aggregation/datasketches/theta/SketchAggregationTest.java @@ -277,6 +277,14 @@ public void testSketchEstimatePostAggregatorSerde() throws Exception 2 ) ); + + assertPostAggregatorSerde( + new SketchEstimatePostAggregator( + "name", + new SketchConstantPostAggregator("name", "AgMDAAAazJMCAAAAAACAPzz9j7pWTMdROWGf15uY1nI="), + null + ) + ); } @Test @@ -293,6 +301,18 @@ public void testSketchSetPostAggregatorSerde() throws Exception ) ) ); + + assertPostAggregatorSerde( + new SketchSetPostAggregator( + "name", + "INTERSECT", + null, + Lists.newArrayList( + new FieldAccessPostAggregator("name1", "fieldName1"), + new SketchConstantPostAggregator("name2", "AgMDAAAazJMCAAAAAACAPzz9j7pWTMdROWGf15uY1nI=") + ) + ) + ); } @Test diff --git a/extensions-core/datasketches/src/test/java/io/druid/query/aggregation/datasketches/theta/SketchAggregationWithSimpleDataTest.java b/extensions-core/datasketches/src/test/java/io/druid/query/aggregation/datasketches/theta/SketchAggregationWithSimpleDataTest.java index 870fa9794b0c..d38166b4b915 100644 --- a/extensions-core/datasketches/src/test/java/io/druid/query/aggregation/datasketches/theta/SketchAggregationWithSimpleDataTest.java +++ b/extensions-core/datasketches/src/test/java/io/druid/query/aggregation/datasketches/theta/SketchAggregationWithSimpleDataTest.java @@ -276,6 +276,51 @@ public void testSimpleDataIngestAndSelectQuery() throws Exception Assert.assertEquals(100, result.getValue().getEvents().size()); Assert.assertEquals("AgMDAAAazJMCAAAAAACAPzz9j7pWTMdROWGf15uY1nI=", result.getValue().getEvents().get(0).getEvent().get("pty_country")); } + + @Test + public void testTopNQueryWithSketchConstant() throws Exception + { + AggregationTestHelper topNQueryAggregationTestHelper = AggregationTestHelper.createTopNQueryAggregationTestHelper( + sm.getJacksonModules(), + tempFolder + ); + + Sequence seq = topNQueryAggregationTestHelper.runQueryOnSegments( + ImmutableList.of(s1, s2), + readFileFromClasspathAsString("topn_query_sketch_const.json") + ); + + Result result = (Result) Iterables.getOnlyElement(seq.toList()); + + Assert.assertEquals(DateTimes.of("2014-10-20T00:00:00.000Z"), result.getTimestamp()); + + DimensionAndMetricValueExtractor value1 = Iterables.get(result.getValue().getValue(), 0); + Assert.assertEquals(38.0, value1.getDoubleMetric("sketch_count"), 0.01); + Assert.assertEquals(38.0, value1.getDoubleMetric("sketchEstimatePostAgg"), 0.01); + Assert.assertEquals(2.0, value1.getDoubleMetric("sketchEstimatePostAggForSketchConstant"), 0.01); + Assert.assertEquals(39.0, value1.getDoubleMetric("sketchUnionPostAggEstimate"), 0.01); + Assert.assertEquals(1.0, value1.getDoubleMetric("sketchIntersectionPostAggEstimate"), 0.01); + Assert.assertEquals(37.0, value1.getDoubleMetric("sketchAnotBPostAggEstimate"), 0.01); + Assert.assertEquals("product_3", value1.getDimensionValue("product")); + + DimensionAndMetricValueExtractor value2 = Iterables.get(result.getValue().getValue(), 1); + Assert.assertEquals(42.0, value2.getDoubleMetric("sketch_count"), 0.01); + Assert.assertEquals(42.0, value2.getDoubleMetric("sketchEstimatePostAgg"), 0.01); + Assert.assertEquals(2.0, value2.getDoubleMetric("sketchEstimatePostAggForSketchConstant"), 0.01); + Assert.assertEquals(42.0, value2.getDoubleMetric("sketchUnionPostAggEstimate"), 0.01); + Assert.assertEquals(2.0, value2.getDoubleMetric("sketchIntersectionPostAggEstimate"), 0.01); + Assert.assertEquals(40.0, value2.getDoubleMetric("sketchAnotBPostAggEstimate"), 0.01); + Assert.assertEquals("product_1", value2.getDimensionValue("product")); + + DimensionAndMetricValueExtractor value3 = Iterables.get(result.getValue().getValue(), 2); + Assert.assertEquals(42.0, value3.getDoubleMetric("sketch_count"), 0.01); + Assert.assertEquals(42.0, value3.getDoubleMetric("sketchEstimatePostAgg"), 0.01); + Assert.assertEquals(2.0, value3.getDoubleMetric("sketchEstimatePostAggForSketchConstant"), 0.01); + Assert.assertEquals(42.0, value3.getDoubleMetric("sketchUnionPostAggEstimate"), 0.01); + Assert.assertEquals(2.0, value3.getDoubleMetric("sketchIntersectionPostAggEstimate"), 0.01); + Assert.assertEquals(40.0, value3.getDoubleMetric("sketchAnotBPostAggEstimate"), 0.01); + Assert.assertEquals("product_2", value3.getDimensionValue("product")); + } public static final String readFileFromClasspathAsString(String fileName) throws IOException { diff --git a/extensions-core/datasketches/src/test/resources/topn_query_sketch_const.json b/extensions-core/datasketches/src/test/resources/topn_query_sketch_const.json new file mode 100644 index 000000000000..3dc47dca86c3 --- /dev/null +++ b/extensions-core/datasketches/src/test/resources/topn_query_sketch_const.json @@ -0,0 +1,104 @@ +{ + "queryType": "topN", + "dataSource": "test_datasource", + "granularity":"ALL", + "metric": { + "type": "inverted", + "metric": "sketch_count" + }, + "dimension": "product", + "threshold": 3, + "aggregations": [ + { + "type": "thetaSketch", + "name": "sketch_count", + "fieldName": "pty_country", + "size": 16384 + } + ], + "postAggregations": [ + { + "type": "thetaSketchEstimate", + "name": "sketchEstimatePostAgg", + "field": { + "type": "fieldAccess", + "fieldName": "sketch_count" + } + }, + { + "type": "thetaSketchEstimate", + "name": "sketchEstimatePostAggForSketchConstant", + "field": { + "type": "thetaSketchConstant", + "name": "theta_sketch_count", + "value": "AgMDAAAazJMCAAAAAACAPzz9j7pWTMdROWGf15uY1nI=" + } + }, + { + "type": "thetaSketchEstimate", + "name": "sketchIntersectionPostAggEstimate", + "field": { + "type": "thetaSketchSetOp", + "name": "sketchIntersectionPostAgg", + "func": "INTERSECT", + "size": 16384, + "fields": [ + { + "type": "fieldAccess", + "fieldName": "sketch_count" + }, + { + "type": "thetaSketchConstant", + "name": "theta_sketch_count", + "value": "AgMDAAAazJMCAAAAAACAPzz9j7pWTMdROWGf15uY1nI=" + } + ] + } + }, + { + "type": "thetaSketchEstimate", + "name": "sketchAnotBPostAggEstimate", + "field": { + "type": "thetaSketchSetOp", + "name": "sketchAnotBUnionPostAgg", + "func": "NOT", + "size": 16384, + "fields": [ + { + "type": "fieldAccess", + "fieldName": "sketch_count" + }, + { + "type": "thetaSketchConstant", + "name": "theta_sketch_count", + "value": "AgMDAAAazJMCAAAAAACAPzz9j7pWTMdROWGf15uY1nI=" + } + ] + } + }, + { + "type": "thetaSketchEstimate", + "name": "sketchUnionPostAggEstimate", + "field": { + "type": "thetaSketchSetOp", + "name": "sketchUnionPostAgg", + "func": "UNION", + "size": 16384, + "fields": [ + { + "type": "fieldAccess", + "fieldName": "sketch_count" + }, + { + "type": "thetaSketchConstant", + "name": "theta_sketch_count", + "value": "AgMDAAAazJMCAAAAAACAPzz9j7pWTMdROWGf15uY1nI=" + } + ] + } + } + ], + "intervals": [ + "2014-10-19T00:00:00.000Z/2014-10-22T00:00:00.000Z" + ] +} diff --git a/processing/src/main/java/io/druid/query/aggregation/post/PostAggregatorIds.java b/processing/src/main/java/io/druid/query/aggregation/post/PostAggregatorIds.java index bfb7d4df108e..3ef660c2cbe4 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/PostAggregatorIds.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/PostAggregatorIds.java @@ -44,4 +44,5 @@ public class PostAggregatorIds public static final byte FINALIZING_FIELD_ACCESS = 20; public static final byte ZTEST = 21; public static final byte PVALUE_FROM_ZTEST = 22; + public static final byte THETA_SKETCH_CONSTANT = 23; } From 0fa9eab2d8adda447ea448e36a60021be66cbefa Mon Sep 17 00:00:00 2001 From: "Laguduwa Sankaran, Senthil Kumar" Date: Tue, 3 Apr 2018 23:48:32 +0530 Subject: [PATCH 2/4] Updated review comments for PR #5551 - Adding thetaSketchConstant --- .../theta/SketchConstantPostAggregator.java | 20 +++++++++---------- .../datasketches/theta/SketchHolder.java | 11 +++++++++- .../post/ConstantPostAggregator.java | 3 +-- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchConstantPostAggregator.java b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchConstantPostAggregator.java index 0d2b48a87852..5ab1e31c4546 100644 --- a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchConstantPostAggregator.java +++ b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchConstantPostAggregator.java @@ -19,6 +19,7 @@ package io.druid.query.aggregation.datasketches.theta; +import java.util.Collections; import java.util.Comparator; import java.util.Map; import java.util.Set; @@ -28,7 +29,6 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; -import com.google.common.collect.Sets; import io.druid.query.aggregation.AggregatorFactory; import io.druid.query.aggregation.PostAggregator; @@ -47,14 +47,13 @@ public class SketchConstantPostAggregator implements PostAggregator public SketchConstantPostAggregator(@JsonProperty("name") String name, @JsonProperty("value") String sketchValue) { this.name = name; - Preconditions.checkNotNull(sketchValue); - this.sketchValue = SketchHolder.deserialize(sketchValue); + this.sketchValue = SketchHolder.deserialize(Preconditions.checkNotNull(sketchValue)); } @Override public Set getDependentFields() { - return Sets.newHashSet(); + return Collections.emptySet(); } @Override @@ -83,15 +82,16 @@ public SketchConstantPostAggregator decorate(Map aggr } @JsonProperty("value") - public String getSketchValue() + public SketchHolder getSketchValue() { - return Base64.encodeBase64String(sketchValue.getSketch().toByteArray()); + return sketchValue; } @Override public String toString() { - return "SketchConstantPostAggregator{" + "name='" + name + '\'' + ", value=" + getSketchValue() + '}'; + return "SketchConstantPostAggregator{name='" + name + "', value='" + + Base64.encodeBase64String(sketchValue.getSketch().toByteArray()) + "'}"; } @Override @@ -106,7 +106,7 @@ public boolean equals(Object o) SketchConstantPostAggregator that = (SketchConstantPostAggregator) o; - if (!getSketchValue().equals(that.getSketchValue())) { + if (!this.sketchValue.equals(that.sketchValue)) { return false; } @@ -121,13 +121,13 @@ public boolean equals(Object o) public int hashCode() { int result = name != null ? name.hashCode() : 0; - result = 37 * result + getSketchValue().hashCode(); + result = 37 * result + sketchValue.hashCode(); return result; } @Override public byte[] getCacheKey() { - return new CacheKeyBuilder(PostAggregatorIds.THETA_SKETCH_CONSTANT).appendInt(getSketchValue().hashCode()).build(); + return new CacheKeyBuilder(PostAggregatorIds.THETA_SKETCH_CONSTANT).appendInt(hashCode()).build(); } } diff --git a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchHolder.java b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchHolder.java index cf5f1a3fb130..ebe565ebead8 100644 --- a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchHolder.java +++ b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchHolder.java @@ -36,6 +36,7 @@ import io.druid.java.util.common.StringUtils; import org.apache.commons.codec.binary.Base64; +import java.util.Arrays; import java.util.Comparator; /** @@ -295,6 +296,14 @@ public boolean equals(Object o) if (o == null || getClass() != o.getClass()) { return false; } - return this.getSketch().equals(((SketchHolder) o).getSketch()); + // Can't use Sketch's equals method because it is not implemented. + // return this.getSketch().equals(((SketchHolder) o).getSketch()); + return Arrays.equals(this.getSketch().toByteArray(), ((SketchHolder) o).getSketch().toByteArray()); + } + + @Override + public int hashCode() + { + return 31 * Base64.encodeBase64(this.getSketch().toByteArray()).hashCode(); } } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/ConstantPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/ConstantPostAggregator.java index b406b83a0fe4..5eb3250c9412 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/ConstantPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/ConstantPostAggregator.java @@ -46,8 +46,7 @@ public ConstantPostAggregator( ) { this.name = name; - this.constantValue = constantValue; - Preconditions.checkNotNull(this.constantValue); + this.constantValue = Preconditions.checkNotNull(constantValue); } @Override From ee3b2abbd62d0fd4a735d10dc37ee61af617fdd2 Mon Sep 17 00:00:00 2001 From: "Laguduwa Sankaran, Senthil Kumar" Date: Wed, 4 Apr 2018 00:58:25 +0530 Subject: [PATCH 3/4] Fixed CI build issue --- .../query/aggregation/datasketches/theta/SketchHolder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchHolder.java b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchHolder.java index ebe565ebead8..42c1c98d0f73 100644 --- a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchHolder.java +++ b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchHolder.java @@ -304,6 +304,6 @@ public boolean equals(Object o) @Override public int hashCode() { - return 31 * Base64.encodeBase64(this.getSketch().toByteArray()).hashCode(); + return 31 * Arrays.hashCode(this.getSketch().toByteArray()); } } From dd34a5db3ade18bbf7aa9a2a274799b7f7199b90 Mon Sep 17 00:00:00 2001 From: "Laguduwa Sankaran, Senthil Kumar" Date: Wed, 4 Apr 2018 16:04:09 +0530 Subject: [PATCH 4/4] Updated review comments 2 for PR #5551 - Adding thetaSketchConstant --- .../theta/SketchConstantPostAggregator.java | 22 +++++++++---------- .../datasketches/theta/SketchHolder.java | 7 ++++-- .../post/ConstantPostAggregator.java | 2 +- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchConstantPostAggregator.java b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchConstantPostAggregator.java index 5ab1e31c4546..510cb78b80bd 100644 --- a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchConstantPostAggregator.java +++ b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchConstantPostAggregator.java @@ -24,7 +24,7 @@ import java.util.Map; import java.util.Set; -import org.apache.commons.codec.binary.Base64; +import org.apache.commons.codec.digest.DigestUtils; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; @@ -41,13 +41,17 @@ public class SketchConstantPostAggregator implements PostAggregator { private final String name; + private final String value; private final SketchHolder sketchValue; @JsonCreator - public SketchConstantPostAggregator(@JsonProperty("name") String name, @JsonProperty("value") String sketchValue) + public SketchConstantPostAggregator(@JsonProperty("name") String name, @JsonProperty("value") String value) { this.name = name; - this.sketchValue = SketchHolder.deserialize(Preconditions.checkNotNull(sketchValue)); + Preconditions.checkArgument(value != null && !value.isEmpty(), + "Constant value cannot be null or empty, expecting base64 encoded sketch string"); + this.value = value; + this.sketchValue = SketchHolder.deserialize(value); } @Override @@ -57,7 +61,7 @@ public Set getDependentFields() } @Override - public Comparator getComparator() + public Comparator getComparator() { return SketchHolder.COMPARATOR; } @@ -90,8 +94,7 @@ public SketchHolder getSketchValue() @Override public String toString() { - return "SketchConstantPostAggregator{name='" + name + "', value='" - + Base64.encodeBase64String(sketchValue.getSketch().toByteArray()) + "'}"; + return "SketchConstantPostAggregator{name='" + name + "', value='" + value + "'}"; } @Override @@ -103,17 +106,13 @@ public boolean equals(Object o) if (o == null || getClass() != o.getClass()) { return false; } - SketchConstantPostAggregator that = (SketchConstantPostAggregator) o; - if (!this.sketchValue.equals(that.sketchValue)) { return false; } - if (name != null ? !name.equals(that.name) : that.name != null) { return false; } - return true; } @@ -128,6 +127,7 @@ public int hashCode() @Override public byte[] getCacheKey() { - return new CacheKeyBuilder(PostAggregatorIds.THETA_SKETCH_CONSTANT).appendInt(hashCode()).build(); + return new CacheKeyBuilder(PostAggregatorIds.THETA_SKETCH_CONSTANT) + .appendString(DigestUtils.sha1Hex(value)).build(); } } diff --git a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchHolder.java b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchHolder.java index 42c1c98d0f73..c273bd4a76aa 100644 --- a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchHolder.java +++ b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchHolder.java @@ -287,6 +287,11 @@ public static SketchHolder sketchSetOperation(Func func, int sketchSize, Object. } } + /** + * Ideally make use of Sketch's equals and hashCode methods but which are not value based implementations. + * And yet need value based equals and hashCode implementations for SketchHolder. + * Hence using Arrays.equals() and Arrays.hashCode(). + */ @Override public boolean equals(Object o) { @@ -296,8 +301,6 @@ public boolean equals(Object o) if (o == null || getClass() != o.getClass()) { return false; } - // Can't use Sketch's equals method because it is not implemented. - // return this.getSketch().equals(((SketchHolder) o).getSketch()); return Arrays.equals(this.getSketch().toByteArray(), ((SketchHolder) o).getSketch().toByteArray()); } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/ConstantPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/ConstantPostAggregator.java index 5eb3250c9412..80e665b9d833 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/ConstantPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/ConstantPostAggregator.java @@ -46,7 +46,7 @@ public ConstantPostAggregator( ) { this.name = name; - this.constantValue = Preconditions.checkNotNull(constantValue); + this.constantValue = Preconditions.checkNotNull(constantValue, "Constant value cannot be null"); } @Override