fix issue where a shard with no data can cause query to fail

add Distinct test with integer data, and because one of the records
had a null value (and was in a shard by itself), it uncovered this
issue. I added a special error type if a view or fragment is not found
when so that we can match against it and ignore it when calculating
the results for a query.

I also added an implementation within executeCount to handle the
SignedRow case, but discovered that handlePrecalls always dumps the
negative data and that will be a bigger thing to fix

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

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) {

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 {

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 {