scriptoriumadmin/featurebase
1
0
Fork 0
mirror of https://github.com/featurebasedb/featurebase.git synced 2026-09-10 23:11:01 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix bug with "having" and "limit" in GroupBy

Browse source 
the limit could get applied before "having" in some cases which could
result in results being discarded which met the having condition while
results were kept which did not, ultimately resulting in GroupBy
falsely reporting fewer results than actually existed.
This commit is contained in:
Matt Jaffee 2021-01-01 21:56:59 -06:00
parent ea539d8241
commit 6094663e7a
No known key found for this signature in database
GPG key ID: 08A3DFFF987B11BF
2 changed files with 16 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
executor.go
View file

@ -2767,7 +2767,8 @@ func (e *executor) executeGroupBy(ctx context.Context, qcx *Qcx, index string, c
// don't want to prematurely limit the results if we're sorting
limit = int(^uint(0) >> 1)
}
if _, hasHaving, err := c.CallArg("having"); err != nil {
having, hasHaving, err := c.CallArg("having")
if err != nil {
return nil, errors.Wrap(err, "getting 'having' argument")
} else if hasHaving {
// don't want to prematurely limit the results if we're filtering some out
@ -2849,7 +2850,7 @@ func (e *executor) executeGroupBy(ctx context.Context, qcx *Qcx, index string, c
// If there's no sorting, we want to apply limits before
// calculating the Distinct aggregate which is expensive on a
// per-result basis.
if sorter == nil {
if sorter == nil && !hasHaving {
results, err = applyLimitAndOffsetToGroupByResult(c, results)
if err != nil {
return nil, errors.Wrap(err, "applying limit/offset")
@ -2904,9 +2905,7 @@ func (e *executor) executeGroupBy(ctx context.Context, qcx *Qcx, index string, c
}
// Apply having.
if having, hasHaving, err := c.CallArg("having"); err != nil {
return nil, err
} else if hasHaving && !opt.Remote {
if hasHaving && !opt.Remote {
// parse the condition as PQL
if having.Name != "Condition" {
return nil, errors.New("the only supported having call is Condition()")
@ -2931,6 +2930,12 @@ func (e *executor) executeGroupBy(ctx context.Context, qcx *Qcx, index string, c
if err != nil {
return nil, errors.Wrap(err, "applying limit/offset")
}
} else if hasHaving && !opt.Remote {
results, err = applyLimitAndOffsetToGroupByResult(c, results)
if err != nil {
return nil, errors.Wrap(err, "applying limit/offset")
}
}
return results, nil

6
executor_test.go
View file

@ -7018,6 +7018,12 @@ zebra,1,0
toucan,1,0
dog,1,0
icecream,6,0
`,
},
{
query: "GroupBy(Rows(field=likes), aggregate=Sum(field=net_worth), limit=2, having=Condition(sum>10))",
csvVerifier: `pangolin,1,100
zebra,1,1000
`,
},
{