diff --git a/api/internal/querylang/parser/parser.go b/api/internal/querylang/parser/parser.go index 01201e7..d1daf4e 100644 --- a/api/internal/querylang/parser/parser.go +++ b/api/internal/querylang/parser/parser.go @@ -41,11 +41,36 @@ func (p *parser) advance() { } func (p *parser) parseQuery() (*ast.Query, error) { - base, err := p.parseBoolExpr() - if err != nil { - return nil, err + q := &ast.Query{} + + // base_search is normally mandatory (see the grammar comment on + // ast.Query), but every pipe-stage keyword is also a valid bare + // Ident, so a query with no leading filter -- e.g. `stats count by + // host`, documented as valid in + // /docs/query-language-reference.md's "stats" section -- used to + // get silently misparsed as a base_search of four ANDed free-text + // terms ("stats", "count", "by", "host"), matching nothing rather + // than erroring or aggregating. Recognizing a leading pipe-stage + // keyword up front and skipping straight to stage-parsing (q.Base + // stays its zero value; compileBoolExpr already treats zero Terms + // as "match everything", so no planner/executor change is needed) + // fixes that without a leading "|". atPipeStageKeyword's comparator + // lookahead keeps a genuine field named e.g. "where" (`where=foo`) + // parsing as a structured filter, same as today. + if p.atPipeStageKeyword() { + stage, err := p.parsePipeStage() + if err != nil { + return nil, err + } + q.Pipes = append(q.Pipes, stage) + } else { + base, err := p.parseBoolExpr() + if err != nil { + return nil, err + } + q.Base = base } - q := &ast.Query{Base: base} + for p.cur.Kind == lexer.Pipe { p.advance() stage, err := p.parsePipeStage() @@ -60,6 +85,24 @@ func (p *parser) parseQuery() (*ast.Query, error) { return q, nil } +// atPipeStageKeyword reports whether the parser is sitting on one of the +// six pipe-stage keywords used as the query's very first token, with no +// preceding base filter. The comparator lookahead disambiguates from a +// structured comparison on a field that happens to share one of these +// names (e.g. `where=foo`), which must keep parsing as a filter, not a +// stage. +func (p *parser) atPipeStageKeyword() bool { + if p.cur.Kind != lexer.Ident || isComparatorStart(p.next.Kind) { + return false + } + switch p.cur.Value { + case "where", "stats", "sort", "fields", "head", "tail": + return true + default: + return false + } +} + func (p *parser) parseBoolExpr() (ast.BoolExpr, error) { var expr ast.BoolExpr term, err := p.parseTerm() diff --git a/api/internal/querylang/parser/parser_test.go b/api/internal/querylang/parser/parser_test.go index 4f78ddd..f92cbab 100644 --- a/api/internal/querylang/parser/parser_test.go +++ b/api/internal/querylang/parser/parser_test.go @@ -266,6 +266,77 @@ func TestParseMultipleAggregations(t *testing.T) { } } +func TestParseLeadingStatsStageWithNoBaseFilter(t *testing.T) { + // Documented as valid in /docs/query-language-reference.md's "stats" + // section -- used to get silently misparsed as four ANDed free-text + // base terms ("stats", "count", "by", "host"), matching nothing. + q, err := Parse(`stats count by host`) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + if len(q.Base.Terms) != 0 { + t.Fatalf("expected an empty base (match-everything), got %+v", q.Base.Terms) + } + if len(q.Pipes) != 1 { + t.Fatalf("expected 1 pipe stage, got %d: %+v", len(q.Pipes), q.Pipes) + } + stats, ok := q.Pipes[0].(ast.StatsStage) + if !ok { + t.Fatalf("expected StatsStage, got %T", q.Pipes[0]) + } + if len(stats.Aggs) != 1 || stats.Aggs[0].Func != "count" || len(stats.By) != 1 || stats.By[0] != "host" { + t.Fatalf("unexpected stats stage: %+v", stats) + } +} + +func TestParseLeadingStatsStageWithParenAggAndNoBaseFilter(t *testing.T) { + // The specific case that first surfaced this: count() (parens, zero + // args) as the very first token of the query. + q, err := Parse(`stats count(), avg(latency_ms) as avg_latency by host, service`) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + stats := q.Pipes[0].(ast.StatsStage) + if len(stats.Aggs) != 2 || stats.Aggs[0].Func != "count" || stats.Aggs[1].Func != "avg" { + t.Fatalf("unexpected stats aggs: %+v", stats.Aggs) + } +} + +func TestParseLeadingWhereStageWithNoBaseFilter(t *testing.T) { + q, err := Parse(`where status>=500`) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + if len(q.Base.Terms) != 0 { + t.Fatalf("expected an empty base, got %+v", q.Base.Terms) + } + where, ok := q.Pipes[0].(ast.WhereStage) + if !ok { + t.Fatalf("expected WhereStage, got %T", q.Pipes[0]) + } + cmp := where.Expr.Terms[0].(ast.Comparison) + if cmp.Field != "status" || cmp.Op != ">=" || cmp.Value != "500" { + t.Fatalf("unexpected where comparison: %+v", cmp) + } +} + +func TestParseFieldNamedLikeAPipeStageKeywordStillFilters(t *testing.T) { + // atPipeStageKeyword's comparator lookahead must not misfire on a + // genuine structured filter for a field that happens to share one + // of the six reserved names. + q, err := Parse(`where=foo`) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + if len(q.Pipes) != 0 { + t.Fatalf("expected no pipe stages, got %+v", q.Pipes) + } + cmp, ok := q.Base.Terms[0].(ast.Comparison) + if !ok || cmp.Field != "where" || cmp.Value != "foo" { + t.Fatalf("expected Comparison(where=foo), got %+v", q.Base.Terms[0]) + } +} + // --- error cases --- func TestParseErrorEmptyQuery(t *testing.T) {