From 6ad39a376e4e0e62a85809140b5736c3d1c51d09 Mon Sep 17 00:00:00 2001 From: Seebs Date: Wed, 20 Nov 2019 14:57:15 -0600 Subject: [PATCH 1/5] handle precalls and cross-index queries better There's two actual changes here, but they're closely related. First, handle named parameters for precalls, not just indexed parameters. Second, when doing translation for a call, check whether it specifies an index, and if it does, use that index instead of the current index for the translation. --- executor.go | 38 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/executor.go b/executor.go index e91ec5f30..31caaf7e7 100644 --- a/executor.go +++ b/executor.go @@ -366,6 +366,14 @@ func (e *executor) handlePreCallChildren(ctx context.Context, index string, c *p return err } } + for _, val := range c.Args { + // Handle Call() operations which exist inside named arguments, too. + if call, ok := val.(*pql.Call); ok { + if err := e.handlePreCalls(ctx, index, call, shards, opt); err != nil { + return err + } + } + } return nil } @@ -3095,7 +3103,20 @@ func (e *executor) translateCalls(ctx context.Context, index string, idx *Index, defer span.Finish() for i := range calls { - if err := e.translateCall(index, idx, calls[i]); err != nil { + // Possibly change to another index for translation, if this + // call crosses index boundaries. + newIdxName := calls[i].CallIndex() + var newIdx *Index + if newIdxName == "" || newIdxName == index { + newIdxName = index + newIdx = idx + } else { + newIdx = idx.holder.indexes[newIdxName] + if newIdx == nil { + return fmt.Errorf("unknown index %q specified in cross-index call", newIdxName) + } + } + if err := e.translateCall(newIdxName, newIdx, calls[i]); err != nil { return err } } @@ -3195,7 +3216,20 @@ func (e *executor) translateCall(index string, idx *Index, c *pql.Call) error { // Translate child calls. for _, child := range c.Children { - if err := e.translateCall(index, idx, child); err != nil { + // Possibly change to another index for translation, if this + // call crosses index boundaries. + newIdxName := child.CallIndex() + var newIdx *Index + if newIdxName == "" || newIdxName == index { + newIdxName = index + newIdx = idx + } else { + newIdx = idx.holder.indexes[newIdxName] + if newIdx == nil { + return fmt.Errorf("unknown index %q specified in cross-index call", newIdxName) + } + } + if err := e.translateCall(newIdxName, newIdx, child); err != nil { return err } } From 26326ac74c47bd7f78f586c3844d83a93060983d Mon Sep 17 00:00:00 2001 From: Seebs Date: Wed, 20 Nov 2019 16:10:51 -0600 Subject: [PATCH 2/5] recompute shards for cross-index queries When computing results on another index, recompute list of shards for that index. --- executor.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/executor.go b/executor.go index 31caaf7e7..0e70de6f0 100644 --- a/executor.go +++ b/executor.go @@ -427,7 +427,7 @@ func (e *executor) execute(ctx context.Context, index string, q *pql.Query, shar // already precomputed by handlePreCallChildren, though, // we don't need this logic in executeCall. if newIndex := call.CallIndex(); newIndex != "" { - v, err = e.executeCall(ctx, newIndex, call, shards, opt) + v, err = e.executeCall(ctx, newIndex, call, nil, opt) } else { v, err = e.executeCall(ctx, index, call, shards, opt) } @@ -458,6 +458,20 @@ func (e *executor) executeCall(ctx context.Context, index string, c *pql.Call, s e.Holder.Logger.Printf("DEPRECATED: Range() is deprecated, please use Row() instead.") } + // If shards are specified, then use that value for shards. If shards aren't + // specified, then include all of them. + if shards == nil && needsShards([]*pql.Call{c}) { + // Round up the number of shards. + idx := e.Holder.Index(index) + if idx == nil { + return nil, ErrIndexNotFound + } + shards = idx.AvailableShards().Slice() + if len(shards) == 0 { + shards = []uint64{0} + } + } + // Special handling for mutation and top-n calls. if op, ok := e.additionalCountOps[c.Name]; ok { e.Holder.Stats.CountWithCustomTags(c.Name, 1, 1.0, []string{indexTag}) From 397d93e84ba220454f4d16dd62dc3b9c3b4659ce Mon Sep 17 00:00:00 2001 From: Seebs Date: Wed, 20 Nov 2019 17:01:22 -0600 Subject: [PATCH 3/5] provide an empty filter when a filter was empty --- executor.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/executor.go b/executor.go index 0e70de6f0..3751a018c 100644 --- a/executor.go +++ b/executor.go @@ -1015,6 +1015,8 @@ func (e *executor) executeGenericFieldShard(ctx context.Context, index string, c filter = row if filter != nil && len(filter.segments) > 0 { filterBitmap = filter.segments[0].data + } else { + filterBitmap = roaring.NewFileBitmap() } } From 36ef82d7acb095e0eb8bd44de83fd97aa739ed7d Mon Sep 17 00:00:00 2001 From: Seebs Date: Fri, 22 Nov 2019 13:52:05 -0600 Subject: [PATCH 4/5] Zero bitmap storage when reusing it for container-as-bitmap If you don't do this, it works fine the first time you use a given storage, but after that you start seeing spurious bits. --- roaring/container_stash.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/roaring/container_stash.go b/roaring/container_stash.go index 8aab2f4d3..89c63ac8f 100644 --- a/roaring/container_stash.go +++ b/roaring/container_stash.go @@ -428,6 +428,9 @@ func (c *Container) AsBitmap(target []uint64) (out []uint64) { out = make([]uint64, 1024) } else { out = target + for i := range out { + out[i] = 0 + } } if c.typeID == containerArray { a := c.array() From d79ecbad86fcedf04bf3c09caab766d4f3dd4312 Mon Sep 17 00:00:00 2001 From: Seebs Date: Fri, 22 Nov 2019 14:18:36 -0600 Subject: [PATCH 5/5] recompute shards for cross-index calls It turns out that we need to recompute the set of shards whenever a query is cross-index. Otherwise we get partial results in unexpected ways sometimes. --- executor.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/executor.go b/executor.go index 3751a018c..8d31c739b 100644 --- a/executor.go +++ b/executor.go @@ -317,6 +317,8 @@ func (e *executor) handlePreCalls(ctx context.Context, index string, c *pql.Call if newIndex != "" && newIndex != index { c.Type = pql.PrecallGlobal index = newIndex + // we need to recompute shards, then + shards = nil } if c.Type == pql.PrecallNone { // otherwise, handle the children