diff --git a/executor.go b/executor.go index 52b0a20b7..1668fa218 100644 --- a/executor.go +++ b/executor.go @@ -1516,7 +1516,10 @@ func executeDistinctShardBSI(ctx context.Context, qcx *Qcx, idx *Index, fieldNam existsBitmap, err := tx.OffsetRange(index, fieldName, view, shard, ShardWidth*shard, ShardWidth*0, ShardWidth*1) if err != nil { - return result, err + if _, ok := errors.Cause(err).(ViewOrFragmentNotFound); ok { + return result, nil + } + return result, errors.Wrap(err, "getting exists bitmap") } if filterBitmap != nil { existsBitmap = existsBitmap.Intersect(filterBitmap) @@ -1527,6 +1530,7 @@ func executeDistinctShardBSI(ctx context.Context, qcx *Qcx, idx *Index, fieldNam signBitmap, err := tx.OffsetRange(index, fieldName, view, shard, ShardWidth*shard, ShardWidth*1, ShardWidth*2) if err != nil { + // TODO wtf... if there's any error getting the sign bitmap we just return an empty result and move on? return result, nil } @@ -4242,12 +4246,20 @@ func (e *executor) executeCount(ctx context.Context, qcx *Qcx, index string, c * if child.Name == "Precomputed" { count := uint64(0) for _, irow := range child.Precomputed { - if row, ok := irow.(*Row); !ok { - return 0, errors.Errorf("unexpected precomputed value type inside count: %+v", irow) - } else { + switch row := irow.(type) { + case *Row: for _, seg := range row.segments { count += seg.n } + case SignedRow: + for _, seg := range row.Pos.segments { + count += seg.n + } + for _, seg := range row.Neg.segments { + count += seg.n + } + default: + return 0, errors.Errorf("unexpected precomputed value type inside count: %+v", row) } } return count, nil diff --git a/executor_test.go b/executor_test.go index cec51220f..dd9363bf7 100644 --- a/executor_test.go +++ b/executor_test.go @@ -6747,7 +6747,7 @@ func TestDistinctOnSetsKeyedIndex(t *testing.T) { c := test.MustRunCluster(t, 3) defer c.Close() - // create and populate "likenums" similar to "likes", but no keys on the field + // Create and populate "likenums" similar to "likes", but without keys on the field. c.CreateField(t, "users", pilosa.IndexOptions{Keys: true, TrackExistence: true}, "likenums") c.ImportIDKey(t, "users", "likenums", []test.KeyID{ {ID: 1, Key: "userA"}, @@ -6764,7 +6764,7 @@ func TestDistinctOnSetsKeyedIndex(t *testing.T) { {ID: 7, Key: "userF"}, }) - // create and populate "likes" field + // Create and populate "likes" field. c.CreateField(t, "users", pilosa.IndexOptions{Keys: true, TrackExistence: true}, "likes", pilosa.OptFieldKeys()) c.ImportKeyKey(t, "users", "likes", [][2]string{ {"molecula", "userA"}, @@ -6781,6 +6781,16 @@ func TestDistinctOnSetsKeyedIndex(t *testing.T) { {"icecream", "userF"}, }) + // Create and populate "affinity" int field with negative, positive, zero and null values. + c.CreateField(t, "users", pilosa.IndexOptions{Keys: true, TrackExistence: true}, "affinity", pilosa.OptFieldTypeInt(-1000, 1000)) + c.ImportIntKey(t, "users", "affinity", []test.IntKey{ + {Val: 10, Key: "userA"}, + {Val: -10, Key: "userB"}, + {Val: 5, Key: "userC"}, + {Val: -5, Key: "userD"}, + {Val: 0, Key: "userE"}, + }) + tests := []struct { query string verifier func(t *testing.T, resp pilosa.QueryResponse) @@ -6817,6 +6827,32 @@ func TestDistinctOnSetsKeyedIndex(t *testing.T) { } }, }, + { + query: "Distinct(field=affinity)", + verifier: func(t *testing.T, resp pilosa.QueryResponse) { + if !reflect.DeepEqual(resp.Results[0].(pilosa.SignedRow).Pos.Columns(), []uint64{0, 5, 10}) { + t.Errorf("wrong positive records: %+v", resp.Results[0].(pilosa.SignedRow).Pos.Columns()) + } + if !reflect.DeepEqual(resp.Results[0].(pilosa.SignedRow).Neg.Columns(), []uint64{5, 10}) { + t.Errorf("wrong negative records: %+v", resp.Results[0].(pilosa.SignedRow).Neg.Columns()) + } + }, + }, + // handling this case properly will require changing the way + // that precomputed data is stored on Call objects. Currently + // if a Distinct is at all nested (e.g. within a Count) it + // gets handled by executor.handlePreCalls which assumes that + // only the positive values are worthwhile. + // { + // query: "Count(Distinct(field=affinity))", + // verifier: func(t *testing.T, resp pilosa.QueryResponse) { + // if resp.Results[0].(uint64) != 5 { + // t.Errorf("wrong number of values: %+v", resp.Results[0]) + // } + // }, + // }, + + // this case doesn't work due to the missing index issue // { // query: "Distinct(field=likes)", // verifier: func(t *testing.T, resp pilosa.QueryResponse) { diff --git a/rrtx.go b/rrtx.go index 0cfb5fba9..0b4d7d1e7 100644 --- a/rrtx.go +++ b/rrtx.go @@ -30,6 +30,7 @@ import ( rbfcfg "github.com/pilosa/pilosa/v2/rbf/cfg" "github.com/pilosa/pilosa/v2/roaring" txkey "github.com/pilosa/pilosa/v2/short_txkey" + //txkey "github.com/pilosa/pilosa/v2/txkey" "github.com/pkg/errors" ) @@ -372,13 +373,13 @@ func (tx *RoaringTx) getFragment(index, field, view string, shard uint64) (*frag v := f.view(view) if v == nil { - return nil, errors.Errorf("view not found: %q", view) + return nil, ViewOrFragmentNotFound(errors.Errorf("view not found: %q", view)) } frag := v.Fragment(shard) if frag == nil { - return nil, fmt.Errorf("fragment not found: %q / %q / %d", field, view, shard) + return nil, ViewOrFragmentNotFound(errors.Errorf("fragment not found: %q / %q / %d", field, view, shard)) } // Note: we cannot cache frag into tx.fragment. @@ -388,6 +389,8 @@ func (tx *RoaringTx) getFragment(index, field, view string, shard uint64) (*frag return frag, nil } +type ViewOrFragmentNotFound error + func (tx *RoaringTx) bitmap(index, field, view string, shard uint64) (*roaring.Bitmap, error) { frag, err := tx.getFragment(index, field, view, shard) if err != nil { diff --git a/test/cluster.go b/test/cluster.go index 0a5d2c451..74b1e31ff 100644 --- a/test/cluster.go +++ b/test/cluster.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "io/ioutil" + "math" "path" "strconv" "strings" @@ -128,6 +129,31 @@ func (c *Cluster) ImportKeyKey(t testing.TB, index, field string, valAndRecKeys } } +// IntKey is a string key and a signed integer value. +type IntKey struct { + Val int64 + Key string +} + +// ImportIntKey imports int data into an index which uses string keys. +func (c *Cluster) ImportIntKey(t testing.TB, index, field string, pairs []IntKey) { + t.Helper() + importRequest := &pilosa.ImportValueRequest{ + Index: index, + Field: field, + Shard: math.MaxUint64, + ColumnKeys: make([]string, len(pairs)), + Values: make([]int64, len(pairs)), + } + for i, pair := range pairs { + importRequest.Values[i] = pair.Val + importRequest.ColumnKeys[i] = pair.Key + } + if err := c.Nodes[0].API.ImportValue(context.Background(), nil, importRequest); err != nil { + t.Fatalf("importing IntKey data: %v", err) + } +} + // KeyID represents a key and an ID for importing data into an index // and field where one uses string keys and the other does not. type KeyID struct {