From f1dc7109ce87243918cfb9c65a892a3daa748695 Mon Sep 17 00:00:00 2001 From: aP0StAl Date: Thu, 26 Mar 2020 14:44:40 +0300 Subject: [PATCH 1/4] fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder --- .../query/aggregation/mean/DoubleMeanHolder.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/mean/DoubleMeanHolder.java b/processing/src/main/java/org/apache/druid/query/aggregation/mean/DoubleMeanHolder.java index f42c993b68aa..06aac1a9bb4a 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/mean/DoubleMeanHolder.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/mean/DoubleMeanHolder.java @@ -30,7 +30,7 @@ public class DoubleMeanHolder { - public static final int MAX_INTERMEDIATE_SIZE = Long.SIZE + Double.SIZE; + public static final int MAX_INTERMEDIATE_SIZE = Long.BYTES + Double.BYTES; public static final Comparator COMPARATOR = (o1, o2) -> Doubles.compare(o1.mean(), o2.mean()); private double sum; @@ -62,16 +62,16 @@ public double mean() public byte[] toBytes() { - ByteBuffer buf = ByteBuffer.allocate(Double.SIZE + Long.SIZE); + ByteBuffer buf = ByteBuffer.allocate(Double.BYTES + Long.BYTES); buf.putDouble(0, sum); - buf.putLong(Double.SIZE, count); + buf.putLong(Double.BYTES, count); return buf.array(); } public static DoubleMeanHolder fromBytes(byte[] data) { ByteBuffer buf = ByteBuffer.wrap(data); - return new DoubleMeanHolder(buf.getDouble(0), buf.getLong(Double.SIZE)); + return new DoubleMeanHolder(buf.getDouble(0), buf.getLong(Double.BYTES)); } public static void init(ByteBuffer buf, int position) @@ -109,12 +109,12 @@ private static double getSum(ByteBuffer buf, int position) private static void writeCount(ByteBuffer buf, int position, long count) { - buf.putLong(position + Double.SIZE, count); + buf.putLong(position + Double.BYTES, count); } private static long getCount(ByteBuffer buf, int position) { - return buf.getLong(position + Double.SIZE); + return buf.getLong(position + Double.BYTES); } public static class Serializer extends JsonSerializer From eea4beda8da56fa35ac191d1d7db3a6b5d2f31c9 Mon Sep 17 00:00:00 2001 From: Stanislav Date: Fri, 27 Mar 2020 21:35:37 +0300 Subject: [PATCH 2/4] byte[] type handling in deserialize and finalizeComputation for DoubleMeanAggregatorFactory --- .../query/aggregation/mean/DoubleMeanAggregatorFactory.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/mean/DoubleMeanAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/mean/DoubleMeanAggregatorFactory.java index 5ed87bee66c2..73170ce6694b 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/mean/DoubleMeanAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/mean/DoubleMeanAggregatorFactory.java @@ -152,6 +152,8 @@ public Object deserialize(Object object) return DoubleMeanHolder.fromBytes(StringUtils.decodeBase64(StringUtils.toUtf8((String) object))); } else if (object instanceof DoubleMeanHolder) { return object; + } else if (object instanceof byte[]) { + return DoubleMeanHolder.fromBytes((byte[]) object); } else { throw new IAE("Unknown object type [%s]", Utils.safeObjectClassGetName(object)); } @@ -165,6 +167,8 @@ public Object finalizeComputation(@Nullable Object object) return ((DoubleMeanHolder) object).mean(); } else if (object == null) { return null; + } else if (object instanceof byte[]) { + return DoubleMeanHolder.fromBytes((byte[]) object).mean(); } else { throw new IAE("Unknown object type [%s]", object.getClass().getName()); } From d6dbea790f69f38d1dda1f10c758e93a875571ad Mon Sep 17 00:00:00 2001 From: Stanislav Date: Fri, 27 Mar 2020 21:36:29 +0300 Subject: [PATCH 3/4] DoubleMeanAggregatorFactory tests: Max Intermediate Size, Deserialize, finalizeComputation --- .../mean/DoubleMeanAggregatorFactoryTest.java | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 processing/src/test/java/org/apache/druid/query/aggregation/mean/DoubleMeanAggregatorFactoryTest.java diff --git a/processing/src/test/java/org/apache/druid/query/aggregation/mean/DoubleMeanAggregatorFactoryTest.java b/processing/src/test/java/org/apache/druid/query/aggregation/mean/DoubleMeanAggregatorFactoryTest.java new file mode 100644 index 000000000000..0e49363f16eb --- /dev/null +++ b/processing/src/test/java/org/apache/druid/query/aggregation/mean/DoubleMeanAggregatorFactoryTest.java @@ -0,0 +1,66 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 org.apache.druid.query.aggregation.mean; + +import org.apache.commons.lang3.builder.EqualsBuilder; +import org.junit.Assert; +import org.junit.Test; + +public class DoubleMeanAggregatorFactoryTest +{ + @Test + public void testMaxIntermediateSize() + { + DoubleMeanAggregatorFactory factory = new DoubleMeanAggregatorFactory("name", "fieldName"); + Assert.assertEquals(Double.BYTES + Long.BYTES, factory.getMaxIntermediateSize()); + Assert.assertEquals(Double.BYTES + Long.BYTES, factory.getMaxIntermediateSizeWithNulls()); + } + + @Test + public void testDeserialyze() + { + DoubleMeanAggregatorFactory factory = new DoubleMeanAggregatorFactory("name", "fieldName"); + DoubleMeanHolder expectedHolder = new DoubleMeanHolder(50.0, 10L); + + DoubleMeanHolder actualHolder = (DoubleMeanHolder) factory.deserialize(expectedHolder); + Assert.assertTrue(EqualsBuilder.reflectionEquals(expectedHolder, actualHolder)); + + actualHolder = (DoubleMeanHolder) factory.deserialize(expectedHolder.toBytes()); + Assert.assertTrue(EqualsBuilder.reflectionEquals(expectedHolder, actualHolder)); + } + + @Test + public void testFinalizeComputation() + { + DoubleMeanAggregatorFactory factory = new DoubleMeanAggregatorFactory("name", "fieldName"); + double sum = 50.0; + long count = 10L; + double expecterMean = sum / count; + DoubleMeanHolder holder = new DoubleMeanHolder(sum, count); + + double actualMean = (Double) factory.finalizeComputation(holder); + Assert.assertEquals("", expecterMean, actualMean, 1e-6); + + actualMean = (Double) factory.finalizeComputation(holder.toBytes()); + Assert.assertEquals("", expecterMean, actualMean, 1e-6); + + Assert.assertNull(factory.finalizeComputation(null)); + } +} From ad2d8842022024417aba6d4d3e2f2db7c3b031c5 Mon Sep 17 00:00:00 2001 From: aP0StAl Date: Fri, 27 Mar 2020 21:59:32 +0300 Subject: [PATCH 4/4] moved byte[] check to first position --- .../mean/DoubleMeanAggregatorFactory.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/mean/DoubleMeanAggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/mean/DoubleMeanAggregatorFactory.java index 73170ce6694b..d1da4d3189f0 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/mean/DoubleMeanAggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/mean/DoubleMeanAggregatorFactory.java @@ -148,12 +148,12 @@ public List getRequiredColumns() @Override public Object deserialize(Object object) { - if (object instanceof String) { + if (object instanceof byte[]) { + return DoubleMeanHolder.fromBytes((byte[]) object); + } else if (object instanceof String) { return DoubleMeanHolder.fromBytes(StringUtils.decodeBase64(StringUtils.toUtf8((String) object))); } else if (object instanceof DoubleMeanHolder) { return object; - } else if (object instanceof byte[]) { - return DoubleMeanHolder.fromBytes((byte[]) object); } else { throw new IAE("Unknown object type [%s]", Utils.safeObjectClassGetName(object)); } @@ -163,12 +163,12 @@ public Object deserialize(Object object) @Override public Object finalizeComputation(@Nullable Object object) { - if (object instanceof DoubleMeanHolder) { + if (object instanceof byte[]) { + return DoubleMeanHolder.fromBytes((byte[]) object).mean(); + } else if (object instanceof DoubleMeanHolder) { return ((DoubleMeanHolder) object).mean(); } else if (object == null) { return null; - } else if (object instanceof byte[]) { - return DoubleMeanHolder.fromBytes((byte[]) object).mean(); } else { throw new IAE("Unknown object type [%s]", object.getClass().getName()); }