From 102c507c3f5931381456e23fc487e52ef1c182e9 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Mon, 20 Nov 2023 19:24:17 +0100 Subject: [PATCH] plan, execution: use only internal vector selector Signed-off-by: Michael Hoffmann --- execution/execution.go | 9 --------- execution/function/absent.go | 6 +++--- logicalplan/distribute.go | 4 ++-- logicalplan/filter.go | 6 +++--- logicalplan/merge_selects.go | 10 +--------- logicalplan/passthrough.go | 2 +- logicalplan/plan.go | 17 ++++++++++++----- logicalplan/propagate_selectors.go | 6 +++--- logicalplan/set_batch_size.go | 5 ----- logicalplan/sort_matchers.go | 2 +- 10 files changed, 26 insertions(+), 41 deletions(-) diff --git a/execution/execution.go b/execution/execution.go index d0b3711e..87209f1f 100644 --- a/execution/execution.go +++ b/execution/execution.go @@ -62,13 +62,6 @@ func newOperator(expr parser.Expr, storage *engstore.SelectorPool, opts *query.O case *parser.NumberLiteral: return scan.NewNumberLiteralSelector(model.NewVectorPool(opts.StepsBatch), opts, e.Val), nil - case *parser.VectorSelector: - start, end := getTimeRangesForVectorSelector(e, opts, 0) - hints.Start = start - hints.End = end - filter := storage.GetSelector(start, end, opts.Step.Milliseconds(), e.LabelMatchers, hints) - return newShardedVectorSelector(filter, opts, e.Offset, 0) - case *logicalplan.VectorSelector: start, end := getTimeRangesForVectorSelector(e.VectorSelector, opts, 0) hints.Start = start @@ -209,8 +202,6 @@ func newOperator(expr parser.Expr, storage *engstore.SelectorPool, opts *query.O func unpackVectorSelector(t *parser.MatrixSelector) (int64, *parser.VectorSelector, []*labels.Matcher, error) { switch t := t.VectorSelector.(type) { - case *parser.VectorSelector: - return 0, t, nil, nil case *logicalplan.VectorSelector: return t.BatchSize, t.VectorSelector, t.Filters, nil default: diff --git a/execution/function/absent.go b/execution/function/absent.go index b29f04dd..7d34dd99 100644 --- a/execution/function/absent.go +++ b/execution/function/absent.go @@ -9,10 +9,10 @@ import ( "time" "github.com/prometheus/prometheus/model/labels" - "github.com/prometheus/prometheus/promql/parser" "github.com/thanos-io/promql-engine/execution/model" + "github.com/thanos-io/promql-engine/logicalplan" ) type absentOperator struct { @@ -49,10 +49,10 @@ func (o *absentOperator) loadSeries() { // https://github.com/prometheus/prometheus/blob/main/promql/functions.go#L1385 var lm []*labels.Matcher switch n := o.funcExpr.Args[0].(type) { - case *parser.VectorSelector: + case *logicalplan.VectorSelector: lm = n.LabelMatchers case *parser.MatrixSelector: - lm = n.VectorSelector.(*parser.VectorSelector).LabelMatchers + lm = n.VectorSelector.(*logicalplan.VectorSelector).LabelMatchers default: o.series = []labels.Labels{labels.EmptyLabels()} return diff --git a/logicalplan/distribute.go b/logicalplan/distribute.go index 23254c50..a4636945 100644 --- a/logicalplan/distribute.go +++ b/logicalplan/distribute.go @@ -371,7 +371,7 @@ func calculateStartOffset(expr *parser.Expr, lookbackDelta time.Duration) time.D if matrixSelector, ok := (*node).(*parser.MatrixSelector); ok { selectRange = matrixSelector.Range } - if vectorSelector, ok := (*node).(*parser.VectorSelector); ok { + if vectorSelector, ok := (*node).(*VectorSelector); ok { offset = vectorSelector.Offset } }) @@ -415,7 +415,7 @@ func matchesExternalLabelSet(expr parser.Expr, externalLabelSet []labels.Labels) } var selectorSet [][]*labels.Matcher traverse(&expr, func(current *parser.Expr) { - vs, ok := (*current).(*parser.VectorSelector) + vs, ok := (*current).(*VectorSelector) if ok { selectorSet = append(selectorSet, vs.LabelMatchers) } diff --git a/logicalplan/filter.go b/logicalplan/filter.go index 7d13d812..f13f68d2 100644 --- a/logicalplan/filter.go +++ b/logicalplan/filter.go @@ -13,8 +13,6 @@ import ( ) // VectorSelector is vector selector with additional configuration set by optimizers. -// TODO(fpetkovski): Consider replacing all VectorSelector nodes with this one as the first step in the plan. -// This should help us avoid dealing with both types in the entire codebase. type VectorSelector struct { *parser.VectorSelector Filters []*labels.Matcher @@ -33,7 +31,9 @@ func (f VectorSelector) String() string { func (f VectorSelector) Pretty(level int) string { return f.String() } -func (f VectorSelector) PositionRange() posrange.PositionRange { return posrange.PositionRange{} } +func (f VectorSelector) PositionRange() posrange.PositionRange { + return f.VectorSelector.PositionRange() +} func (f VectorSelector) Type() parser.ValueType { return parser.ValueTypeVector } diff --git a/logicalplan/merge_selects.go b/logicalplan/merge_selects.go index 0dac5282..80448040 100644 --- a/logicalplan/merge_selects.go +++ b/logicalplan/merge_selects.go @@ -32,7 +32,7 @@ func (m MergeSelectsOptimizer) Optimize(expr parser.Expr, _ *query.Options) pars func extractSelectors(selectors matcherHeap, expr parser.Expr) { traverse(&expr, func(node *parser.Expr) { - e, ok := (*node).(*parser.VectorSelector) + e, ok := (*node).(*VectorSelector) if !ok { return } @@ -48,8 +48,6 @@ func replaceMatchers(selectors matcherHeap, expr *parser.Expr) { traverse(expr, func(node *parser.Expr) { var matchers []*labels.Matcher switch e := (*node).(type) { - case *parser.VectorSelector: - matchers = e.LabelMatchers case *VectorSelector: matchers = e.LabelMatchers default: @@ -84,12 +82,6 @@ func replaceMatchers(selectors matcherHeap, expr *parser.Expr) { } switch e := (*node).(type) { - case *parser.VectorSelector: - e.LabelMatchers = replacement - *node = &VectorSelector{ - VectorSelector: e, - Filters: filters, - } case *VectorSelector: e.LabelMatchers = replacement e.Filters = filters diff --git a/logicalplan/passthrough.go b/logicalplan/passthrough.go index 5f4b82fc..3d05b1dd 100644 --- a/logicalplan/passthrough.go +++ b/logicalplan/passthrough.go @@ -62,7 +62,7 @@ func (m PassthroughOptimizer) Optimize(plan parser.Expr, opts *query.Options) pa matchingLabelsEngines := make([]api.RemoteEngine, 0, len(engines)) TraverseBottomUp(nil, &plan, func(parent, current *parser.Expr) (stop bool) { - if vs, ok := (*current).(*parser.VectorSelector); ok { + if vs, ok := (*current).(*VectorSelector); ok { for _, e := range engines { if !labelSetsMatch(vs.LabelMatchers, e.LabelSets()...) { continue diff --git a/logicalplan/plan.go b/logicalplan/plan.go index 9d533e4d..03e13c9c 100644 --- a/logicalplan/plan.go +++ b/logicalplan/plan.go @@ -46,6 +46,14 @@ func New(expr parser.Expr, opts *query.Options) Plan { setOffsetForAtModifier(opts.Start.UnixMilli(), expr) setOffsetForInnerSubqueries(expr, opts) + // replace all vector selectors by our internal ones so we only need to handle them + traverse(&expr, func(e *parser.Expr) { + switch node := (*e).(type) { + case *parser.VectorSelector: + *e = &VectorSelector{VectorSelector: node} + } + }) + return &plan{ expr: expr, opts: opts, @@ -67,19 +75,20 @@ func (p *plan) Expr() parser.Expr { func traverse(expr *parser.Expr, transform func(*parser.Expr)) { switch node := (*expr).(type) { case *parser.StepInvariantExpr: - transform(&node.Expr) - case *parser.VectorSelector: - transform(expr) + traverse(&node.Expr, transform) case *VectorSelector: var x parser.Expr = node.VectorSelector transform(expr) traverse(&x, transform) + case *parser.VectorSelector: + transform(expr) case *parser.MatrixSelector: transform(expr) traverse(&node.VectorSelector, transform) case *parser.AggregateExpr: transform(expr) traverse(&node.Expr, transform) + traverse(&node.Param, transform) case *parser.Call: for i := range node.Args { traverse(&(node.Args[i]), transform) @@ -103,8 +112,6 @@ func TraverseBottomUp(parent *parser.Expr, current *parser.Expr, transform func( return false case *parser.StepInvariantExpr: return TraverseBottomUp(current, &node.Expr, transform) - case *parser.VectorSelector: - return transform(parent, current) case *VectorSelector: if stop := transform(parent, current); stop { return stop diff --git a/logicalplan/propagate_selectors.go b/logicalplan/propagate_selectors.go index 1401cc93..aab1e24d 100644 --- a/logicalplan/propagate_selectors.go +++ b/logicalplan/propagate_selectors.go @@ -46,11 +46,11 @@ func (m PropagateMatchersOptimizer) Optimize(expr parser.Expr, _ *query.Options) } func propagateMatchers(binOp *parser.BinaryExpr) { - lhSelector, ok := binOp.LHS.(*parser.VectorSelector) + lhSelector, ok := binOp.LHS.(*VectorSelector) if !ok { return } - rhSelector, ok := binOp.RHS.(*parser.VectorSelector) + rhSelector, ok := binOp.RHS.(*VectorSelector) if !ok { return } @@ -105,7 +105,7 @@ func makeUnion(lhMatchers map[string]*labels.Matcher, rhMatchers map[string]*lab return union, false } -func toMatcherMap(lhSelector *parser.VectorSelector) map[string]*labels.Matcher { +func toMatcherMap(lhSelector *VectorSelector) map[string]*labels.Matcher { lhMatchers := make(map[string]*labels.Matcher) for _, m := range lhSelector.LabelMatchers { lhMatchers[m.Name] = m diff --git a/logicalplan/set_batch_size.go b/logicalplan/set_batch_size.go index 0ba26117..523cc159 100644 --- a/logicalplan/set_batch_size.go +++ b/logicalplan/set_batch_size.go @@ -38,11 +38,6 @@ func (m SelectorBatchSize) Optimize(expr parser.Expr, _ *query.Options) parser.E e.BatchSize = m.Size } canBatch = false - case *parser.VectorSelector: - if canBatch { - *current = &VectorSelector{VectorSelector: e, BatchSize: m.Size} - } - canBatch = false } }) return expr diff --git a/logicalplan/sort_matchers.go b/logicalplan/sort_matchers.go index 3f6371c0..02d4dd82 100644 --- a/logicalplan/sort_matchers.go +++ b/logicalplan/sort_matchers.go @@ -18,7 +18,7 @@ type SortMatchers struct{} func (m SortMatchers) Optimize(expr parser.Expr, _ *query.Options) parser.Expr { traverse(&expr, func(node *parser.Expr) { - e, ok := (*node).(*parser.VectorSelector) + e, ok := (*node).(*VectorSelector) if !ok { return }