From 964f14a3a1a260ab1aacfc41b77a862e60b608ca Mon Sep 17 00:00:00 2001
From: reesporte <45641995+reesporte@users.noreply.github.com>
Date: Thu, 26 May 2022 16:45:10 -0500
Subject: [PATCH] If the field doesn't exist return ErrFieldNotFound (#2081)

This way we don't return seemingly valid data for calls on non-existent fields. See
also [FB-237](https://molecula.atlassian.net/browse/FB-237).
---
 executor.go      | 10 +++++-----
 executor_test.go | 34 +++++++++++++++++++++++++++++++++-
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/executor.go b/executor.go
index a92aaf86d..d21cfbb5b 100644
--- a/executor.go
+++ b/executor.go
@@ -1861,7 +1861,7 @@ func (e *executor) executeSumCountShard(ctx context.Context, qcx *Qcx, index str
 
 	field := e.Holder.Field(index, fieldName)
 	if field == nil {
-		return ValCount{}, nil
+		return ValCount{}, ErrFieldNotFound
 	}
 
 	bsig := field.bsiGroup(fieldName)
@@ -1919,7 +1919,7 @@ func (e *executor) executeMinShard(ctx context.Context, qcx *Qcx, index string,
 
 	field := e.Holder.Field(index, fieldName)
 	if field == nil {
-		return ValCount{}, nil
+		return ValCount{}, ErrFieldNotFound
 	}
 
 	tx, finisher, err := qcx.GetTx(Txo{Write: !writable, Index: idx, Shard: shard})
@@ -1951,7 +1951,7 @@ func (e *executor) executeMaxShard(ctx context.Context, qcx *Qcx, index string,
 
 	field := e.Holder.Field(index, fieldName)
 	if field == nil {
-		return ValCount{}, nil
+		return ValCount{}, ErrFieldNotFound
 	}
 
 	tx, finisher, err := qcx.GetTx(Txo{Write: !writable, Index: idx, Shard: shard})
@@ -1978,7 +1978,7 @@ func (e *executor) executeMinRowShard(ctx context.Context, qcx *Qcx, index strin
 	fieldName, _ := c.Args["field"].(string)
 	field := e.Holder.Field(index, fieldName)
 	if field == nil {
-		return PairField{}, nil
+		return PairField{}, ErrFieldNotFound
 	}
 
 	fragment := e.Holder.fragment(index, fieldName, viewStandard, shard)
@@ -2023,7 +2023,7 @@ func (e *executor) executeMaxRowShard(ctx context.Context, qcx *Qcx, index strin
 	fieldName, _ := c.Args["field"].(string)
 	field := e.Holder.Field(index, fieldName)
 	if field == nil {
-		return PairField{}, nil
+		return PairField{}, ErrFieldNotFound
 	}
 
 	fragment := e.Holder.fragment(index, fieldName, viewStandard, shard)
diff --git a/executor_test.go b/executor_test.go
index a64382728..51214aa3e 100644
--- a/executor_test.go
+++ b/executor_test.go
@@ -2664,7 +2664,12 @@ func TestExecutor_Execute_MinMaxRow(t *testing.T) {
 				t.Fatalf("unexpected result %v != %v", target, result.Results[0])
 			}
 		})
-
+		t.Run("MinRowNonExistent", func(t *testing.T) {
+			_, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: "MinRow(field=fake)"})
+			if got, exp := err.Error(), "executing: executeMinRow: mapping on primary node: field not found"; got != exp {
+				t.Fatalf("expected %v, got %v", exp, got)
+			}
+		})
 		t.Run("MaxRow", func(t *testing.T) {
 			result, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: "MaxRow(field=f)"})
 			if err != nil {
@@ -2678,6 +2683,12 @@ func TestExecutor_Execute_MinMaxRow(t *testing.T) {
 				t.Fatalf("unexpected result %v != %v", target, result.Results[0])
 			}
 		})
+		t.Run("MaxRowNonExistent", func(t *testing.T) {
+			_, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: "MaxRow(field=fake)"})
+			if got, exp := err.Error(), "executing: executeMaxRow: mapping on primary node: field not found"; got != exp {
+				t.Fatalf("expected %v, got %v", exp, got)
+			}
+		})
 	})
 
 	t.Run("RowKey", func(t *testing.T) {
@@ -2827,6 +2838,13 @@ func TestExecutor_Execute_Sum(t *testing.T) {
 			})
 		})
 
+		t.Run("SumNonExistent", func(t *testing.T) {
+			_, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Sum(field=fake)`})
+			if err.Error() != "executing: executeSum: mapping on primary node: field not found" {
+				t.Fatal(err)
+			}
+		})
+
 		t.Run("Decimal", func(t *testing.T) {
 			t.Run("NoFilter", func(t *testing.T) {
 				if result, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Sum(field=dec)`}); err != nil {
@@ -6627,6 +6645,20 @@ func TestExecutor_Execute_MinMaxCountEqual(t *testing.T) {
 			}
 		}
 	})
+	t.Run("MinNonExistent", func(t *testing.T) {
+		pql := `Min(field=fake)`
+		_, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: pql})
+		if err.Error() != "executing: executeMin: mapping on primary node: field not found" {
+			t.Fatal(err)
+		}
+	})
+	t.Run("MaxNonExistent", func(t *testing.T) {
+		pql := `Max(field=fake)`
+		_, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: pql})
+		if err.Error() != "executing: executeMax: mapping on primary node: field not found" {
+			t.Fatal(err)
+		}
+	})
 	t.Run("MinDec", func(t *testing.T) {
 		tests := []struct {
 			filter string