diff --git a/promql/ast.go b/promql/ast.go index 98725fb07..803c9892e 100644 --- a/promql/ast.go +++ b/promql/ast.go @@ -113,7 +113,9 @@ type Call struct { // MatrixSelector represents a Matrix selection. type MatrixSelector struct { - VectorSelector *VectorSelector + // It is safe to assume that this is an VectorSelector + // if the parser hasn't returned an error. + VectorSelector Expr Range time.Duration EndPos Pos diff --git a/promql/engine.go b/promql/engine.go index 2603fcfc6..d43fb93f4 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -638,7 +638,7 @@ func (ng *Engine) populateSeries(ctx context.Context, q storage.Queryable, s *Ev if maxOffset < n.Range+subqOffset { maxOffset = n.Range + subqOffset } - if m := n.VectorSelector.Offset + n.Range + subqOffset; m > maxOffset { + if m := n.VectorSelector.(*VectorSelector).Offset + n.Range + subqOffset; m > maxOffset { maxOffset = m } } @@ -1004,15 +1004,16 @@ func (ev *evaluator) rangeEval(f func([]Value, *EvalNodeHelper) Vector, exprs .. // evaluated MatrixSelector in its place. Note that the Name and LabelMatchers are not set. func (ev *evaluator) evalSubquery(subq *SubqueryExpr) *MatrixSelector { val := ev.eval(subq).(Matrix) + vs := &VectorSelector{ + Offset: subq.Offset, + series: make([]storage.Series, 0, len(val)), + } ms := &MatrixSelector{ - Range: subq.Range, - VectorSelector: &VectorSelector{ - Offset: subq.Offset, - series: make([]storage.Series, 0, len(val)), - }, + Range: subq.Range, + VectorSelector: vs, } for _, s := range val { - ms.VectorSelector.series = append(ms.VectorSelector.series, NewStorageSeries(s)) + vs.series = append(vs.series, NewStorageSeries(s)) } return ms } @@ -1095,7 +1096,7 @@ func (ev *evaluator) eval(expr Expr) Value { } sel := e.Args[matrixArgIndex].(*MatrixSelector) - selVS := sel.VectorSelector + selVS := sel.VectorSelector.(*VectorSelector) checkForSeriesSetExpansion(ev.ctx, sel) mat := make(Matrix, 0, len(selVS.series)) // Output matrix. @@ -1422,15 +1423,17 @@ func putPointSlice(p []Point) { func (ev *evaluator) matrixSelector(node *MatrixSelector) Matrix { checkForSeriesSetExpansion(ev.ctx, node) + vs := node.VectorSelector.(*VectorSelector) + var ( - offset = durationMilliseconds(node.VectorSelector.Offset) + offset = durationMilliseconds(vs.Offset) maxt = ev.startTimestamp - offset mint = maxt - durationMilliseconds(node.Range) - matrix = make(Matrix, 0, len(node.VectorSelector.series)) + matrix = make(Matrix, 0, len(vs.series)) ) it := storage.NewBuffer(durationMilliseconds(node.Range)) - series := node.VectorSelector.series + series := vs.series for i, s := range series { if err := contextDone(ev.ctx, "expression evaluation"); err != nil { diff --git a/promql/functions.go b/promql/functions.go index 88803f04c..4765965ab 100644 --- a/promql/functions.go +++ b/promql/functions.go @@ -64,11 +64,12 @@ func funcTime(vals []Value, args Expressions, enh *EvalNodeHelper) Vector { // the result as either per-second (if isRate is true) or overall. func extrapolatedRate(vals []Value, args Expressions, enh *EvalNodeHelper, isCounter bool, isRate bool) Vector { ms := args[0].(*MatrixSelector) + vs := ms.VectorSelector.(*VectorSelector) var ( samples = vals[0].(Matrix)[0] - rangeStart = enh.ts - durationMilliseconds(ms.Range+ms.VectorSelector.Offset) - rangeEnd = enh.ts - durationMilliseconds(ms.VectorSelector.Offset) + rangeStart = enh.ts - durationMilliseconds(ms.Range+vs.Offset) + rangeEnd = enh.ts - durationMilliseconds(vs.Offset) ) // No sense in trying to compute a rate without at least two points. Drop @@ -1243,7 +1244,7 @@ func createLabelsForAbsentFunction(expr Expr) labels.Labels { case *VectorSelector: lm = n.LabelMatchers case *MatrixSelector: - lm = n.VectorSelector.LabelMatchers + lm = n.VectorSelector.(*VectorSelector).LabelMatchers default: return m } diff --git a/promql/generated_parser.y b/promql/generated_parser.y index dc7d3698d..0cb98df75 100644 --- a/promql/generated_parser.y +++ b/promql/generated_parser.y @@ -40,7 +40,7 @@ import ( } -%token +%token ASSIGN BLANK COLON @@ -159,15 +159,15 @@ START_METRIC_SELECTOR %% -start : +start : START_METRIC metric { yylex.(*parser).generatedParserResult = $2 } | START_SERIES_DESCRIPTION series_description | START_EXPRESSION /* empty */ EOF - { yylex.(*parser).failf(PositionRange{}, "no expression found in input")} + { yylex.(*parser).addParseErrf(PositionRange{}, "no expression found in input")} | START_EXPRESSION expr { yylex.(*parser).generatedParserResult = $2 } - | START_METRIC_SELECTOR vector_selector + | START_METRIC_SELECTOR vector_selector { yylex.(*parser).generatedParserResult = $2 } | start EOF | error /* If none of the more detailed error messages are triggered, we fall back to this. */ @@ -240,7 +240,7 @@ binary_expr : expr ADD bin_modifier expr { $$ = yylex.(*parser).newBinar | expr SUB bin_modifier expr { $$ = yylex.(*parser).newBinaryExpression($1, $2, $3, $4) } ; -// Using left recursion for the modifier rules, helps to keep the parser stack small and +// Using left recursion for the modifier rules, helps to keep the parser stack small and // reduces allocations bin_modifier : group_modifiers; @@ -261,13 +261,13 @@ on_or_ignoring : bool_modifier IGNORING grouping_labels { $$ = $1 $$.(*BinaryExpr).VectorMatching.MatchingLabels = $3 - } + } | bool_modifier ON grouping_labels { $$ = $1 $$.(*BinaryExpr).VectorMatching.MatchingLabels = $3 $$.(*BinaryExpr).VectorMatching.On = true - } + } ; group_modifiers: bool_modifier /* empty */ @@ -321,13 +321,13 @@ grouping_label : maybe_label /* * Function calls. */ - + function_call : IDENTIFIER function_call_body { fn, exist := getFunction($1.Val) if !exist{ - yylex.(*parser).failf($1.PositionRange(),"unknown function with name %q", $1.Val) - } + yylex.(*parser).addParseErrf($1.PositionRange(),"unknown function with name %q", $1.Val) + } $$ = &Call{ Func: fn, Args: $2.(Expressions), @@ -347,11 +347,11 @@ function_call_body: LEFT_PAREN function_call_args RIGHT_PAREN function_call_args: function_call_args COMMA expr { $$ = append($1.(Expressions), $3.(Expr)) } - | expr + | expr { $$ = Expressions{$1.(Expr)} } | function_call_args COMMA { - yylex.(*parser).failf($2.PositionRange(), "trailing commas not allowed in function call args") + yylex.(*parser).addParseErrf($2.PositionRange(), "trailing commas not allowed in function call args") $$ = $1 } ; @@ -393,11 +393,11 @@ matrix_selector : expr LEFT_BRACKET duration RIGHT_BRACKET if errMsg != ""{ errRange := mergeRanges(&$2, &$4) - yylex.(*parser).failf(errRange, errMsg) + yylex.(*parser).addParseErrf(errRange, errMsg) } $$ = &MatrixSelector{ - VectorSelector: vs, + VectorSelector: $1.(Expr), Range: $3, EndPos: yylex.(*parser).lastClosing, } @@ -410,7 +410,7 @@ subquery_expr : expr LEFT_BRACKET duration COLON maybe_duration RIGHT_BRACKET Expr: $1.(Expr), Range: $3, Step: $5, - + EndPos: $6.Pos + 1, } } @@ -430,7 +430,7 @@ subquery_expr : expr LEFT_BRACKET duration COLON maybe_duration RIGHT_BRACKET unary_expr : /* gives the rule the same precedence as MUL. This aligns with mathematical conventions */ - unary_op expr %prec MUL + unary_op expr %prec MUL { if nl, ok := $2.(*NumberLiteral); ok { if $1.Typ == SUB { @@ -449,15 +449,15 @@ unary_expr : */ vector_selector: metric_identifier label_matchers - { + { vs := $2.(*VectorSelector) - vs.PosRange = mergeRanges(&$1, vs) + vs.PosRange = mergeRanges(&$1, vs) vs.Name = $1.Val yylex.(*parser).assembleVectorSelector(vs) $$ = vs } - | metric_identifier - { + | metric_identifier + { vs := &VectorSelector{ Name: $1.Val, LabelMatchers: []*labels.Matcher{}, @@ -467,7 +467,7 @@ vector_selector: metric_identifier label_matchers $$ = vs } | label_matchers - { + { vs := $1.(*VectorSelector) yylex.(*parser).assembleVectorSelector(vs) $$ = vs @@ -498,7 +498,7 @@ label_matchers : LEFT_BRACE label_match_list RIGHT_BRACE ; label_match_list: label_match_list COMMA label_matcher - { + { if $1 != nil{ $$ = append($1, $3) } else { @@ -515,19 +515,19 @@ label_matcher : IDENTIFIER match_op STRING { $$ = yylex.(*parser).newLabelMatcher($1, $2, $3); } | IDENTIFIER match_op error { yylex.(*parser).unexpected("label matching", "string"); $$ = nil} - | IDENTIFIER error - { yylex.(*parser).unexpected("label matching", "label matching operator"); $$ = nil } + | IDENTIFIER error + { yylex.(*parser).unexpected("label matching", "label matching operator"); $$ = nil } | error { yylex.(*parser).unexpected("label matching", "identifier or \"}\""); $$ = nil} ; - + /* * Metric descriptions. */ metric : metric_identifier label_set { $$ = append($2, labels.Label{Name: labels.MetricName, Value: $1.Val}); sort.Sort($$) } - | label_set + | label_set {$$ = $1} ; @@ -550,11 +550,11 @@ label_set_list : label_set_list COMMA label_set_item { $$ = []labels.Label{$1} } | label_set_list error { yylex.(*parser).unexpected("label set", "\",\" or \"}\"", ); $$ = $1 } - + ; label_set_item : IDENTIFIER EQL STRING - { $$ = labels.Label{Name: $1.Val, Value: yylex.(*parser).unquoteString($3.Val) } } + { $$ = labels.Label{Name: $1.Val, Value: yylex.(*parser).unquoteString($3.Val) } } | IDENTIFIER EQL error { yylex.(*parser).unexpected("label set", "string"); $$ = labels.Label{}} | IDENTIFIER error @@ -645,7 +645,7 @@ match_op : EQL | NEQ | EQL_REGEX | NEQ_REGEX ; * Literals. */ -number_literal : NUMBER +number_literal : NUMBER { $$ = &NumberLiteral{ Val: yylex.(*parser).number($1.Val), @@ -665,7 +665,7 @@ uint : NUMBER var err error $$, err = strconv.ParseUint($1.Val, 10, 64) if err != nil { - yylex.(*parser).failf($1.PositionRange(), "invalid repetition in series values: %s", err) + yylex.(*parser).addParseErrf($1.PositionRange(), "invalid repetition in series values: %s", err) } } ; @@ -675,7 +675,7 @@ duration : DURATION var err error $$, err = parseDuration($1.Val) if err != nil { - yylex.(*parser).fail($1.PositionRange(), err) + yylex.(*parser).addParseErr($1.PositionRange(), err) } } ; diff --git a/promql/generated_parser.y.go b/promql/generated_parser.y.go index 2f7608da7..84412d64f 100644 --- a/promql/generated_parser.y.go +++ b/promql/generated_parser.y.go @@ -794,7 +794,7 @@ yydefault: yyDollar = yyS[yypt-2 : yypt+1] //line promql/generated_parser.y:167 { - yylex.(*parser).failf(PositionRange{}, "no expression found in input") + yylex.(*parser).addParseErrf(PositionRange{}, "no expression found in input") } case 4: yyDollar = yyS[yypt-2 : yypt+1] @@ -1060,7 +1060,7 @@ yydefault: { fn, exist := getFunction(yyDollar[1].item.Val) if !exist { - yylex.(*parser).failf(yyDollar[1].item.PositionRange(), "unknown function with name %q", yyDollar[1].item.Val) + yylex.(*parser).addParseErrf(yyDollar[1].item.PositionRange(), "unknown function with name %q", yyDollar[1].item.Val) } yyVAL.node = &Call{ Func: fn, @@ -1099,7 +1099,7 @@ yydefault: yyDollar = yyS[yypt-2 : yypt+1] //line promql/generated_parser.y:353 { - yylex.(*parser).failf(yyDollar[2].item.PositionRange(), "trailing commas not allowed in function call args") + yylex.(*parser).addParseErrf(yyDollar[2].item.PositionRange(), "trailing commas not allowed in function call args") yyVAL.node = yyDollar[1].node } case 64: @@ -1136,11 +1136,11 @@ yydefault: if errMsg != "" { errRange := mergeRanges(&yyDollar[2].item, &yyDollar[4].item) - yylex.(*parser).failf(errRange, errMsg) + yylex.(*parser).addParseErrf(errRange, errMsg) } yyVAL.node = &MatrixSelector{ - VectorSelector: vs, + VectorSelector: yyDollar[1].node.(Expr), Range: yyDollar[3].duration, EndPos: yylex.(*parser).lastClosing, } @@ -1506,7 +1506,7 @@ yydefault: var err error yyVAL.uint, err = strconv.ParseUint(yyDollar[1].item.Val, 10, 64) if err != nil { - yylex.(*parser).failf(yyDollar[1].item.PositionRange(), "invalid repetition in series values: %s", err) + yylex.(*parser).addParseErrf(yyDollar[1].item.PositionRange(), "invalid repetition in series values: %s", err) } } case 160: @@ -1516,7 +1516,7 @@ yydefault: var err error yyVAL.duration, err = parseDuration(yyDollar[1].item.Val) if err != nil { - yylex.(*parser).fail(yyDollar[1].item.PositionRange(), err) + yylex.(*parser).addParseErr(yyDollar[1].item.PositionRange(), err) } } case 161: diff --git a/promql/parse.go b/promql/parse.go index fcacb04a4..1f6b2b17b 100644 --- a/promql/parse.go +++ b/promql/parse.go @@ -116,7 +116,7 @@ func ParseExpr(input string) (expr Expr, err error) { // Only typecheck when there are no syntax errors. if len(p.parseErrors) == 0 { - p.checkType(expr) + p.checkAST(expr) } if len(p.parseErrors) != 0 { @@ -220,13 +220,13 @@ func parseSeriesDesc(input string) (labels labels.Labels, values []sequenceValue return labels, values, err } -// failf formats the error and terminates processing. -func (p *parser) failf(positionRange PositionRange, format string, args ...interface{}) { - p.fail(positionRange, errors.Errorf(format, args...)) +// addParseErrf formats the error and and appends it to the list of parsing errors. +func (p *parser) addParseErrf(positionRange PositionRange, format string, args ...interface{}) { + p.addParseErr(positionRange, errors.Errorf(format, args...)) } -// fail terminates processing. -func (p *parser) fail(positionRange PositionRange, err error) { +// addParseErr appends the provided error to the list of parsing errors. +func (p *parser) addParseErr(positionRange PositionRange, err error) { perr := ParseErr{ PositionRange: positionRange, Err: err, @@ -255,7 +255,7 @@ func (p *parser) unexpected(context string, expected string) { errMsg.WriteString(expected) } - p.fail(p.yyParser.lval.item.PositionRange(), errors.New(errMsg.String())) + p.addParseErr(p.yyParser.lval.item.PositionRange(), errors.New(errMsg.String())) } var errUnexpected = errors.New("unexpected error") @@ -304,7 +304,7 @@ func (p *parser) Lex(lval *yySymType) int { switch typ { case ERROR: - p.failf(lval.item.PositionRange(), "%s", lval.item.Val) + p.addParseErrf(lval.item.PositionRange(), "%s", lval.item.Val) p.InjectItem(0) case EOF: lval.item.Typ = EOF @@ -355,48 +355,29 @@ func (p *parser) newBinaryExpression(lhs Node, op Item, modifiers Node, rhs Node func (p *parser) assembleVectorSelector(vs *VectorSelector) { if vs.Name != "" { - - for _, m := range vs.LabelMatchers { - if m != nil && m.Name == labels.MetricName { - p.failf(vs.PositionRange(), "metric name must not be set twice: %q or %q", vs.Name, m.Value) - } - } - nameMatcher, err := labels.NewMatcher(labels.MatchEqual, labels.MetricName, vs.Name) if err != nil { panic(err) // Must not happen with labels.MatchEqual } vs.LabelMatchers = append(vs.LabelMatchers, nameMatcher) } - - // A Vector selector must contain at least one non-empty matcher to prevent - // implicit selection of all metrics (e.g. by a typo). - notEmpty := false - for _, lm := range vs.LabelMatchers { - if lm != nil && !lm.Matches("") { - notEmpty = true - break - } - } - if !notEmpty { - p.failf(vs.PositionRange(), "vector selector must contain at least one non-empty matcher") - } } func (p *parser) newAggregateExpr(op Item, modifier Node, args Node) (ret *AggregateExpr) { ret = modifier.(*AggregateExpr) arguments := args.(Expressions) + ret.PosRange = PositionRange{ + Start: op.Pos, + End: p.lastClosing, + } + ret.Op = op.Typ if len(arguments) == 0 { - p.failf(ret.PositionRange(), "no arguments for aggregate expression provided") + p.addParseErrf(ret.PositionRange(), "no arguments for aggregate expression provided") - // Currently p.failf() panics, so this return is not needed - // at the moment. - // However, this behaviour is likely to be changed in the - // future. In case of having non-panicking errors this - // return prevents invalid array accesses + // Prevents invalid array accesses. return } @@ -408,17 +389,12 @@ func (p *parser) newAggregateExpr(op Item, modifier Node, args Node) (ret *Aggre } if len(arguments) != desiredArgs { - p.failf(ret.PositionRange(), "wrong number of arguments for aggregate expression provided, expected %d, got %d", desiredArgs, len(arguments)) + p.addParseErrf(ret.PositionRange(), "wrong number of arguments for aggregate expression provided, expected %d, got %d", desiredArgs, len(arguments)) return } ret.Expr = arguments[desiredArgs-1] - ret.PosRange = PositionRange{ - Start: op.Pos, - End: p.lastClosing, - } - return ret } @@ -430,7 +406,7 @@ func (p *parser) number(val string) float64 { f, err = strconv.ParseFloat(val, 64) } if err != nil { - p.failf(p.yyParser.lval.item.PositionRange(), "error parsing number: %s", err) + p.addParseErrf(p.yyParser.lval.item.PositionRange(), "error parsing number: %s", err) } return f } @@ -438,18 +414,14 @@ func (p *parser) number(val string) float64 { // expectType checks the type of the node and raises an error if it // is not of the expected type. func (p *parser) expectType(node Node, want ValueType, context string) { - t := p.checkType(node) + t := p.checkAST(node) if t != want { - p.failf(node.PositionRange(), "expected type %s in %s, got %s", documentedType(want), context, documentedType(t)) + p.addParseErrf(node.PositionRange(), "expected type %s in %s, got %s", documentedType(want), context, documentedType(t)) } } -// check the types of the children of each node and raise an error -// if they do not form a valid node. -// -// Some of these checks are redundant as the parsing stage does not allow -// them, but the costs are small and might reveal errors when making changes. -func (p *parser) checkType(node Node) (typ ValueType) { +// checkAST checks the sanity of the provided AST. This includes type checking. +func (p *parser) checkAST(node Node) (typ ValueType) { // For expressions the type is determined by their Type function. // Lists do not have a type but are not invalid either. switch n := node.(type) { @@ -458,28 +430,28 @@ func (p *parser) checkType(node Node) (typ ValueType) { case Expr: typ = n.Type() default: - p.failf(node.PositionRange(), "unknown node type: %T", node) + p.addParseErrf(node.PositionRange(), "unknown node type: %T", node) } // Recursively check correct typing for child nodes and raise // errors in case of bad typing. switch n := node.(type) { case *EvalStmt: - ty := p.checkType(n.Expr) + ty := p.checkAST(n.Expr) if ty == ValueTypeNone { - p.failf(n.Expr.PositionRange(), "evaluation statement must have a valid expression type but got %s", documentedType(ty)) + p.addParseErrf(n.Expr.PositionRange(), "evaluation statement must have a valid expression type but got %s", documentedType(ty)) } case Expressions: for _, e := range n { - ty := p.checkType(e) + ty := p.checkAST(e) if ty == ValueTypeNone { - p.failf(e.PositionRange(), "expression must have a valid expression type but got %s", documentedType(ty)) + p.addParseErrf(e.PositionRange(), "expression must have a valid expression type but got %s", documentedType(ty)) } } case *AggregateExpr: if !n.Op.isAggregator() { - p.failf(n.PositionRange(), "aggregation operator expected in aggregation expression but got %q", n.Op) + p.addParseErrf(n.PositionRange(), "aggregation operator expected in aggregation expression but got %q", n.Op) } p.expectType(n.Expr, ValueTypeVector, "aggregation expression") if n.Op == TOPK || n.Op == BOTTOMK || n.Op == QUANTILE { @@ -490,8 +462,8 @@ func (p *parser) checkType(node Node) (typ ValueType) { } case *BinaryExpr: - lt := p.checkType(n.LHS) - rt := p.checkType(n.RHS) + lt := p.checkAST(n.LHS) + rt := p.checkAST(n.RHS) // opRange returns the PositionRange of the operator part of the BinaryExpr. // This is made a function instead of a variable, so it is lazily evaluated on demand. @@ -505,11 +477,11 @@ func (p *parser) checkType(node Node) (typ ValueType) { } if n.ReturnBool && !n.Op.isComparisonOperator() { - p.failf(opRange(), "bool modifier can only be used on comparison operators") + p.addParseErrf(opRange(), "bool modifier can only be used on comparison operators") } if n.Op.isComparisonOperator() && !n.ReturnBool && n.RHS.Type() == ValueTypeScalar && n.LHS.Type() == ValueTypeScalar { - p.failf(opRange(), "comparisons between scalars must use BOOL modifier") + p.addParseErrf(opRange(), "comparisons between scalars must use BOOL modifier") } if n.Op.isSetOperator() && n.VectorMatching.Card == CardOneToOne { @@ -519,54 +491,54 @@ func (p *parser) checkType(node Node) (typ ValueType) { for _, l1 := range n.VectorMatching.MatchingLabels { for _, l2 := range n.VectorMatching.Include { if l1 == l2 && n.VectorMatching.On { - p.failf(opRange(), "label %q must not occur in ON and GROUP clause at once", l1) + p.addParseErrf(opRange(), "label %q must not occur in ON and GROUP clause at once", l1) } } } if !n.Op.isOperator() { - p.failf(n.PositionRange(), "binary expression does not support operator %q", n.Op) + p.addParseErrf(n.PositionRange(), "binary expression does not support operator %q", n.Op) } if lt != ValueTypeScalar && lt != ValueTypeVector { - p.failf(n.LHS.PositionRange(), "binary expression must contain only scalar and instant vector types") + p.addParseErrf(n.LHS.PositionRange(), "binary expression must contain only scalar and instant vector types") } if rt != ValueTypeScalar && rt != ValueTypeVector { - p.failf(n.RHS.PositionRange(), "binary expression must contain only scalar and instant vector types") + p.addParseErrf(n.RHS.PositionRange(), "binary expression must contain only scalar and instant vector types") } if (lt != ValueTypeVector || rt != ValueTypeVector) && n.VectorMatching != nil { if len(n.VectorMatching.MatchingLabels) > 0 { - p.failf(n.PositionRange(), "vector matching only allowed between instant vectors") + p.addParseErrf(n.PositionRange(), "vector matching only allowed between instant vectors") } n.VectorMatching = nil } else { // Both operands are Vectors. if n.Op.isSetOperator() { if n.VectorMatching.Card == CardOneToMany || n.VectorMatching.Card == CardManyToOne { - p.failf(n.PositionRange(), "no grouping allowed for %q operation", n.Op) + p.addParseErrf(n.PositionRange(), "no grouping allowed for %q operation", n.Op) } if n.VectorMatching.Card != CardManyToMany { - p.failf(n.PositionRange(), "set operations must always be many-to-many") + p.addParseErrf(n.PositionRange(), "set operations must always be many-to-many") } } } if (lt == ValueTypeScalar || rt == ValueTypeScalar) && n.Op.isSetOperator() { - p.failf(n.PositionRange(), "set operator %q not allowed in binary scalar expression", n.Op) + p.addParseErrf(n.PositionRange(), "set operator %q not allowed in binary scalar expression", n.Op) } case *Call: nargs := len(n.Func.ArgTypes) if n.Func.Variadic == 0 { if nargs != len(n.Args) { - p.failf(n.PositionRange(), "expected %d argument(s) in call to %q, got %d", nargs, n.Func.Name, len(n.Args)) + p.addParseErrf(n.PositionRange(), "expected %d argument(s) in call to %q, got %d", nargs, n.Func.Name, len(n.Args)) } } else { na := nargs - 1 if na > len(n.Args) { - p.failf(n.PositionRange(), "expected at least %d argument(s) in call to %q, got %d", na, n.Func.Name, len(n.Args)) + p.addParseErrf(n.PositionRange(), "expected at least %d argument(s) in call to %q, got %d", na, n.Func.Name, len(n.Args)) } else if nargsmax := na + n.Func.Variadic; n.Func.Variadic > 0 && nargsmax < len(n.Args) { - p.failf(n.PositionRange(), "expected at most %d argument(s) in call to %q, got %d", nargsmax, n.Func.Name, len(n.Args)) + p.addParseErrf(n.PositionRange(), "expected at most %d argument(s) in call to %q, got %d", nargsmax, n.Func.Name, len(n.Args)) } } @@ -578,29 +550,54 @@ func (p *parser) checkType(node Node) (typ ValueType) { } case *ParenExpr: - p.checkType(n.Expr) + p.checkAST(n.Expr) case *UnaryExpr: if n.Op != ADD && n.Op != SUB { - p.failf(n.PositionRange(), "only + and - operators allowed for unary expressions") + p.addParseErrf(n.PositionRange(), "only + and - operators allowed for unary expressions") } - if t := p.checkType(n.Expr); t != ValueTypeScalar && t != ValueTypeVector { - p.failf(n.PositionRange(), "unary expression only allowed on expressions of type scalar or instant vector, got %q", documentedType(t)) + if t := p.checkAST(n.Expr); t != ValueTypeScalar && t != ValueTypeVector { + p.addParseErrf(n.PositionRange(), "unary expression only allowed on expressions of type scalar or instant vector, got %q", documentedType(t)) } case *SubqueryExpr: - ty := p.checkType(n.Expr) + ty := p.checkAST(n.Expr) if ty != ValueTypeVector { - p.failf(n.PositionRange(), "subquery is only allowed on instant vector, got %s in %q instead", ty, n.String()) + p.addParseErrf(n.PositionRange(), "subquery is only allowed on instant vector, got %s in %q instead", ty, n.String()) } case *MatrixSelector: - p.checkType(n.VectorSelector) + p.checkAST(n.VectorSelector) - case *NumberLiteral, *StringLiteral, *VectorSelector: + case *VectorSelector: + // A Vector selector must contain at least one non-empty matcher to prevent + // implicit selection of all metrics (e.g. by a typo). + notEmpty := false + for _, lm := range n.LabelMatchers { + if lm != nil && !lm.Matches("") { + notEmpty = true + break + } + } + if !notEmpty { + p.addParseErrf(n.PositionRange(), "vector selector must contain at least one non-empty matcher") + } + + if n.Name != "" { + // In this case the last LabelMatcher is checking for the metric name + // set outside the braces. This checks if the name has already been set + // previously + for _, m := range n.LabelMatchers[0 : len(n.LabelMatchers)-1] { + if m != nil && m.Name == labels.MetricName { + p.addParseErrf(n.PositionRange(), "metric name must not be set twice: %q or %q", n.Name, m.Value) + } + } + } + + case *NumberLiteral, *StringLiteral: // Nothing to do for terminals. default: - p.failf(n.PositionRange(), "unknown node type: %T", node) + p.addParseErrf(n.PositionRange(), "unknown node type: %T", node) } return } @@ -608,7 +605,7 @@ func (p *parser) checkType(node Node) (typ ValueType) { func (p *parser) unquoteString(s string) string { unquoted, err := strutil.Unquote(s) if err != nil { - p.failf(p.yyParser.lval.item.PositionRange(), "error unquoting string %q: %s", s, err) + p.addParseErrf(p.yyParser.lval.item.PositionRange(), "error unquoting string %q: %s", s, err) } return unquoted } @@ -659,7 +656,7 @@ func (p *parser) newLabelMatcher(label Item, operator Item, value Item) *labels. m, err := labels.NewMatcher(matchType, label.Val, val) if err != nil { - p.fail(mergeRanges(&label, &value), err) + p.addParseErr(mergeRanges(&label, &value), err) } return m @@ -674,20 +671,22 @@ func (p *parser) addOffset(e Node, offset time.Duration) { offsetp = &s.Offset endPosp = &s.PosRange.End case *MatrixSelector: - offsetp = &s.VectorSelector.Offset + if vs, ok := s.VectorSelector.(*VectorSelector); ok { + offsetp = &vs.Offset + } endPosp = &s.EndPos case *SubqueryExpr: offsetp = &s.Offset endPosp = &s.EndPos default: - p.failf(e.PositionRange(), "offset modifier must be preceded by an instant or range selector, but follows a %T instead", e) + p.addParseErrf(e.PositionRange(), "offset modifier must be preceded by an instant or range selector, but follows a %T instead", e) return } // it is already ensured by parseDuration func that there never will be a zero offset modifier if *offsetp != 0 { - p.failf(e.PositionRange(), "offset may not be set multiple times") - } else { + p.addParseErrf(e.PositionRange(), "offset may not be set multiple times") + } else if offsetp != nil { *offsetp = offset } diff --git a/promql/printer.go b/promql/printer.go index 0f6800ae2..6310cdfee 100644 --- a/promql/printer.go +++ b/promql/printer.go @@ -113,7 +113,7 @@ func (node *Call) String() string { func (node *MatrixSelector) String() string { // Copy the Vector selector before changing the offset - var vecSelector VectorSelector = *node.VectorSelector + var vecSelector VectorSelector = *node.VectorSelector.(*VectorSelector) offset := "" if vecSelector.Offset != time.Duration(0) { offset = fmt.Sprintf(" offset %s", model.Duration(vecSelector.Offset))