diff --git a/promql/ast.go b/promql/ast.go index af3ed4189..0c8a3ad2a 100644 --- a/promql/ast.go +++ b/promql/ast.go @@ -237,57 +237,80 @@ type VectorMatching struct { // Visitor allows visiting a Node and its child nodes. The Visit method is // invoked for each node with the path leading to the node provided additionally. -// If the result visitor w is not nil, Walk visits each of the children +// If the result visitor w is not nil and no error, Walk visits each of the children // of node with the visitor w, followed by a call of w.Visit(nil, nil). type Visitor interface { - Visit(node Node, path []Node) (w Visitor) + Visit(node Node, path []Node) (w Visitor, err error) } // Walk traverses an AST in depth-first order: It starts by calling // v.Visit(node, path); node must not be nil. If the visitor w returned by -// v.Visit(node, path) is not nil, Walk is invoked recursively with visitor -// w for each of the non-nil children of node, followed by a call of -// w.Visit(nil). +// v.Visit(node, path) is not nil and the visitor returns no error, Walk is +// invoked recursively with visitor w for each of the non-nil children of node, +// followed by a call of w.Visit(nil), returning an error // As the tree is descended the path of previous nodes is provided. -func Walk(v Visitor, node Node, path []Node) { - if v = v.Visit(node, path); v == nil { - return +func Walk(v Visitor, node Node, path []Node) error { + var err error + if v, err = v.Visit(node, path); v == nil || err != nil { + return err } path = append(path, node) switch n := node.(type) { case Statements: for _, s := range n { - Walk(v, s, path) + if err := Walk(v, s, path); err != nil { + return err + } } case *AlertStmt: - Walk(v, n.Expr, path) + if err := Walk(v, n.Expr, path); err != nil { + return err + } case *EvalStmt: - Walk(v, n.Expr, path) + if err := Walk(v, n.Expr, path); err != nil { + return err + } case *RecordStmt: - Walk(v, n.Expr, path) + if err := Walk(v, n.Expr, path); err != nil { + return err + } case Expressions: for _, e := range n { - Walk(v, e, path) + if err := Walk(v, e, path); err != nil { + return err + } } case *AggregateExpr: - Walk(v, n.Expr, path) + if err := Walk(v, n.Expr, path); err != nil { + return err + } case *BinaryExpr: - Walk(v, n.LHS, path) - Walk(v, n.RHS, path) + if err := Walk(v, n.LHS, path); err != nil { + return err + } + if err := Walk(v, n.RHS, path); err != nil { + return err + } case *Call: - Walk(v, n.Args, path) + if err := Walk(v, n.Args, path); err != nil { + return err + } case *ParenExpr: - Walk(v, n.Expr, path) + if err := Walk(v, n.Expr, path); err != nil { + return err + } case *UnaryExpr: - Walk(v, n.Expr, path) + if err := Walk(v, n.Expr, path); err != nil { + return err + } case *MatrixSelector, *NumberLiteral, *StringLiteral, *VectorSelector: // nothing to do @@ -296,21 +319,23 @@ func Walk(v Visitor, node Node, path []Node) { panic(fmt.Errorf("promql.Walk: unhandled node type %T", node)) } - v.Visit(nil, nil) + _, err = v.Visit(nil, nil) + return err } -type inspector func(Node, []Node) bool +type inspector func(Node, []Node) error -func (f inspector) Visit(node Node, path []Node) Visitor { - if f(node, path) { - return f +func (f inspector) Visit(node Node, path []Node) (Visitor, error) { + if err := f(node, path); err == nil { + return f, nil + } else { + return nil, err } - return nil } // Inspect traverses an AST in depth-first order: It starts by calling -// f(node, path); node must not be nil. If f returns true, Inspect invokes f +// f(node, path); node must not be nil. If f returns a nil error, Inspect invokes f // for all the non-nil children of node, recursively. -func Inspect(node Node, f func(Node, []Node) bool) { +func Inspect(node Node, f inspector) { Walk(inspector(f), node, nil) } diff --git a/promql/engine.go b/promql/engine.go index aeea2b5d3..48679c515 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -451,7 +451,7 @@ func (ng *Engine) execEvalStmt(ctx context.Context, query *query, s *EvalStmt) ( func (ng *Engine) populateSeries(ctx context.Context, q storage.Queryable, s *EvalStmt) (storage.Querier, error) { var maxOffset time.Duration - Inspect(s.Expr, func(node Node, _ []Node) bool { + Inspect(s.Expr, func(node Node, _ []Node) error { switch n := node.(type) { case *VectorSelector: if maxOffset < LookbackDelta { @@ -468,7 +468,7 @@ func (ng *Engine) populateSeries(ctx context.Context, q storage.Queryable, s *Ev maxOffset = n.Offset + n.Range } } - return true + return nil }) mint := s.Start.Add(-maxOffset) @@ -478,7 +478,7 @@ func (ng *Engine) populateSeries(ctx context.Context, q storage.Queryable, s *Ev return nil, err } - Inspect(s.Expr, func(node Node, path []Node) bool { + Inspect(s.Expr, func(node Node, path []Node) error { var set storage.SeriesSet params := &storage.SelectParams{ Step: int64(s.Interval / time.Millisecond), @@ -491,13 +491,13 @@ func (ng *Engine) populateSeries(ctx context.Context, q storage.Queryable, s *Ev set, err = querier.Select(params, n.LabelMatchers...) if err != nil { level.Error(ng.logger).Log("msg", "error selecting series set", "err", err) - return false + return err } n.series, err = expandSeriesSet(set) if err != nil { // TODO(fabxc): use multi-error. level.Error(ng.logger).Log("msg", "error expanding series set", "err", err) - return false + return err } case *MatrixSelector: @@ -506,15 +506,15 @@ func (ng *Engine) populateSeries(ctx context.Context, q storage.Queryable, s *Ev set, err = querier.Select(params, n.LabelMatchers...) if err != nil { level.Error(ng.logger).Log("msg", "error selecting series set", "err", err) - return false + return err } n.series, err = expandSeriesSet(set) if err != nil { level.Error(ng.logger).Log("msg", "error expanding series set", "err", err) - return false + return err } } - return true + return nil }) return querier, err }