Merge pull request #1970 from jaffee/1967-groupby-filter-strings

Fix filter calls in GroupBy not being translated

api.go

@@ -897,11 +897,14 @@ func (api *API) Import(ctx context.Context, req *ImportRequest, opts ...ImportOp
 				if _, ok := m[shard]; !ok {
 					m[shard] = make([]Bit, 0)
 				}
-				m[shard] = append(m[shard], Bit{
-					RowID:     req.RowIDs[i],
-					ColumnID:  colID,
-					Timestamp: req.Timestamps[i],
-				})
+				bit := Bit{
+					RowID:    req.RowIDs[i],
+					ColumnID: colID,
+				}
+				if len(req.Timestamps) > 0 {
+					bit.Timestamp = req.Timestamps[i]
+				}
+				m[shard] = append(m[shard], bit)
 			}
 
 			// Signal to the receiving nodes to ignore checking for key translation.

executor.go

@@ -2536,6 +2536,16 @@ func (e *executor) translateGroupByCall(index string, idx *Index, c *pql.Call) e
 		}
 	}
 
+	if filter, ok, err := c.CallArg("filter"); ok {
+		if err != nil {
+			return errors.Wrap(err, "getting filter call")
+		}
+		err = e.translateCall(index, idx, filter)
+		if err != nil {
+			return errors.Wrap(err, "translating filter call")
+		}
+	}
+
 	prev, ok := c.Args["previous"]
 	if !ok {
 		return nil // nothing else to be translated

executor_test.go

@@ -3243,7 +3243,56 @@ func TestExecutor_Execute_Query_Error(t *testing.T) {
 			}
 		})
 	}
+}
 
+func TestExecutor_GroupByStrings(t *testing.T) {
+	c := test.MustRunCluster(t, 1)
+	defer c.Close()
+	c.CreateField(t, "istring", pilosa.IndexOptions{Keys: true}, "generals", pilosa.OptFieldKeys())
+
+	req := &pilosa.ImportRequest{
+		Index:      "istring",
+		Field:      "generals",
+		Shard:      0,
+		RowKeys:    []string{"r1", "r2", "r1", "r2", "r1", "r2", "r1", "r2", "r1", "r2"},
+		ColumnKeys: []string{"c1", "c2", "c3", "c4", "c5", "c6", "c7", "c8", "c9", "c10"},
+	}
+	if err := c[0].API.Import(context.Background(), req); err != nil {
+		t.Fatalf("importing: %v", err)
+	}
+
+	tests := []struct {
+		query    string
+		expected []pilosa.GroupCount
+	}{
+		{
+			query: "GroupBy(Rows(generals))",
+			expected: []pilosa.GroupCount{
+				{Group: []pilosa.FieldRow{{Field: "generals", RowID: 1, RowKey: "r1"}}, Count: 5},
+				{Group: []pilosa.FieldRow{{Field: "generals", RowID: 2, RowKey: "r2"}}, Count: 5},
+			},
+		},
+		{
+			query: "GroupBy(Rows(generals), filter=Row(generals=r2))",
+			expected: []pilosa.GroupCount{
+				{Group: []pilosa.FieldRow{{Field: "generals", RowID: 2, RowKey: "r2"}}, Count: 5},
+			},
+		},
+	}
+
+	for i, tst := range tests {
+		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+			r, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{
+				Index: "istring",
+				Query: tst.query,
+			})
+			if err != nil {
+				t.Fatalf("got an error %v", err)
+			}
+			results := r.Results[0].([]pilosa.GroupCount)
+			test.CheckGroupBy(t, tst.expected, results)
+		})
+	}
 }
 
 func TestExecutor_Execute_Rows_Keys(t *testing.T) {

go.sum

@@ -109,6 +109,8 @@ github.com/uber/jaeger-client-go v2.16.0+incompatible h1:Q2Pp6v3QYiocMxomCaJuwQG
 github.com/uber/jaeger-client-go v2.16.0+incompatible/go.mod h1:WVhlPFC8FDjOFMMWRy2pZqQJSXxYSwNYOkTr/Z6d3Kk=
 github.com/uber/jaeger-lib v1.5.0 h1:OHbgr8l656Ub3Fw5k9SWnBfIEwvoHQ+W2y+Aa9D1Uyo=
 github.com/uber/jaeger-lib v1.5.0/go.mod h1:ComeNDZlWwrWnDv8aPp0Ba6+uUTzImX/AauajbLI56U=
+github.com/uber/jaeger-lib v2.0.0+incompatible h1:iMSCV0rmXEogjNWPh2D0xk9YVKvrtGoHJNe9ebLu/pw=
+github.com/uber/jaeger-lib v2.0.0+incompatible/go.mod h1:ComeNDZlWwrWnDv8aPp0Ba6+uUTzImX/AauajbLI56U=
 github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0=
 github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q=
 go.uber.org/atomic v1.3.2 h1:2Oa65PReHzfn29GpvgsYwloV9AVFHPDk8tYxt2c2tr4=