Skip to content

Commit

Permalink
Bugfix: Apply LIMIT to output of GROUP BY (#107)
Browse files Browse the repository at this point in the history
Also added && shortcut operator (Similar to the || operator)
  • Loading branch information
scudette authored Oct 9, 2024
1 parent 1f95429 commit 76c3a28
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 11 deletions.
12 changes: 11 additions & 1 deletion fixtures/vql_queries.golden
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,17 @@
"'A' || '' || 'B'": "A"
}
],
"076 Whitespace in the query: SELECT * FROM test()": [
"076 Alternatives with AND shortcut operator: SELECT NULL \u0026\u0026 '', TRUE \u0026\u0026 'XX', 'A' \u0026\u0026 'B', 'A' \u0026\u0026 FALSE, ((FALSE \u0026\u0026 1) || 2), TRUE \u0026\u0026 1 || 2 FROM scope()": [
{
"NULL \u0026\u0026 ''": false,
"TRUE \u0026\u0026 'XX'": "XX",
"'A' \u0026\u0026 'B'": "B",
"'A' \u0026\u0026 FALSE": false,
"((FALSE \u0026\u0026 1) || 2)": 2,
"TRUE \u0026\u0026 1 || 2": 1
}
],
"077 Whitespace in the query: SELECT * FROM test()": [
{
"foo": 0,
"bar": 0
Expand Down
33 changes: 23 additions & 10 deletions vfilter.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ var (
`|(?ims)(?P<AND>\bAND\b)` +
`|(?ims)(?P<OR>\bOR\b)` +
`|(?ims)(?P<AlternativeOR>\|+)` +
`|(?ims)(?P<AlternativeAND>&&)` +
`|(?ims)(?P<FROM>\bFROM\b)` +
`|(?ims)(?P<NOT>\bNOT\b)` +
`|(?ims)(?P<AS>\bAS\b)` +
Expand Down Expand Up @@ -475,12 +476,10 @@ func (self *_Select) Eval(ctx context.Context, scope types.Scope) <-chan Row {
// Start query evaluation
scope.Explainer().StartQuery(self)

if self.GroupBy != nil {
return self.EvalGroupBy(ctx, scope)
}

output_chan := make(chan Row)

// Limits occur before the group by so we can cut the group by
// result short according to the limit clause.
if self.Limit != nil {
go func() {
defer close(output_chan)
Expand Down Expand Up @@ -511,6 +510,12 @@ func (self *_Select) Eval(ctx context.Context, scope types.Scope) <-chan Row {
return output_chan
}

// Group by occurs before order by so we can order the grouped by
// results.
if self.GroupBy != nil {
return self.EvalGroupBy(ctx, scope)
}

if self.OrderBy != nil {
desc := false
if self.OrderByDesc != nil {
Expand Down Expand Up @@ -813,7 +818,7 @@ type _AndExpression struct {
}

type _OpAndTerm struct {
Operator string ` @AND `
Operator string ` ( @AND | @AlternativeAND ) `
Term *_OrExpression `@@`
}

Expand Down Expand Up @@ -1268,22 +1273,30 @@ func (self *_AndExpression) IsAggregate(scope types.Scope) bool {
}

func (self *_AndExpression) Reduce(ctx context.Context, scope types.Scope) Any {
result := self.Left.Reduce(ctx, scope)
left := self.Left.Reduce(ctx, scope)
if self.Right == nil {
return result
return left
}

if scope.Bool(result) == false {
if scope.Bool(left) == false {
return false
}

last := left
for _, term := range self.Right {
if scope.Bool(term.Term.Reduce(ctx, scope)) == false {
right := term.Term.Reduce(ctx, scope)
if scope.Bool(right) == false {
return false
}

if term.Operator == "&&" {
last = right
} else {
last = true
}
}

return true
return last
}

func (self *_OrExpression) IsAggregate(scope types.Scope) bool {
Expand Down
3 changes: 3 additions & 0 deletions vfilter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,9 @@ b={
{"Alternatives with the OR shortcut operator false",
"SELECT NULL || '', NULL || FALSE, NULL || 'X', 'A' || 'B', 'A' || FALSE, 'A' || '' || 'B' FROM scope()"},

{"Alternatives with AND shortcut operator",
"SELECT NULL && '', TRUE && 'XX', 'A' && 'B', 'A' && FALSE, ((FALSE && 1) || 2), TRUE && 1 || 2 FROM scope()"},

{"Whitespace in the query",
"SELECT * FROM\ntest()"},
}
Expand Down

0 comments on commit 76c3a28

Please sign in to comment.