From 898c1c21af2e200c04088d0f1f46fcf091c43a3e Mon Sep 17 00:00:00 2001 From: Navis Ryu Date: Wed, 26 Oct 2016 02:31:51 +0900 Subject: [PATCH] More best-effort parse long (#3603) * More best-effort parse long * addressed comments --- .../io/druid/common/guava/GuavaUtils.java | 13 ++++++ .../main/java/io/druid/math/expr/Evals.java | 4 +- .../io/druid/common/guava/GuavaUtilsTest.java | 40 +++++++++++++++++++ .../io/druid/query/filter/BoundDimFilter.java | 6 +-- .../io/druid/query/filter/InDimFilter.java | 4 +- .../druid/query/filter/SelectorDimFilter.java | 4 +- .../query/ordering/StringComparators.java | 6 +-- .../java/io/druid/segment/filter/Filters.java | 4 +- 8 files changed, 67 insertions(+), 14 deletions(-) create mode 100644 common/src/test/java/io/druid/common/guava/GuavaUtilsTest.java diff --git a/common/src/main/java/io/druid/common/guava/GuavaUtils.java b/common/src/main/java/io/druid/common/guava/GuavaUtils.java index 01697ef0e0f..d81ea621df1 100644 --- a/common/src/main/java/io/druid/common/guava/GuavaUtils.java +++ b/common/src/main/java/io/druid/common/guava/GuavaUtils.java @@ -21,9 +21,11 @@ package io.druid.common.guava; import com.google.common.base.Charsets; import com.google.common.base.Function; +import com.google.common.base.Strings; import com.google.common.collect.Iterables; import com.google.common.io.CharStreams; import com.google.common.io.InputSupplier; +import com.google.common.primitives.Longs; import javax.annotation.Nullable; import java.io.BufferedReader; @@ -95,4 +97,15 @@ public class GuavaUtils } }; } + + /** + * To fix semantic difference of Longs.tryParse() from Long.parseLong (Longs.tryParse() returns null for '+' started value) + */ + @Nullable + public static Long tryParseLong(@Nullable String string) + { + return Strings.isNullOrEmpty(string) + ? null + : Longs.tryParse(string.charAt(0) == '+' ? string.substring(1) : string); + } } diff --git a/common/src/main/java/io/druid/math/expr/Evals.java b/common/src/main/java/io/druid/math/expr/Evals.java index c608c1e9978..2ba6eff8c99 100644 --- a/common/src/main/java/io/druid/math/expr/Evals.java +++ b/common/src/main/java/io/druid/math/expr/Evals.java @@ -19,7 +19,7 @@ package io.druid.math.expr; -import com.google.common.primitives.Longs; +import io.druid.common.guava.GuavaUtils; /** */ @@ -34,7 +34,7 @@ public class Evals return (Number) value; } String stringValue = String.valueOf(value); - Long longValue = Longs.tryParse(stringValue); + Long longValue = GuavaUtils.tryParseLong(stringValue); if (longValue == null) { return Double.valueOf(stringValue); } diff --git a/common/src/test/java/io/druid/common/guava/GuavaUtilsTest.java b/common/src/test/java/io/druid/common/guava/GuavaUtilsTest.java new file mode 100644 index 00000000000..39af5fbd93d --- /dev/null +++ b/common/src/test/java/io/druid/common/guava/GuavaUtilsTest.java @@ -0,0 +1,40 @@ +/* + * 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.common.guava; + +import com.google.common.primitives.Longs; +import org.junit.Assert; +import org.junit.Test; + +public class GuavaUtilsTest +{ + @Test + public void testParsLong() + { + Assert.assertNull(Longs.tryParse("+100")); + Assert.assertNull(GuavaUtils.tryParseLong("")); + Assert.assertNull(GuavaUtils.tryParseLong(null)); + Assert.assertNull(GuavaUtils.tryParseLong("+")); + Assert.assertNull(GuavaUtils.tryParseLong("++100")); + Assert.assertEquals((Object) Long.parseLong("+100"), GuavaUtils.tryParseLong("+100")); + Assert.assertEquals((Object) Long.parseLong("-100"), GuavaUtils.tryParseLong("-100")); + Assert.assertNotEquals(new Long(100), GuavaUtils.tryParseLong("+101")); + } +} diff --git a/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java b/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java index 5b9388cba58..98db495d6e7 100644 --- a/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java +++ b/processing/src/main/java/io/druid/query/filter/BoundDimFilter.java @@ -28,7 +28,7 @@ import com.google.common.collect.BoundType; import com.google.common.collect.Range; import com.google.common.collect.RangeSet; import com.google.common.collect.TreeRangeSet; -import com.google.common.primitives.Longs; +import io.druid.common.guava.GuavaUtils; import io.druid.java.util.common.StringUtils; import io.druid.query.extraction.ExtractionFn; import io.druid.query.ordering.StringComparator; @@ -334,7 +334,7 @@ public class BoundDimFilter implements DimFilter return; } - Long lowerLong = Longs.tryParse(Strings.nullToEmpty(lower)); + Long lowerLong = GuavaUtils.tryParseLong(lower); if (hasLowerBound() && lowerLong != null) { hasLowerLongBoundVolatile = true; lowerLongBoundVolatile = lowerLong; @@ -342,7 +342,7 @@ public class BoundDimFilter implements DimFilter hasLowerLongBoundVolatile = false; } - Long upperLong = Longs.tryParse(Strings.nullToEmpty(upper)); + Long upperLong = GuavaUtils.tryParseLong(upper); if (hasUpperBound() && upperLong != null) { hasUpperLongBoundVolatile = true; upperLongBoundVolatile = upperLong; diff --git a/processing/src/main/java/io/druid/query/filter/InDimFilter.java b/processing/src/main/java/io/druid/query/filter/InDimFilter.java index bde619e946e..cdf649366a5 100644 --- a/processing/src/main/java/io/druid/query/filter/InDimFilter.java +++ b/processing/src/main/java/io/druid/query/filter/InDimFilter.java @@ -30,7 +30,7 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Range; import com.google.common.collect.RangeSet; import com.google.common.collect.TreeRangeSet; -import com.google.common.primitives.Longs; +import io.druid.common.guava.GuavaUtils; import io.druid.java.util.common.StringUtils; import io.druid.query.extraction.ExtractionFn; import io.druid.query.lookup.LookupExtractionFn; @@ -254,7 +254,7 @@ public class InDimFilter implements DimFilter List longs = new ArrayList<>(); for (String value : values) { - Long longValue = Longs.tryParse(value); + Long longValue = GuavaUtils.tryParseLong(value); if (longValue != null) { longs.add(longValue); } diff --git a/processing/src/main/java/io/druid/query/filter/SelectorDimFilter.java b/processing/src/main/java/io/druid/query/filter/SelectorDimFilter.java index 1c9cd405cfa..320b51dd737 100644 --- a/processing/src/main/java/io/druid/query/filter/SelectorDimFilter.java +++ b/processing/src/main/java/io/druid/query/filter/SelectorDimFilter.java @@ -28,7 +28,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Range; import com.google.common.collect.RangeSet; import com.google.common.collect.TreeRangeSet; -import com.google.common.primitives.Longs; +import io.druid.common.guava.GuavaUtils; import io.druid.java.util.common.StringUtils; import io.druid.query.extraction.ExtractionFn; import io.druid.segment.filter.DimensionPredicateFilter; @@ -221,7 +221,7 @@ public class SelectorDimFilter implements DimFilter if (longsInitialized) { return; } - valueAsLong = Longs.tryParse(value); + valueAsLong = GuavaUtils.tryParseLong(value); longsInitialized = true; } } diff --git a/processing/src/main/java/io/druid/query/ordering/StringComparators.java b/processing/src/main/java/io/druid/query/ordering/StringComparators.java index c97e9a4f402..03f15b111d2 100644 --- a/processing/src/main/java/io/druid/query/ordering/StringComparators.java +++ b/processing/src/main/java/io/druid/query/ordering/StringComparators.java @@ -21,8 +21,8 @@ package io.druid.query.ordering; import com.google.common.collect.Ordering; import com.google.common.primitives.Ints; -import com.google.common.primitives.Longs; import com.google.common.primitives.UnsignedBytes; +import io.druid.common.guava.GuavaUtils; import io.druid.java.util.common.StringUtils; import java.math.BigDecimal; @@ -387,8 +387,8 @@ public class StringComparators // Creating a BigDecimal from a String is expensive (involves copying the String into a char[]) // Converting the String to a Long first is faster. // We optimize here with the assumption that integer values are more common than floating point. - Long long1 = Longs.tryParse(o1); - Long long2 = Longs.tryParse(o2); + Long long1 = GuavaUtils.tryParseLong(o1); + Long long2 = GuavaUtils.tryParseLong(o2); if (long1 != null && long2 != null) { return Long.compare(long1, long2); diff --git a/processing/src/main/java/io/druid/segment/filter/Filters.java b/processing/src/main/java/io/druid/segment/filter/Filters.java index ce991ff58c7..d35bae5e97b 100644 --- a/processing/src/main/java/io/druid/segment/filter/Filters.java +++ b/processing/src/main/java/io/druid/segment/filter/Filters.java @@ -24,8 +24,8 @@ import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; -import com.google.common.primitives.Longs; import com.metamx.collections.bitmap.ImmutableBitmap; +import io.druid.common.guava.GuavaUtils; import io.druid.java.util.common.guava.FunctionalIterable; import io.druid.query.Query; import io.druid.query.filter.BitmapIndexSelector; @@ -171,7 +171,7 @@ public class Filters return new BooleanValueMatcher(false); } - final Long longValue = Longs.tryParse(value.toString()); + final Long longValue = GuavaUtils.tryParseLong(value.toString()); if (longValue == null) { return new BooleanValueMatcher(false); }