From f921c5ded076685399b587ff1bb93e466f589962 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kuba=20Podg=C3=B3rski?= <kuba--@users.noreply.github.com>
Date: Tue, 30 Jun 2020 21:07:54 +0200
Subject: [PATCH 1/2] Add test for Rows on bool

---
 executor.go               | 27 +++++++++----------
 executor_internal_test.go | 56 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 14 deletions(-)

diff --git a/executor.go b/executor.go
index 57bc2abca..d89823b56 100644
--- a/executor.go
+++ b/executor.go
@@ -724,7 +724,6 @@ func (e *executor) executeFieldValueCall(ctx context.Context, index string, c *p
 	} else {
 		id, ok, err := c.UintArg("column")
 		if !ok || err != nil {
-			// TODO: this error is getting swallowed somewhere (via curl)
 			return ValCount{}, errors.Wrap(err, "getting column argument")
 		}
 		colID = id
@@ -4041,20 +4040,20 @@ func (e *executor) translateCall(ctx context.Context, indexName string, c *pql.C
 		// are only two possible values. Instead, they are handled
 		// directly.
 		if field.Type() == FieldTypeBool {
-			// TODO: This code block doesn't make sense for a `Rows()`
-			// queries on a `bool` field. Need to review this better,
-			// include it in tests, and probably back-port it to Pilosa.
-			if c.Name != "Rows" {
-				boolVal, err := callArgBool(c, rowKey)
-				if err != nil {
-					return errors.Wrap(err, "getting bool key")
-				}
-				rowID := falseRowID
-				if boolVal {
-					rowID = trueRowID
-				}
-				c.Args[rowKey] = rowID
+			if c.Name == "Rows" {
+				// TranslateInfo for Rows returns "previous" as rowKey,
+				// so for bool fields we would get "missing bool argument" error
+				return nil
 			}
+			boolVal, err := callArgBool(c, rowKey)
+			if err != nil {
+				return errors.Wrapf(err, "getting bool key (%+v)", rowKey)
+			}
+			rowID := falseRowID
+			if boolVal {
+				rowID = trueRowID
+			}
+			c.Args[rowKey] = rowID
 		} else if field.Keys() {
 			foreignIndexName := field.ForeignIndex()
 			if c.Args[rowKey] != nil && isCondition(c.Args[rowKey]) {
diff --git a/executor_internal_test.go b/executor_internal_test.go
index 86ff6cdde..20c4f19c4 100644
--- a/executor_internal_test.go
+++ b/executor_internal_test.go
@@ -133,6 +133,62 @@ func TestExecutor_TranslateGroupByCall(t *testing.T) {
 	}
 }
 
+func TestExecutor_TranslateRowsOnBool(t *testing.T) {
+	holder := NewHolder(DefaultPartitionN)
+	defer holder.Close()
+
+	e := &executor{
+		Holder:  holder,
+		Cluster: NewTestCluster(1),
+	}
+	e.Holder.Path, _ = ioutil.TempDir(*TempDir, "")
+	err := e.Holder.Open()
+	if err != nil {
+		t.Fatalf("opening holder: %v", err)
+	}
+
+	idx, err := e.Holder.CreateIndex("i", IndexOptions{})
+	if err != nil {
+		t.Fatalf("creating index: %v", err)
+	}
+
+	fb, errb := idx.CreateField("b", OptFieldTypeBool())
+	_, errbk := idx.CreateField("bk", OptFieldTypeBool(), OptFieldKeys())
+	if errb != nil || errbk != nil {
+		t.Fatalf("creating fields %v, %v", errb, errbk)
+	}
+
+	_, err1 := fb.SetBit(1, 1, nil)
+	_, err2 := fb.SetBit(2, 2, nil)
+	_, err3 := fb.SetBit(3, 3, nil)
+	if err1 != nil || err2 != nil || err3 != nil {
+		t.Fatalf("seeting bit %v, %v, %v", err1, err2, err3)
+	}
+
+	tests := []struct {
+		pql string
+	}{
+		{pql: "Rows(b)"},
+		{pql: "GroupBy(Rows(b))"},
+		{pql: "Set(4, b=true)"},
+	}
+
+	for _, test := range tests {
+		t.Run(test.pql, func(t *testing.T) {
+			query, err := pql.ParseString(test.pql)
+			if err != nil {
+				t.Fatalf("parsing query: %v", err)
+			}
+
+			c := query.Calls[0]
+			err = e.translateCall(context.Background(), "i", c, make(map[string]map[string]uint64))
+			if err != nil {
+				t.Fatalf("translating call: %v", err)
+			}
+		})
+	}
+}
+
 func isInt(a interface{}) bool {
 	switch a.(type) {
 	case int, int64, uint, uint64:

From 58964947d48d1c7fe4fe9c38bce3e2104e21984f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kuba=20Podg=C3=B3rski?= <kuba--@users.noreply.github.com>
Date: Wed, 1 Jul 2020 10:17:30 +0200
Subject: [PATCH 2/2] Update executor_internal_test.go

Co-authored-by: Travis Turner <travis@pilosa.com>
---
 executor_internal_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/executor_internal_test.go b/executor_internal_test.go
index 20c4f19c4..68b0f1f4e 100644
--- a/executor_internal_test.go
+++ b/executor_internal_test.go
@@ -162,7 +162,7 @@ func TestExecutor_TranslateRowsOnBool(t *testing.T) {
 	_, err2 := fb.SetBit(2, 2, nil)
 	_, err3 := fb.SetBit(3, 3, nil)
 	if err1 != nil || err2 != nil || err3 != nil {
-		t.Fatalf("seeting bit %v, %v, %v", err1, err2, err3)
+		t.Fatalf("setting bit %v, %v, %v", err1, err2, err3)
 	}
 
 	tests := []struct {