From 3ccadff800b350940cca0aae72cf35f2822bcc58 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 22 Mar 2018 14:49:40 +0100 Subject: [PATCH] mssql: fix timeGroup macro so that it properly creates correct groups Earlier the division of interval was done using whole numbers resulting in that important information was lost/too many time series merged to the same group. Now using division of floating point and rounding up to solve the problem --- pkg/tsdb/mssql/macros.go | 2 +- pkg/tsdb/mssql/macros_test.go | 4 +- pkg/tsdb/mssql/mssql_test.go | 72 ++++++++++++++++++++--------------- 3 files changed, 44 insertions(+), 34 deletions(-) diff --git a/pkg/tsdb/mssql/macros.go b/pkg/tsdb/mssql/macros.go index 92c6ede148e..108faee85b4 100644 --- a/pkg/tsdb/mssql/macros.go +++ b/pkg/tsdb/mssql/macros.go @@ -113,7 +113,7 @@ func (m *MsSqlMacroEngine) evaluateMacro(name string, args []string) (string, er m.Query.Model.Set("fillValue", floatVal) } } - return fmt.Sprintf("cast(cast(DATEDIFF(second, {d '1970-01-01'}, DATEADD(second, DATEDIFF(second,GETDATE(),GETUTCDATE()), %s))/%.0f as int)*%.0f as int)", args[0], interval.Seconds(), interval.Seconds()), nil + return fmt.Sprintf("CAST(ROUND(DATEDIFF(second, '1970-01-01', %s)/%.1f, 0) as bigint)*%.0f", args[0], interval.Seconds(), interval.Seconds()), nil case "__unixEpochFilter": if len(args) == 0 { return "", fmt.Errorf("missing time column argument for macro %v", name) diff --git a/pkg/tsdb/mssql/macros_test.go b/pkg/tsdb/mssql/macros_test.go index db1f5670924..c07bcbf498c 100644 --- a/pkg/tsdb/mssql/macros_test.go +++ b/pkg/tsdb/mssql/macros_test.go @@ -57,14 +57,14 @@ func TestMacroEngine(t *testing.T) { sql, err := engine.Interpolate(query, timeRange, "GROUP BY $__timeGroup(time_column,'5m')") So(err, ShouldBeNil) - So(sql, ShouldEqual, "GROUP BY cast(cast(DATEDIFF(second, {d '1970-01-01'}, DATEADD(second, DATEDIFF(second,GETDATE(),GETUTCDATE()), time_column))/300 as int)*300 as int)") + So(sql, ShouldEqual, "GROUP BY CAST(ROUND(DATEDIFF(second, '1970-01-01', time_column)/300.0, 0) as bigint)*300") }) Convey("interpolate __timeGroup function with spaces around arguments", func() { sql, err := engine.Interpolate(query, timeRange, "GROUP BY $__timeGroup(time_column , '5m')") So(err, ShouldBeNil) - So(sql, ShouldEqual, "GROUP BY cast(cast(DATEDIFF(second, {d '1970-01-01'}, DATEADD(second, DATEDIFF(second,GETDATE(),GETUTCDATE()), time_column))/300 as int)*300 as int)") + So(sql, ShouldEqual, "GROUP BY CAST(ROUND(DATEDIFF(second, '1970-01-01', time_column)/300.0, 0) as bigint)*300") }) Convey("interpolate __timeGroup function with fill (value = NULL)", func() { diff --git a/pkg/tsdb/mssql/mssql_test.go b/pkg/tsdb/mssql/mssql_test.go index 7ac135ec2f5..8e8b22d254f 100644 --- a/pkg/tsdb/mssql/mssql_test.go +++ b/pkg/tsdb/mssql/mssql_test.go @@ -211,22 +211,32 @@ func TestMSSQL(t *testing.T) { } resp, err := endpoint.Query(nil, nil, query) - queryResult := resp.Results["A"] So(err, ShouldBeNil) + queryResult := resp.Results["A"] So(queryResult.Error, ShouldBeNil) points := queryResult.Series[0].Points + So(len(points), ShouldEqual, 6) - So(len(points), ShouldEqual, 4) - actualValueFirst := points[0][0].Float64 - actualTimeFirst := time.Unix(int64(points[0][1].Float64)/1000, 0) - So(actualValueFirst, ShouldEqual, 15) - So(actualTimeFirst, ShouldEqual, fromStart) + dt := fromStart - actualValueLast := points[3][0].Float64 - actualTimeLast := time.Unix(int64(points[3][1].Float64)/1000, 0) - So(actualValueLast, ShouldEqual, 20) - So(actualTimeLast, ShouldEqual, fromStart.Add(25*time.Minute)) + for i := 0; i < 3; i++ { + aValue := points[i][0].Float64 + aTime := time.Unix(int64(points[i][1].Float64)/1000, 0) + So(aValue, ShouldEqual, 15) + So(aTime, ShouldEqual, dt) + dt = dt.Add(5 * time.Minute) + } + + // adjust for 5 minute gap + dt = dt.Add(5 * time.Minute) + for i := 3; i < 6; i++ { + aValue := points[i][0].Float64 + aTime := time.Unix(int64(points[i][1].Float64)/1000, 0) + So(aValue, ShouldEqual, 20) + So(aTime, ShouldEqual, dt) + dt = dt.Add(5 * time.Minute) + } }) Convey("When doing a metric query using timeGroup with NULL fill enabled", func() { @@ -247,33 +257,34 @@ func TestMSSQL(t *testing.T) { } resp, err := endpoint.Query(nil, nil, query) - queryResult := resp.Results["A"] So(err, ShouldBeNil) + queryResult := resp.Results["A"] So(queryResult.Error, ShouldBeNil) points := queryResult.Series[0].Points - So(len(points), ShouldEqual, 7) - actualValueFirst := points[0][0].Float64 - actualTimeFirst := time.Unix(int64(points[0][1].Float64)/1000, 0) - So(actualValueFirst, ShouldEqual, 15) - So(actualTimeFirst, ShouldEqual, fromStart) - actualNullPoint := points[3][0] - actualNullTime := time.Unix(int64(points[3][1].Float64)/1000, 0) - So(actualNullPoint.Valid, ShouldBeFalse) - So(actualNullTime, ShouldEqual, fromStart.Add(15*time.Minute)) + dt := fromStart - actualValueLast := points[5][0].Float64 - actualTimeLast := time.Unix(int64(points[5][1].Float64)/1000, 0) - So(actualValueLast, ShouldEqual, 20) - So(actualTimeLast, ShouldEqual, fromStart.Add(25*time.Minute)) + for i := 0; i < 3; i++ { + aValue := points[i][0].Float64 + aTime := time.Unix(int64(points[i][1].Float64)/1000, 0) + So(aValue, ShouldEqual, 15) + So(aTime, ShouldEqual, dt) + dt = dt.Add(5 * time.Minute) + } - actualLastNullPoint := points[6][0] - actualLastNullTime := time.Unix(int64(points[6][1].Float64)/1000, 0) - So(actualLastNullPoint.Valid, ShouldBeFalse) - So(actualLastNullTime, ShouldEqual, fromStart.Add(30*time.Minute)) + So(points[3][0].Valid, ShouldBeFalse) + // adjust for 5 minute gap + dt = dt.Add(5 * time.Minute) + for i := 4; i < 7; i++ { + aValue := points[i][0].Float64 + aTime := time.Unix(int64(points[i][1].Float64)/1000, 0) + So(aValue, ShouldEqual, 20) + So(aTime, ShouldEqual, dt) + dt = dt.Add(5 * time.Minute) + } }) Convey("When doing a metric query using timeGroup with float fill enabled", func() { @@ -294,13 +305,12 @@ func TestMSSQL(t *testing.T) { } resp, err := endpoint.Query(nil, nil, query) - queryResult := resp.Results["A"] So(err, ShouldBeNil) + queryResult := resp.Results["A"] So(queryResult.Error, ShouldBeNil) points := queryResult.Series[0].Points - - So(points[6][0].Float64, ShouldEqual, 1.5) + So(points[3][0].Float64, ShouldEqual, 1.5) }) })