From 38f459a6d431c219c6829df33f234a1e4a2f75f7 Mon Sep 17 00:00:00 2001
From: Matt Jaffee <matthew.jaffee@gmail.com>
Date: Fri, 12 Oct 2018 18:58:27 -0500
Subject: [PATCH] add GroupBy(Rows(column)) test and fix comments

---
 executor.go      |  7 ++++---
 executor_test.go | 13 +++++++++++--
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/executor.go b/executor.go
index 8bb413f09..40589f6e2 100644
--- a/executor.go
+++ b/executor.go
@@ -839,9 +839,10 @@ func (e *executor) executeGroupBy(ctx context.Context, index string, c *pql.Call
 		limit = int(lim)
 	}
 
-	// perform Rows queries - TODO, call async? run per shard in
-	// executeGroupByShard? (note: can only do this for Rows queries which do
-	// not include "column" arg)
+	// perform necessary Rows queries (any that have limit or columns args) -
+	// TODO, call async? would only help if multiple Rows queries had a column
+	// or limit arg.
+	// TODO support TopN in here would be really cool - and pretty easy I think.
 	childRows := make([]RowIDs, len(c.Children))
 	for i, child := range c.Children {
 		if child.Name != "Rows" {
diff --git a/executor_test.go b/executor_test.go
index 6b903acc2..a5e7b984a 100644
--- a/executor_test.go
+++ b/executor_test.go
@@ -2518,6 +2518,17 @@ func TestExecutor_Execute_GroupBy(t *testing.T) {
 			checkGroupBy(t, expected, results)
 		})
 
+		t.Run("distinct rows in different shards with column arg", func(t *testing.T) {
+			results := c.Query(t, "i", fmt.Sprintf(`GroupBy(Rows(field=ma), Rows(field=mb, column=%d), limit=5)`, ShardWidth)).Results[0].([]pilosa.GroupCount)
+			expected := []pilosa.GroupCount{
+				{Group: []pilosa.FieldRow{{Field: "ma", RowID: 1}, {Field: "mb", RowID: 1}}, Count: 1},
+				{Group: []pilosa.FieldRow{{Field: "ma", RowID: 1}, {Field: "mb", RowID: 3}}, Count: 1},
+				{Group: []pilosa.FieldRow{{Field: "ma", RowID: 3}, {Field: "mb", RowID: 1}}, Count: 1},
+				{Group: []pilosa.FieldRow{{Field: "ma", RowID: 3}, {Field: "mb", RowID: 3}}, Count: 1},
+			}
+			checkGroupBy(t, expected, results)
+		})
+
 		c.CreateField(t, "i", pilosa.IndexOptions{}, "na")
 		c.CreateField(t, "i", pilosa.IndexOptions{}, "nb")
 		c.ImportBits(t, "i", "na", [][2]uint64{
@@ -2544,8 +2555,6 @@ func TestExecutor_Execute_GroupBy(t *testing.T) {
 
 		})
 
-		// TODO test column queries to row call (also with multiple shards)
-
 		// test paging over results using previous. set the same bits in three
 		// fields
 		c.CreateField(t, "i", pilosa.IndexOptions{}, "ppa")