Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check switches on syntax.Expr for exhaustiveness. #11113

Merged
merged 5 commits into from
Nov 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ linters:
- goimports
- gosimple
- staticcheck
- gochecksumtype
disable:
- unused
- unparam
Expand Down
2 changes: 1 addition & 1 deletion pkg/logql/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ func (q *query) evalSample(ctx context.Context, expr syntax.SampleExpr) (promql_

func (q *query) checkIntervalLimit(expr syntax.SampleExpr, limit time.Duration) error {
var err error
expr.Walk(func(e interface{}) {
expr.Walk(func(e syntax.Expr) {
switch e := e.(type) {
case *syntax.RangeAggregationExpr:
if e.Left == nil || e.Left.Interval <= limit {
Expand Down
2 changes: 1 addition & 1 deletion pkg/logql/evaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func Sortable(q Params) (bool, error) {
if err != nil {
return false, err
}
expr.Walk(func(e interface{}) {
expr.Walk(func(e syntax.Expr) {
rangeExpr, ok := e.(*syntax.VectorAggregationExpr)
if !ok {
return
Expand Down
4 changes: 2 additions & 2 deletions pkg/logql/optimize.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import "github.com/grafana/loki/pkg/logql/syntax"
func optimizeSampleExpr(expr syntax.SampleExpr) (syntax.SampleExpr, error) {
var skip bool
// we skip sharding AST for now, it's not easy to clone them since they are not part of the language.
expr.Walk(func(e interface{}) {
expr.Walk(func(e syntax.Expr) {
switch e.(type) {
case *ConcatSampleExpr, *DownstreamSampleExpr:
skip = true
Expand All @@ -28,7 +28,7 @@ func optimizeSampleExpr(expr syntax.SampleExpr) (syntax.SampleExpr, error) {

// removeLineformat removes unnecessary line_format within a SampleExpr.
func removeLineformat(expr syntax.SampleExpr) {
expr.Walk(func(e interface{}) {
expr.Walk(func(e syntax.Expr) {
rangeExpr, ok := e.(*syntax.RangeAggregationExpr)
if !ok {
return
Expand Down
8 changes: 4 additions & 4 deletions pkg/logql/rangemapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func (m RangeMapper) Map(expr syntax.SampleExpr, vectorAggrPushdown *syntax.Vect
// Example: expression `count_over_time({app="foo"}[10m])` returns 10m
func getRangeInterval(expr syntax.SampleExpr) time.Duration {
var rangeInterval time.Duration
expr.Walk(func(e interface{}) {
expr.Walk(func(e syntax.Expr) {
switch concrete := e.(type) {
case *syntax.RangeAggregationExpr:
rangeInterval = concrete.Left.Interval
Expand All @@ -190,7 +190,7 @@ func getRangeInterval(expr syntax.SampleExpr) time.Duration {
// such as `| json` or `| logfmt`, that would result in an exploding amount of series in downstream queries.
func hasLabelExtractionStage(expr syntax.SampleExpr) bool {
found := false
expr.Walk(func(e interface{}) {
expr.Walk(func(e syntax.Expr) {
switch concrete := e.(type) {
case *syntax.LogfmtParserExpr:
found = true
Expand Down Expand Up @@ -278,7 +278,7 @@ func (m RangeMapper) vectorAggrWithRangeDownstreams(expr *syntax.RangeAggregatio
// Returns the updated downstream ConcatSampleExpr.
func appendDownstream(downstreams *ConcatSampleExpr, expr syntax.SampleExpr, interval time.Duration, offset time.Duration) *ConcatSampleExpr {
sampleExpr := clone(expr)
sampleExpr.Walk(func(e interface{}) {
sampleExpr.Walk(func(e syntax.Expr) {
switch concrete := e.(type) {
case *syntax.RangeAggregationExpr:
concrete.Left.Interval = interval
Expand All @@ -300,7 +300,7 @@ func getOffsets(expr syntax.SampleExpr) []time.Duration {
// Expect to always find at most 1 offset, so preallocate it accordingly
offsets := make([]time.Duration, 0, 1)

expr.Walk(func(e interface{}) {
expr.Walk(func(e syntax.Expr) {
switch concrete := e.(type) {
case *syntax.RangeAggregationExpr:
offsets = append(offsets, concrete.Left.Offset)
Expand Down
6 changes: 5 additions & 1 deletion pkg/logql/syntax/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (
)

// Expr is the root expression which can be a SampleExpr or LogSelectorExpr
//
//sumtype:decl
type Expr interface {
logQLExpr() // ensure it's not implemented accidentally
Shardable() bool // A recursive check on the AST to see if it's shardable.
Expand Down Expand Up @@ -2106,7 +2108,7 @@ func (e *VectorExpr) MatcherGroups() ([]MatcherRange, error) { return nil, e.er
func (e *VectorExpr) Extractor() (log.SampleExtractor, error) { return nil, nil }

func ReducesLabels(e Expr) (conflict bool) {
e.Walk(func(e interface{}) {
e.Walk(func(e Expr) {
switch expr := e.(type) {
case *RangeAggregationExpr:
if groupingReducesLabels(expr.Grouping) {
Expand Down Expand Up @@ -2135,6 +2137,8 @@ func ReducesLabels(e Expr) (conflict bool) {
break
}
}
default:
return
}
})
return
Expand Down
2 changes: 1 addition & 1 deletion pkg/logql/syntax/walk.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package syntax

type WalkFn = func(e interface{})
type WalkFn = func(e Expr)

func walkAll(f WalkFn, xs ...Walkable) {
for _, x := range xs {
Expand Down
4 changes: 2 additions & 2 deletions pkg/logql/syntax/walk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func Test_Walkable(t *testing.T) {
require.Nil(t, err)

var cnt int
expr.Walk(func(_ interface{}) { cnt++ })
expr.Walk(func(_ Expr) { cnt++ })
require.Equal(t, test.want, cnt)
})
}
Expand Down Expand Up @@ -77,7 +77,7 @@ func Test_AppendMatchers(t *testing.T) {
expr, err := ParseExpr(test.expr)
require.NoError(t, err)

expr.Walk(func(e interface{}) {
expr.Walk(func(e Expr) {
switch me := e.(type) {
case *MatchersExpr:
me.AppendMatchers(test.matchers)
Expand Down
2 changes: 1 addition & 1 deletion pkg/querier/multi_tenant_querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ func removeTenantSelector(params logql.SelectSampleParams, tenantIDs []string) (
// replaceMatchers traverses the passed expression and replaces all matchers.
func replaceMatchers(expr syntax.Expr, matchers []*labels.Matcher) syntax.Expr {
expr, _ = syntax.Clone(expr)
expr.Walk(func(e interface{}) {
expr.Walk(func(e syntax.Expr) {
switch concrete := e.(type) {
case *syntax.MatchersExpr:
concrete.Mts = matchers
Expand Down
2 changes: 1 addition & 1 deletion pkg/querier/queryrange/split_by_interval.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ func maxRangeVectorAndOffsetDuration(q string) (time.Duration, time.Duration, er
}

var maxRVDuration, maxOffset time.Duration
expr.Walk(func(e interface{}) {
expr.Walk(func(e syntax.Expr) {
if r, ok := e.(*syntax.LogRange); ok {
if r.Interval > maxRVDuration {
maxRVDuration = r.Interval
Expand Down
Loading