From 6ca1e58d98181ab517ece4601247cb085c59e357 Mon Sep 17 00:00:00 2001
From: Aliaksandr Valialkin <valyala@gmail.com>
Date: Mon, 24 Feb 2020 13:24:18 +0200
Subject: [PATCH] app/vmselect/promql: properly take into account the first
 datapoint when calculating `rollup_candlestick`

---
 app/vmselect/promql/exec_test.go |  4 ++--
 app/vmselect/promql/rollup.go    | 36 +++++++++++++++++++++-----------
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/app/vmselect/promql/exec_test.go b/app/vmselect/promql/exec_test.go
index b8141364d..69f384d69 100644
--- a/app/vmselect/promql/exec_test.go
+++ b/app/vmselect/promql/exec_test.go
@@ -4750,7 +4750,7 @@ func TestExecSuccess(t *testing.T) {
 		}}
 		r2 := netstorage.Result{
 			MetricName: metricNameExpected,
-			Values:     []float64{0.85, 0.15, 0.43, 0.76, 0.47, 0.21},
+			Values:     []float64{0.9, 0.32, 0.82, 0.13, 0.28, 0.86},
 			Timestamps: timestampsExpected,
 		}
 		r2.MetricName.Tags = []storage.Tag{{
@@ -4768,7 +4768,7 @@ func TestExecSuccess(t *testing.T) {
 		}}
 		r4 := netstorage.Result{
 			MetricName: metricNameExpected,
-			Values:     []float64{0.85, 0.94, 0.97, 0.93, 0.98, 0.92},
+			Values:     []float64{0.9, 0.94, 0.97, 0.93, 0.98, 0.92},
 			Timestamps: timestampsExpected,
 		}
 		r4.MetricName.Tags = []storage.Tag{{
diff --git a/app/vmselect/promql/rollup.go b/app/vmselect/promql/rollup.go
index b3cce3a9a..96b943c03 100644
--- a/app/vmselect/promql/rollup.go
+++ b/app/vmselect/promql/rollup.go
@@ -1434,17 +1434,21 @@ func getCandlestickValues(rfa *rollupFuncArg) []float64 {
 	return rfa.values[:len(timestamps)]
 }
 
-func getPrevValueForCandlestick(rfa *rollupFuncArg) float64 {
-	if rfa.prevTimestamp+rfa.step == rfa.currTimestamp {
+func getFirstValueForCandlestick(rfa *rollupFuncArg) float64 {
+	if rfa.prevTimestamp+rfa.step >= rfa.currTimestamp {
 		return rfa.prevValue
 	}
 	return nan
 }
 
 func rollupOpen(rfa *rollupFuncArg) float64 {
+	v := getFirstValueForCandlestick(rfa)
+	if !math.IsNaN(v) {
+		return v
+	}
 	values := getCandlestickValues(rfa)
 	if len(values) == 0 {
-		return getPrevValueForCandlestick(rfa)
+		return nan
 	}
 	return values[0]
 }
@@ -1452,18 +1456,22 @@ func rollupOpen(rfa *rollupFuncArg) float64 {
 func rollupClose(rfa *rollupFuncArg) float64 {
 	values := getCandlestickValues(rfa)
 	if len(values) == 0 {
-		return getPrevValueForCandlestick(rfa)
+		return getFirstValueForCandlestick(rfa)
 	}
 	return values[len(values)-1]
 }
 
 func rollupHigh(rfa *rollupFuncArg) float64 {
 	values := getCandlestickValues(rfa)
-	if len(values) == 0 {
-		return getPrevValueForCandlestick(rfa)
+	max := getFirstValueForCandlestick(rfa)
+	if math.IsNaN(max) {
+		if len(values) == 0 {
+			return nan
+		}
+		max = values[0]
+		values = values[1:]
 	}
-	max := values[0]
-	for _, v := range values[1:] {
+	for _, v := range values {
 		if v > max {
 			max = v
 		}
@@ -1473,11 +1481,15 @@ func rollupHigh(rfa *rollupFuncArg) float64 {
 
 func rollupLow(rfa *rollupFuncArg) float64 {
 	values := getCandlestickValues(rfa)
-	if len(values) == 0 {
-		return getPrevValueForCandlestick(rfa)
+	min := getFirstValueForCandlestick(rfa)
+	if math.IsNaN(min) {
+		if len(values) == 0 {
+			return nan
+		}
+		min = values[0]
+		values = values[1:]
 	}
-	min := values[0]
-	for _, v := range values[1:] {
+	for _, v := range values {
 		if v < min {
 			min = v
 		}