From bea321019ae87296b3fe1e53ace4b3b576012a5b Mon Sep 17 00:00:00 2001
From: Suneet Saldanha <suneet.saldanha@imply.io>
Date: Wed, 10 Jun 2020 01:22:16 -0700
Subject: [PATCH 1/9] ROUND and having comparators correctly handle doubles

Double.NaN, Double.POSITIVE_INFINITY and Double.NEGATIVE_INFINITY are not real
numbers. Because of this, they can not be converted to BigDecimal and instead
throw a NumberFormatException.

This change adds support for calculations that produce these numbers either
for use in the `ROUND` function or the HavingSpecMetricComparator by not
attempting to convert the number to a BigDecimal.

The bug in ROUND was first introduced in #7224 where we added the ability to
round to any decimal place. This PR changes the behavior back to using
`Math.round` if we recognize a number that can not be converted to a
BigDecimal.
---
 .../org/apache/druid/math/expr/Function.java  |  7 ++-
 docs/misc/math-expr.md                        |  2 +-
 docs/querying/sql.md                          |  2 +-
 .../having/HavingSpecMetricComparator.java    |  9 ++-
 .../HavingSpecMetricComparatorTest.java       | 60 ++++++++++++++++++
 .../sql/calcite/planner/DruidRexExecutor.java |  1 +
 .../calcite/expression/ExpressionsTest.java   | 63 +++++++++++++++++++
 .../expression/GreatestExpressionTest.java    | 17 +++++
 .../expression/LeastExpressionTest.java       | 18 ++++++
 9 files changed, 175 insertions(+), 4 deletions(-)

diff --git a/core/src/main/java/org/apache/druid/math/expr/Function.java b/core/src/main/java/org/apache/druid/math/expr/Function.java
index af52555793b7..d22e40c1becd 100644
--- a/core/src/main/java/org/apache/druid/math/expr/Function.java
+++ b/core/src/main/java/org/apache/druid/math/expr/Function.java
@@ -737,7 +737,12 @@ private ExprEval eval(ExprEval param, int scale)
       if (param.type() == ExprType.LONG) {
         return ExprEval.of(BigDecimal.valueOf(param.asLong()).setScale(scale, RoundingMode.HALF_UP).longValue());
       } else if (param.type() == ExprType.DOUBLE) {
-        return ExprEval.of(BigDecimal.valueOf(param.asDouble()).setScale(scale, RoundingMode.HALF_UP).doubleValue());
+        double val = param.asDouble();
+        if (Double.isNaN(val) || val == Double.POSITIVE_INFINITY || val == Double.NEGATIVE_INFINITY) {
+          // This is the behavior of Math.round()
+          return ExprEval.of(0L);
+        }
+        return ExprEval.of(BigDecimal.valueOf(val).setScale(scale, RoundingMode.HALF_UP).doubleValue());
       } else {
         return ExprEval.of(null);
       }
diff --git a/docs/misc/math-expr.md b/docs/misc/math-expr.md
index 249eb1ef209c..ed81058d4961 100644
--- a/docs/misc/math-expr.md
+++ b/docs/misc/math-expr.md
@@ -141,7 +141,7 @@ See javadoc of java.lang.Math for detailed explanation for each function.
 |pow|pow(x, y) would return the value of the x raised to the power of y|
 |remainder|remainder(x, y) would return the remainder operation on two arguments as prescribed by the IEEE 754 standard|
 |rint|rint(x) would return value that is closest in value to x and is equal to a mathematical integer|
-|round|round(x, y) would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points.|
+|round|round(x, y) would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If x is either Nan or infinity, this will reurn 0. |
 |scalb|scalb(d, sf) would return d * 2^sf rounded as if performed by a single correctly rounded floating-point multiply to a member of the double value set|
 |signum|signum(x) would return the signum function of the argument x|
 |sin|sin(x) would return the trigonometric sine of an angle x|
diff --git a/docs/querying/sql.md b/docs/querying/sql.md
index dff5c7176e1f..bad5667f0ce0 100644
--- a/docs/querying/sql.md
+++ b/docs/querying/sql.md
@@ -287,7 +287,7 @@ to FLOAT. At runtime, Druid will widen 32-bit floats to 64-bit for most expressi
 |`SQRT(expr)`|Square root.|
 |`TRUNCATE(expr[, digits])`|Truncate expr to a specific number of decimal digits. If digits is negative, then this truncates that many places to the left of the decimal point. Digits defaults to zero if not specified.|
 |`TRUNC(expr[, digits])`|Synonym for `TRUNCATE`.|
-|`ROUND(expr[, digits])`|`ROUND(x, y)` would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points.|
+|`ROUND(expr[, digits])`|`ROUND(x, y)` would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If `expr` evaluates to either NaN or infinity, this will return 0. |
 |`x + y`|Addition.|
 |`x - y`|Subtraction.|
 |`x * y`|Multiplication.|
diff --git a/processing/src/main/java/org/apache/druid/query/groupby/having/HavingSpecMetricComparator.java b/processing/src/main/java/org/apache/druid/query/groupby/having/HavingSpecMetricComparator.java
index 88f50efb8488..e4ed95d3b72d 100644
--- a/processing/src/main/java/org/apache/druid/query/groupby/having/HavingSpecMetricComparator.java
+++ b/processing/src/main/java/org/apache/druid/query/groupby/having/HavingSpecMetricComparator.java
@@ -19,6 +19,7 @@
 
 package org.apache.druid.query.groupby.having;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.primitives.Doubles;
 import com.google.common.primitives.Longs;
 import org.apache.druid.java.util.common.ISE;
@@ -89,10 +90,16 @@ static int compare(String aggregationName, Number value, Map<String, AggregatorF
     }
   }
 
-  private static int compareDoubleToLong(final double a, final long b)
+  @VisibleForTesting
+  static int compareDoubleToLong(final double a, final long b)
   {
     // Use BigDecimal when comparing integers vs floating points, a convenient way to handle all cases (like
     // fractional values, values out of range of max long/max int) without worrying about them ourselves.
+    // The only edge case we need to handle is doubles that can not be converted to a BigDecimal, so fall back to using
+    // Double.compare
+    if (Double.isNaN(a) || a == Double.POSITIVE_INFINITY || a == Double.NEGATIVE_INFINITY) {
+      return Double.compare(a, b);
+    }
     return BigDecimal.valueOf(a).compareTo(BigDecimal.valueOf(b));
   }
 }
diff --git a/processing/src/test/java/org/apache/druid/query/groupby/having/HavingSpecMetricComparatorTest.java b/processing/src/test/java/org/apache/druid/query/groupby/having/HavingSpecMetricComparatorTest.java
index 135328df0035..46ba97583951 100644
--- a/processing/src/test/java/org/apache/druid/query/groupby/having/HavingSpecMetricComparatorTest.java
+++ b/processing/src/test/java/org/apache/druid/query/groupby/having/HavingSpecMetricComparatorTest.java
@@ -41,4 +41,64 @@ public void testLongRegex()
     Assert.assertFalse(HavingSpecMetricComparator.LONG_PAT.matcher("").matches());
     Assert.assertFalse(HavingSpecMetricComparator.LONG_PAT.matcher("xyz").matches());
   }
+
+  @Test
+  public void testCompareDoubleToLongWithNanReturns1()
+  {
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Double.NaN, 1));
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Double.NaN, -1));
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Double.NaN, Long.MAX_VALUE));
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Double.NaN, Long.MIN_VALUE));
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Double.NaN, 0L));
+
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Float.NaN, 1));
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Float.NaN, -1));
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Float.NaN, Long.MAX_VALUE));
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Float.NaN, Long.MIN_VALUE));
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Float.NaN, 0L));
+  }
+
+  @Test
+  public void testCompareDoubleToLongWithInfinityReturns1()
+  {
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Double.POSITIVE_INFINITY, 1));
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Double.POSITIVE_INFINITY, -1));
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Double.POSITIVE_INFINITY, Long.MAX_VALUE));
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Double.POSITIVE_INFINITY, Long.MIN_VALUE));
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Double.POSITIVE_INFINITY, 0L));
+
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Float.POSITIVE_INFINITY, 1));
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Float.POSITIVE_INFINITY, -1));
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Float.POSITIVE_INFINITY, Long.MAX_VALUE));
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Float.POSITIVE_INFINITY, Long.MIN_VALUE));
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(Float.POSITIVE_INFINITY, 0L));
+  }
+
+  @Test
+  public void testCompareDoubleToLongWithInfinityReturnsNegative1()
+  {
+    Assert.assertEquals(-1, HavingSpecMetricComparator.compareDoubleToLong(Double.NEGATIVE_INFINITY, 1));
+    Assert.assertEquals(-1, HavingSpecMetricComparator.compareDoubleToLong(Double.NEGATIVE_INFINITY, -1));
+    Assert.assertEquals(-1, HavingSpecMetricComparator.compareDoubleToLong(Double.NEGATIVE_INFINITY, Long.MAX_VALUE));
+    Assert.assertEquals(-1, HavingSpecMetricComparator.compareDoubleToLong(Double.NEGATIVE_INFINITY, Long.MIN_VALUE));
+    Assert.assertEquals(-1, HavingSpecMetricComparator.compareDoubleToLong(Double.NEGATIVE_INFINITY, 0L));
+
+    Assert.assertEquals(-1, HavingSpecMetricComparator.compareDoubleToLong(Float.NEGATIVE_INFINITY, 1));
+    Assert.assertEquals(-1, HavingSpecMetricComparator.compareDoubleToLong(Float.NEGATIVE_INFINITY, -1));
+    Assert.assertEquals(-1, HavingSpecMetricComparator.compareDoubleToLong(Float.NEGATIVE_INFINITY, Long.MAX_VALUE));
+    Assert.assertEquals(-1, HavingSpecMetricComparator.compareDoubleToLong(Float.NEGATIVE_INFINITY, Long.MIN_VALUE));
+    Assert.assertEquals(-1, HavingSpecMetricComparator.compareDoubleToLong(Float.NEGATIVE_INFINITY, 0L));
+  }
+
+  @Test
+  public void testCompareDoubleToLongWithNumbers()
+  {
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong(1 + 1e-6, 1));
+    Assert.assertEquals(-1, HavingSpecMetricComparator.compareDoubleToLong(1 - 1e-6, 1));
+    Assert.assertEquals(0, HavingSpecMetricComparator.compareDoubleToLong(10D, 10));
+    Assert.assertEquals(0, HavingSpecMetricComparator.compareDoubleToLong(0D, 0));
+    Assert.assertEquals(0, HavingSpecMetricComparator.compareDoubleToLong(-0D, 0));
+    Assert.assertEquals(1, HavingSpecMetricComparator.compareDoubleToLong((double) Long.MAX_VALUE + 1, Long.MAX_VALUE));
+    Assert.assertEquals(-1, HavingSpecMetricComparator.compareDoubleToLong((double) Long.MIN_VALUE - 1, Long.MIN_VALUE));
+  }
 }
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java
index 31244d13b497..a0099a46f765 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java
@@ -122,6 +122,7 @@ public void reduce(
             if (exprResult.type() == ExprType.LONG) {
               bigDecimal = BigDecimal.valueOf(exprResult.asLong());
             } else {
+              // if exprResult evaluates to Nan or infinity, this will throw a NumberFormatException
               bigDecimal = BigDecimal.valueOf(exprResult.asDouble());
             }
 
diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java
index 25034521f99d..2db5ffffcbbf 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java
@@ -77,6 +77,12 @@ public class ExpressionsTest extends ExpressionTestBase
       .add("y", ValueType.LONG)
       .add("z", ValueType.FLOAT)
       .add("s", ValueType.STRING)
+      .add("nan", ValueType.DOUBLE)
+      .add("inf", ValueType.DOUBLE)
+      .add("-inf", ValueType.DOUBLE)
+      .add("fnan", ValueType.FLOAT)
+      .add("finf", ValueType.FLOAT)
+      .add("-finf", ValueType.FLOAT)
       .add("hexstr", ValueType.STRING)
       .add("intstr", ValueType.STRING)
       .add("spacey", ValueType.STRING)
@@ -92,6 +98,13 @@ public class ExpressionsTest extends ExpressionTestBase
       .put("x", 2.25)
       .put("y", 3.0)
       .put("z", -2.25)
+      .put("o", 0)
+      .put("nan", Double.NaN)
+      .put("inf", Double.POSITIVE_INFINITY)
+      .put("-inf", Double.NEGATIVE_INFINITY)
+      .put("fnan", Float.NaN)
+      .put("finf", Float.POSITIVE_INFINITY)
+      .put("-finf", Float.NEGATIVE_INFINITY)
       .put("s", "foo")
       .put("hexstr", "EF")
       .put("intstr", "-100")
@@ -920,6 +933,56 @@ public void testRoundWithInvalidSecondArgument()
     );
   }
 
+  @Test
+  public void testRoundWithNanShouldRoundTo0()
+  {
+    final SqlFunction roundFunction = new RoundOperatorConversion().calciteOperator();
+
+    testHelper.testExpression(
+        roundFunction,
+        testHelper.makeInputRef("nan"),
+        DruidExpression.fromExpression("round(\"nan\")"),
+        0L
+    );
+    testHelper.testExpression(
+        roundFunction,
+        testHelper.makeInputRef("fnan"),
+        DruidExpression.fromExpression("round(\"fnan\")"),
+        0L
+    );
+  }
+
+  @Test
+  public void testRoundWithInfinityShouldRoundTo0()
+  {
+    final SqlFunction roundFunction = new RoundOperatorConversion().calciteOperator();
+
+    testHelper.testExpression(
+        roundFunction,
+        testHelper.makeInputRef("inf"),
+        DruidExpression.fromExpression("round(\"inf\")"),
+        0L
+    );
+    testHelper.testExpression(
+        roundFunction,
+        testHelper.makeInputRef("-inf"),
+        DruidExpression.fromExpression("round(\"-inf\")"),
+        0L
+    );
+    testHelper.testExpression(
+        roundFunction,
+        testHelper.makeInputRef("finf"),
+        DruidExpression.fromExpression("round(\"finf\")"),
+        0L
+    );
+    testHelper.testExpression(
+        roundFunction,
+        testHelper.makeInputRef("-finf"),
+        DruidExpression.fromExpression("round(\"-finf\")"),
+        0L
+    );
+  }
+
   @Test
   public void testDateTrunc()
   {
diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/expression/GreatestExpressionTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/expression/GreatestExpressionTest.java
index fcd96911aecb..58278232ddd4 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/expression/GreatestExpressionTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/expression/GreatestExpressionTest.java
@@ -212,6 +212,23 @@ public void testDecimal()
     );
   }
 
+  @Test
+  public void testDecimalWithNullShouldReturnString()
+  {
+    testExpression(
+        Arrays.asList(
+            testHelper.makeLiteral(BigDecimal.valueOf(1.2)),
+            testHelper.getConstantNull(),
+            testHelper.makeLiteral(BigDecimal.valueOf(3.4))
+        ),
+        buildExpectedExpression(
+            1.2,
+            null,
+            3.4
+        ),
+        "3.4"
+    );
+  }
   @Test
   public void testTimestamp()
   {
diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/expression/LeastExpressionTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/expression/LeastExpressionTest.java
index 1405eac207c4..6eed91bb241c 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/expression/LeastExpressionTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/expression/LeastExpressionTest.java
@@ -212,6 +212,24 @@ public void testDecimal()
     );
   }
 
+  @Test
+  public void testDecimalWithNullShouldReturnString()
+  {
+    testExpression(
+        Arrays.asList(
+            testHelper.makeLiteral(BigDecimal.valueOf(1.2)),
+            testHelper.makeLiteral(BigDecimal.valueOf(3.4)),
+            testHelper.getConstantNull()
+        ),
+        buildExpectedExpression(
+            1.2,
+            3.4,
+            null
+        ),
+        "1.2"
+    );
+  }
+
   @Test
   public void testTimestamp()
   {

From ac09d884279cbc3eb366504c3492c067a6d177e6 Mon Sep 17 00:00:00 2001
From: Suneet Saldanha <suneet.saldanha@imply.io>
Date: Wed, 10 Jun 2020 07:41:21 -0700
Subject: [PATCH 2/9] Add tests and fix spellcheck

---
 .../org/apache/druid/math/expr/Function.java  |  12 +-
 .../apache/druid/math/expr/FunctionTest.java  | 132 +++++++++++++++++-
 docs/misc/math-expr.md                        |   2 +-
 docs/querying/sql.md                          |   2 +-
 4 files changed, 137 insertions(+), 11 deletions(-)

diff --git a/core/src/main/java/org/apache/druid/math/expr/Function.java b/core/src/main/java/org/apache/druid/math/expr/Function.java
index d22e40c1becd..dc967734c090 100644
--- a/core/src/main/java/org/apache/druid/math/expr/Function.java
+++ b/core/src/main/java/org/apache/druid/math/expr/Function.java
@@ -705,7 +705,11 @@ public ExprEval apply(List<Expr> args, Expr.ObjectBinding bindings)
     {
       ExprEval value1 = args.get(0).eval(bindings);
       if (value1.type() != ExprType.LONG && value1.type() != ExprType.DOUBLE) {
-        throw new IAE("The first argument to the function[%s] should be integer or double type but get the %s type", name(), value1.type());
+        throw new IAE(
+            "The first argument to the function[%s] should be integer or double type but got the type: %s",
+            name(),
+            value1.type()
+        );
       }
 
       if (args.size() == 1) {
@@ -713,7 +717,11 @@ public ExprEval apply(List<Expr> args, Expr.ObjectBinding bindings)
       } else {
         ExprEval value2 = args.get(1).eval(bindings);
         if (value2.type() != ExprType.LONG) {
-          throw new IAE("The second argument to the function[%s] should be integer type but get the %s type", name(), value2.type());
+          throw new IAE(
+              "The second argument to the function[%s] should be integer type but got the type: %s",
+              name(),
+              value2.type()
+          );
         }
         return eval(value1, value2.asInt());
       }
diff --git a/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java b/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java
index 2fe52490c400..921ed7113cc3 100644
--- a/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java
+++ b/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java
@@ -20,13 +20,17 @@
 package org.apache.druid.math.expr;
 
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import org.apache.druid.common.config.NullHandling;
+import org.apache.druid.java.util.common.Pair;
 import org.apache.druid.testing.InitializedNullHandlingTest;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
 import javax.annotation.Nullable;
+import java.util.Locale;
+import java.util.Set;
 
 public class FunctionTest extends InitializedNullHandlingTest
 {
@@ -35,13 +39,21 @@ public class FunctionTest extends InitializedNullHandlingTest
   @Before
   public void setup()
   {
-    ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder();
-    builder.put("x", "foo");
-    builder.put("y", 2);
-    builder.put("z", 3.1);
-    builder.put("a", new String[] {"foo", "bar", "baz", "foobar"});
-    builder.put("b", new Long[] {1L, 2L, 3L, 4L, 5L});
-    builder.put("c", new Double[] {3.1, 4.2, 5.3});
+    ImmutableMap.Builder<String, Object> builder = ImmutableMap.<String, Object>builder()
+        .put("x", "foo")
+        .put("y", 2)
+        .put("z", 3.1)
+        .put("d", 34.56D)
+        .put("f", 12.34F)
+        .put("nan", Double.NaN)
+        .put("inf", Double.POSITIVE_INFINITY)
+        .put("-inf", Double.NEGATIVE_INFINITY)
+        .put("o", 0)
+        .put("od", 0D)
+        .put("of", 0F)
+        .put("a", new String[] {"foo", "bar", "baz", "foobar"})
+        .put("b", new Long[] {1L, 2L, 3L, 4L, 5L})
+        .put("c", new Double[] {3.1, 4.2, 5.3});
     bindings = Parser.withMap(builder.build());
   }
 
@@ -293,6 +305,112 @@ public void testArrayPrepend()
     assertArrayExpr("array_prepend(1, <DOUBLE>[])", new Double[]{1.0});
   }
 
+  @Test
+  public void testRoundWithNonNumericValuesShouldReturn0()
+  {
+    assertExpr("round(nan)", 0L);
+    assertExpr("round(nan, 5)", 0L);
+    assertExpr("round(inf)", 0L);
+    assertExpr("round(inf, 4)", 0L);
+    assertExpr("round(-inf)", 0L);
+    assertExpr("round(-inf, 3)", 0L);
+
+    // Calculations that result in non numeric numbers
+    assertExpr("round(0/od)", 0L);
+    assertExpr("round(od/od)", 0L);
+    assertExpr("round(1/od)", 0L);
+    assertExpr("round(-1/od)", 0L);
+
+    assertExpr("round(0/of)", 0L);
+    assertExpr("round(of/of)", 0L);
+    assertExpr("round(1/of)", 0L);
+    assertExpr("round(-1/of)", 0L);
+  }
+
+  @Test
+  public void testRoundWithLong()
+  {
+    assertExpr("round(y)", 2L);
+    assertExpr("round(y, 2)", 2L);
+    assertExpr("round(y, -1)", 0L);
+  }
+
+  @Test
+  public void testRoundWithDouble()
+  {
+    assertExpr("round(d)", 35D);
+    assertExpr("round(d, 2)", 34.56D);
+    assertExpr("round(d, y)", 34.56D);
+    assertExpr("round(d, 1)", 34.6D);
+    assertExpr("round(d, -1)", 30D);
+  }
+
+  @Test
+  public void testRoundWithFloat()
+  {
+    assertExpr("round(f)", 12D);
+    assertExpr("round(f, 2)", 12.34D);
+    assertExpr("round(f, y)", 12.34D);
+    assertExpr("round(f, 1)", 12.3D);
+    assertExpr("round(f, -1)", 10D);
+  }
+
+  @Test
+  public void testRoundWithInvalidFirstArgument()
+  {
+    Set<Pair<String, String>> invalidArguments = ImmutableSet.of(
+        Pair.of("b", "LONG_ARRAY"),
+        Pair.of("x", "STRING"),
+        Pair.of("c", "DOUBLE_ARRAY"),
+        Pair.of("a", "STRING_ARRAY")
+
+    );
+    for (Pair<String, String> argAndType : invalidArguments) {
+      try {
+        assertExpr(String.format(Locale.ENGLISH, "round(%s)", argAndType.lhs), null);
+        Assert.fail("Did not throw IllegalArgumentException");
+      }
+      catch (IllegalArgumentException e) {
+        Assert.assertEquals(
+            String.format(
+                Locale.ENGLISH,
+                "The first argument to the function[round] should be integer or double type but got the type: %s",
+                argAndType.rhs
+            ),
+            e.getMessage()
+        );
+      }
+    }
+  }
+
+  @Test
+  public void testRoundWithInvalidSecondArgument()
+  {
+    Set<Pair<String, String>> invalidArguments = ImmutableSet.of(
+        Pair.of("1.2", "DOUBLE"),
+        Pair.of("x", "STRING"),
+        Pair.of("a", "STRING_ARRAY"),
+        Pair.of("c", "DOUBLE_ARRAY")
+
+    );
+    for (Pair<String, String> argAndType : invalidArguments) {
+      try {
+        assertExpr(String.format(Locale.ENGLISH, "round(d, %s)", argAndType.lhs), null);
+        Assert.fail("Did not throw IllegalArgumentException");
+      }
+      catch (IllegalArgumentException e) {
+        Assert.assertEquals(
+            String.format(
+                Locale.ENGLISH,
+                "The second argument to the function[round] should be integer type but got the type: %s",
+                argAndType.rhs
+            ),
+            e.getMessage()
+        );
+      }
+    }
+  }
+
   @Test
   public void testGreatest()
   {
diff --git a/docs/misc/math-expr.md b/docs/misc/math-expr.md
index ed81058d4961..4a70fd170cc7 100644
--- a/docs/misc/math-expr.md
+++ b/docs/misc/math-expr.md
@@ -141,7 +141,7 @@ See javadoc of java.lang.Math for detailed explanation for each function.
 |pow|pow(x, y) would return the value of the x raised to the power of y|
 |remainder|remainder(x, y) would return the remainder operation on two arguments as prescribed by the IEEE 754 standard|
 |rint|rint(x) would return value that is closest in value to x and is equal to a mathematical integer|
-|round|round(x, y) would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If x is either Nan or infinity, this will reurn 0. |
+|round|round(x, y) would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If x is either `NaN` or infinity, this will return 0. |
 |scalb|scalb(d, sf) would return d * 2^sf rounded as if performed by a single correctly rounded floating-point multiply to a member of the double value set|
 |signum|signum(x) would return the signum function of the argument x|
 |sin|sin(x) would return the trigonometric sine of an angle x|
diff --git a/docs/querying/sql.md b/docs/querying/sql.md
index bad5667f0ce0..e563df3989cc 100644
--- a/docs/querying/sql.md
+++ b/docs/querying/sql.md
@@ -287,7 +287,7 @@ to FLOAT. At runtime, Druid will widen 32-bit floats to 64-bit for most expressi
 |`SQRT(expr)`|Square root.|
 |`TRUNCATE(expr[, digits])`|Truncate expr to a specific number of decimal digits. If digits is negative, then this truncates that many places to the left of the decimal point. Digits defaults to zero if not specified.|
 |`TRUNC(expr[, digits])`|Synonym for `TRUNCATE`.|
-|`ROUND(expr[, digits])`|`ROUND(x, y)` would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If `expr` evaluates to either NaN or infinity, this will return 0. |
+|`ROUND(expr[, digits])`|`ROUND(x, y)` would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If `expr` evaluates to either `NaN` or infinity, this will return 0. |
 |`x + y`|Addition.|
 |`x - y`|Subtraction.|
 |`x * y`|Multiplication.|

From 526964cc2fad25cce996a475825f2f80f83b6bab Mon Sep 17 00:00:00 2001
From: Suneet Saldanha <suneet.saldanha@imply.io>
Date: Wed, 10 Jun 2020 08:52:12 -0700
Subject: [PATCH 3/9] update error message in ExpressionsTest

---
 .../apache/druid/sql/calcite/expression/ExpressionsTest.java  | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java
index 2db5ffffcbbf..9980b15c46db 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java
@@ -903,7 +903,7 @@ public void testRoundWithInvalidArgument()
 
     expectException(
         IAE.class,
-        "The first argument to the function[round] should be integer or double type but get the STRING type"
+        "The first argument to the function[round] should be integer or double type but got the type: STRING"
     );
     testHelper.testExpression(
         roundFunction,
@@ -920,7 +920,7 @@ public void testRoundWithInvalidSecondArgument()
 
     expectException(
         IAE.class,
-        "The second argument to the function[round] should be integer type but get the STRING type"
+        "The second argument to the function[round] should be integer type but got the type: STRING"
     );
     testHelper.testExpression(
         roundFunction,

From 2c3f405b83dd9b1c9cc04cd5e96adf53c934868d Mon Sep 17 00:00:00 2001
From: Suneet Saldanha <suneet.saldanha@imply.io>
Date: Mon, 15 Jun 2020 16:00:14 -0700
Subject: [PATCH 4/9] Address comments

---
 .../org/apache/druid/math/expr/Function.java  |  2 +-
 docs/misc/math-expr.md                        |  2 +-
 docs/querying/sql.md                          |  2 +-
 .../having/HavingSpecMetricComparator.java    |  2 +-
 .../sql/calcite/planner/DruidRexExecutor.java |  6 +++--
 .../druid/sql/calcite/CalciteQueryTest.java   | 24 +++++++++++++++++++
 6 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/core/src/main/java/org/apache/druid/math/expr/Function.java b/core/src/main/java/org/apache/druid/math/expr/Function.java
index dc967734c090..7c67a3766355 100644
--- a/core/src/main/java/org/apache/druid/math/expr/Function.java
+++ b/core/src/main/java/org/apache/druid/math/expr/Function.java
@@ -746,7 +746,7 @@ private ExprEval eval(ExprEval param, int scale)
         return ExprEval.of(BigDecimal.valueOf(param.asLong()).setScale(scale, RoundingMode.HALF_UP).longValue());
       } else if (param.type() == ExprType.DOUBLE) {
         double val = param.asDouble();
-        if (Double.isNaN(val) || val == Double.POSITIVE_INFINITY || val == Double.NEGATIVE_INFINITY) {
+        if (Double.isNaN(val) || Double.isInfinite(val)) {
           // This is the behavior of Math.round()
           return ExprEval.of(0L);
         }
diff --git a/docs/misc/math-expr.md b/docs/misc/math-expr.md
index e8bff9a7dc22..54eaf12027d8 100644
--- a/docs/misc/math-expr.md
+++ b/docs/misc/math-expr.md
@@ -141,7 +141,7 @@ See javadoc of java.lang.Math for detailed explanation for each function.
 |pow|pow(x, y) would return the value of the x raised to the power of y|
 |remainder|remainder(x, y) would return the remainder operation on two arguments as prescribed by the IEEE 754 standard|
 |rint|rint(x) would return value that is closest in value to x and is equal to a mathematical integer|
-|round|round(x, y) would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If x is either `NaN` or infinity, this will return 0. |
+|round|round(x, y) would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If x is either `NaN` or infinity (positive or negative), this will return 0. |
 |scalb|scalb(d, sf) would return d * 2^sf rounded as if performed by a single correctly rounded floating-point multiply to a member of the double value set|
 |signum|signum(x) would return the signum function of the argument x|
 |sin|sin(x) would return the trigonometric sine of an angle x|
diff --git a/docs/querying/sql.md b/docs/querying/sql.md
index c0af25b74baa..de332f523a59 100644
--- a/docs/querying/sql.md
+++ b/docs/querying/sql.md
@@ -287,7 +287,7 @@ to FLOAT. At runtime, Druid will widen 32-bit floats to 64-bit for most expressi
 |`SQRT(expr)`|Square root.|
 |`TRUNCATE(expr[, digits])`|Truncate expr to a specific number of decimal digits. If digits is negative, then this truncates that many places to the left of the decimal point. Digits defaults to zero if not specified.|
 |`TRUNC(expr[, digits])`|Synonym for `TRUNCATE`.|
-|`ROUND(expr[, digits])`|`ROUND(x, y)` would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If `expr` evaluates to either `NaN` or infinity, this will return 0. |
+|`ROUND(expr[, digits])`|`ROUND(x, y)` would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If `expr` evaluates to either `NaN` or infinity (positive or negative), this will return 0. |
 |`x + y`|Addition.|
 |`x - y`|Subtraction.|
 |`x * y`|Multiplication.|
diff --git a/processing/src/main/java/org/apache/druid/query/groupby/having/HavingSpecMetricComparator.java b/processing/src/main/java/org/apache/druid/query/groupby/having/HavingSpecMetricComparator.java
index e4ed95d3b72d..e3f786e1f8aa 100644
--- a/processing/src/main/java/org/apache/druid/query/groupby/having/HavingSpecMetricComparator.java
+++ b/processing/src/main/java/org/apache/druid/query/groupby/having/HavingSpecMetricComparator.java
@@ -97,7 +97,7 @@ static int compareDoubleToLong(final double a, final long b)
     // fractional values, values out of range of max long/max int) without worrying about them ourselves.
     // The only edge case we need to handle is doubles that can not be converted to a BigDecimal, so fall back to using
     // Double.compare
-    if (Double.isNaN(a) || a == Double.POSITIVE_INFINITY || a == Double.NEGATIVE_INFINITY) {
+    if (Double.isNaN(a) || Double.isInfinite(a)) {
       return Double.compare(a, b);
     }
     return BigDecimal.valueOf(a).compareTo(BigDecimal.valueOf(b));
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java
index a0099a46f765..c181cb8c885a 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java
@@ -121,11 +121,13 @@ public void reduce(
           } else {
             if (exprResult.type() == ExprType.LONG) {
               bigDecimal = BigDecimal.valueOf(exprResult.asLong());
+
             } else {
-              // if exprResult evaluates to Nan or infinity, this will throw a NumberFormatException
+              // if exprResult evaluates to Nan or infinity, this will throw a NumberFormatException.
+              // If you find yourself in such a position, consider casting the literal to a BIGINT so that
+              // the query can execute.
               bigDecimal = BigDecimal.valueOf(exprResult.asDouble());
             }
-
             literal = rexBuilder.makeLiteral(bigDecimal, constExp.getType(), true);
           }
         } else if (sqlTypeName == SqlTypeName.ARRAY) {
diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
index 083285870044..9b391a716f1b 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
@@ -139,6 +139,30 @@ public void testSelectConstantExpression() throws Exception
     );
   }
 
+  @Test
+  public void testSelectNonNumericNumberLiterals() throws Exception
+  {
+    // Tests to convert NaN, positive infinity and negative infinity as literals.
+    testQuery(
+        "SELECT"
+        + " CAST(1 / 0.0 AS BIGINT),"
+        + " CAST(1 / -0.0 AS BIGINT),"
+        + " CAST(-1 / 0.0 AS BIGINT),"
+        + " CAST(-1 / -0.0 AS BIGINT),"
+        + " CAST(0/ 0.0 AS BIGINT)",
+        ImmutableList.of(),
+        ImmutableList.of(
+            new Object[] {
+                Long.MAX_VALUE,
+                Long.MAX_VALUE,
+                Long.MIN_VALUE,
+                Long.MIN_VALUE,
+                0L
+            }
+        )
+    );
+  }
+
   @Test
   public void testSelectConstantExpressionFromTable() throws Exception
   {

From 7199359cfb938f79cd1d640faefd50d74eb3a078 Mon Sep 17 00:00:00 2001
From: Suneet Saldanha <suneet.saldanha@imply.io>
Date: Tue, 16 Jun 2020 08:21:51 -0700
Subject: [PATCH 5/9] fix up round for infinity

---
 .../org/apache/druid/math/expr/Function.java  |  3 +-
 .../apache/druid/math/expr/FunctionTest.java  | 34 ++++++++++++++-----
 docs/misc/math-expr.md                        |  2 +-
 docs/querying/sql.md                          |  2 +-
 4 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/core/src/main/java/org/apache/druid/math/expr/Function.java b/core/src/main/java/org/apache/druid/math/expr/Function.java
index 7c67a3766355..db5d77d73e4a 100644
--- a/core/src/main/java/org/apache/druid/math/expr/Function.java
+++ b/core/src/main/java/org/apache/druid/math/expr/Function.java
@@ -747,8 +747,7 @@ private ExprEval eval(ExprEval param, int scale)
       } else if (param.type() == ExprType.DOUBLE) {
         double val = param.asDouble();
         if (Double.isNaN(val) || Double.isInfinite(val)) {
-          // This is the behavior of Math.round()
-          return ExprEval.of(0L);
+          return ExprEval.of(Math.round(val));
         }
         return ExprEval.of(BigDecimal.valueOf(val).setScale(scale, RoundingMode.HALF_UP).doubleValue());
       } else {
diff --git a/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java b/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java
index 53d5b11d6ce3..699444c916b7 100644
--- a/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java
+++ b/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java
@@ -29,6 +29,8 @@
 import org.junit.Test;
 
 import javax.annotation.Nullable;
+import java.math.BigDecimal;
+import java.math.RoundingMode;
 import java.util.Locale;
 import java.util.Set;
 
@@ -44,6 +46,8 @@ public void setup()
         .put("y", 2)
         .put("z", 3.1)
         .put("d", 34.56D)
+        .put("maxLong", Long.MAX_VALUE)
+        .put("minLong", Long.MIN_VALUE)
         .put("f", 12.34F)
         .put("nan", Double.NaN)
         .put("inf", Double.POSITIVE_INFINITY)
@@ -337,21 +341,21 @@ public void testRoundWithNonNumericValuesShouldReturn0()
   {
     assertExpr("round(nan)", 0L);
     assertExpr("round(nan, 5)", 0L);
-    assertExpr("round(inf)", 0L);
-    assertExpr("round(inf, 4)", 0L);
-    assertExpr("round(-inf)", 0L);
-    assertExpr("round(-inf, 3)", 0L);
+    assertExpr("round(inf)", Long.MAX_VALUE);
+    assertExpr("round(inf, 4)", Long.MAX_VALUE);
+    assertExpr("round(-inf)", Long.MIN_VALUE);
+    assertExpr("round(-inf, 3)", Long.MIN_VALUE);
 
     // Calculations that result in non numeric numbers
     assertExpr("round(0/od)", 0L);
     assertExpr("round(od/od)", 0L);
-    assertExpr("round(1/od)", 0L);
-    assertExpr("round(-1/od)", 0L);
+    assertExpr("round(1/od)", Long.MAX_VALUE);
+    assertExpr("round(-1/od)", Long.MIN_VALUE);
 
     assertExpr("round(0/of)", 0L);
     assertExpr("round(of/of)", 0L);
-    assertExpr("round(1/of)", 0L);
-    assertExpr("round(-1/of)", 0L);
+    assertExpr("round(1/of)", Long.MAX_VALUE);
+    assertExpr("round(-1/of)", Long.MIN_VALUE);
   }
 
   @Test
@@ -382,6 +386,20 @@ public void testRoundWithFloat()
     assertExpr("round(f, -1)", 10D);
   }
 
+  @Test
+  public void testRoundWithExtremeNumbers()
+  {
+    assertExpr("round(maxLong)", BigDecimal.valueOf(Long.MAX_VALUE).setScale(0, RoundingMode.HALF_UP).longValue());
+    assertExpr("round(minLong)", BigDecimal.valueOf(Long.MIN_VALUE).setScale(0, RoundingMode.HALF_UP).longValue());
+    // overflow
+    assertExpr("round(maxLong + 1, 1)", BigDecimal.valueOf(Long.MIN_VALUE).setScale(1, RoundingMode.HALF_UP).longValue());
+    // underflow
+    assertExpr("round(minLong - 1, -2)", BigDecimal.valueOf(Long.MAX_VALUE).setScale(-2, RoundingMode.HALF_UP).longValue());
+
+    assertExpr("round(CAST(maxLong, 'DOUBLE') + 1, 1)", BigDecimal.valueOf(((double) Long.MAX_VALUE) + 1).setScale(1, RoundingMode.HALF_UP).doubleValue());
+    assertExpr("round(CAST(minLong, 'DOUBLE') - 1, -2)", BigDecimal.valueOf(((double) Long.MIN_VALUE) - 1).setScale(-2, RoundingMode.HALF_UP).doubleValue());
+  }
+
   @Test
   public void testRoundWithInvalidFirstArgument()
   {
diff --git a/docs/misc/math-expr.md b/docs/misc/math-expr.md
index 54eaf12027d8..7438145a64d7 100644
--- a/docs/misc/math-expr.md
+++ b/docs/misc/math-expr.md
@@ -141,7 +141,7 @@ See javadoc of java.lang.Math for detailed explanation for each function.
 |pow|pow(x, y) would return the value of the x raised to the power of y|
 |remainder|remainder(x, y) would return the remainder operation on two arguments as prescribed by the IEEE 754 standard|
 |rint|rint(x) would return value that is closest in value to x and is equal to a mathematical integer|
-|round|round(x, y) would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If x is either `NaN` or infinity (positive or negative), this will return 0. |
+|round|round(x, y) would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If x is `NaN`, this will return 0. If x is positive or negative infinity, this will return `Long.MAX_VALUE` or `Long.MIN_VALUE` respectively. |
 |scalb|scalb(d, sf) would return d * 2^sf rounded as if performed by a single correctly rounded floating-point multiply to a member of the double value set|
 |signum|signum(x) would return the signum function of the argument x|
 |sin|sin(x) would return the trigonometric sine of an angle x|
diff --git a/docs/querying/sql.md b/docs/querying/sql.md
index de332f523a59..569e4f5d7ac5 100644
--- a/docs/querying/sql.md
+++ b/docs/querying/sql.md
@@ -287,7 +287,7 @@ to FLOAT. At runtime, Druid will widen 32-bit floats to 64-bit for most expressi
 |`SQRT(expr)`|Square root.|
 |`TRUNCATE(expr[, digits])`|Truncate expr to a specific number of decimal digits. If digits is negative, then this truncates that many places to the left of the decimal point. Digits defaults to zero if not specified.|
 |`TRUNC(expr[, digits])`|Synonym for `TRUNCATE`.|
-|`ROUND(expr[, digits])`|`ROUND(x, y)` would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If `expr` evaluates to either `NaN` or infinity (positive or negative), this will return 0. |
+|`ROUND(expr[, digits])`|`ROUND(x, y)` would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If `expr` evaluates to either `NaN`, this will return 0. If `expr` is positive or negative infinity, this will return `Long.MAX_VALUE` or `Long.MIN_VALUE` respectively. |
 |`x + y`|Addition.|
 |`x - y`|Subtraction.|
 |`x * y`|Multiplication.|

From 81fd9e80020eb483867a45e2505986354537b11a Mon Sep 17 00:00:00 2001
From: Suneet Saldanha <suneet.saldanha@imply.io>
Date: Tue, 16 Jun 2020 11:08:32 -0700
Subject: [PATCH 6/9] round non numeric doubles returns a double

---
 .../org/apache/druid/math/expr/Function.java  | 26 +++++++++++--
 .../apache/druid/math/expr/FunctionTest.java  | 37 +++++++++++--------
 docs/misc/math-expr.md                        |  2 +-
 docs/querying/sql.md                          |  2 +-
 .../calcite/expression/ExpressionsTest.java   | 14 ++++---
 5 files changed, 54 insertions(+), 27 deletions(-)

diff --git a/core/src/main/java/org/apache/druid/math/expr/Function.java b/core/src/main/java/org/apache/druid/math/expr/Function.java
index db5d77d73e4a..bcc267dc18b4 100644
--- a/core/src/main/java/org/apache/druid/math/expr/Function.java
+++ b/core/src/main/java/org/apache/druid/math/expr/Function.java
@@ -694,6 +694,11 @@ protected ExprEval eval(double param)
 
   class Round implements Function
   {
+    //CHECKSTYLE.OFF: Regexp
+    private static final BigDecimal MAX_FINITE_VALUE = BigDecimal.valueOf(Double.MAX_VALUE);
+    private static final BigDecimal MIN_FINITE_VALUE = BigDecimal.valueOf(-1 * Double.MAX_VALUE);
+    //CHECKSTYLE.ON: Regexp
+
     @Override
     public String name()
     {
@@ -746,14 +751,27 @@ private ExprEval eval(ExprEval param, int scale)
         return ExprEval.of(BigDecimal.valueOf(param.asLong()).setScale(scale, RoundingMode.HALF_UP).longValue());
       } else if (param.type() == ExprType.DOUBLE) {
         double val = param.asDouble();
-        if (Double.isNaN(val) || Double.isInfinite(val)) {
-          return ExprEval.of(Math.round(val));
-        }
-        return ExprEval.of(BigDecimal.valueOf(val).setScale(scale, RoundingMode.HALF_UP).doubleValue());
+        BigDecimal decimal = safeGetFromDouble(param.asDouble());
+        return ExprEval.of(decimal.setScale(scale, RoundingMode.HALF_UP).doubleValue());
       } else {
         return ExprEval.of(null);
       }
     }
+
+    /**
+     * Converts non-finite doubles to BigDecimal values instead of throwing a NumberFormatException.
+     */
+    private static BigDecimal safeGetFromDouble(double val)
+    {
+      if (Double.isNaN(val)) {
+        return BigDecimal.ZERO;
+      } else if (val == Double.POSITIVE_INFINITY) {
+        return MAX_FINITE_VALUE;
+      } else if (val == Double.NEGATIVE_INFINITY) {
+        return MIN_FINITE_VALUE;
+      }
+      return BigDecimal.valueOf(val);
+    }
   }
 
   class Signum extends UnivariateMathFunction
diff --git a/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java b/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java
index 699444c916b7..bd755ba7e0ec 100644
--- a/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java
+++ b/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java
@@ -339,23 +339,30 @@ public void testArrayPrepend()
   @Test
   public void testRoundWithNonNumericValuesShouldReturn0()
   {
-    assertExpr("round(nan)", 0L);
-    assertExpr("round(nan, 5)", 0L);
-    assertExpr("round(inf)", Long.MAX_VALUE);
-    assertExpr("round(inf, 4)", Long.MAX_VALUE);
-    assertExpr("round(-inf)", Long.MIN_VALUE);
-    assertExpr("round(-inf, 3)", Long.MIN_VALUE);
+    assertExpr("round(nan)", 0D);
+    assertExpr("round(nan, 5)", 0D);
+    //CHECKSTYLE.OFF: Regexp
+    assertExpr("round(inf)", Double.MAX_VALUE);
+    assertExpr("round(inf, 4)", Double.MAX_VALUE);
+    assertExpr("round(-inf)", -1 * Double.MAX_VALUE);
+    assertExpr("round(-inf, 3)", -1 * Double.MAX_VALUE);
+    assertExpr("round(-inf, -5)", -1 * Double.MAX_VALUE);
+    //CHECKSTYLE.ON: Regexp
 
     // Calculations that result in non numeric numbers
-    assertExpr("round(0/od)", 0L);
-    assertExpr("round(od/od)", 0L);
-    assertExpr("round(1/od)", Long.MAX_VALUE);
-    assertExpr("round(-1/od)", Long.MIN_VALUE);
-
-    assertExpr("round(0/of)", 0L);
-    assertExpr("round(of/of)", 0L);
-    assertExpr("round(1/of)", Long.MAX_VALUE);
-    assertExpr("round(-1/of)", Long.MIN_VALUE);
+    assertExpr("round(0/od)", 0D);
+    assertExpr("round(od/od)", 0D);
+    //CHECKSTYLE.OFF: Regexp
+    assertExpr("round(1/od)", Double.MAX_VALUE);
+    assertExpr("round(-1/od)", -1 * Double.MAX_VALUE);
+    //CHECKSTYLE.ON: Regexp
+
+    assertExpr("round(0/of)", 0D);
+    assertExpr("round(of/of)", 0D);
+    //CHECKSTYLE.OFF: Regexp
+    assertExpr("round(1/of)", Double.MAX_VALUE);
+    assertExpr("round(-1/of)", -1 * Double.MAX_VALUE);
+    //CHECKSTYLE.ON: Regexp
   }
 
   @Test
diff --git a/docs/misc/math-expr.md b/docs/misc/math-expr.md
index 7438145a64d7..d3036995f36c 100644
--- a/docs/misc/math-expr.md
+++ b/docs/misc/math-expr.md
@@ -141,7 +141,7 @@ See javadoc of java.lang.Math for detailed explanation for each function.
 |pow|pow(x, y) would return the value of the x raised to the power of y|
 |remainder|remainder(x, y) would return the remainder operation on two arguments as prescribed by the IEEE 754 standard|
 |rint|rint(x) would return value that is closest in value to x and is equal to a mathematical integer|
-|round|round(x, y) would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If x is `NaN`, this will return 0. If x is positive or negative infinity, this will return `Long.MAX_VALUE` or `Long.MIN_VALUE` respectively. |
+|round|round(x, y) would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If x is `NaN`, this will return 0. |
 |scalb|scalb(d, sf) would return d * 2^sf rounded as if performed by a single correctly rounded floating-point multiply to a member of the double value set|
 |signum|signum(x) would return the signum function of the argument x|
 |sin|sin(x) would return the trigonometric sine of an angle x|
diff --git a/docs/querying/sql.md b/docs/querying/sql.md
index 569e4f5d7ac5..25458ca035d8 100644
--- a/docs/querying/sql.md
+++ b/docs/querying/sql.md
@@ -287,7 +287,7 @@ to FLOAT. At runtime, Druid will widen 32-bit floats to 64-bit for most expressi
 |`SQRT(expr)`|Square root.|
 |`TRUNCATE(expr[, digits])`|Truncate expr to a specific number of decimal digits. If digits is negative, then this truncates that many places to the left of the decimal point. Digits defaults to zero if not specified.|
 |`TRUNC(expr[, digits])`|Synonym for `TRUNCATE`.|
-|`ROUND(expr[, digits])`|`ROUND(x, y)` would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If `expr` evaluates to either `NaN`, this will return 0. If `expr` is positive or negative infinity, this will return `Long.MAX_VALUE` or `Long.MIN_VALUE` respectively. |
+|`ROUND(expr[, digits])`|`ROUND(x, y)` would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If `expr` evaluates to either `NaN`, this will return 0. |
 |`x + y`|Addition.|
 |`x - y`|Subtraction.|
 |`x * y`|Multiplication.|
diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java
index 9980b15c46db..9975cffe64cf 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java
@@ -942,13 +942,13 @@ public void testRoundWithNanShouldRoundTo0()
         roundFunction,
         testHelper.makeInputRef("nan"),
         DruidExpression.fromExpression("round(\"nan\")"),
-        0L
+        0D
     );
     testHelper.testExpression(
         roundFunction,
         testHelper.makeInputRef("fnan"),
         DruidExpression.fromExpression("round(\"fnan\")"),
-        0L
+        0D
     );
   }
 
@@ -957,30 +957,32 @@ public void testRoundWithInfinityShouldRoundTo0()
   {
     final SqlFunction roundFunction = new RoundOperatorConversion().calciteOperator();
 
+    //CHECKSTYLE.OFF: Regexp
     testHelper.testExpression(
         roundFunction,
         testHelper.makeInputRef("inf"),
         DruidExpression.fromExpression("round(\"inf\")"),
-        0L
+        Double.MAX_VALUE
     );
     testHelper.testExpression(
         roundFunction,
         testHelper.makeInputRef("-inf"),
         DruidExpression.fromExpression("round(\"-inf\")"),
-        0L
+        -1 * Double.MAX_VALUE
     );
     testHelper.testExpression(
         roundFunction,
         testHelper.makeInputRef("finf"),
         DruidExpression.fromExpression("round(\"finf\")"),
-        0L
+        Double.MAX_VALUE
     );
     testHelper.testExpression(
         roundFunction,
         testHelper.makeInputRef("-finf"),
         DruidExpression.fromExpression("round(\"-finf\")"),
-        0L
+        -1 * Double.MAX_VALUE
     );
+    //CHECKSTYLE.ON: Regexp
   }
 
   @Test

From eb8fb51e1a637b04ac794f4ed82e357a0797b96e Mon Sep 17 00:00:00 2001
From: Suneet Saldanha <suneet.saldanha@imply.io>
Date: Tue, 16 Jun 2020 12:32:13 -0700
Subject: [PATCH 7/9] fix spotbugs

---
 core/src/main/java/org/apache/druid/math/expr/Function.java | 1 -
 1 file changed, 1 deletion(-)

diff --git a/core/src/main/java/org/apache/druid/math/expr/Function.java b/core/src/main/java/org/apache/druid/math/expr/Function.java
index bcc267dc18b4..a20863929875 100644
--- a/core/src/main/java/org/apache/druid/math/expr/Function.java
+++ b/core/src/main/java/org/apache/druid/math/expr/Function.java
@@ -750,7 +750,6 @@ private ExprEval eval(ExprEval param, int scale)
       if (param.type() == ExprType.LONG) {
         return ExprEval.of(BigDecimal.valueOf(param.asLong()).setScale(scale, RoundingMode.HALF_UP).longValue());
       } else if (param.type() == ExprType.DOUBLE) {
-        double val = param.asDouble();
         BigDecimal decimal = safeGetFromDouble(param.asDouble());
         return ExprEval.of(decimal.setScale(scale, RoundingMode.HALF_UP).doubleValue());
       } else {

From 75c3581bf297319b8b05e24f403db47da4275db9 Mon Sep 17 00:00:00 2001
From: Suneet Saldanha <44787917+suneet-s@users.noreply.github.com>
Date: Tue, 16 Jun 2020 14:38:48 -0700
Subject: [PATCH 8/9] Update docs/misc/math-expr.md

---
 docs/misc/math-expr.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/misc/math-expr.md b/docs/misc/math-expr.md
index d3036995f36c..dc356479ad58 100644
--- a/docs/misc/math-expr.md
+++ b/docs/misc/math-expr.md
@@ -141,7 +141,7 @@ See javadoc of java.lang.Math for detailed explanation for each function.
 |pow|pow(x, y) would return the value of the x raised to the power of y|
 |remainder|remainder(x, y) would return the remainder operation on two arguments as prescribed by the IEEE 754 standard|
 |rint|rint(x) would return value that is closest in value to x and is equal to a mathematical integer|
-|round|round(x, y) would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If x is `NaN`, this will return 0. |
+|round|round(x, y) would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If x is `NaN`, x will return 0. If x is infinity, x will be converted to the nearest finite double. |
 |scalb|scalb(d, sf) would return d * 2^sf rounded as if performed by a single correctly rounded floating-point multiply to a member of the double value set|
 |signum|signum(x) would return the signum function of the argument x|
 |sin|sin(x) would return the trigonometric sine of an angle x|

From 1d69625603f5eff7a1a4f83b4d98b1450a58b967 Mon Sep 17 00:00:00 2001
From: Suneet Saldanha <44787917+suneet-s@users.noreply.github.com>
Date: Tue, 16 Jun 2020 14:38:55 -0700
Subject: [PATCH 9/9] Update docs/querying/sql.md

---
 docs/querying/sql.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/querying/sql.md b/docs/querying/sql.md
index 25458ca035d8..acbc3ae0e726 100644
--- a/docs/querying/sql.md
+++ b/docs/querying/sql.md
@@ -287,7 +287,7 @@ to FLOAT. At runtime, Druid will widen 32-bit floats to 64-bit for most expressi
 |`SQRT(expr)`|Square root.|
 |`TRUNCATE(expr[, digits])`|Truncate expr to a specific number of decimal digits. If digits is negative, then this truncates that many places to the left of the decimal point. Digits defaults to zero if not specified.|
 |`TRUNC(expr[, digits])`|Synonym for `TRUNCATE`.|
-|`ROUND(expr[, digits])`|`ROUND(x, y)` would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If `expr` evaluates to either `NaN`, this will return 0. |
+|`ROUND(expr[, digits])`|`ROUND(x, y)` would return the value of the x rounded to the y decimal places. While x can be an integer or floating-point number, y must be an integer. The type of the return value is specified by that of x. y defaults to 0 if omitted. When y is negative, x is rounded on the left side of the y decimal points. If `expr` evaluates to either `NaN`, `expr` will be converted to 0. If `expr` is infinity, `expr` will be converted to the nearest finite double. |
 |`x + y`|Addition.|
 |`x - y`|Subtraction.|
 |`x * y`|Multiplication.|