diff --git a/api/src/test/java/io/druid/jackson/JacksonExtremeDoubleValuesSerdeTest.java b/api/src/test/java/io/druid/jackson/JacksonExtremeDoubleValuesSerdeTest.java new file mode 100644 index 000000000000..789fd82def97 --- /dev/null +++ b/api/src/test/java/io/druid/jackson/JacksonExtremeDoubleValuesSerdeTest.java @@ -0,0 +1,41 @@ +/* + * 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.jackson; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.Assert; +import org.junit.Test; + +import java.io.IOException; + +public class JacksonExtremeDoubleValuesSerdeTest +{ + @Test + public void testExtremeDoubleValuesSerde() throws IOException + { + ObjectMapper objectMapper = new ObjectMapper(); + for (double value : new double[] {Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY}) { + String serialized = objectMapper.writeValueAsString(value); + Assert.assertEquals(new Double(value), objectMapper.readValue(serialized, Double.class)); + } + String negativeInfinityString = objectMapper.writeValueAsString(Double.NaN); + Assert.assertTrue(objectMapper.readValue(negativeInfinityString, Double.class).isNaN()); + } +} diff --git a/bytebuffer-collections/src/main/java/io/druid/collections/spatial/Node.java b/bytebuffer-collections/src/main/java/io/druid/collections/spatial/Node.java index 1206083799c1..e93df949c433 100755 --- a/bytebuffer-collections/src/main/java/io/druid/collections/spatial/Node.java +++ b/bytebuffer-collections/src/main/java/io/druid/collections/spatial/Node.java @@ -151,9 +151,9 @@ public boolean enclose() { boolean retVal = false; float[] minCoords = new float[getNumDims()]; - Arrays.fill(minCoords, Float.MAX_VALUE); + Arrays.fill(minCoords, Float.POSITIVE_INFINITY); float[] maxCoords = new float[getNumDims()]; - Arrays.fill(maxCoords, -Float.MAX_VALUE); + Arrays.fill(maxCoords, Float.NEGATIVE_INFINITY); for (Node child : getChildren()) { for (int i = 0; i < getNumDims(); i++) { diff --git a/bytebuffer-collections/src/main/java/io/druid/collections/spatial/RTree.java b/bytebuffer-collections/src/main/java/io/druid/collections/spatial/RTree.java index 74ad9426802a..6cce6b90e75f 100755 --- a/bytebuffer-collections/src/main/java/io/druid/collections/spatial/RTree.java +++ b/bytebuffer-collections/src/main/java/io/druid/collections/spatial/RTree.java @@ -128,8 +128,8 @@ private Node buildRoot(boolean isLeaf) { float[] initMinCoords = new float[numDims]; float[] initMaxCoords = new float[numDims]; - Arrays.fill(initMinCoords, -Float.MAX_VALUE); - Arrays.fill(initMaxCoords, Float.MAX_VALUE); + Arrays.fill(initMinCoords, Float.NEGATIVE_INFINITY); + Arrays.fill(initMaxCoords, Float.POSITIVE_INFINITY); return new Node(initMinCoords, initMaxCoords, isLeaf, bitmapFactory); } @@ -178,7 +178,7 @@ private Node chooseLeaf(Node node, Point point) return node; } - double minCost = Double.MAX_VALUE; + double minCost = Double.POSITIVE_INFINITY; Node optimal = node.getChildren().get(0); for (Node child : node.getChildren()) { double cost = RTreeUtils.getExpansionCost(child, point); diff --git a/bytebuffer-collections/src/main/java/io/druid/collections/spatial/split/LinearGutmanSplitStrategy.java b/bytebuffer-collections/src/main/java/io/druid/collections/spatial/split/LinearGutmanSplitStrategy.java index a193f466cff5..e4249f35f39a 100755 --- a/bytebuffer-collections/src/main/java/io/druid/collections/spatial/split/LinearGutmanSplitStrategy.java +++ b/bytebuffer-collections/src/main/java/io/druid/collections/spatial/split/LinearGutmanSplitStrategy.java @@ -58,10 +58,10 @@ public Node[] pickSeeds(List nodes) double bestNormalized = 0.0; for (int i = 0; i < numDims; i++) { - float minCoord = Float.MAX_VALUE; - float maxCoord = -Float.MAX_VALUE; - float highestLowSide = -Float.MAX_VALUE; - float lowestHighside = Float.MAX_VALUE; + float minCoord = Float.POSITIVE_INFINITY; + float maxCoord = Float.NEGATIVE_INFINITY; + float lowestHighside = Float.POSITIVE_INFINITY; + float highestLowSide = Float.NEGATIVE_INFINITY; int highestLowSideIndex = 0; int lowestHighSideIndex = 0; diff --git a/bytebuffer-collections/src/main/java/io/druid/collections/spatial/split/QuadraticGutmanSplitStrategy.java b/bytebuffer-collections/src/main/java/io/druid/collections/spatial/split/QuadraticGutmanSplitStrategy.java index 444fc5ecaeb0..a1808919d13c 100755 --- a/bytebuffer-collections/src/main/java/io/druid/collections/spatial/split/QuadraticGutmanSplitStrategy.java +++ b/bytebuffer-collections/src/main/java/io/druid/collections/spatial/split/QuadraticGutmanSplitStrategy.java @@ -37,7 +37,7 @@ public QuadraticGutmanSplitStrategy(int minNumChildren, int maxNumChildren, Bitm @Override public Node[] pickSeeds(List nodes) { - double highestCost = Double.MIN_VALUE; + double highestCost = Double.NEGATIVE_INFINITY; int[] highestCostIndices = new int[2]; for (int i = 0; i < nodes.size() - 1; i++) { @@ -58,7 +58,7 @@ public Node[] pickSeeds(List nodes) @Override public Node pickNext(List nodes, Node[] groups) { - double highestCost = Double.MIN_VALUE; + double highestCost = Double.NEGATIVE_INFINITY; Node costlyNode = null; int counter = 0; int index = -1; diff --git a/codestyle/checkstyle.xml b/codestyle/checkstyle.xml index fc79f49e9084..0c994b818c03 100644 --- a/codestyle/checkstyle.xml +++ b/codestyle/checkstyle.xml @@ -71,10 +71,31 @@ + + + + + + + + + + + + + + + + + + + + + diff --git a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/ApproximateHistogram.java b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/ApproximateHistogram.java index 50d941f0af22..da4b08513751 100644 --- a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/ApproximateHistogram.java +++ b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/ApproximateHistogram.java @@ -335,11 +335,11 @@ public void offer(float value) // or merge existing bins before inserting the new one int minPos = minDeltaIndex(); - float minDelta = minPos >= 0 ? positions[minPos + 1] - positions[minPos] : Float.MAX_VALUE; + float minDelta = minPos >= 0 ? positions[minPos + 1] - positions[minPos] : Float.POSITIVE_INFINITY; // determine the distance of new value to the nearest bins - final float deltaRight = insertAt < binCount ? positions[insertAt] - value : Float.MAX_VALUE; - final float deltaLeft = insertAt > 0 ? value - positions[insertAt - 1] : Float.MAX_VALUE; + final float deltaRight = insertAt < binCount ? positions[insertAt] - value : Float.POSITIVE_INFINITY; + final float deltaLeft = insertAt > 0 ? value - positions[insertAt - 1] : Float.POSITIVE_INFINITY; boolean mergeValue = false; if (deltaRight < minDelta) { @@ -368,7 +368,7 @@ public void offer(float value) protected int minDeltaIndex() { // determine minimum distance between existing bins - float minDelta = Float.MAX_VALUE; + float minDelta = Float.POSITIVE_INFINITY; int minPos = -1; for (int i = 0; i < binCount - 1; ++i) { float delta = (positions[i + 1] - positions[i]); @@ -886,9 +886,6 @@ private static void mergeBins( while (i < numMerge) { // find the smallest delta within the range used for bins - // pick minimum delta by scanning array - //int currentIndex = minIndex(deltas, lastValidIndex); - // pick minimum delta index using min-heap int currentIndex = heap[0]; @@ -908,17 +905,13 @@ private static void mergeBins( final float mm0 = (m0 - m1) * w + m1; mergedPositions[currentIndex] = mm0; - //mergedPositions[nextIndex] = Float.MAX_VALUE; // for debugging mergedBins[currentIndex] = sum | APPROX_FLAG_BIT; - //mergedBins[nextIndex] = -1; // for debugging // update deltas and min-heap if (nextIndex == lastValidIndex) { // merged bin is the last => remove the current bin delta from the heap heapSize = heapDelete(heap, reverseIndex, heapSize, reverseIndex[currentIndex], deltas); - - //deltas[currentIndex] = Float.MAX_VALUE; // for debugging } else { // merged bin is not the last => remove the merged bin delta from the heap heapSize = heapDelete(heap, reverseIndex, heapSize, reverseIndex[nextIndex], deltas); @@ -938,9 +931,6 @@ private static void mergeBins( siftDown(heap, reverseIndex, reverseIndex[prevIndex], heapSize - 1, deltas); } - // mark the merged bin as invalid - // deltas[nextIndex] = Float.MAX_VALUE; // for debugging - // update last valid index if we merged the last bin if (nextIndex == lastValidIndex) { lastValidIndex = currentIndex; @@ -1037,7 +1027,7 @@ private static int heapDelete(int[] heap, int[] reverseIndex, int count, int hea private static int minIndex(float[] deltas, int lastValidIndex) { int minIndex = -1; - float min = Float.MAX_VALUE; + float min = Float.POSITIVE_INFINITY; for (int k = 0; k < lastValidIndex; ++k) { float value = deltas[k]; if (value < min) { diff --git a/hll/src/main/java/io/druid/hll/HyperLogLogCollector.java b/hll/src/main/java/io/druid/hll/HyperLogLogCollector.java index f5465bb53e1c..e09f35d602ed 100644 --- a/hll/src/main/java/io/druid/hll/HyperLogLogCollector.java +++ b/hll/src/main/java/io/druid/hll/HyperLogLogCollector.java @@ -146,7 +146,7 @@ public static double applyCorrection(double e, int zeroCount) final double ratio = e / TWO_TO_THE_SIXTY_FOUR; if (ratio >= 1) { // handle very unlikely case that value is > 2^64 - return Double.MAX_VALUE; + return Double.POSITIVE_INFINITY; } else { return -TWO_TO_THE_SIXTY_FOUR * Math.log(1 - ratio); } diff --git a/hll/src/test/java/io/druid/hll/HyperLogLogCollectorTest.java b/hll/src/test/java/io/druid/hll/HyperLogLogCollectorTest.java index 5c7744963ed8..fc3b05ae4a69 100644 --- a/hll/src/test/java/io/druid/hll/HyperLogLogCollectorTest.java +++ b/hll/src/test/java/io/druid/hll/HyperLogLogCollectorTest.java @@ -693,7 +693,7 @@ public void testHighBits() throws Exception fillBuckets(collector, (byte) 0, (byte) 63); collector.add(new byte[]{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}); - Assert.assertEquals(Double.MAX_VALUE, collector.estimateCardinality(), 1000); + Assert.assertEquals(Double.POSITIVE_INFINITY, collector.estimateCardinality(), 1000); } @Test diff --git a/processing/src/main/java/io/druid/query/aggregation/DoubleMaxAggregatorFactory.java b/processing/src/main/java/io/druid/query/aggregation/DoubleMaxAggregatorFactory.java index db928b4b686f..a6dc784b9115 100644 --- a/processing/src/main/java/io/druid/query/aggregation/DoubleMaxAggregatorFactory.java +++ b/processing/src/main/java/io/druid/query/aggregation/DoubleMaxAggregatorFactory.java @@ -86,7 +86,13 @@ public BufferAggregator factorizeBuffered(ColumnSelectorFactory metricFactory) private FloatColumnSelector getFloatColumnSelector(ColumnSelectorFactory metricFactory) { - return AggregatorUtil.getFloatColumnSelector(metricFactory, macroTable, fieldName, expression, Float.MIN_VALUE); + return AggregatorUtil.getFloatColumnSelector( + metricFactory, + macroTable, + fieldName, + expression, + Float.NEGATIVE_INFINITY + ); } @Override diff --git a/processing/src/main/java/io/druid/query/aggregation/DoubleMinAggregatorFactory.java b/processing/src/main/java/io/druid/query/aggregation/DoubleMinAggregatorFactory.java index 1ef0e12cd841..cd5963046081 100644 --- a/processing/src/main/java/io/druid/query/aggregation/DoubleMinAggregatorFactory.java +++ b/processing/src/main/java/io/druid/query/aggregation/DoubleMinAggregatorFactory.java @@ -87,7 +87,13 @@ public BufferAggregator factorizeBuffered(ColumnSelectorFactory metricFactory) private FloatColumnSelector getFloatColumnSelector(ColumnSelectorFactory metricFactory) { - return AggregatorUtil.getFloatColumnSelector(metricFactory, macroTable, fieldName, expression, Float.MAX_VALUE); + return AggregatorUtil.getFloatColumnSelector( + metricFactory, + macroTable, + fieldName, + expression, + Float.POSITIVE_INFINITY + ); } @Override diff --git a/processing/src/main/java/io/druid/query/aggregation/Histogram.java b/processing/src/main/java/io/druid/query/aggregation/Histogram.java index b77d2607f70d..3beda4ea7abe 100644 --- a/processing/src/main/java/io/druid/query/aggregation/Histogram.java +++ b/processing/src/main/java/io/druid/query/aggregation/Histogram.java @@ -42,8 +42,8 @@ public Histogram(float[] breaks) { this.breaks = breaks; this.bins = new long[this.breaks.length + 1]; this.count = 0; - this.min = Float.MAX_VALUE; - this.max = Float.MIN_VALUE; + this.min = Float.POSITIVE_INFINITY; + this.max = Float.NEGATIVE_INFINITY; } public Histogram(float[] breaks, long[] bins, float min, float max) { diff --git a/processing/src/main/java/io/druid/query/aggregation/HistogramBufferAggregator.java b/processing/src/main/java/io/druid/query/aggregation/HistogramBufferAggregator.java index 4a6dd9a85087..66b2a2fbd0e5 100644 --- a/processing/src/main/java/io/druid/query/aggregation/HistogramBufferAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/HistogramBufferAggregator.java @@ -50,8 +50,8 @@ public void init(ByteBuffer buf, int position) final long[] bins = new long[breaks.length + 1]; mutationBuffer.asLongBuffer().put(bins); - mutationBuffer.putFloat(position + minOffset, Float.MAX_VALUE); - mutationBuffer.putFloat(position + maxOffset, Float.MIN_VALUE); + mutationBuffer.putFloat(position + minOffset, Float.POSITIVE_INFINITY); + mutationBuffer.putFloat(position + maxOffset, Float.NEGATIVE_INFINITY); } @Override diff --git a/processing/src/main/java/io/druid/segment/FloatDimensionIndexer.java b/processing/src/main/java/io/druid/segment/FloatDimensionIndexer.java index a7f5f75450c5..763f4edd9801 100644 --- a/processing/src/main/java/io/druid/segment/FloatDimensionIndexer.java +++ b/processing/src/main/java/io/druid/segment/FloatDimensionIndexer.java @@ -69,13 +69,13 @@ public Indexed getSortedIndexedValues() @Override public Float getMinValue() { - return Float.MIN_VALUE; + return Float.NEGATIVE_INFINITY; } @Override public Float getMaxValue() { - return Float.MAX_VALUE; + return Float.POSITIVE_INFINITY; } @Override diff --git a/processing/src/test/java/io/druid/query/aggregation/HistogramTest.java b/processing/src/test/java/io/druid/query/aggregation/HistogramTest.java index 1ac89cd857b9..78ec75fa7cf9 100644 --- a/processing/src/test/java/io/druid/query/aggregation/HistogramTest.java +++ b/processing/src/test/java/io/druid/query/aggregation/HistogramTest.java @@ -45,6 +45,25 @@ public void testOffer() { Assert.assertEquals("histogram matches expected histogram", hExpected, h); } + /** + * This test differs from {@link #testOffer()} only in that it offers only negative values into Histogram. It's to + * expose the issue of using Float's MIN_VALUE that is actually positive as initial value for {@link Histogram#max}. + */ + @Test + public void testOfferOnlyNegative() { + final float[] values = {-0.3f, -.1f, -0.8f, -.7f, -.5f, -3f}; + final float[] breaks = {-1f, -0.5f, 0.0f, 0.5f, 1f}; + + Histogram hExpected = new Histogram(breaks, new long[]{1,3,2,0,0,0}, -3f, -0.1f); + + Histogram h = new Histogram(breaks); + for(float v : values) { + h.offer(v); + } + + Assert.assertEquals("histogram matches expected histogram", hExpected, h); + } + @Test public void testToFromBytes() { float[] breaks = {-1f, -0.5f, 0.0f, 0.5f, 1f}; diff --git a/processing/src/test/java/io/druid/segment/data/CompressedFloatsSerdeTest.java b/processing/src/test/java/io/druid/segment/data/CompressedFloatsSerdeTest.java index bc043a3a905f..9b9abd278677 100644 --- a/processing/src/test/java/io/druid/segment/data/CompressedFloatsSerdeTest.java +++ b/processing/src/test/java/io/druid/segment/data/CompressedFloatsSerdeTest.java @@ -69,8 +69,8 @@ public static Iterable compressionStrategies() private final float values5[] = {123.16f, 1.12f, 62.00f, 462.12f, 517.71f, 56.54f, 971.32f, 824.22f, 472.12f, 625.26f}; private final float values6[] = {1000000f, 1000001f, 1000002f, 1000003f, 1000004f, 1000005f, 1000006f, 1000007f, 1000008f}; private final float values7[] = { - Float.MAX_VALUE, Float.MIN_VALUE, 12378.5734f, -12718243.7496f, -93653653.1f, 12743153.385534f, 21431.414538f, - 65487435436632.123f, -43734526234564.65f + Float.POSITIVE_INFINITY, Float.NEGATIVE_INFINITY, 12378.5734f, -12718243.7496f, -93653653.1f, 12743153.385534f, + 21431.414538f, 65487435436632.123f, -43734526234564.65f }; public CompressedFloatsSerdeTest( diff --git a/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexTest.java b/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexTest.java index afac0e2a6d3a..1b1856d3cba0 100644 --- a/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexTest.java +++ b/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexTest.java @@ -214,7 +214,7 @@ public void testNullDimensionTransform() throws IndexSizeExceededException Lists.newArrayList("string", "float", "long"), ImmutableMap.of( "string", Arrays.asList("A", null, ""), - "float", Arrays.asList(Float.MAX_VALUE, null, ""), + "float", Arrays.asList(Float.POSITIVE_INFINITY, null, ""), "long", Arrays.asList(Long.MIN_VALUE, null, "") ) ) @@ -223,7 +223,7 @@ public void testNullDimensionTransform() throws IndexSizeExceededException Row row = index.iterator().next(); Assert.assertEquals(Arrays.asList(new String[]{"", "", "A"}), row.getRaw("string")); - Assert.assertEquals(Arrays.asList(new String[]{"", "", String.valueOf(Float.MAX_VALUE)}), row.getRaw("float")); + Assert.assertEquals(Arrays.asList(new String[]{"", "", String.valueOf(Float.POSITIVE_INFINITY)}), row.getRaw("float")); Assert.assertEquals(Arrays.asList(new String[]{"", "", String.valueOf(Long.MIN_VALUE)}), row.getRaw("long")); }