index planning: Remove unnecessary error handling (#12738)

The `estimatePredicateCardinality` function wasn't actually returning
any errors in its implementation, so this simplifies the function
signature by removing the unused error return value.

This cleanup removes dead code and simplifies the calling sites.

---------

Signed-off-by: Dimitar Dimitrov <[email protected]>

Author: dimitarvdimitrov

pkg/ingester/lookupplan/plan.go

@@ -4,7 +4,6 @@ package lookupplan
 
 import (
 	"context"
-	"fmt"
 	"slices"
 
 	"github.com/prometheus/prometheus/model/labels"
@@ -31,22 +30,19 @@ type plan struct {
 }
 
 // newScanOnlyPlan returns a plan in which all predicates would be used to scan and none to reach from the index.
-func newScanOnlyPlan(ctx context.Context, stats index.Statistics, matchers []*labels.Matcher) (plan, error) {
+func newScanOnlyPlan(ctx context.Context, stats index.Statistics, matchers []*labels.Matcher) plan {
 	p := plan{
 		predicates:     make([]planPredicate, 0, len(matchers)),
 		indexPredicate: make([]bool, 0, len(matchers)),
 		totalSeries:    stats.TotalSeries(),
 	}
 	for _, m := range matchers {
-		pred, err := newPlanPredicate(ctx, m, stats)
-		if err != nil {
-			return plan{}, fmt.Errorf("error converting matcher to plan predicate: %w", err)
-		}
+		pred := newPlanPredicate(ctx, m, stats)
 		p.predicates = append(p.predicates, pred)
 		p.indexPredicate = append(p.indexPredicate, false)
 	}
 
-	return p, nil
+	return p
 }
 
 func (p plan) IndexMatchers() []*labels.Matcher {

pkg/ingester/lookupplan/plan_test.go

@@ -10,7 +10,6 @@ import (
 
 	"github.com/prometheus/prometheus/model/labels"
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 
 	"github.com/grafana/mimir/pkg/util"
 )
@@ -74,8 +73,7 @@ func TestPlanCosts(t *testing.T) {
 			allMatchers = append(allMatchers, tc.scanMatchers...)
 
 			// Create a scan-only plan with all matchers
-			p, err := newScanOnlyPlan(ctx, stats, allMatchers)
-			require.NoError(t, err)
+			p := newScanOnlyPlan(ctx, stats, allMatchers)
 
 			// Use index for the first N predicates (corresponding to index matchers)
 			for i := 0; i < len(tc.indexMatchers); i++ {

pkg/ingester/lookupplan/planner.go

@@ -55,10 +55,7 @@ func (p CostBasedPlanner) PlanIndexLookup(ctx context.Context, inPlan index.Look
 		return inPlan, errTooManyMatchers
 	}
 
-	allPlansUnordered, err := p.generatePlans(ctx, p.stats, matchers)
-	if err != nil {
-		return nil, fmt.Errorf("error generating plans: %w", err)
-	}
+	allPlansUnordered := p.generatePlans(ctx, p.stats, matchers)
 
 	type planWithCost struct {
 		plan
@@ -93,14 +90,11 @@ func (p CostBasedPlanner) PlanIndexLookup(ctx context.Context, inPlan index.Look
 
 var errTooManyMatchers = errors.New("too many matchers to generate plans")
 
-func (p CostBasedPlanner) generatePlans(ctx context.Context, statistics index.Statistics, matchers []*labels.Matcher) ([]plan, error) {
-	noopPlan, err := newScanOnlyPlan(ctx, statistics, matchers)
-	if err != nil {
-		return nil, fmt.Errorf("error generating index lookup plan: %w", err)
-	}
+func (p CostBasedPlanner) generatePlans(ctx context.Context, statistics index.Statistics, matchers []*labels.Matcher) []plan {
+	noopPlan := newScanOnlyPlan(ctx, statistics, matchers)
 	allPlans := make([]plan, 0, 1<<uint(len(matchers)))
 
-	return generatePredicateCombinations(allPlans, noopPlan, 0), nil
+	return generatePredicateCombinations(allPlans, noopPlan, 0)
 }
 
 // generatePredicateCombinations recursively generates all possible plans with their predicates toggled as index or as scan predicates.

pkg/ingester/lookupplan/predicate.go

@@ -4,7 +4,6 @@ package lookupplan
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/prometheus/prometheus/model/labels"
 	"github.com/prometheus/prometheus/tsdb/index"
@@ -27,26 +26,18 @@ type planPredicate struct {
 	indexScanCost float64
 }
 
-func newPlanPredicate(ctx context.Context, m *labels.Matcher, stats index.Statistics) (planPredicate, error) {
-	var err error
+func newPlanPredicate(ctx context.Context, m *labels.Matcher, stats index.Statistics) planPredicate {
 	pred := planPredicate{
 		matcher:         m,
 		singleMatchCost: m.SingleMatchCost(),
 	}
 	pred.labelNameUniqueVals = stats.LabelValuesCount(ctx, m.Name)
-	if err != nil {
-		return planPredicate{}, fmt.Errorf("error getting label values count for label %s: %w", m.Name, err)
-	}
 	pred.selectivity = m.EstimateSelectivity(pred.labelNameUniqueVals)
 
-	pred.cardinality, err = estimatePredicateCardinality(ctx, m, stats, pred.selectivity)
-	if err != nil {
-		return planPredicate{}, fmt.Errorf("error estimating cardinality for label %s: %w", m.Name, err)
-	}
-
+	pred.cardinality = estimatePredicateCardinality(ctx, m, stats, pred.selectivity)
 	pred.indexScanCost = estimatePredicateIndexScanCost(pred, m)
 
-	return pred, nil
+	return pred
 }
 
 func estimatePredicateIndexScanCost(pred planPredicate, m *labels.Matcher) float64 {
@@ -75,11 +66,10 @@ func estimatePredicateIndexScanCost(pred planPredicate, m *labels.Matcher) float
 	panic("estimatePredicateIndexScanCost called with unhandled matcher type: " + m.Type.String() + m.String())
 }
 
-func estimatePredicateCardinality(ctx context.Context, m *labels.Matcher, stats index.Statistics, selectivity float64) (uint64, error) {
+func estimatePredicateCardinality(ctx context.Context, m *labels.Matcher, stats index.Statistics, selectivity float64) uint64 {
 	var (
 		seriesBehindSelectedValues uint64
 		matchesAnyValues           bool
-		err                        error
 	)
 
 	switch m.Type {
@@ -107,18 +97,15 @@ func estimatePredicateCardinality(ctx context.Context, m *labels.Matcher, stats
 			seriesBehindSelectedValues += uint64(float64(labelNameCardinality) * selectivity)
 		}
 	}
-	if err != nil {
-		return 0, fmt.Errorf("error getting series per label value for label %s: %w", m.Name, err)
-	}
 	switch m.Type {
 	case labels.MatchNotEqual, labels.MatchNotRegexp:
 		if !matchesAnyValues {
 			// This label name doesn't exist. This means that negating this will select everything.
-			return stats.TotalSeries(), nil
+			return stats.TotalSeries()
 		}
-		return stats.TotalSeries() - seriesBehindSelectedValues, nil
+		return stats.TotalSeries() - seriesBehindSelectedValues
 	}
-	return seriesBehindSelectedValues, nil
+	return seriesBehindSelectedValues
 }
 
 func (pr planPredicate) indexLookupCost() float64 {

pkg/ingester/lookupplan/predicate_test.go

@@ -46,8 +46,7 @@ func TestFilterCost(t *testing.T) {
 	for _, tc := range testCases {
 		testName := fmt.Sprintf("%d_series_%s", tc.seriesCount, tc.matcher)
 		t.Run(testName, func(t *testing.T) {
-			pred, err := newPlanPredicate(ctx, tc.matcher, stats)
-			assert.NoError(t, err)
+			pred := newPlanPredicate(ctx, tc.matcher, stats)
 
 			cost := pred.filterCost(tc.seriesCount)
 			assert.GreaterOrEqual(t, cost, 0.0, "can't have negative costs")
@@ -89,8 +88,7 @@ func TestIndexLookupCost(t *testing.T) {
 
 	for tcIdx, tc := range testCases {
 		t.Run(tc.matcher.String(), func(t *testing.T) {
-			pred, err := newPlanPredicate(ctx, tc.matcher, stats)
-			assert.NoError(t, err)
+			pred := newPlanPredicate(ctx, tc.matcher, stats)
 
 			actualCost := pred.indexLookupCost()
 			assert.GreaterOrEqual(t, actualCost, 0.0, "can't have negative cost")
@@ -135,9 +133,7 @@ func TestCardinalityEstimation(t *testing.T) {
 
 	for tcIdx, tc := range testCases {
 		t.Run(tc.matcher.String(), func(t *testing.T) {
-			pred, err := newPlanPredicate(ctx, tc.matcher, stats)
-			assert.NoError(t, err)
-
+			pred := newPlanPredicate(ctx, tc.matcher, stats)
 			actualCardinality := pred.cardinality
 
 			if tc.deltaTolerance > 0 {