From 2ac7e49924df7632c96ea434563b7628b7509e50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 10 Jul 2018 09:34:17 +0200 Subject: [PATCH] Fix broken NaN check in MovingFunctions#stdDev() (#31888) The initial check will never be true, because of the special semantics of NaN, where no value is equal to Nan, including NaN. Thus, x == Double.NaN always evaluates to false. The method still works correct because later computations will also return NaN if the avg argument is NaN, but the intended shortcut doesn't work. --- .../search/aggregations/pipeline/movfn/MovingFunctions.java | 2 +- .../pipeline/movfn/MovFnWhitelistedFunctionTests.java | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/movfn/MovingFunctions.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/movfn/MovingFunctions.java index c38ce27cd8ad2..020189d461935 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/movfn/MovingFunctions.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/movfn/MovingFunctions.java @@ -85,7 +85,7 @@ public static double unweightedAvg(double[] values) { * The average is based on the count of non-null, non-NaN values. */ public static double stdDev(double[] values, double avg) { - if (avg == Double.NaN) { + if (Double.isNaN(avg)) { return Double.NaN; } else { long count = 0; diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/movfn/MovFnWhitelistedFunctionTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/movfn/MovFnWhitelistedFunctionTests.java index 0a0f9d6ae3759..6d0e388e64325 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/movfn/MovFnWhitelistedFunctionTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/movfn/MovFnWhitelistedFunctionTests.java @@ -313,6 +313,10 @@ public void testEmptySimpleStdDev() { assertThat(actual, equalTo(Double.NaN)); } + public void testStdDevNaNAvg() { + assertThat(MovingFunctions.stdDev(new double[] { 1.0, 2.0, 3.0 }, Double.NaN), equalTo(Double.NaN)); + } + public void testLinearMovAvg() { int numValues = randomIntBetween(1, 100);