chore: updated comment

pkg/query-service/app/parser.go

@@ -887,7 +887,7 @@ func ParseQueryRangeParams(r *http.Request) (*v3.QueryRangeParamsV3, *model.ApiE
 
 			keys := make([]string, 0, len(queryRangeParams.Variables))
 
-			querytemplate.AssignReservedVarsV3(queryRangeParams)
+			querytemplate.AssignReservedVars(queryRangeParams.Variables, queryRangeParams.Start, queryRangeParams.End)
 
 			for k := range queryRangeParams.Variables {
 				keys = append(keys, k)
@@ -927,7 +927,7 @@ func ParseQueryRangeParams(r *http.Request) (*v3.QueryRangeParamsV3, *model.ApiE
 				continue
 			}
 
-			querytemplate.AssignReservedVarsV3(queryRangeParams)
+			querytemplate.AssignReservedVars(queryRangeParams.Variables, queryRangeParams.Start, queryRangeParams.End)
 
 			keys := make([]string, 0, len(queryRangeParams.Variables))
 

pkg/query-service/converter/converter.go

@@ -1,8 +1,17 @@
 package converter
 
+import "github.com/SigNoz/signoz/pkg/errors"
+
 // Unit represents a unit of measurement
 type Unit string
 
+func (u Unit) Validate() error {
+	if !IsValidUnit(u) {
+		return errors.NewInvalidInputf(errors.CodeInvalidInput, "invalid unit: %s", u)
+	}
+	return nil
+}
+
 // Value represents a value with a unit of measurement
 type Value struct {
 	F float64
@@ -60,6 +69,27 @@ func FromUnit(u Unit) Converter {
 	}
 }
 
+// IsValidUnit returns true if the given unit is valid
+func IsValidUnit(u Unit) bool {
+	switch u {
+	// Duration unit
+	case "ns", "us", "µs", "ms", "s", "m", "h", "d", "min",
+		// Data unit
+		"bytes", "decbytes", "bits", "decbits", "kbytes", "decKbytes", "deckbytes", "mbytes", "decMbytes", "decmbytes", "gbytes", "decGbytes", "decgbytes", "tbytes", "decTbytes", "dectbytes", "pbytes", "decPbytes", "decpbytes", "By", "kBy", "MBy", "GBy", "TBy", "PBy",
+		// Data rate unit
+		"binBps", "Bps", "binbps", "bps", "KiBs", "Kibits", "KBs", "Kbits", "MiBs", "Mibits", "MBs", "Mbits", "GiBs", "Gibits", "GBs", "Gbits", "TiBs", "Tibits", "TBs", "Tbits", "PiBs", "Pibits", "PBs", "Pbits", "By/s", "kBy/s", "MBy/s", "GBy/s", "TBy/s", "PBy/s", "bit/s", "kbit/s", "Mbit/s", "Gbit/s", "Tbit/s", "Pbit/s",
+		// Percent unit
+		"percent", "percentunit", "%",
+		// Bool unit
+		"bool", "bool_yes_no", "bool_true_false", "bool_1_0",
+		// Throughput unit
+		"cps", "ops", "reqps", "rps", "wps", "iops", "cpm", "opm", "rpm", "wpm", "{count}/s", "{ops}/s", "{req}/s", "{read}/s", "{write}/s", "{iops}/s", "{count}/min", "{ops}/min", "{read}/min", "{write}/min":
+		return true
+	default:
+		return false
+	}
+}
+
 func UnitToName(u string) string {
 	switch u {
 	case "ns":

pkg/query-service/model/v3/v3.go

@@ -9,8 +9,9 @@ import (
 	"strings"
 	"time"
 
+	"github.com/SigNoz/signoz/pkg/errors"
+	"github.com/SigNoz/signoz/pkg/query-service/converter"
 	"github.com/SigNoz/signoz/pkg/valuer"
-	"github.com/pkg/errors"
 	"go.uber.org/zap"
 
 	qbtypes "github.com/SigNoz/signoz/pkg/types/querybuildertypes/querybuildertypesv5"
@@ -601,43 +602,170 @@ func (c *CompositeQuery) Sanitize() {
 
 func (c *CompositeQuery) Validate() error {
 	if c == nil {
-		return fmt.Errorf("composite query is required")
+		return errors.NewInvalidInputf(
+			errors.CodeInvalidInput,
+			"composite query is required",
+		)
 	}
 
-	if c.BuilderQueries == nil && c.ClickHouseQueries == nil && c.PromQueries == nil && len(c.Queries) == 0 {
-		return fmt.Errorf("composite query must contain at least one query type")
-	}
-
-	if c.QueryType == QueryTypeBuilder {
-		for name, query := range c.BuilderQueries {
-			if err := query.Validate(c.PanelType); err != nil {
-				return fmt.Errorf("builder query %s is invalid: %w", name, err)
-			}
-		}
-	}
-
-	if c.QueryType == QueryTypeClickHouseSQL {
-		for name, query := range c.ClickHouseQueries {
-			if err := query.Validate(); err != nil {
-				return fmt.Errorf("clickhouse query %s is invalid: %w", name, err)
-			}
-		}
-	}
-
-	if c.QueryType == QueryTypePromQL {
-		for name, query := range c.PromQueries {
-			if err := query.Validate(); err != nil {
-				return fmt.Errorf("prom query %s is invalid: %w", name, err)
-			}
+	// Validate unit if supplied
+	if c.Unit != "" {
+		unit := converter.Unit(c.Unit)
+		err := unit.Validate()
+		if err != nil {
+			return errors.NewInvalidInputf(
+				errors.CodeInvalidInput,
+				"invalid unit: %s",
+				err.Error(),
+			)
 		}
 	}
 
 	if err := c.PanelType.Validate(); err != nil {
-		return fmt.Errorf("panel type is invalid: %w", err)
+		return errors.NewInvalidInputf(
+			errors.CodeInvalidInput,
+			"panel type is invalid: %s",
+			err.Error(),
+		)
 	}
 
 	if err := c.QueryType.Validate(); err != nil {
-		return fmt.Errorf("query type is invalid: %w", err)
+		return errors.NewInvalidInputf(
+			errors.CodeInvalidInput,
+			"query type is invalid: %s",
+			err.Error(),
+		)
+	}
+
+	if len(c.Queries) == 0 {
+		return errors.NewInvalidInputf(
+			errors.CodeInvalidInput,
+			"at least one query is required",
+		)
+	}
+
+	// Validate each query
+	for i, envelope := range c.Queries {
+		queryId := qbtypes.GetQueryIdentifier(envelope, i)
+
+		switch envelope.Type {
+		case qbtypes.QueryTypeBuilder, qbtypes.QueryTypeSubQuery:
+			switch spec := envelope.Spec.(type) {
+			case qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation]:
+				if err := spec.Validate(qbtypes.RequestTypeTimeSeries); err != nil {
+					return errors.NewInvalidInputf(
+						errors.CodeInvalidInput,
+						"invalid %s: %s",
+						queryId,
+						err.Error(),
+					)
+				}
+			case qbtypes.QueryBuilderQuery[qbtypes.LogAggregation]:
+				if err := spec.Validate(qbtypes.RequestTypeTimeSeries); err != nil {
+					return errors.NewInvalidInputf(
+						errors.CodeInvalidInput,
+						"invalid %s: %s",
+						queryId,
+						err.Error(),
+					)
+				}
+			case qbtypes.QueryBuilderQuery[qbtypes.MetricAggregation]:
+				if err := spec.Validate(qbtypes.RequestTypeTimeSeries); err != nil {
+					return errors.NewInvalidInputf(
+						errors.CodeInvalidInput,
+						"invalid %s: %s",
+						queryId,
+						err.Error(),
+					)
+				}
+			default:
+				return errors.NewInvalidInputf(
+					errors.CodeInvalidInput,
+					"unknown query spec type for %s",
+					queryId,
+				)
+			}
+		case qbtypes.QueryTypePromQL:
+			spec, ok := envelope.Spec.(qbtypes.PromQuery)
+			if !ok {
+				return errors.NewInvalidInputf(
+					errors.CodeInvalidInput,
+					"invalid spec for %s",
+					queryId,
+				)
+			}
+			if spec.Query == "" {
+				return errors.NewInvalidInputf(
+					errors.CodeInvalidInput,
+					"query expression is required for %s",
+					queryId,
+				)
+			}
+			if err := validatePromQLQuery(spec.Query); err != nil {
+				return err
+			}
+		case qbtypes.QueryTypeClickHouseSQL:
+			spec, ok := envelope.Spec.(qbtypes.ClickHouseQuery)
+			if !ok {
+				return errors.NewInvalidInputf(
+					errors.CodeInvalidInput,
+					"invalid spec for %s",
+					queryId,
+				)
+			}
+			if spec.Query == "" {
+				return errors.NewInvalidInputf(
+					errors.CodeInvalidInput,
+					"query expression is required for %s",
+					queryId,
+				)
+			}
+			if err := validateClickHouseQuery(spec.Query); err != nil {
+				return err
+			}
+		case qbtypes.QueryTypeFormula:
+			spec, ok := envelope.Spec.(qbtypes.QueryBuilderFormula)
+			if !ok {
+				return errors.NewInvalidInputf(
+					errors.CodeInvalidInput,
+					"invalid spec for %s",
+					queryId,
+				)
+			}
+			if err := spec.Validate(); err != nil {
+				return err
+			}
+		case qbtypes.QueryTypeTraceOperator:
+			spec, ok := envelope.Spec.(qbtypes.QueryBuilderTraceOperator)
+			if !ok {
+				return errors.NewInvalidInputf(
+					errors.CodeInvalidInput,
+					"invalid spec for %s",
+					queryId,
+				)
+			}
+			err := spec.ValidateTraceOperator(c.Queries)
+			if err != nil {
+				return err
+			}
+		default:
+			return errors.NewInvalidInputf(
+				errors.CodeInvalidInput,
+				"unknown query type '%s' for %s",
+				envelope.Type,
+				queryId,
+			).WithAdditional(
+				"Valid query types are: builder_query, builder_sub_query, builder_formula, builder_join, promql, clickhouse_sql, trace_operator",
+			)
+		}
+	}
+
+	// Check if all queries are disabled
+	if allDisabled := checkQueriesDisabled(c); allDisabled {
+		return errors.NewInvalidInputf(
+			errors.CodeInvalidInput,
+			"all queries are disabled - at least one query must be enabled",
+		)
 	}
 
 	return nil
@@ -1203,7 +1331,7 @@ func (f *FilterSet) Scan(src interface{}) error {
 func (f *FilterSet) Value() (driver.Value, error) {
 	filterSetJson, err := json.Marshal(f)
 	if err != nil {
-		return nil, errors.Wrap(err, "could not serialize FilterSet to JSON")
+		return nil, errors.Wrapf(err, errors.TypeInternal, errors.CodeInternal, "could not serialize FilterSet to JSON")
 	}
 	return filterSetJson, nil
 }

pkg/query-service/model/v3/validation.go

@@ -0,0 +1,137 @@
+package v3
+
+import (
+	"bytes"
+	"fmt"
+	"text/template"
+	"time"
+
+	clickhouse "github.com/AfterShip/clickhouse-sql-parser/parser"
+	"github.com/SigNoz/signoz/pkg/errors"
+	querytemplate "github.com/SigNoz/signoz/pkg/query-service/utils/queryTemplate"
+	qbtypes "github.com/SigNoz/signoz/pkg/types/querybuildertypes/querybuildertypesv5"
+	"github.com/prometheus/prometheus/promql/parser"
+)
+
+type QueryParseError struct {
+	StartPosition *int
+	EndPosition   *int
+	ErrorMessage  string
+	Query         string
+}
+
+func (e *QueryParseError) Error() string {
+	if e.StartPosition != nil && e.EndPosition != nil {
+		return fmt.Sprintf("query parse error: %s at position %d:%d", e.ErrorMessage, *e.StartPosition, *e.EndPosition)
+	}
+	return fmt.Sprintf("query parse error: %s", e.ErrorMessage)
+}
+
+// validatePromQLQuery validates a PromQL query syntax using the Prometheus parser
+func validatePromQLQuery(query string) error {
+	_, err := parser.ParseExpr(query)
+	if err != nil {
+		if syntaxErrs, ok := err.(parser.ParseErrors); ok {
+			syntaxErr := syntaxErrs[0]
+			startPosition := int(syntaxErr.PositionRange.Start)
+			endPosition := int(syntaxErr.PositionRange.End)
+			return &QueryParseError{
+				StartPosition: &startPosition,
+				EndPosition:   &endPosition,
+				ErrorMessage:  syntaxErr.Error(),
+				Query:         query,
+			}
+		}
+	}
+	return err
+}
+
+// validateClickHouseQuery validates a ClickHouse SQL query syntax using the ClickHouse parser
+func validateClickHouseQuery(query string) error {
+	// Assign the default template variables with dummy values
+	variables := make(map[string]interface{})
+	start := time.Now().UnixMilli()
+	end := start + 1000
+	querytemplate.AssignReservedVars(variables, start, end)
+
+	// Apply the values for default template variables before parsing the query
+	tmpl := template.New("clickhouse-query")
+	tmpl, err := tmpl.Parse(query)
+	if err != nil {
+		return errors.NewInvalidInputf(
+			errors.CodeInvalidInput,
+			"failed to parse clickhouse query: %s",
+			err.Error(),
+		)
+	}
+	var queryBuffer bytes.Buffer
+	err = tmpl.Execute(&queryBuffer, variables)
+	if err != nil {
+		return errors.NewInvalidInputf(
+			errors.CodeInvalidInput,
+			"failed to execute clickhouse query template: %s",
+			err.Error(),
+		)
+	}
+
+	// Parse the ClickHouse query with the default template variables applied
+	p := clickhouse.NewParser(queryBuffer.String())
+	_, err = p.ParseStmts()
+	if err != nil {
+		// TODO: errors returned here is errors.errorString
+		// we should think on using some other library that parses the CH query in more accurate manner,
+		// current CH parser does very minimal checks and on just the known keywords, without validating the syntax of given query.
+		// Sample Error: "line 0:36 expected table name or subquery, got ;\nSELECT department, avg(salary) FROM ;\n                                    ^\n"
+		return &QueryParseError{
+			ErrorMessage: err.Error(),
+			Query:        query,
+		}
+	}
+	return nil
+}
+
+// checkQueriesDisabled checks if all queries are disabled. Returns true if all queries are disabled, false otherwise.
+func checkQueriesDisabled(compositeQuery *CompositeQuery) bool {
+	for _, envelope := range compositeQuery.Queries {
+		switch envelope.Type {
+		case qbtypes.QueryTypeBuilder, qbtypes.QueryTypeSubQuery:
+			switch spec := envelope.Spec.(type) {
+			case qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation]:
+				if !spec.Disabled {
+					return false
+				}
+			case qbtypes.QueryBuilderQuery[qbtypes.LogAggregation]:
+				if !spec.Disabled {
+					return false
+				}
+			case qbtypes.QueryBuilderQuery[qbtypes.MetricAggregation]:
+				if !spec.Disabled {
+					return false
+				}
+			}
+		case qbtypes.QueryTypeFormula:
+			if spec, ok := envelope.Spec.(qbtypes.QueryBuilderFormula); ok && !spec.Disabled {
+				return false
+			}
+		case qbtypes.QueryTypeTraceOperator:
+			if spec, ok := envelope.Spec.(qbtypes.QueryBuilderTraceOperator); ok && !spec.Disabled {
+				return false
+			}
+		case qbtypes.QueryTypeJoin:
+			if spec, ok := envelope.Spec.(qbtypes.QueryBuilderJoin); ok && !spec.Disabled {
+				return false
+			}
+		case qbtypes.QueryTypePromQL:
+			if spec, ok := envelope.Spec.(qbtypes.PromQuery); ok && !spec.Disabled {
+				return false
+			}
+		case qbtypes.QueryTypeClickHouseSQL:
+			if spec, ok := envelope.Spec.(qbtypes.ClickHouseQuery); ok && !spec.Disabled {
+				return false
+			}
+		}
+	}
+
+	// If we reach here, all queries are disabled
+	return true
+}

pkg/query-service/model/v3/validation_test.go

@@ -0,0 +1,528 @@
+package v3
+
+import (
+	"testing"
+
+	qbtypes "github.com/SigNoz/signoz/pkg/types/querybuildertypes/querybuildertypesv5"
+	"github.com/SigNoz/signoz/pkg/types/telemetrytypes"
+	"github.com/SigNoz/signoz/pkg/valuer"
+	"github.com/stretchr/testify/require"
+)
+
+func TestValidateCompositeQuery(t *testing.T) {
+
+	tests := []struct {
+		name           string
+		compositeQuery *CompositeQuery
+		wantErr        bool
+		errContains    string
+	}{
+		{
+			name:           "nil composite query should return error",
+			compositeQuery: nil,
+			wantErr:        true,
+			errContains:    "composite query is required",
+		},
+		{
+			name: "empty queries array should return error",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Queries: []qbtypes.QueryEnvelope{},
+			},
+			wantErr:     true,
+			errContains: "at least one query is required",
+		},
+		{
+			name: "invalid input error",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Unit: "some_invalid_unit",
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypePromQL,
+						Spec: qbtypes.PromQuery{
+							Name:  "prom_query",
+							Query: "rate(http_requests_total[5m])",
+						},
+					},
+				},
+			},
+			wantErr:     true,
+			errContains: "invalid unit",
+		},
+		{
+			name: "valid metric builder query should pass",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Unit: "bytes", // valid unit
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypeBuilder,
+						Spec: qbtypes.QueryBuilderQuery[qbtypes.MetricAggregation]{
+							Name:   "metric_query",
+							Signal: telemetrytypes.SignalMetrics,
+							Aggregations: []qbtypes.MetricAggregation{
+								{
+									MetricName: "cpu_usage",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "valid log builder query should pass",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Unit: "µs", // valid unit
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypeBuilder,
+						Spec: qbtypes.QueryBuilderQuery[qbtypes.LogAggregation]{
+							Name:   "log_query",
+							Signal: telemetrytypes.SignalLogs,
+							Aggregations: []qbtypes.LogAggregation{
+								{
+									Expression: "count()",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "valid trace builder query should pass",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Unit: "MBs", // valid unit
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypeBuilder,
+						Spec: qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation]{
+							Name:   "trace_query",
+							Signal: telemetrytypes.SignalTraces,
+							Aggregations: []qbtypes.TraceAggregation{
+								{
+									Expression: "count()",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "valid PromQL query should pass",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Unit: "{req}/s", // valid unit
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypePromQL,
+						Spec: qbtypes.PromQuery{
+							Name:  "prom_query",
+							Query: "rate(http_requests_total[5m])",
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "valid ClickHouse query should pass",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypeClickHouseSQL,
+						Spec: qbtypes.ClickHouseQuery{
+							Name:  "ch_query",
+							Query: "SELECT count(*) FROM metrics WHERE metric_name = 'cpu_usage'",
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "valid formula query should pass",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypeFormula,
+						Spec: qbtypes.QueryBuilderFormula{
+							Name:       "formula_query",
+							Expression: "A + B",
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		// We've not added support for join query yet
+		{
+			name: "valid join query should pass",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypeJoin,
+						Spec: qbtypes.QueryBuilderJoin{
+							Name:  "join_query",
+							Left:  qbtypes.QueryRef{Name: "A"},
+							Right: qbtypes.QueryRef{Name: "B"},
+							Type:  qbtypes.JoinTypeInner,
+							On:    "service_name",
+						},
+					},
+				},
+			},
+			wantErr: true,
+			errContains: "unknown query type",
+		},
+		{
+			name: "valid trace operator query should pass",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypeBuilder,
+						Spec: qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation]{
+							Name:   "A",
+							Signal: telemetrytypes.SignalTraces,
+							Aggregations: []qbtypes.TraceAggregation{
+								{
+									Expression: "count()",
+								},
+							},
+						},
+					},
+					{
+						Type: qbtypes.QueryTypeBuilder,
+						Spec: qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation]{
+							Name:   "B",
+							Signal: telemetrytypes.SignalTraces,
+							Aggregations: []qbtypes.TraceAggregation{
+								{
+									Expression: "count()",
+								},
+							},
+						},
+					},
+					{
+						Type: qbtypes.QueryTypeTraceOperator,
+						Spec: qbtypes.QueryBuilderTraceOperator{
+							Name:       "trace_operator",
+							Expression: "A && B",
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "invalid metric builder query - missing aggregation should return error",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypeBuilder,
+						Spec: qbtypes.QueryBuilderQuery[qbtypes.MetricAggregation]{
+							Name:         "metric_query",
+							Signal:       telemetrytypes.SignalMetrics,
+							Aggregations: []qbtypes.MetricAggregation{},
+						},
+					},
+				},
+			},
+			wantErr:     true,
+			errContains: "invalid",
+		},
+		{
+			name: "invalid PromQL query - empty query should return error",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypePromQL,
+						Spec: qbtypes.PromQuery{
+							Name:  "prom_query",
+							Query: "",
+						},
+					},
+				},
+			},
+			wantErr:     true,
+			errContains: "query expression is required",
+		},
+		{
+			name: "invalid PromQL query - syntax error should return error",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypePromQL,
+						Spec: qbtypes.PromQuery{
+							Name:  "prom_query",
+							Query: "rate(http_requests_total[5m",
+						},
+					},
+				},
+			},
+			wantErr:     true,
+			errContains: "unclosed left parenthesis",
+		},
+		{
+			name: "invalid ClickHouse query - empty query should return error",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypeClickHouseSQL,
+						Spec: qbtypes.ClickHouseQuery{
+							Name:  "ch_query",
+							Query: "",
+						},
+					},
+				},
+			},
+			wantErr:     true,
+			errContains: "query expression is required",
+		},
+		{
+			name: "invalid ClickHouse query - syntax error should return error",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypeClickHouseSQL,
+						Spec: qbtypes.ClickHouseQuery{
+							Name:  "ch_query",
+							Query: "SELECT * FROM metrics WHERE",
+						},
+					},
+				},
+			},
+			wantErr:     true,
+			errContains: "query parse error",
+		},
+		{
+			name: "invalid formula query - empty expression should return error",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypeFormula,
+						Spec: qbtypes.QueryBuilderFormula{
+							Name:       "formula_query",
+							Expression: "",
+						},
+					},
+				},
+			},
+			wantErr:     true,
+			errContains: "formula expression cannot be blank",
+		},
+		{
+			name: "invalid trace operator query - empty expression should return error",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypeTraceOperator,
+						Spec: qbtypes.QueryBuilderTraceOperator{
+							Name:       "trace_operator",
+							Expression: "",
+						},
+					},
+				},
+			},
+			wantErr:     true,
+			errContains: "expression cannot be empty",
+		},
+		{
+			name: "all queries disabled should return error",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypeBuilder,
+						Spec: qbtypes.QueryBuilderQuery[qbtypes.MetricAggregation]{
+							Name:     "metric_query",
+							Disabled: true,
+							Signal:   telemetrytypes.SignalMetrics,
+							Aggregations: []qbtypes.MetricAggregation{
+								{
+									MetricName: "cpu_usage",
+								},
+							},
+						},
+					},
+					{
+						Type: qbtypes.QueryTypePromQL,
+						Spec: qbtypes.PromQuery{
+							Name:     "prom_query",
+							Query:    "rate(http_requests_total[5m])",
+							Disabled: true,
+						},
+					},
+				},
+			},
+			wantErr:     true,
+			errContains: "all queries are disabled",
+		},
+		{
+			name: "mixed disabled and enabled queries should pass",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypeBuilder,
+						Spec: qbtypes.QueryBuilderQuery[qbtypes.MetricAggregation]{
+							Name:     "metric_query",
+							Disabled: true,
+							Signal:   telemetrytypes.SignalMetrics,
+							Aggregations: []qbtypes.MetricAggregation{
+								{
+									MetricName: "cpu_usage",
+								},
+							},
+						},
+					},
+					{
+						Type: qbtypes.QueryTypePromQL,
+						Spec: qbtypes.PromQuery{
+							Name:     "prom_query",
+							Query:    "rate(http_requests_total[5m])",
+							Disabled: false,
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "multiple valid queries should pass",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypeBuilder,
+						Spec: qbtypes.QueryBuilderQuery[qbtypes.MetricAggregation]{
+							Name:   "metric_query",
+							Signal: telemetrytypes.SignalMetrics,
+							Aggregations: []qbtypes.MetricAggregation{
+								{
+									MetricName: "cpu_usage",
+								},
+							},
+						},
+					},
+					{
+						Type: qbtypes.QueryTypePromQL,
+						Spec: qbtypes.PromQuery{
+							Name:  "prom_query",
+							Query: "rate(http_requests_total[5m])",
+						},
+					},
+					{
+						Type: qbtypes.QueryTypeClickHouseSQL,
+						Spec: qbtypes.ClickHouseQuery{
+							Name:  "ch_query",
+							Query: "SELECT count(*) FROM metrics WHERE metric_name = 'cpu_usage'",
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "invalid query in multiple queries should return error",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryTypeBuilder,
+						Spec: qbtypes.QueryBuilderQuery[qbtypes.MetricAggregation]{
+							Name:   "metric_query",
+							Signal: telemetrytypes.SignalMetrics,
+							Aggregations: []qbtypes.MetricAggregation{
+								{
+									MetricName: "cpu_usage",
+								},
+							},
+						},
+					},
+					{
+						Type: qbtypes.QueryTypePromQL,
+						Spec: qbtypes.PromQuery{
+							Name:  "prom_query",
+							Query: "invalid promql syntax [",
+						},
+					},
+				},
+			},
+			wantErr:     true,
+			errContains: "query parse error",
+		},
+		{
+			name: "unknown query type should return error",
+			compositeQuery: &CompositeQuery{
+				QueryType: QueryTypeBuilder,
+				PanelType: PanelTypeGraph,
+				Queries: []qbtypes.QueryEnvelope{
+					{
+						Type: qbtypes.QueryType{String: valuer.NewString("invalid_query_type")},
+						Spec: qbtypes.PromQuery{
+							Name:  "prom_query",
+							Query: "rate(http_requests_total[5m])",
+						},
+					},
+				},
+			},
+			wantErr:     true,
+			errContains: "unknown query type",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := tt.compositeQuery.Validate()
+			if tt.wantErr {
+				require.Error(t, err)
+				if tt.errContains != "" {
+					require.Contains(t, err.Error(), tt.errContains)
+				}
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}

pkg/query-service/rules/threshold_rule.go

@@ -133,7 +133,7 @@ func (r *ThresholdRule) prepareQueryRange(ctx context.Context, ts time.Time) (*v
 			Variables: make(map[string]interface{}, 0),
 			NoCache:   true,
 		}
-		querytemplate.AssignReservedVarsV3(params)
+		querytemplate.AssignReservedVars(params.Variables, start, end)
 		for name, chQuery := range r.ruleCondition.CompositeQuery.ClickHouseQueries {
 			if chQuery.Disabled {
 				continue

pkg/query-service/utils/queryTemplate/vars.go

@@ -2,26 +2,21 @@ package querytemplate
 
 import (
 	"fmt"
-
-	v3 "github.com/SigNoz/signoz/pkg/query-service/model/v3"
 )
 
-// AssignReservedVars assigns values for go template vars. assumes that
-// model.QueryRangeParamsV3.Start and End are Unix Nano timestamps
-func AssignReservedVarsV3(queryRangeParams *v3.QueryRangeParamsV3) {
-	queryRangeParams.Variables["start_timestamp"] = queryRangeParams.Start / 1000
-	queryRangeParams.Variables["end_timestamp"] = queryRangeParams.End / 1000
+func AssignReservedVars(variables map[string]interface{}, start int64, end int64) {
+	variables["start_timestamp"] = start / 1000
+	variables["end_timestamp"] = end / 1000
 
-	queryRangeParams.Variables["start_timestamp_ms"] = queryRangeParams.Start
-	queryRangeParams.Variables["end_timestamp_ms"] = queryRangeParams.End
+	variables["start_timestamp_ms"] = start
+	variables["end_timestamp_ms"] = end
 
-	queryRangeParams.Variables["SIGNOZ_START_TIME"] = queryRangeParams.Start
-	queryRangeParams.Variables["SIGNOZ_END_TIME"] = queryRangeParams.End
+	variables["SIGNOZ_START_TIME"] = start
+	variables["SIGNOZ_END_TIME"] = end
 
-	queryRangeParams.Variables["start_timestamp_nano"] = queryRangeParams.Start * 1e6
-	queryRangeParams.Variables["end_timestamp_nano"] = queryRangeParams.End * 1e6
-
-	queryRangeParams.Variables["start_datetime"] = fmt.Sprintf("toDateTime(%d)", queryRangeParams.Start/1000)
-	queryRangeParams.Variables["end_datetime"] = fmt.Sprintf("toDateTime(%d)", queryRangeParams.End/1000)
+	variables["start_timestamp_nano"] = start * 1e6
+	variables["end_timestamp_nano"] = end * 1e6
 
+	variables["start_datetime"] = fmt.Sprintf("toDateTime(%d)", start/1000)
+	variables["end_datetime"] = fmt.Sprintf("toDateTime(%d)", end/1000)
 }

pkg/types/querybuildertypes/querybuildertypesv5/builder_elements.go

@@ -553,6 +553,11 @@ func (f Function) Copy() Function {
 	return c
 }
 
+// Validate validates the Function by calling Validate on its Name
+func (f Function) Validate() error {
+	return f.Name.Validate()
+}
+
 type LimitBy struct {
 	// keys to limit by
 	Keys []string `json:"keys"`

pkg/types/querybuildertypes/querybuildertypesv5/formula.go

@@ -73,6 +73,43 @@ func (f *QueryBuilderFormula) UnmarshalJSON(data []byte) error {
 	return nil
 }
 
+// Validate checks if the QueryBuilderFormula fields are valid
+func (f QueryBuilderFormula) Validate() error {
+	// Validate name is not blank
+	if strings.TrimSpace(f.Name) == "" {
+		return errors.NewInvalidInputf(
+			errors.CodeInvalidInput,
+			"formula name cannot be blank",
+		)
+	}
+
+	// Validate expression is not blank
+	if strings.TrimSpace(f.Expression) == "" {
+		return errors.NewInvalidInputf(
+			errors.CodeInvalidInput,
+			"formula expression cannot be blank",
+		)
+	}
+
+	// Validate functions if present
+	for i, fn := range f.Functions {
+		if err := fn.Validate(); err != nil {
+			fnId := fmt.Sprintf("function #%d", i+1)
+			if f.Name != "" {
+				fnId = fmt.Sprintf("function #%d in formula '%s'", i+1, f.Name)
+			}
+			return errors.NewInvalidInputf(
+				errors.CodeInvalidInput,
+				"invalid %s: %s",
+				fnId,
+				err.Error(),
+			)
+		}
+	}
+
+	return nil
+}
+
 // small container to store the query name and index or alias reference
 // for a variable in the formula expression
 // read below for more details on aggregation references

pkg/types/querybuildertypes/querybuildertypesv5/functions.go

@@ -5,6 +5,7 @@ import (
 	"slices"
 	"strconv"
 
+	"github.com/SigNoz/signoz/pkg/errors"
 	"github.com/SigNoz/signoz/pkg/valuer"
 )
 
@@ -33,6 +34,37 @@ var (
 	FunctionNameFillZero      = FunctionName{valuer.NewString("fillZero")}
 )
 
+// Validate checks if the FunctionName is valid and one of the known types
+func (fn FunctionName) Validate() error {
+	switch fn {
+	case FunctionNameCutOffMin,
+		FunctionNameCutOffMax,
+		FunctionNameClampMin,
+		FunctionNameClampMax,
+		FunctionNameAbsolute,
+		FunctionNameRunningDiff,
+		FunctionNameLog2,
+		FunctionNameLog10,
+		FunctionNameCumulativeSum,
+		FunctionNameEWMA3,
+		FunctionNameEWMA5,
+		FunctionNameEWMA7,
+		FunctionNameMedian3,
+		FunctionNameMedian5,
+		FunctionNameMedian7,
+		FunctionNameTimeShift,
+		FunctionNameAnomaly,
+		FunctionNameFillZero:
+		return nil
+	default:
+		return errors.NewInvalidInputf(
+			errors.CodeInvalidInput,
+			"invalid function name: %s",
+			fn.StringValue(),
+		)
+	}
+}
+
 // ApplyFunction applies the given function to the result data
 func ApplyFunction(fn Function, result *TimeSeries) *TimeSeries {
 	// Extract the function name and arguments

pkg/types/querybuildertypes/querybuildertypesv5/validation.go

@@ -10,8 +10,8 @@ import (
 	"github.com/SigNoz/signoz/pkg/types/telemetrytypes"
 )
 
-// getQueryIdentifier returns a friendly identifier for a query based on its type and name/content
-func getQueryIdentifier(envelope QueryEnvelope, index int) string {
+// GetQueryIdentifier returns a friendly identifier for a query based on its type and name/content
+func GetQueryIdentifier(envelope QueryEnvelope, index int) string {
 	switch envelope.Type {
 	case QueryTypeBuilder, QueryTypeSubQuery:
 		switch spec := envelope.Spec.(type) {
@@ -567,7 +567,7 @@ func (r *QueryRangeRequest) validateCompositeQuery() error {
 			switch spec := envelope.Spec.(type) {
 			case QueryBuilderQuery[TraceAggregation]:
 				if err := spec.Validate(r.RequestType); err != nil {
-					queryId := getQueryIdentifier(envelope, i)
+					queryId := GetQueryIdentifier(envelope, i)
 					return wrapValidationError(err, queryId, "invalid %s: %s")
 				}
 				// Check name uniqueness for non-formula context
@@ -583,7 +583,7 @@ func (r *QueryRangeRequest) validateCompositeQuery() error {
 				}
 			case QueryBuilderQuery[LogAggregation]:
 				if err := spec.Validate(r.RequestType); err != nil {
-					queryId := getQueryIdentifier(envelope, i)
+					queryId := GetQueryIdentifier(envelope, i)
 					return wrapValidationError(err, queryId, "invalid %s: %s")
 				}
 				// Check name uniqueness for non-formula context
@@ -599,7 +599,7 @@ func (r *QueryRangeRequest) validateCompositeQuery() error {
 				}
 			case QueryBuilderQuery[MetricAggregation]:
 				if err := spec.Validate(r.RequestType); err != nil {
-					queryId := getQueryIdentifier(envelope, i)
+					queryId := GetQueryIdentifier(envelope, i)
 					return wrapValidationError(err, queryId, "invalid %s: %s")
 				}
 				// Check name uniqueness for non-formula context
@@ -614,7 +614,7 @@ func (r *QueryRangeRequest) validateCompositeQuery() error {
 					queryNames[spec.Name] = true
 				}
 			default:
-				queryId := getQueryIdentifier(envelope, i)
+				queryId := GetQueryIdentifier(envelope, i)
 				return errors.NewInvalidInputf(
 					errors.CodeInvalidInput,
 					"unknown spec type for %s",
@@ -625,7 +625,7 @@ func (r *QueryRangeRequest) validateCompositeQuery() error {
 			// Formula validation is handled separately
 			spec, ok := envelope.Spec.(QueryBuilderFormula)
 			if !ok {
-				queryId := getQueryIdentifier(envelope, i)
+				queryId := GetQueryIdentifier(envelope, i)
 				return errors.NewInvalidInputf(
 					errors.CodeInvalidInput,
 					"invalid spec for %s",
@@ -633,7 +633,7 @@ func (r *QueryRangeRequest) validateCompositeQuery() error {
 				)
 			}
 			if spec.Expression == "" {
-				queryId := getQueryIdentifier(envelope, i)
+				queryId := GetQueryIdentifier(envelope, i)
 				return errors.NewInvalidInputf(
 					errors.CodeInvalidInput,
 					"expression is required for %s",
@@ -644,7 +644,7 @@ func (r *QueryRangeRequest) validateCompositeQuery() error {
 			// Join validation is handled separately
 			_, ok := envelope.Spec.(QueryBuilderJoin)
 			if !ok {
-				queryId := getQueryIdentifier(envelope, i)
+				queryId := GetQueryIdentifier(envelope, i)
 				return errors.NewInvalidInputf(
 					errors.CodeInvalidInput,
 					"invalid spec for %s",
@@ -654,7 +654,7 @@ func (r *QueryRangeRequest) validateCompositeQuery() error {
 		case QueryTypeTraceOperator:
 			spec, ok := envelope.Spec.(QueryBuilderTraceOperator)
 			if !ok {
-				queryId := getQueryIdentifier(envelope, i)
+				queryId := GetQueryIdentifier(envelope, i)
 				return errors.NewInvalidInputf(
 					errors.CodeInvalidInput,
 					"invalid spec for %s",
@@ -662,7 +662,7 @@ func (r *QueryRangeRequest) validateCompositeQuery() error {
 				)
 			}
 			if spec.Expression == "" {
-				queryId := getQueryIdentifier(envelope, i)
+				queryId := GetQueryIdentifier(envelope, i)
 				return errors.NewInvalidInputf(
 					errors.CodeInvalidInput,
 					"expression is required for %s",
@@ -673,7 +673,7 @@ func (r *QueryRangeRequest) validateCompositeQuery() error {
 			// PromQL validation is handled separately
 			spec, ok := envelope.Spec.(PromQuery)
 			if !ok {
-				queryId := getQueryIdentifier(envelope, i)
+				queryId := GetQueryIdentifier(envelope, i)
 				return errors.NewInvalidInputf(
 					errors.CodeInvalidInput,
 					"invalid spec for %s",
@@ -681,7 +681,7 @@ func (r *QueryRangeRequest) validateCompositeQuery() error {
 				)
 			}
 			if spec.Query == "" {
-				queryId := getQueryIdentifier(envelope, i)
+				queryId := GetQueryIdentifier(envelope, i)
 				return errors.NewInvalidInputf(
 					errors.CodeInvalidInput,
 					"query expression is required for %s",
@@ -692,7 +692,7 @@ func (r *QueryRangeRequest) validateCompositeQuery() error {
 			// ClickHouse SQL validation is handled separately
 			spec, ok := envelope.Spec.(ClickHouseQuery)
 			if !ok {
-				queryId := getQueryIdentifier(envelope, i)
+				queryId := GetQueryIdentifier(envelope, i)
 				return errors.NewInvalidInputf(
 					errors.CodeInvalidInput,
 					"invalid spec for %s",
@@ -700,7 +700,7 @@ func (r *QueryRangeRequest) validateCompositeQuery() error {
 				)
 			}
 			if spec.Query == "" {
-				queryId := getQueryIdentifier(envelope, i)
+				queryId := GetQueryIdentifier(envelope, i)
 				return errors.NewInvalidInputf(
 					errors.CodeInvalidInput,
 					"query expression is required for %s",
@@ -708,7 +708,7 @@ func (r *QueryRangeRequest) validateCompositeQuery() error {
 				)
 			}
 		default:
-			queryId := getQueryIdentifier(envelope, i)
+			queryId := GetQueryIdentifier(envelope, i)
 			return errors.NewInvalidInputf(
 				errors.CodeInvalidInput,
 				"unknown query type '%s' for %s",
@@ -735,7 +735,7 @@ func (c *CompositeQuery) Validate(requestType RequestType) error {
 	// Validate each query
 	for i, envelope := range c.Queries {
 		if err := validateQueryEnvelope(envelope, requestType); err != nil {
-			queryId := getQueryIdentifier(envelope, i)
+			queryId := GetQueryIdentifier(envelope, i)
 			return wrapValidationError(err, queryId, "invalid %s: %s")
 		}
 	}

pkg/types/ruletypes/api_params_test.go

@@ -405,7 +405,7 @@ func TestParseIntoRuleSchemaVersioning(t *testing.T) {
 						"spec": [{
 							"name": "existing_threshold",
 							"target": 50.0,
-							"targetUnit": "MB",
+							"targetUnit": "MBs",
 							"ruleUnit": "bytes",
 							"matchType": "1",
 							"op": "1"

pkg/types/ruletypes/threshold.go

@@ -252,6 +252,15 @@ func (b BasicRuleThreshold) Validate() error {
 		errs = append(errs, errors.NewInvalidInputf(errors.CodeInvalidInput, "invalid match type: %s", string(b.MatchType)))
 	}
 
+	// Only validate unit if specified
+	if b.TargetUnit != "" {
+		unit := converter.Unit(b.TargetUnit)
+		err := unit.Validate()
+		if err != nil {
+			errs = append(errs, errors.NewInvalidInputf(errors.CodeInvalidInput, "invalid unit"))
+		}
+	}
+
 	return errors.Join(errs...)
 }