From f824117df9a125522fcda72251d0f23517ad6e64 Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Thu, 3 Feb 2022 10:41:23 -0700 Subject: [PATCH] Fix GroupBy with multiple offset int groups --- executor.go | 11 +++++++++++ executor_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/executor.go b/executor.go index 163c845e1..b79b448b7 100644 --- a/executor.go +++ b/executor.go @@ -3658,9 +3658,20 @@ func (e *executor) executeGroupByShard(ctx context.Context, qcx *Qcx, index stri } // Apply bases. + // + // SUP-139: The group value is shared across multiple groups so we can't + // add the base to each one. Instead, we need to track which ones have been + // seen already and avoid adding to those again in the future. for i, base := range bases { + m := make(map[*int64]struct{}) + for _, r := range results { + if _, ok := m[r.Group[i].Value]; ok { + continue + } + *r.Group[i].Value += base + m[r.Group[i].Value] = struct{}{} } } diff --git a/executor_test.go b/executor_test.go index 5233d07a7..f1da0402f 100644 --- a/executor_test.go +++ b/executor_test.go @@ -6150,6 +6150,34 @@ func TestExecutor_Execute_GroupBy(t *testing.T) { test.CheckGroupByOnKey(t, expected, results) }) + // SUP-139: GroupBy returns incorrect results when two or more Integer Range Fields are used to define the grouping + t.Run("CountByIntegersWithMinMax", func(t *testing.T) { + c.CreateField(t, "cbimm", pilosa.IndexOptions{}, "year", pilosa.OptFieldTypeInt(2019, 2020)) + c.CreateField(t, "cbimm", pilosa.IndexOptions{}, "quarter", pilosa.OptFieldTypeInt(1, 4)) + + c.ImportIntID(t, "cbimm", "year", []test.IntID{{ID: 1, Val: 2019}, {ID: 2, Val: 2019}, {ID: 3, Val: 2019}, {ID: 4, Val: 2019}}) + c.ImportIntID(t, "cbimm", "quarter", []test.IntID{{ID: 1, Val: 1}, {ID: 2, Val: 1}, {ID: 3, Val: 1}, {ID: 4, Val: 2}}) + + year2019 := int64(2019) + quarter1, quarter2 := int64(1), int64(2) + + results := c.Query(t, "cbimm", `GroupBy(Rows(year), Rows(quarter))`).Results[0].(*pilosa.GroupCounts).Groups() + + test.CheckGroupBy(t, + []pilosa.GroupCount{ + {Group: []pilosa.FieldRow{ + {Field: "year", RowID: 0, Value: &year2019}, + {Field: "quarter", RowID: 0, Value: &quarter1}, + }, Count: 3}, + {Group: []pilosa.FieldRow{ + {Field: "year", RowID: 0, Value: &year2019}, + {Field: "quarter", RowID: 0, Value: &quarter2}, + }, Count: 1}, + }, + results, + ) + + }) } for _, size := range []int{1, 3} { t.Run(fmt.Sprintf("%d_nodes", size), func(t *testing.T) {