From 95ddfda894a1775c510d431d964ad311ee9cefa3 Mon Sep 17 00:00:00 2001
From: Roman Khavronenko <hagen1778@gmail.com>
Date: Thu, 30 Sep 2021 12:24:17 +0300
Subject: [PATCH] app/vmselect: fix binary comparison func (#1667)

The fix makes the binary comparison func to check for NaNs
before executing the actual comparison. This prevents VM
to return values for non-existing samples for expressions
which contain bool comparisons. Please see added test
for example.
---
 app/vmselect/promql/binary_op.go |  6 +++---
 app/vmselect/promql/exec_test.go | 22 ++++++++++++++++++++++
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/app/vmselect/promql/binary_op.go b/app/vmselect/promql/binary_op.go
index 37300ab9c5..48b682cfe2 100644
--- a/app/vmselect/promql/binary_op.go
+++ b/app/vmselect/promql/binary_op.go
@@ -59,12 +59,12 @@ func newBinaryOpCmpFunc(cf func(left, right float64) bool) binaryOpFunc {
 			}
 			return nan
 		}
-		if cf(left, right) {
-			return 1
-		}
 		if math.IsNaN(left) {
 			return nan
 		}
+		if cf(left, right) {
+			return 1
+		}
 		return 0
 	}
 	return newBinaryOpFunc(cfe)
diff --git a/app/vmselect/promql/exec_test.go b/app/vmselect/promql/exec_test.go
index 87ffc2a0cf..dd73676a13 100644
--- a/app/vmselect/promql/exec_test.go
+++ b/app/vmselect/promql/exec_test.go
@@ -2180,6 +2180,28 @@ func TestExecSuccess(t *testing.T) {
 		resultExpected := []netstorage.Result{r}
 		f(q, resultExpected)
 	})
+	t.Run(`nan!=bool scalar`, func(t *testing.T) {
+		t.Parallel()
+		q := `(time() > 1234) !=bool 1400`
+		r := netstorage.Result{
+			MetricName: metricNameExpected,
+			Values:     []float64{nan, nan, 0, 1, 1, 1},
+			Timestamps: timestampsExpected,
+		}
+		resultExpected := []netstorage.Result{r}
+		f(q, resultExpected)
+	})
+	t.Run(`scalar!=bool nan`, func(t *testing.T) {
+		t.Parallel()
+		q := `1400 !=bool (time() > 1234)`
+		r := netstorage.Result{
+			MetricName: metricNameExpected,
+			Values:     []float64{nan, nan, 0, 1, 1, 1},
+			Timestamps: timestampsExpected,
+		}
+		resultExpected := []netstorage.Result{r}
+		f(q, resultExpected)
+	})
 	t.Run(`scalar > time()`, func(t *testing.T) {
 		t.Parallel()
 		q := `123 > time()`