Skip to content

Commit

Permalink
properly wrap prefix series selector into parens if it is used as t…
Browse files Browse the repository at this point in the history
…he right arg of binary operation

This fixes parsing for `q op group_left() (prefix)` queries
  • Loading branch information
valyala committed Jul 18, 2023
1 parent eba05b9 commit 026c425
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 6 deletions.
3 changes: 3 additions & 0 deletions optimizer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,9 @@ func TestOptimize(t *testing.T) {
f(`{a="b"} + ({c="d"} * on(e) group_right() {e="f"})`, `{a="b",e="f"} + ({c="d",e="f"} * on(e) group_right() {e="f"})`)
f(`{a="b"} + ({c="d"} * on(x) group_right() {e="f"})`, `{a="b",e="f"} + ({c="d"} * on(x) group_right() {e="f"})`)
f(`{a="b" or c="d"} + ({c="d"} * on(x) group_right() {e="f"})`, `{a="b",e="f" or c="d",e="f"} + ({c="d"} * on(x) group_right() {e="f"})`)
f(`a + on(x) group_left(*) (prefix{x="a"})`, `a{x="a"} + on(x) group_left(*) (prefix{x="a"})`)
f(`a + on(x) group_right(*) prefix "foo_" b{x="a"}`, `a{x="a"} + on(x) group_right(*) prefix "foo_" b{x="a"}`)
f(`a{x="a"} + on(x) group_right(*) prefix "foo_" prefix`, `a{x="a"} + on(x) group_right(*) prefix "foo_" (prefix{x="a"})`)

// specially handled binary expressions
f(`foo{a="b"} or bar{x="y"}`, `foo{a="b"} or bar{x="y"}`)
Expand Down
8 changes: 6 additions & 2 deletions parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ func (p *parser) parseExpr() (Expr, error) {
if err := p.parseModifierExpr(&be.JoinModifier, true); err != nil {
return nil, err
}
if strings.ToLower(p.lex.Token) == "prefix" {
if isPrefixModifier(p.lex.Token) {
if err := p.lex.Next(); err != nil {
return nil, fmt.Errorf("cannot read prefix for %s: %w", be.JoinModifier.AppendString(nil), err)
}
Expand Down Expand Up @@ -1758,7 +1758,11 @@ func needBinaryOpArgParens(arg Expr) bool {
}

func isReservedBinaryOpIdent(s string) bool {
return isBinaryOpGroupModifier(s) || isBinaryOpJoinModifier(s) || isBinaryOpBoolModifier(s)
return isBinaryOpGroupModifier(s) || isBinaryOpJoinModifier(s) || isBinaryOpBoolModifier(s) || isPrefixModifier(s)
}

func isPrefixModifier(s string) bool {
return strings.ToLower(s) == "prefix"
}

func appendArgInParens(dst []byte, arg Expr) []byte {
Expand Down
15 changes: 11 additions & 4 deletions parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,10 @@ func TestParseSuccess(t *testing.T) {
same(`a + on() group_left(*) b`)
same(`a + on() group_right(*) b`)
same(`a + on() group_left(*) prefix "foo" b`)
another(`a + group_left`, `a + (group_left)`)
another(`a + group_left / b`, `a + (group_left / b)`)
same(`a + on(x) (group_left)`)
same(`a + on(x) group_left() (prefix)`)
another(`a + oN() gROUp_rigHt(*) PREfix "bar" b`, `a + on() group_right(*) prefix "bar" b`)
same(`a + on(a) group_left(x,y) prefix "foo" b`)
same(`a + on(a,b) group_right(z) prefix "bar" b`)
Expand Down Expand Up @@ -731,10 +735,13 @@ func TestParseError(t *testing.T) {
f(`foo == bool $$`)
f(`"foo" + bar`)
f(`(foo + `)
f(`a + on(*) b`) // star cannot be used inside on()
f(`a + ignoring(*) b`) // star cannot be used inside ignoring()
f(`a + on() group_left(*,x) b`) // star cannot be mixed with other labels inside group_left()
f(`a + on() group_right(x,*) b`) // star cannot be mixed with other labels inside group_right()
f(`a + on(*) b`) // star cannot be used inside on()
f(`a + ignoring(*) b`) // star cannot be used inside ignoring()
f(`a + prefix "b" c`) // missing group_left()/group_right()
f(`a + on() prefix "b" c`) // missing group_left()/group_right()
f(`a + ignoring(foo) prefix "b" c`) // missing group_left()/group_right()
f(`a + on() group_left(*,x) b`) // star cannot be mixed with other labels inside group_left()
f(`a + on() group_right(x,*) b`) // star cannot be mixed with other labels inside group_right()

// invalid parensExpr
f(`(`)
Expand Down

0 comments on commit 026c425

Please sign in to comment.