fix: apply having and limit clause on formulas (#5201)

pkg/query-service/app/http_handler.go

@@ -3056,6 +3056,7 @@ func (aH *APIHandler) queryRangeV3(ctx context.Context, queryRangeParams *v3.Que
 		return
 	}
 
+	postprocess.ApplyHavingClause(result, queryRangeParams)
 	postprocess.ApplyMetricLimit(result, queryRangeParams)
 
 	sendQueryResultEvents(r, result, queryRangeParams)
@@ -3065,6 +3066,10 @@ func (aH *APIHandler) queryRangeV3(ctx context.Context, queryRangeParams *v3.Que
 		postprocess.ApplyFunctions(result, queryRangeParams)
 	}
 
+	if queryRangeParams.CompositeQuery.FillGaps {
+		postprocess.FillGaps(result, queryRangeParams)
+	}
+
 	resp := v3.QueryRangeResponse{
 		Result: result,
 	}

pkg/query-service/postprocess/having.go

@@ -6,14 +6,17 @@ import (
 	v3 "go.signoz.io/signoz/pkg/query-service/model/v3"
 )
 
-// applyHavingClause applies the having clause to the result
+// ApplyHavingClause applies the having clause to the result
 // each query has its own having clause
 // there can be multiple having clauses for each query
-func applyHavingClause(result []*v3.Result, queryRangeParams *v3.QueryRangeParamsV3) {
+func ApplyHavingClause(result []*v3.Result, queryRangeParams *v3.QueryRangeParamsV3) {
 	for _, result := range result {
 		builderQueries := queryRangeParams.CompositeQuery.BuilderQueries
 
-		if builderQueries != nil && (builderQueries[result.QueryName].DataSource == v3.DataSourceMetrics) {
+		// apply having clause for metrics and formula
+		if builderQueries != nil &&
+			(builderQueries[result.QueryName].DataSource == v3.DataSourceMetrics ||
+				builderQueries[result.QueryName].QueryName != builderQueries[result.QueryName].Expression) {
 			havingClause := builderQueries[result.QueryName].Having
 
 			for i := 0; i < len(result.Series); i++ {
@@ -37,46 +40,38 @@ func evaluateHavingClause(having []v3.Having, value float64) bool {
 		return true
 	}
 
+	satisfied := true
+
 	for _, h := range having {
 		switch h.Operator {
 		case v3.HavingOperatorEqual:
-			if value == h.Value.(float64) {
-				return true
+			if value != h.Value.(float64) {
+				satisfied = false
 			}
 		case v3.HavingOperatorNotEqual:
-			if value != h.Value.(float64) {
-				return true
+			if value == h.Value.(float64) {
+				satisfied = false
 			}
 		case v3.HavingOperatorGreaterThan:
-			if value > h.Value.(float64) {
-				return true
+			if value <= h.Value.(float64) {
+				satisfied = false
 			}
 		case v3.HavingOperatorGreaterThanOrEq:
-			if value >= h.Value.(float64) {
-				return true
+			if value < h.Value.(float64) {
+				satisfied = false
 			}
 		case v3.HavingOperatorLessThan:
-			if value < h.Value.(float64) {
-				return true
+			if value >= h.Value.(float64) {
+				satisfied = false
 			}
 		case v3.HavingOperatorLessThanOrEq:
-			if value <= h.Value.(float64) {
-				return true
+			if value > h.Value.(float64) {
+				satisfied = false
 			}
 		case v3.HavingOperatorIn, v3.HavingOperator(strings.ToLower(string(v3.HavingOperatorIn))):
 			values, ok := h.Value.([]interface{})
 			if !ok {
-				return false
-			}
-			for _, v := range values {
-				if value == v.(float64) {
-					return true
-				}
-			}
-		case v3.HavingOperatorNotIn, v3.HavingOperator(strings.ToLower(string(v3.HavingOperatorNotIn))):
-			values, ok := h.Value.([]interface{})
-			if !ok {
-				return true
+				satisfied = false
 			}
 			found := false
 			for _, v := range values {
@@ -86,9 +81,24 @@ func evaluateHavingClause(having []v3.Having, value float64) bool {
 				}
 			}
 			if !found {
-				return true
+				satisfied = false
+			}
+		case v3.HavingOperatorNotIn, v3.HavingOperator(strings.ToLower(string(v3.HavingOperatorNotIn))):
+			values, ok := h.Value.([]interface{})
+			if !ok {
+				satisfied = false
+			}
+			found := false
+			for _, v := range values {
+				if value == v.(float64) {
+					found = true
+					break
+				}
+			}
+			if found {
+				satisfied = false
 			}
 		}
 	}
-	return false
+	return satisfied
 }

pkg/query-service/postprocess/having_test.go

@@ -248,22 +248,86 @@ func TestApplyHavingCaluse(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "test multiple having clause",
+			results: []*v3.Result{
+				{
+					QueryName: "A",
+					Series: []*v3.Series{
+						{
+							Points: []v3.Point{
+								{
+									Value: 0.5,
+								},
+								{
+									Value: 0.4,
+								},
+								{
+									Value: 0.3,
+								},
+								{
+									Value: 0.2,
+								},
+								{
+									Value: 0.1,
+								},
+							},
+						},
+					},
+				},
+			},
+			params: &v3.QueryRangeParamsV3{
+				CompositeQuery: &v3.CompositeQuery{
+					BuilderQueries: map[string]*v3.BuilderQuery{
+						"A": {
+							DataSource: v3.DataSourceMetrics,
+							Having: []v3.Having{
+								{
+									Operator: v3.HavingOperatorGreaterThanOrEq,
+									Value:    0.3,
+								},
+								{
+									Operator: v3.HavingOperatorLessThanOrEq,
+									Value:    0.4,
+								},
+							},
+						},
+					},
+				},
+			},
+			want: []*v3.Result{
+				{
+					Series: []*v3.Series{
+						{
+							Points: []v3.Point{
+								{
+									Value: 0.4,
+								},
+								{
+									Value: 0.3,
+								},
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 
-			applyHavingClause(tc.results, tc.params)
+			ApplyHavingClause(tc.results, tc.params)
 
 			got := tc.results
 
 			if len(got) != len(tc.want) {
-				t.Errorf("got %v, want %v", got, tc.want)
+				t.Errorf("got %v, want %v", len(got), len(tc.want))
 			}
 
 			for i := range got {
 				if len(got[i].Series) != len(tc.want[i].Series) {
-					t.Errorf("got %v, want %v", got, tc.want)
+					t.Errorf("got %v, want %v", len(got[i].Series), len(tc.want[i].Series))
 				}
 
 				for j := range got[i].Series {
@@ -273,7 +337,7 @@ func TestApplyHavingCaluse(t *testing.T) {
 
 					for k := range got[i].Series[j].Points {
 						if got[i].Series[j].Points[k].Value != tc.want[i].Series[j].Points[k].Value {
-							t.Errorf("got %v, want %v", got, tc.want)
+							t.Errorf("got %v, want %v", got[i].Series[j].Points[k].Value, tc.want[i].Series[j].Points[k].Value)
 						}
 					}
 				}

pkg/query-service/postprocess/limit.go

@@ -17,7 +17,10 @@ func ApplyMetricLimit(results []*v3.Result, queryRangeParams *v3.QueryRangeParam
 	for _, result := range results {
 		builderQueries := queryRangeParams.CompositeQuery.BuilderQueries
 
-		if builderQueries != nil && (builderQueries[result.QueryName].DataSource == v3.DataSourceMetrics) {
+		// apply limit for metrics and formula
+		if builderQueries != nil &&
+			(builderQueries[result.QueryName].DataSource == v3.DataSourceMetrics ||
+				builderQueries[result.QueryName].QueryName != builderQueries[result.QueryName].Expression) {
 			limit := builderQueries[result.QueryName].Limit
 
 			orderByList := builderQueries[result.QueryName].OrderBy

pkg/query-service/postprocess/process.go

@@ -18,7 +18,7 @@ func PostProcessResult(result []*v3.Result, queryRangeParams *v3.QueryRangeParam
 	// It's not included in the query because it doesn't work nicely with caching
 	// With this change, if you have a query with a having clause, and then you change the having clause
 	// to something else, the query will still be cached.
-	applyHavingClause(result, queryRangeParams)
+	ApplyHavingClause(result, queryRangeParams)
 	// We apply the metric limit here because it's not part of the clickhouse query
 	// The limit in the context of the time series query is the number of time series
 	// So for the limit to work, we need to know what series to keep and what to discard
@@ -64,6 +64,8 @@ func PostProcessResult(result []*v3.Result, queryRangeParams *v3.QueryRangeParam
 				return nil, err
 			}
 			formulaResult.QueryName = query.QueryName
+			ApplyHavingClause([]*v3.Result{formulaResult}, queryRangeParams)
+			ApplyMetricLimit([]*v3.Result{formulaResult}, queryRangeParams)
 			result = append(result, formulaResult)
 		}
 	}

pkg/query-service/postprocess/process_test.go

@@ -0,0 +1,222 @@
+package postprocess
+
+import (
+	"reflect"
+	"testing"
+
+	v3 "go.signoz.io/signoz/pkg/query-service/model/v3"
+)
+
+func TestPostProcessResult(t *testing.T) {
+	testCases := []struct {
+		name             string
+		results          []*v3.Result
+		queryRangeParams *v3.QueryRangeParamsV3
+		want             []*v3.Result
+	}{
+		{
+			name: "test1",
+			results: []*v3.Result{
+				{
+					QueryName: "A",
+					Series: []*v3.Series{
+						{
+							Labels: map[string]string{
+								"service_name": "frontend",
+							},
+							Points: []v3.Point{
+								{
+									Timestamp: 1,
+									Value:     10,
+								},
+								{
+									Timestamp: 2,
+									Value:     20,
+								},
+							},
+						},
+						{
+							Labels: map[string]string{
+								"service_name": "redis",
+							},
+							Points: []v3.Point{
+								{
+									Timestamp: 1,
+									Value:     12,
+								},
+								{
+									Timestamp: 2,
+									Value:     45,
+								},
+							},
+						},
+						{
+							Labels: map[string]string{
+								"service_name": "route",
+							},
+							Points: []v3.Point{
+								{
+									Timestamp: 1,
+									Value:     2,
+								},
+								{
+									Timestamp: 2,
+									Value:     45,
+								},
+							},
+						},
+					},
+				},
+				{
+					QueryName: "B",
+					Series: []*v3.Series{
+						{
+							Labels: map[string]string{
+								"service_name": "redis",
+							},
+							Points: []v3.Point{
+								{
+									Timestamp: 1,
+									Value:     6,
+								},
+								{
+									Timestamp: 2,
+									Value:     9,
+								},
+							},
+						},
+					},
+				},
+			},
+			want: []*v3.Result{
+				{
+					QueryName: "A",
+					Series: []*v3.Series{
+						{
+							Labels: map[string]string{
+								"service_name": "redis",
+							},
+							Points: []v3.Point{
+								{
+									Timestamp: 1,
+									Value:     12,
+								},
+								{
+									Timestamp: 2,
+									Value:     45,
+								},
+							},
+						},
+						{
+							Labels: map[string]string{
+								"service_name": "route",
+							},
+							Points: []v3.Point{
+								{
+									Timestamp: 1,
+									Value:     2,
+								},
+								{
+									Timestamp: 2,
+									Value:     45,
+								},
+							},
+						},
+						{
+							Labels: map[string]string{
+								"service_name": "frontend",
+							},
+							Points: []v3.Point{
+								{
+									Timestamp: 1,
+									Value:     10,
+								},
+								{
+									Timestamp: 2,
+									Value:     20,
+								},
+							},
+						},
+					},
+				},
+				{
+					QueryName: "B",
+					Series: []*v3.Series{
+						{
+							Labels: map[string]string{
+								"service_name": "redis",
+							},
+							Points: []v3.Point{
+								{
+									Timestamp: 1,
+									Value:     6,
+								},
+								{
+									Timestamp: 2,
+									Value:     9,
+								},
+							},
+						},
+					},
+				},
+				{
+					QueryName: "F1",
+					Series: []*v3.Series{
+						{
+							Labels: map[string]string{
+								"service_name": "redis",
+							},
+							LabelsArray: []map[string]string{
+								{
+									"service_name": "redis",
+								},
+							},
+							Points: []v3.Point{
+								{
+									Timestamp: 1,
+									Value:     0.5,
+								},
+								{
+									Timestamp: 2,
+									Value:     0.2,
+								},
+							},
+						},
+					},
+				},
+			},
+			queryRangeParams: &v3.QueryRangeParamsV3{
+				CompositeQuery: &v3.CompositeQuery{
+					BuilderQueries: map[string]*v3.BuilderQuery{
+						"A": {
+							DataSource: v3.DataSourceMetrics,
+							QueryName:  "A",
+							Expression: "A",
+						},
+						"B": {
+							DataSource: v3.DataSourceMetrics,
+							QueryName:  "B",
+							Expression: "B",
+						},
+						"F1": {
+							Expression: "B/A",
+							QueryName:  "F1",
+							Limit:      1,
+						},
+					},
+				},
+			},
+		},
+	}
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := PostProcessResult(tt.results, tt.queryRangeParams)
+			if err != nil {
+				t.Errorf("PostProcessResult() error = %v", err)
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("PostProcessResult() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}