From 603b0e5369eef89f53276cceecd32cb5e01ed739 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Tue, 9 Oct 2018 19:27:10 -0500 Subject: [PATCH] fix logic bug applying limit to group by rows check in failing test showing how applying the limit to each rows query can cause the query to falsely return no results --- executor.go | 2 +- executor_test.go | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/executor.go b/executor.go index 8cac5c034..1cd95eed4 100644 --- a/executor.go +++ b/executor.go @@ -851,7 +851,7 @@ func (e *executor) executeGroupBy(ctx context.Context, index string, c *pql.Call } if limit, hasLimit, err := child.UintArg("limit"); err != nil { return nil, err - } else if hasLimit && int(limit) > gbLimit { + } else if !hasLimit || int(limit) > gbLimit { child.Args["limit"] = uint64(gbLimit) } var err error diff --git a/executor_test.go b/executor_test.go index 072dff0f4..12bf0b1e8 100644 --- a/executor_test.go +++ b/executor_test.go @@ -2416,6 +2416,26 @@ func TestExecutor_Execute_GroupBy(t *testing.T) { checkGroupBy(t, expected, results) }) + + c.CreateField(t, "i", pilosa.IndexOptions{}, "a") + c.CreateField(t, "i", pilosa.IndexOptions{}, "b") + c.ImportBits(t, "i", "a", [][2]uint64{ + {0, 1}, + {1, ShardWidth + 1}, + }) + c.ImportBits(t, "i", "b", [][2]uint64{ + {0, ShardWidth + 1}, + {1, 1}, + }) + + t.Run("tricky data", func(t *testing.T) { + expected := []pilosa.GroupCount{ + {Group: []pilosa.FieldRow{{Field: "a", RowID: 0}, {Field: "b", RowID: 1}}, Count: 1}, + } + + results := c.Query(t, "i", `GroupBy(Rows(field=a), Rows(field=b), limit=1)`).Results[0].([]pilosa.GroupCount) + checkGroupBy(t, expected, results) + }) } func checkGroupBy(t *testing.T, expected, results []pilosa.GroupCount) { @@ -2434,7 +2454,7 @@ func checkGroupBy(t *testing.T, expected, results []pilosa.GroupCount) { } for _, result := range results { if notIn(result, expected) { - t.Fatalf("unexpected grouping: \n%+v\n\n\n%+v\n", result, expected) + t.Fatalf("unexpected results: \n got:%+v\nwant:%+v\n", results, expected) } } }