From 97eb94397f09064093cd458cec0cadbcbffaa3fe Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Fri, 21 Dec 2018 11:55:25 -0600 Subject: [PATCH] fix Rows bug where Pilosa would crash without 'field' argument. --- executor.go | 5 ++++- executor_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/executor.go b/executor.go index bbb8c2297..57aadf3a5 100644 --- a/executor.go +++ b/executor.go @@ -2761,7 +2761,10 @@ func newGroupByIterator(rowIDs []RowIDs, children []*pql.Call, filter *Row, inde ignorePrev := false for i, call := range children { - fieldName := call.Args["field"].(string) // this has already been validated by this point + fieldName, ok := call.Args["field"].(string) + if !ok { + return nil, errors.Errorf("%s call must have 'field' argument", call.Name) + } gbi.fields[i].Field = fieldName // Fetch fragment. frag := holder.fragment(index, fieldName, viewStandard, shard) diff --git a/executor_test.go b/executor_test.go index 141533d7f..3924657da 100644 --- a/executor_test.go +++ b/executor_test.go @@ -2672,6 +2672,39 @@ func TestExecutor_Execute_Rows(t *testing.T) { if !reflect.DeepEqual(rows, pilosa.RowIdentifiers{Rows: []uint64{11, 12}}) { t.Fatalf("unexpected rows: %+v", rows) } + +} + +func TestExecutor_Execute_Rows_Error(t *testing.T) { + c := test.MustRunCluster(t, 3) + defer c.Close() + c.CreateField(t, "i", pilosa.IndexOptions{}, "general") + + tests := []struct { + query string + error string + }{ + { + query: "GroupBy(Rows())", + error: "Rows call must have 'field' argument", + }, + } + + for i, test := range tests { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + r, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{ + Index: "i", + Query: test.query, + }) + if err == nil { + t.Fatalf("should have gotten an error on invalid rows query, but got %#v", r) + } + if !strings.Contains(err.Error(), test.error) { + t.Fatalf("unexpected error message: %s", err.Error()) + } + }) + } + } func TestExecutor_Execute_Rows_Keys(t *testing.T) {