honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Watcher comparisons don't deal with NaN correctly (elastic/x-pack-elasticsearch#4133) 

Aggregations may return `NaN`, and the comparison code would return `true` if this result was passed to a `gte` or `lte` condition.

Original commit: elastic/x-pack-elasticsearch@3b16ae6675
This commit is contained in:
Alan Woodward 2018-03-20 15:55:59 +00:00 committed by GitHub
parent 063ed78c42
commit c16e5f1f92
2 changed files with 42 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/condition/LenientCompare.java
View File

@ -28,6 +28,10 @@ public class LenientCompare {
return null; return null;
} }
if (v1.equals(Double.NaN) || v2.equals(Double.NaN) || v1.equals(Float.NaN) || v2.equals(Float.NaN)) {
return null;
}
// special case for numbers. If v1 is not a number, we'll try to convert it to a number // special case for numbers. If v1 is not a number, we'll try to convert it to a number
if (v2 instanceof Number) { if (v2 instanceof Number) {
if (!(v1 instanceof Number)) { if (!(v1 instanceof Number)) {

38
plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/condition/CompareConditionTests.java
View File

@ -46,6 +46,13 @@ public class CompareConditionTests extends ESTestCase {
assertThat(CompareCondition.Op.EQ.eval(singletonMap("k", "v"), singletonMap("k1", "v1")), is(false)); assertThat(CompareCondition.Op.EQ.eval(singletonMap("k", "v"), singletonMap("k1", "v1")), is(false));
assertThat(CompareCondition.Op.EQ.eval(Arrays.asList("k", "v"), Arrays.asList("k", "v")), is(true)); assertThat(CompareCondition.Op.EQ.eval(Arrays.asList("k", "v"), Arrays.asList("k", "v")), is(true));
assertThat(CompareCondition.Op.EQ.eval(Arrays.asList("k", "v"), Arrays.asList("k1", "v1")), is(false)); assertThat(CompareCondition.Op.EQ.eval(Arrays.asList("k", "v"), Arrays.asList("k1", "v1")), is(false));
assertThat(CompareCondition.Op.EQ.eval(Double.NaN, 250000), is(false));
assertThat(CompareCondition.Op.EQ.eval(250000, Double.NaN), is(false));
assertThat(CompareCondition.Op.EQ.eval(Double.NaN, Double.NaN), is(true));
assertThat(CompareCondition.Op.EQ.eval(Float.NaN, 250000), is(false));
assertThat(CompareCondition.Op.EQ.eval(250000, Float.NaN), is(false));
assertThat(CompareCondition.Op.EQ.eval(Float.NaN, Float.NaN), is(true));
} }
public void testOpEvalNotEQ() throws Exception { public void testOpEvalNotEQ() throws Exception {
@ -65,6 +72,12 @@ public class CompareConditionTests extends ESTestCase {
assertThat(CompareCondition.Op.NOT_EQ.eval(singletonMap("k", "v"), singletonMap("k1", "v1")), is(true)); assertThat(CompareCondition.Op.NOT_EQ.eval(singletonMap("k", "v"), singletonMap("k1", "v1")), is(true));
assertThat(CompareCondition.Op.NOT_EQ.eval(Arrays.asList("k", "v"), Arrays.asList("k", "v")), is(false)); assertThat(CompareCondition.Op.NOT_EQ.eval(Arrays.asList("k", "v"), Arrays.asList("k", "v")), is(false));
assertThat(CompareCondition.Op.NOT_EQ.eval(Arrays.asList("k", "v"), Arrays.asList("k1", "v1")), is(true)); assertThat(CompareCondition.Op.NOT_EQ.eval(Arrays.asList("k", "v"), Arrays.asList("k1", "v1")), is(true));
assertThat(CompareCondition.Op.NOT_EQ.eval(Double.NaN, 250000), is(true));
assertThat(CompareCondition.Op.NOT_EQ.eval(250000, Double.NaN), is(true));
assertThat(CompareCondition.Op.NOT_EQ.eval(Double.NaN, Double.NaN), is(false));
assertThat(CompareCondition.Op.NOT_EQ.eval(Float.NaN, 250000), is(true));
assertThat(CompareCondition.Op.NOT_EQ.eval(250000, Float.NaN), is(true));
assertThat(CompareCondition.Op.NOT_EQ.eval(Float.NaN, Float.NaN), is(false));
} }
public void testOpEvalGTE() throws Exception { public void testOpEvalGTE() throws Exception {
@ -79,6 +92,12 @@ public class CompareConditionTests extends ESTestCase {
assertThat(CompareCondition.Op.GTE.eval("a", "aa"), is(false)); assertThat(CompareCondition.Op.GTE.eval("a", "aa"), is(false));
assertThat(CompareCondition.Op.GTE.eval("a", "a"), is(true)); assertThat(CompareCondition.Op.GTE.eval("a", "a"), is(true));
assertThat(CompareCondition.Op.GTE.eval("aa", "ab"), is(false)); assertThat(CompareCondition.Op.GTE.eval("aa", "ab"), is(false));
assertThat(CompareCondition.Op.GTE.eval(Double.NaN, 250000), is(false));
assertThat(CompareCondition.Op.GTE.eval(250000, Double.NaN), is(false));
assertThat(CompareCondition.Op.GTE.eval(Double.NaN, Double.NaN), is(true));
assertThat(CompareCondition.Op.GTE.eval(Float.NaN, 250000), is(false));
assertThat(CompareCondition.Op.GTE.eval(250000, Float.NaN), is(false));
assertThat(CompareCondition.Op.GTE.eval(Float.NaN, Float.NaN), is(true));
} }
public void testOpEvalGT() throws Exception { public void testOpEvalGT() throws Exception {
@ -93,6 +112,13 @@ public class CompareConditionTests extends ESTestCase {
assertThat(CompareCondition.Op.GT.eval("a", "aa"), is(false)); assertThat(CompareCondition.Op.GT.eval("a", "aa"), is(false));
assertThat(CompareCondition.Op.GT.eval("a", "a"), is(false)); assertThat(CompareCondition.Op.GT.eval("a", "a"), is(false));
assertThat(CompareCondition.Op.GT.eval("aa", "ab"), is(false)); assertThat(CompareCondition.Op.GT.eval("aa", "ab"), is(false));
assertThat(CompareCondition.Op.GT.eval(Double.NaN, 250000), is(false));
assertThat(CompareCondition.Op.GT.eval(250000, Double.NaN), is(false));
assertThat(CompareCondition.Op.GT.eval(Double.NaN, Double.NaN), is(false));
assertThat(CompareCondition.Op.GT.eval(Float.NaN, 250000), is(false));
assertThat(CompareCondition.Op.GT.eval(250000, Float.NaN), is(false));
assertThat(CompareCondition.Op.GT.eval(Float.NaN, Float.NaN), is(false));
} }
public void testOpEvalLTE() throws Exception { public void testOpEvalLTE() throws Exception {
@ -107,6 +133,12 @@ public class CompareConditionTests extends ESTestCase {
assertThat(CompareCondition.Op.LTE.eval("a", "aa"), is(true)); assertThat(CompareCondition.Op.LTE.eval("a", "aa"), is(true));
assertThat(CompareCondition.Op.LTE.eval("a", "a"), is(true)); assertThat(CompareCondition.Op.LTE.eval("a", "a"), is(true));
assertThat(CompareCondition.Op.LTE.eval("aa", "ab"), is(true)); assertThat(CompareCondition.Op.LTE.eval("aa", "ab"), is(true));
assertThat(CompareCondition.Op.LTE.eval(Double.NaN, 250000), is(false));
assertThat(CompareCondition.Op.LTE.eval(250000, Double.NaN), is(false));
assertThat(CompareCondition.Op.LTE.eval(Double.NaN, Double.NaN), is(true));
assertThat(CompareCondition.Op.LTE.eval(Float.NaN, 250000), is(false));
assertThat(CompareCondition.Op.LTE.eval(250000, Float.NaN), is(false));
assertThat(CompareCondition.Op.LTE.eval(Float.NaN, Float.NaN), is(true));
} }
public void testOpEvalLT() throws Exception { public void testOpEvalLT() throws Exception {
@ -121,6 +153,12 @@ public class CompareConditionTests extends ESTestCase {
assertThat(CompareCondition.Op.LT.eval("a", "aa"), is(true)); assertThat(CompareCondition.Op.LT.eval("a", "aa"), is(true));
assertThat(CompareCondition.Op.LT.eval("a", "a"), is(false)); assertThat(CompareCondition.Op.LT.eval("a", "a"), is(false));
assertThat(CompareCondition.Op.LT.eval("aa", "ab"), is(true)); assertThat(CompareCondition.Op.LT.eval("aa", "ab"), is(true));
assertThat(CompareCondition.Op.LT.eval(Double.NaN, 250000), is(false));
assertThat(CompareCondition.Op.LT.eval(250000, Double.NaN), is(false));
assertThat(CompareCondition.Op.LT.eval(Double.NaN, Double.NaN), is(false));
assertThat(CompareCondition.Op.LT.eval(Float.NaN, 250000), is(false));
assertThat(CompareCondition.Op.LT.eval(250000, Float.NaN), is(false));
assertThat(CompareCondition.Op.LT.eval(Float.NaN, Float.NaN), is(false));
} }
public void testExecute() throws Exception { public void testExecute() throws Exception {