From 1115c2f235d231af8da7d329b0f426b31a10e5ce Mon Sep 17 00:00:00 2001
From: Aliaksandr Valialkin <valyala@victoriametrics.com>
Date: Fri, 24 Sep 2021 00:46:22 +0300
Subject: [PATCH] app/vmselect/promql: align the behavior of `or`, `and` and
 `unless` operators with `on (labels)` modifier to Prometheus

Previously VictoriaMetrics could return unexpected result of the right-hand side operand
had multiple time series with the given set of labels mentioned in `on(labels)`.

See https://github.com/VictoriaMetrics/VictoriaMetrics/pull/1643
---
 app/vmselect/promql/binary_op.go | 38 ++++++++++++++++++++++----------
 docs/CHANGELOG.md                |  2 ++
 2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/app/vmselect/promql/binary_op.go b/app/vmselect/promql/binary_op.go
index 512d326636..37300ab9c5 100644
--- a/app/vmselect/promql/binary_op.go
+++ b/app/vmselect/promql/binary_op.go
@@ -303,12 +303,18 @@ func binaryOpAnd(bfa *binaryOpFuncArg) ([]*timeseries, error) {
 		if tssLeft == nil {
 			continue
 		}
-		// Add gaps to tssLeft if there are gaps at valuesRight.
-		valuesRight := tssRight[0].Values
+		// Add gaps to tssLeft if there are gaps at tssRight.
 		for _, tsLeft := range tssLeft {
 			valuesLeft := tsLeft.Values
-			for i, v := range valuesRight {
-				if math.IsNaN(v) {
+			for i := range valuesLeft {
+				hasValue := false
+				for _, tsRight := range tssRight {
+					if !math.IsNaN(tsRight.Values[i]) {
+						hasValue = true
+						break
+					}
+				}
+				if !hasValue {
 					valuesLeft[i] = nan
 				}
 			}
@@ -333,12 +339,18 @@ func binaryOpOr(bfa *binaryOpFuncArg) ([]*timeseries, error) {
 		}
 		// Fill gaps in tssLeft with values from tssRight as Prometheus does.
 		// See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/552
-		valuesRight := tssRight[0].Values
 		for _, tsLeft := range tssLeft {
 			valuesLeft := tsLeft.Values
 			for i, v := range valuesLeft {
-				if math.IsNaN(v) {
-					valuesLeft[i] = valuesRight[i]
+				if !math.IsNaN(v) {
+					continue
+				}
+				for _, tsRight := range tssRight {
+					vRight := tsRight.Values[i]
+					if !math.IsNaN(vRight) {
+						valuesLeft[i] = vRight
+						break
+					}
 				}
 			}
 		}
@@ -355,13 +367,15 @@ func binaryOpUnless(bfa *binaryOpFuncArg) ([]*timeseries, error) {
 			rvs = append(rvs, tssLeft...)
 			continue
 		}
-		// Add gaps to tssLeft if the are no gaps at valuesRight.
-		valuesRight := tssRight[0].Values
+		// Add gaps to tssLeft if the are no gaps at tssRight.
 		for _, tsLeft := range tssLeft {
 			valuesLeft := tsLeft.Values
-			for i, v := range valuesRight {
-				if !math.IsNaN(v) {
-					valuesLeft[i] = nan
+			for i := range valuesLeft {
+				for _, tsRight := range tssRight {
+					if !math.IsNaN(tsRight.Values[i]) {
+						valuesLeft[i] = nan
+						break
+					}
 				}
 			}
 		}
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index 7da886d449..47c6c5a06d 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -6,6 +6,8 @@ sort: 15
 
 ## tip
 
+* BUGFIX: align behavior of the queries `a or on (labels) b`, `a and on (labels) b` and `a unless on (labels) b` where `b` has multiple time series with the given `labels` to Prometheus behavior. See [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/1643).
+
 
 ## [v1.66.2](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.66.2)