LogQL: [optimization] syntax: Replace "panic" in "/pkg/logql/syntax" with "error"

pkg/ingester/instance.go

@@ -425,11 +425,14 @@ func (i *instance) QuerySample(ctx context.Context, req logql.SelectSampleParams
 	if len(shards) == 1 {
 		shard = &shards[0]
 	}
-
+	selector, err := expr.Selector()
+	if err != nil {
+		return nil, err
+	}
 	err = i.forMatchingStreams(
 		ctx,
 		req.Start,
-		expr.Selector().Matchers(),
+		selector.Matchers(),
 		shard,
 		func(stream *stream) error {
 			iter, err := stream.SampleIterator(ctx, stats, req.Start, req.End, extractor.ForStream(stream.labels))

pkg/logql/engine.go

@@ -99,7 +99,7 @@ func (s SelectSampleParams) LogSelector() (syntax.LogSelectorExpr, error) {
 	if err != nil {
 		return nil, err
 	}
-	return expr.Selector(), nil
+	return expr.Selector()
 }
 
 // Querier allows a LogQL expression to fetch an EntryIterator for a
@@ -403,9 +403,13 @@ func (q *query) evalSample(ctx context.Context, expr syntax.SampleExpr) (promql_
 }
 
 func (q *query) evalLiteral(_ context.Context, expr *syntax.LiteralExpr) (promql_parser.Value, error) {
+	value, err := expr.Value()
+	if err != nil {
+		return nil, err
+	}
 	s := promql.Scalar{
 		T: q.params.Start().UnixNano() / int64(time.Millisecond),
-		V: expr.Value(),
+		V: value,
 	}
 
 	if GetRangeType(q.params) == InstantType {

pkg/logql/evaluator.go

@@ -472,9 +472,13 @@ func rangeAggEvaluator(
 		return nil, err
 	}
 	if expr.Operation == syntax.OpRangeTypeAbsent {
+		absentLabels, err := absentLabels(expr)
+		if err != nil {
+			return nil, err
+		}
 		return &absentRangeVectorEvaluator{
 			iter: iter,
-			lbs:  absentLabels(expr),
+			lbs:  absentLabels,
 		}, nil
 	}
 	return &rangeVectorEvaluator{
@@ -771,8 +775,11 @@ func vectorBinop(op string, opts *syntax.BinOpOptions, lhs, rhs promql.Vector, l
 				ls, rs = rs, ls
 			}
 		}
-
-		if merged := syntax.MergeBinOp(op, ls, rs, filter, syntax.IsComparisonOperator(op)); merged != nil {
+		merged, err := syntax.MergeBinOp(op, ls, rs, filter, syntax.IsComparisonOperator(op))
+		if err != nil {
+			return nil, err
+		}
+		if merged != nil {
 			// replace with labels specified by expr
 			merged.Metric = metric
 			results = append(results, *merged)
@@ -886,37 +893,52 @@ func literalStepEvaluator(
 	inverted bool,
 	returnBool bool,
 ) (StepEvaluator, error) {
+	val, err := lit.Value()
+	if err != nil {
+		return nil, err
+	}
+	var mergeErr error
+
 	return newStepEvaluator(
 		func() (bool, int64, promql.Vector) {
 			ok, ts, vec := eval.Next()
-
 			results := make(promql.Vector, 0, len(vec))
 			for _, sample := range vec {
+
 				literalPoint := promql.Sample{
 					Metric: sample.Metric,
-					Point:  promql.Point{T: ts, V: lit.Value()},
+					Point:  promql.Point{T: ts, V: val},
 				}
 
 				left, right := &literalPoint, &sample
 				if inverted {
 					left, right = right, left
 				}
-
-				if merged := syntax.MergeBinOp(
+				merged, err := syntax.MergeBinOp(
 					op,
 					left,
 					right,
 					!returnBool,
 					syntax.IsComparisonOperator(op),
-				); merged != nil {
+				)
+				if err != nil {
+					mergeErr = err
+					return false, 0, nil
+				}
+				if merged != nil {
 					results = append(results, *merged)
 				}
 			}
 
 			return ok, ts, results
 		},
 		eval.Close,
-		eval.Error,
+		func() error {
+			if mergeErr != nil {
+				return mergeErr
+			}
+			return eval.Error()
+		},
 	)
 }
 
@@ -1010,12 +1032,16 @@ func labelReplaceEvaluator(
 }
 
 // This is to replace missing timeseries during absent_over_time aggregation.
-func absentLabels(expr syntax.SampleExpr) labels.Labels {
+func absentLabels(expr syntax.SampleExpr) (labels.Labels, error) {
 	m := labels.Labels{}
 
-	lm := expr.Selector().Matchers()
+	selector, err := expr.Selector()
+	if err != nil {
+		return nil, err
+	}
+	lm := selector.Matchers()
 	if len(lm) == 0 {
-		return m
+		return m, nil
 	}
 
 	empty := []string{}
@@ -1033,5 +1059,5 @@ func absentLabels(expr syntax.SampleExpr) labels.Labels {
 	for _, v := range empty {
 		m = labels.NewBuilder(m).Del(v).Labels(nil)
 	}
-	return m
+	return m, nil
 }

pkg/logql/evaluator_test.go

@@ -11,7 +11,7 @@ import (
 )
 
 func TestDefaultEvaluator_DivideByZero(t *testing.T) {
-	require.Equal(t, true, math.IsNaN(syntax.MergeBinOp(syntax.OpTypeDiv,
+	op, err := syntax.MergeBinOp(syntax.OpTypeDiv,
 		&promql.Sample{
 			Point: promql.Point{T: 1, V: 1},
 		},
@@ -20,9 +20,11 @@ func TestDefaultEvaluator_DivideByZero(t *testing.T) {
 		},
 		false,
 		false,
-	).Point.V))
+	)
+	require.NoError(t, err)
 
-	require.Equal(t, true, math.IsNaN(syntax.MergeBinOp(syntax.OpTypeMod,
+	require.Equal(t, true, math.IsNaN(op.Point.V))
+	binOp, err := syntax.MergeBinOp(syntax.OpTypeMod,
 		&promql.Sample{
 			Point: promql.Point{T: 1, V: 1},
 		},
@@ -31,7 +33,9 @@ func TestDefaultEvaluator_DivideByZero(t *testing.T) {
 		},
 		false,
 		false,
-	).Point.V))
+	)
+	require.NoError(t, err)
+	require.Equal(t, true, math.IsNaN(binOp.Point.V))
 }
 func TestDefaultEvaluator_Sortable(t *testing.T) {
 	logqlSort := `sort(rate(({app=~"foo|bar"} |~".+bar")[1m])) `
@@ -216,20 +220,33 @@ func TestEvaluator_mergeBinOpComparisons(t *testing.T) {
 		t.Run(tc.desc, func(t *testing.T) {
 			// comparing a binop should yield the unfiltered (non-nil variant) regardless
 			// of whether this is a vector-vector comparison or not.
-			require.Equal(t, tc.expected, syntax.MergeBinOp(tc.op, tc.lhs, tc.rhs, false, false))
-			require.Equal(t, tc.expected, syntax.MergeBinOp(tc.op, tc.lhs, tc.rhs, false, true))
+			op, err := syntax.MergeBinOp(tc.op, tc.lhs, tc.rhs, false, false)
+			require.NoError(t, err)
+			require.Equal(t, tc.expected, op)
+			op2, err := syntax.MergeBinOp(tc.op, tc.lhs, tc.rhs, false, true)
+			require.NoError(t, err)
+			require.Equal(t, tc.expected, op2)
 
-			require.Nil(t, syntax.MergeBinOp(tc.op, tc.lhs, nil, false, true))
+			op3, err := syntax.MergeBinOp(tc.op, tc.lhs, nil, false, true)
+			require.NoError(t, err)
+			require.Nil(t, op3)
 
 			//  test filtered variants
 			if tc.expected.V == 0 {
 				//  ensure zeroed predicates are filtered out
-				require.Nil(t, syntax.MergeBinOp(tc.op, tc.lhs, tc.rhs, true, false))
-				require.Nil(t, syntax.MergeBinOp(tc.op, tc.lhs, tc.rhs, true, true))
+
+				op, err := syntax.MergeBinOp(tc.op, tc.lhs, tc.rhs, true, false)
+				require.NoError(t, err)
+				require.Nil(t, op)
+				op2, err := syntax.MergeBinOp(tc.op, tc.lhs, tc.rhs, true, true)
+				require.NoError(t, err)
+				require.Nil(t, op2)
 
 				// for vector-vector comparisons, ensure that nil right hand sides
 				// translate into nil results
-				require.Nil(t, syntax.MergeBinOp(tc.op, tc.lhs, nil, true, true))
+				op3, err := syntax.MergeBinOp(tc.op, tc.lhs, nil, true, true)
+				require.NoError(t, err)
+				require.Nil(t, op3)
 
 			}
 		})

pkg/logql/log/label_filter.go

@@ -269,6 +269,7 @@ type NumericLabelFilter struct {
 	Name  string
 	Value float64
 	Type  LabelFilterType
+	err   error
 }
 
 // NewNumericLabelFilter creates a new label filterer which parses float64 string representation (5.2)