From aa0d64047d06da328d8ed931a14b638625bcf33d Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Fri, 21 Dec 2018 17:40:50 -0600 Subject: [PATCH 1/4] add horrifying code to skip rows with count 0 in Group By --- executor.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/executor.go b/executor.go index 08eae1d49..a6a5fcf6b 100644 --- a/executor.go +++ b/executor.go @@ -2836,6 +2836,7 @@ func newGroupByIterator(rowIDs []RowIDs, children []*pql.Call, filter *Row, inde // nextAtIdx is a recursive helper method for getting the next row for the field // at index i, and then updating the rows in the "higher" fields if it wraps. func (gbi *groupByIterator) nextAtIdx(i int) { +TOP: nr, rowID, wrapped := gbi.rowIters[i].Next() if nr == nil { gbi.done = true @@ -2852,11 +2853,18 @@ func (gbi *groupByIterator) nextAtIdx(i int) { gbi.rows[i].row = nr.Intersect(gbi.rows[i-1].row) } gbi.rows[i].id = rowID + + if gbi.rows[i].row.Count() == 0 { + goto TOP // I wanted to just call nextAtIdx again, but if a bunch of + // rows in a row were 0, I was worried we'd get into a stack + // overflow situation + } } // Next returns a GroupCount representing the next group by record. When there // are no more records it will return an empty GroupCount and done==true. func (gbi *groupByIterator) Next() (ret GroupCount, done bool) { +TOPNEXT: if gbi.done { return ret, true } @@ -2865,6 +2873,10 @@ func (gbi *groupByIterator) Next() (ret GroupCount, done bool) { } else { ret.Count = gbi.rows[len(gbi.rows)-1].row.intersectionCount(gbi.rows[len(gbi.rows)-2].row) } + if ret.Count == 0 { + gbi.nextAtIdx(len(gbi.rows) - 1) + goto TOPNEXT + } ret.Group = make([]FieldRow, len(gbi.rows)) copy(ret.Group, gbi.fields) @@ -2873,6 +2885,7 @@ func (gbi *groupByIterator) Next() (ret GroupCount, done bool) { } // set up for next call + gbi.nextAtIdx(len(gbi.rows) - 1) return ret, false From 216fd0964a42cdf1f007f2afedbca719411329e7 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Sat, 22 Dec 2018 06:12:06 -0600 Subject: [PATCH 2/4] a quicker empty check for group by --- executor.go | 2 +- row.go | 15 +++++++++++++++ row_test.go | 14 ++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/executor.go b/executor.go index a6a5fcf6b..0c61e0714 100644 --- a/executor.go +++ b/executor.go @@ -2854,7 +2854,7 @@ TOP: } gbi.rows[i].id = rowID - if gbi.rows[i].row.Count() == 0 { + if gbi.rows[i].row.IsEmpty() { goto TOP // I wanted to just call nextAtIdx again, but if a bunch of // rows in a row were 0, I was worried we'd get into a stack // overflow situation diff --git a/row.go b/row.go index 9f8f9a403..4a19dc051 100644 --- a/row.go +++ b/row.go @@ -16,6 +16,7 @@ package pilosa import ( "encoding/json" + "fmt" "sort" "github.com/pilosa/pilosa/roaring" @@ -42,6 +43,20 @@ func NewRow(columns ...uint64) *Row { return r } +func (r *Row) IsEmpty() bool { + fmt.Println("what", len(r.segments)) + if len(r.segments) == 0 { + return true + } + for i := range r.segments { + if r.segments[i].n > 0 { + return false + } + + } + return true +} + // Merge merges data from other into r. func (r *Row) Merge(other *Row) { var segments []rowSegment diff --git a/row_test.go b/row_test.go index 7f1279ceb..49559f131 100644 --- a/row_test.go +++ b/row_test.go @@ -111,3 +111,17 @@ func TestRow_Difference_Segment(t *testing.T) { t.Fatalf("Test 2 Difference Results %v != expected %v\n", res.Columns(), exp) } } + +func TestRow_IsEmpty(t *testing.T) { + r1 := pilosa.NewRow(1, ShardWidth) + r2 := pilosa.NewRow(0, 2*ShardWidth) + res := r2.Intersect(r1) + + if r1.IsEmpty() { + t.Fatal("r1 Should Not Be Empty\n") + } + if !res.IsEmpty() { + t.Fatal("Result Should Be Empty\n") + } + +} From 8b06ed95942b29b13a102568f7d811cda7b0f84f Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Sat, 22 Dec 2018 17:47:41 -0600 Subject: [PATCH 3/4] removed some debug --- row.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/row.go b/row.go index 4a19dc051..255c3e847 100644 --- a/row.go +++ b/row.go @@ -16,7 +16,6 @@ package pilosa import ( "encoding/json" - "fmt" "sort" "github.com/pilosa/pilosa/roaring" @@ -44,7 +43,6 @@ func NewRow(columns ...uint64) *Row { } func (r *Row) IsEmpty() bool { - fmt.Println("what", len(r.segments)) if len(r.segments) == 0 { return true } From 5b14227e08d033bd750784519077657cc559b9d5 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Wed, 2 Jan 2019 14:25:25 -0600 Subject: [PATCH 4/4] convert gotos to for loops --- executor.go | 69 ++++++++++++++++++++++++++++------------------------- 1 file changed, 36 insertions(+), 33 deletions(-) diff --git a/executor.go b/executor.go index 0c61e0714..eea9a76a2 100644 --- a/executor.go +++ b/executor.go @@ -2836,46 +2836,49 @@ func newGroupByIterator(rowIDs []RowIDs, children []*pql.Call, filter *Row, inde // nextAtIdx is a recursive helper method for getting the next row for the field // at index i, and then updating the rows in the "higher" fields if it wraps. func (gbi *groupByIterator) nextAtIdx(i int) { -TOP: - nr, rowID, wrapped := gbi.rowIters[i].Next() - if nr == nil { - gbi.done = true - return - } - if wrapped && i != 0 { - gbi.nextAtIdx(i - 1) - } - if i == 0 && gbi.filter != nil { - gbi.rows[i].row = nr.Intersect(gbi.filter) - } else if i == 0 || i == len(gbi.rows)-1 { - gbi.rows[i].row = nr - } else { - gbi.rows[i].row = nr.Intersect(gbi.rows[i-1].row) - } - gbi.rows[i].id = rowID + // loop until we find a non-empty row. This is an optimization - the loop and if/break can be removed. + for { + nr, rowID, wrapped := gbi.rowIters[i].Next() + if nr == nil { + gbi.done = true + return + } + if wrapped && i != 0 { + gbi.nextAtIdx(i - 1) + } + if i == 0 && gbi.filter != nil { + gbi.rows[i].row = nr.Intersect(gbi.filter) + } else if i == 0 || i == len(gbi.rows)-1 { + gbi.rows[i].row = nr + } else { + gbi.rows[i].row = nr.Intersect(gbi.rows[i-1].row) + } + gbi.rows[i].id = rowID - if gbi.rows[i].row.IsEmpty() { - goto TOP // I wanted to just call nextAtIdx again, but if a bunch of - // rows in a row were 0, I was worried we'd get into a stack - // overflow situation + if !gbi.rows[i].row.IsEmpty() { + break + } } } // Next returns a GroupCount representing the next group by record. When there // are no more records it will return an empty GroupCount and done==true. func (gbi *groupByIterator) Next() (ret GroupCount, done bool) { -TOPNEXT: - if gbi.done { - return ret, true - } - if len(gbi.rows) == 1 { - ret.Count = gbi.rows[len(gbi.rows)-1].row.Count() - } else { - ret.Count = gbi.rows[len(gbi.rows)-1].row.intersectionCount(gbi.rows[len(gbi.rows)-2].row) - } - if ret.Count == 0 { - gbi.nextAtIdx(len(gbi.rows) - 1) - goto TOPNEXT + // loop until we find a result with count > 0 + for { + if gbi.done { + return ret, true + } + if len(gbi.rows) == 1 { + ret.Count = gbi.rows[len(gbi.rows)-1].row.Count() + } else { + ret.Count = gbi.rows[len(gbi.rows)-1].row.intersectionCount(gbi.rows[len(gbi.rows)-2].row) + } + if ret.Count == 0 { + gbi.nextAtIdx(len(gbi.rows) - 1) + continue + } + break } ret.Group = make([]FieldRow, len(gbi.rows))