Skip to content

Commit

Permalink
feat(traceql): define error types with position
Browse files Browse the repository at this point in the history
  • Loading branch information
tdakkota committed Aug 2, 2023
1 parent c23bf64 commit 8a04002
Show file tree
Hide file tree
Showing 10 changed files with 222 additions and 118 deletions.
28 changes: 28 additions & 0 deletions internal/traceql/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package traceql

import (
"fmt"
"text/scanner"
)

// SyntaxError is a syntax error.
type SyntaxError struct {
Msg string
Pos scanner.Position
}

// Error implements error.
func (e *SyntaxError) Error() string {
return fmt.Sprintf("at %s: %s", e.Pos, e.Msg)
}

// TypeError is a type checking error.
type TypeError struct {
Msg string
Pos scanner.Position
}

// Error implements error.
func (e *TypeError) Error() string {
return fmt.Sprintf("at %s: %s", e.Pos, e.Msg)
}
2 changes: 1 addition & 1 deletion internal/traceql/field_expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package traceql
// FieldExpr is a field expression.
type FieldExpr interface {
fieldExpr()
ValueType() StaticType
TypedExpr
}

func (*BinaryFieldExpr) fieldExpr() {}
Expand Down
20 changes: 15 additions & 5 deletions internal/traceql/parser.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package traceql

import (
"fmt"
"strconv"
"time"

Expand Down Expand Up @@ -60,15 +61,24 @@ func (p *parser) unread() {

func (p *parser) unexpectedToken(t lexer.Token) error {
if t.Type == lexer.EOF {
return errors.New("unexpected EOF")
return &SyntaxError{
Msg: "unexpected EOF",
Pos: t.Pos,
}
}
return &SyntaxError{
Msg: fmt.Sprintf("unexpected token %q", t.Type),
Pos: t.Pos,
}
return errors.Errorf("unexpected token %q at %s", t.Type, t.Pos)
}

func (p *parser) consumeText(tt lexer.TokenType) (string, error) {
func (p *parser) consumeText(expect lexer.TokenType) (string, error) {
t := p.next()
if t.Type != tt {
return "", errors.Wrapf(p.unexpectedToken(t), "expected %q", tt)
if t.Type != expect {
return "", &SyntaxError{
Msg: fmt.Sprintf("expected %q, got %q", expect, t.Type),
Pos: t.Pos,
}
}
return t.Text, nil
}
Expand Down
18 changes: 11 additions & 7 deletions internal/traceql/parser_field_expr.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package traceql

import (
"fmt"
"strings"

"github.com/go-faster/errors"
"go.opentelemetry.io/collector/pdata/ptrace"

"github.com/go-faster/oteldb/internal/traceql/lexer"
Expand Down Expand Up @@ -48,10 +48,12 @@ func (p *parser) parseFieldExpr1() (FieldExpr, error) {
op = OpNeg
}

if err := CheckUnaryExpr(op, expr); err != nil {
return nil, errors.Wrapf(err, "at %s", pos)
if t := expr.ValueType(); !op.CheckType(t) {
return nil, &TypeError{
Msg: fmt.Sprintf("unary operator %q not defined on %q", op, t),
Pos: pos,
}
}

return &UnaryFieldExpr{
Expr: expr,
Op: op,
Expand Down Expand Up @@ -95,15 +97,17 @@ func (p *parser) parseBinaryFieldExpr(left FieldExpr, minPrecedence int) (FieldE
if !ok || op.Precedence() < minPrecedence {
return left, nil
}
// Consume op.
p.next()
// Consume op and get op token position.
opPos := p.next().Pos

// Get right expression position.
rightPos := p.peek().Pos
right, err := p.parseFieldExpr1()
if err != nil {
return nil, err
}

if err := CheckBinaryExpr(left, op, right); err != nil {
if err := p.checkBinaryExpr(left, op, opPos, right, rightPos); err != nil {
return nil, err
}

Expand Down
12 changes: 8 additions & 4 deletions internal/traceql/parser_pipeline.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package traceql

import (
"github.com/go-faster/errors"

"github.com/go-faster/oteldb/internal/traceql/lexer"
)

Expand Down Expand Up @@ -39,7 +37,10 @@ func (p *parser) parsePipeline() (stages []PipelineStage, rerr error) {
stages = append(stages, op)
case lexer.Coalesce:
if len(stages) < 1 {
return stages, errors.Errorf("coalesce cannot be first operation: at %s", t.Pos)
return stages, &SyntaxError{
Msg: "coalesce cannot be first operation",
Pos: t.Pos,
}
}
p.next()

Expand Down Expand Up @@ -143,7 +144,10 @@ func (p *parser) parseSpansetExpr1() (SpansetExpr, error) {
switch fieldExpr.ValueType() {
case TypeBool, TypeAttribute:
default:
return nil, errors.Errorf("filter expression must evaluate to boolean: at %s", t2.Pos)
return nil, &TypeError{
Msg: "filter expression must evaluate to boolean",
Pos: t2.Pos,
}
}
filter.Expr = fieldExpr
} else {
Expand Down
15 changes: 9 additions & 6 deletions internal/traceql/parser_scalar_expr.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package traceql

import (
"github.com/go-faster/errors"

"github.com/go-faster/oteldb/internal/traceql/lexer"
)

Expand Down Expand Up @@ -75,7 +73,10 @@ func (p *parser) parseAggregateScalarExpr() (expr *AggregateScalarExpr, _ error)
expr.Field = field

if t := field.ValueType(); t != TypeAttribute && !t.IsNumeric() {
return nil, errors.Errorf("aggregate expression must evaluate to numeric: at %s", pos)
return nil, &TypeError{
Msg: "aggregate expression must evaluate to numeric",
Pos: pos,
}
}
}

Expand All @@ -92,15 +93,17 @@ func (p *parser) parseBinaryScalarExpr(left ScalarExpr, minPrecedence int) (Scal
if !ok || !op.IsArithmetic() || op.Precedence() < minPrecedence {
return left, nil
}
// Consume op.
p.next()
// Consume op and get op token position.
opPos := p.next().Pos

// Get right expression position.
rightPos := p.peek().Pos
right, err := p.parseScalarExpr1()
if err != nil {
return nil, err
}

if err := CheckBinaryExpr(left, op, right); err != nil {
if err := p.checkBinaryExpr(left, op, opPos, right, rightPos); err != nil {
return nil, err
}

Expand Down
88 changes: 8 additions & 80 deletions internal/traceql/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -823,85 +823,6 @@ var tests = []TestCase{
{`coalesce()`, nil, true},
// Scalar filter must be a part of pipeline.
{`max() > 3 && { }`, nil, true},

// Type check errors.
// Spanset expression must evaluate to boolean.
// Static.
{`{ "foo" }`, nil, true},
{`{ 1 }`, nil, true},
{`{ 3.14 }`, nil, true},
{`{ nil }`, nil, true},
{`{ 5h }`, nil, true},
{`{ ok }`, nil, true},
{`{ client }`, nil, true},
// Attribute.
{`{ duration }`, nil, true},
{`{ childCount }`, nil, true},
{`{ name }`, nil, true},
{`{ status }`, nil, true},
{`{ kind }`, nil, true},
{`{ parent }`, nil, true},
{`{ rootName }`, nil, true},
{`{ rootServiceName }`, nil, true},
{`{ traceDuration }`, nil, true},
// Aggregate expression must evaluate to number.
{`{ .a } | sum(true) > 10`, nil, true},
{`{ .a } | sum("foo") > 10`, nil, true},
// Illegal operand.
{`{ -"foo" =~ "foo" }`, nil, true},
{`{ !"foo" =~ "foo" }`, nil, true},
{`{ 1 + "foo" }`, nil, true},
{`{ 1 + true }`, nil, true},
{`{ 1 + nil }`, nil, true},
{`{ 1 + ok }`, nil, true},
{`{ 1 + client }`, nil, true},
{`{ "foo" > 10 }`, nil, true},
{`{ "foo" =~ 10 }`, nil, true},
// Illegal operation.
// String.
{`{ "foo" && "foo" }`, nil, true},
{`{ "foo" || "foo" }`, nil, true},
{`{ "foo" + "foo" }`, nil, true},
{`{ "foo" % "foo" }`, nil, true},
// Int.
{`{ 1 && 1 }`, nil, true},
{`{ 1 || 1 }`, nil, true},
{`{ 1 =~ 1 }`, nil, true},
{`{ 1 !~ 1 }`, nil, true},
// Number.
{`{ 3.14 && 3.14 }`, nil, true},
{`{ 3.14 || 3.14 }`, nil, true},
{`{ 3.14 =~ 3.14 }`, nil, true},
{`{ 3.14 !~ 3.14 }`, nil, true},
// Bool.
{`{ true / true }`, nil, true},
{`{ true < true }`, nil, true},
{`{ true =~ true }`, nil, true},
{`{ true !~ true }`, nil, true},
// Nil.
{`{ nil - nil }`, nil, true},
{`{ nil =~ nil }`, nil, true},
{`{ nil !~ nil }`, nil, true},
{`{ nil < nil }`, nil, true},
// Duration.
{`{ 5h && 5h }`, nil, true},
{`{ 5h || 5h }`, nil, true},
{`{ 5h =~ 5h }`, nil, true},
{`{ 5h !~ 5h }`, nil, true},
// Status.
{`{ ok && ok }`, nil, true},
{`{ ok || ok }`, nil, true},
{`{ ok ^ ok }`, nil, true},
{`{ ok =~ ok }`, nil, true},
{`{ ok !~ ok }`, nil, true},
{`{ ok <= ok }`, nil, true},
// Kind.
{`{ client && client }`, nil, true},
{`{ client || client }`, nil, true},
{`{ client * client }`, nil, true},
{`{ client =~ client }`, nil, true},
{`{ client !~ client }`, nil, true},
{`{ client >= client }`, nil, true},
}

func TestParse(t *testing.T) {
Expand All @@ -915,8 +836,12 @@ func TestParse(t *testing.T) {
}()

got, err := Parse(tt.input)
if err != nil {
t.Logf("Error: \n%+v", err)
}
if tt.wantErr {
require.Error(t, err)
var se *SyntaxError
require.ErrorAs(t, err, &se)
return
}
require.NoError(t, err)
Expand All @@ -929,6 +854,9 @@ func FuzzParse(f *testing.F) {
for _, tt := range tests {
f.Add(tt.input)
}
for _, tt := range typeTests {
f.Add(tt.input)
}
f.Fuzz(func(t *testing.T, input string) {
defer func() {
if r := recover(); r != nil || t.Failed() {
Expand Down
2 changes: 1 addition & 1 deletion internal/traceql/scalar_expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package traceql
// ScalarExpr is a scalar expression.
type ScalarExpr interface {
scalarExpr()
ValueType() StaticType
TypedExpr
}

func (*BinaryScalarExpr) scalarExpr() {}
Expand Down
38 changes: 24 additions & 14 deletions internal/traceql/type.go
Original file line number Diff line number Diff line change
@@ -1,32 +1,42 @@
package traceql

import "github.com/go-faster/errors"
import (
"fmt"
"text/scanner"
)

// TypedExpr is an interface for typed expression.
type TypedExpr interface {
ValueType() StaticType
}

// CheckBinaryExpr typechecks binary expression.
func CheckBinaryExpr(left TypedExpr, op BinaryOp, right TypedExpr) error {
// checkBinaryExpr typechecks binary expression.
func (p *parser) checkBinaryExpr(
left TypedExpr,
op BinaryOp,
opPos scanner.Position,
right TypedExpr,
rightPos scanner.Position,
) error {
lt := left.ValueType()
rt := right.ValueType()
if !op.CheckType(lt) {
return errors.Errorf("can't apply op %q to %q", op, lt)
return &TypeError{
Msg: fmt.Sprintf("binary operator %q not defined on %q", op, lt),
Pos: opPos,
}
}
if !op.CheckType(rt) {
return errors.Errorf("can't apply op %q to %q", op, rt)
return &TypeError{
Msg: fmt.Sprintf("binary operator %q not defined on %q", op, rt),
Pos: opPos,
}
}
if !lt.CheckOperand(rt) {
return errors.Errorf("invalid operand types: %q and %q", lt, rt)
}
return nil
}

// CheckUnaryExpr typechecks unary expression.
func CheckUnaryExpr(op UnaryOp, expr TypedExpr) error {
if t := expr.ValueType(); !op.CheckType(t) {
return errors.Errorf("can't apply op %q to %q", op, t)
return &TypeError{
Msg: fmt.Sprintf("operand types mismatch: %q and %q", lt, rt),
Pos: rightPos,
}
}
return nil
}
Loading

0 comments on commit 8a04002

Please sign in to comment.