From 41d334aeaf7cc850cb9e2cbe83c839d7e8a3b4bc Mon Sep 17 00:00:00 2001 From: Travis Date: Wed, 8 Mar 2017 16:09:56 -0600 Subject: [PATCH] If a top-n query contains BitmapIDs, then treat N as having value 0. --- executor.go | 4 ---- executor_test.go | 4 ++-- fragment.go | 18 ++++++++++++------ 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/executor.go b/executor.go index 774e2443f..5862fbe3b 100644 --- a/executor.go +++ b/executor.go @@ -189,10 +189,6 @@ func (e *Executor) executeTopN(ctx context.Context, db string, c *pql.Call, slic // Only the original caller should refetch the full counts. other := c.Clone() - // Double the size of n for other calls in order to... - // TODO: travis review - other.Args["n"] = len(bitmapIDs) * 2 - ids := Pairs(pairs).Keys() sort.Sort(uint64Slice(ids)) other.Args["ids"] = ids diff --git a/executor_test.go b/executor_test.go index 1cc0985ac..196c2e815 100644 --- a/executor_test.go +++ b/executor_test.go @@ -241,7 +241,7 @@ func TestExecutor_Execute_TopN_fill(t *testing.T) { idx := MustOpenIndex() defer idx.Close() - // Set bits for bitmaps 0, 10, & 20 across two slices. + // Set bits for bitmaps 0 & 1 across two slices. idx.MustCreateFragmentIfNotExists("d", "f", 0).SetBit(0, 0) idx.MustCreateFragmentIfNotExists("d", "f", 0).SetBit(0, 1) idx.MustCreateFragmentIfNotExists("d", "f", 0).SetBit(0, 2) @@ -601,7 +601,7 @@ func TestExecutor_Execute_Remote_TopN(t *testing.T) { t.Fatalf("unexpected query(0): %s", query.String()) } case 1: - if query.String() != `TopN(frame="f", ids=[0,10,30], n=0)` { + if query.String() != `TopN(frame="f", ids=[0,10,30], n=3)` { t.Fatalf("unexpected query(1): %s", query.String()) } default: diff --git a/fragment.go b/fragment.go index e70dab80c..82f2e50f2 100644 --- a/fragment.go +++ b/fragment.go @@ -476,6 +476,11 @@ func (f *Fragment) Top(opt TopOptions) ([]Pair, error) { // Retrieve pairs. If no bitmap ids specified then return from cache. pairs := f.topBitmapPairs(opt.BitmapIDs) + // If BitmapIDs are provided, we don't want to truncate the result set + if len(opt.BitmapIDs) > 0 { + opt.N = 0 + } + // Create a fast lookup of filter values. var filters map[interface{}]struct{} if opt.FilterField != "" && len(opt.FilterValues) > 0 { @@ -489,10 +494,10 @@ func (f *Fragment) Top(opt TopOptions) ([]Pair, error) { //results := make(PairHeap, 0, opt.N) results := &PairHeap{} for _, pair := range pairs { - bitmapID, n := pair.ID, pair.Count + bitmapID, cnt := pair.ID, pair.Count // Ignore empty bitmaps. - if n <= 0 { + if cnt <= 0 { continue } @@ -513,7 +518,7 @@ func (f *Fragment) Top(opt TopOptions) ([]Pair, error) { // The initial n pairs should simply be added to the results. if opt.N == 0 || results.Len() < opt.N { // Calculate count and append. - count := n + count := cnt if opt.Src != nil { count = opt.Src.IntersectionCount(f.Bitmap(bitmapID)) } @@ -530,6 +535,7 @@ func (f *Fragment) Top(opt TopOptions) ([]Pair, error) { break } } + continue } @@ -538,14 +544,13 @@ func (f *Fragment) Top(opt TopOptions) ([]Pair, error) { threshold := results.Pairs[0].Count // If the bitmap doesn't have enough bits set before the intersection - // then we can assume that any remaing bitmaps also have a count too low. - if n < threshold { + // then we can assume that any remaining bitmaps also have a count too low. + if cnt < threshold { break } // Calculate the intersecting bit count and skip if it's below our // last bitmap in our current result set. - count := opt.Src.IntersectionCount(f.Bitmap(bitmapID)) if count < threshold { continue @@ -569,6 +574,7 @@ func (f *Fragment) topBitmapPairs(bitmapIDs []uint64) []BitmapPair { f.mu.Lock() defer f.mu.Unlock() f.cache.Invalidate() + // TODO: make sure this is just returning a reference to the entire cache return f.cache.Top() }