From a495b6c227cb72e1c88a70a984617ebb4e0e2b8b Mon Sep 17 00:00:00 2001 From: Seebs Date: Wed, 11 Mar 2020 11:03:24 -0500 Subject: [PATCH 1/2] make Distinct work across nodes, probably Problem: A top-level bare "Distinct" call returns results only for shards on the current node. Analysis: We don't actually want to limit Distinct calls to "available" shards at all. We just want to run them on everything. But we already did that in generating the precomputed results; all we need to do is, if we get a non-shard-specific request for precomputed values, just return all the values. It's pretty hard to create logic for this using our fancy mapReduce, but also we could just... not do that. --- executor.go | 33 +++++---------------------------- 1 file changed, 5 insertions(+), 28 deletions(-) diff --git a/executor.go b/executor.go index 482a10e6e..36b1a6cda 100644 --- a/executor.go +++ b/executor.go @@ -1006,37 +1006,14 @@ func (e *executor) executeMaxRow(ctx context.Context, index string, c *pql.Call, // executePrecomputedCall pretends to execute a call that we have a precomputed value for. func (e *executor) executePrecomputedCall(ctx context.Context, index string, c *pql.Call, shards []uint64, opt *execOptions) (*Row, error) { - span, ctx := tracing.StartSpanFromContext(ctx, "Executor.executePrecomputedCall") + span, _ := tracing.StartSpanFromContext(ctx, "Executor.executePrecomputedCall") defer span.Finish() + result := NewRow() - // Execute calls in bulk on each remote node and merge. - mapFn := func(shard uint64) (interface{}, error) { - if c.Precomputed != nil { - return c.Precomputed[shard], nil - } - // This might not be an error -- if there were no values, we will not have created - // the corresponding row. - return NewRow(), nil + for _, row := range c.Precomputed { + result.Merge(row.(*Row)) } - - // Merge returned results at coordinating node. - reduceFn := func(prev, v interface{}) interface{} { - other, _ := prev.(*Row) - if other == nil { - other = NewRow() - } - other.Merge(v.(*Row)) - return other - } - - other, err := e.mapReduce(ctx, index, shards, c, opt, mapFn, reduceFn) - if err != nil { - return nil, errors.Wrap(err, "map reduce") - } - - row, _ := other.(*Row) - - return row, nil + return result, nil } // executeBitmapCall executes a call that returns a bitmap. From 0374bda45f917fed3c01facf3cfd9a96c8771a61 Mon Sep 17 00:00:00 2001 From: Travis Date: Mon, 30 Mar 2020 11:58:55 -0500 Subject: [PATCH 2/2] Adjust bare-distinct logic. If an index is provided to a bare distinct which happens to be the index handling the query, then the query needs to behave as if no index argument was provided. For example: When querying against index `i`, ``` Distinct(index="i", field="ints")` ``` should behave exactly like ``` Distinct(field="ints") ``` --- executor.go | 2 +- executor_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/executor.go b/executor.go index 36b1a6cda..51c308aae 100644 --- a/executor.go +++ b/executor.go @@ -438,7 +438,7 @@ func (e *executor) execute(ctx context.Context, index string, q *pql.Query, shar // still need to handle them. Since everything else was // already precomputed by handlePreCallChildren, though, // we don't need this logic in executeCall. - if newIndex := call.CallIndex(); newIndex != "" { + if newIndex := call.CallIndex(); newIndex != "" && newIndex != index { v, err = e.executeCall(ctx, newIndex, call, nil, opt) } else { v, err = e.executeCall(ctx, index, call, shards, opt) diff --git a/executor_test.go b/executor_test.go index 3e81bd156..9af5a478f 100644 --- a/executor_test.go +++ b/executor_test.go @@ -5082,3 +5082,32 @@ func TestExecutor_Execute_CountDistinct(t *testing.T) { } }) } + +// Ensure that a top-level, bare distinct on multiple nodes +// is handled correctly. +func TestExecutor_BareDistinct(t *testing.T) { + t.Helper() + c := test.MustRunCluster(t, 2) + defer c.Close() + + c.CreateField(t, "i", pilosa.IndexOptions{}, "ints", + pilosa.OptFieldTypeInt(0, math.MaxInt64), + ) + + // Populate integer data. + c.Query(t, "i", fmt.Sprintf(` + Set(0, ints=1) + Set(%d, ints=2) + `, ShardWidth)) + + for _, pql := range []string{ + `Distinct(field="ints")`, + `Distinct(index="i", field="ints")`, + } { + exp := []uint64{1, 2} + res := c.Query(t, "i", pql).Results[0].(pilosa.SignedRow) + if got := res.Pos.Columns(); !reflect.DeepEqual(exp, got) { + t.Fatalf("expected: %v, but got: %v", exp, got) + } + } +}