diff --git a/executor.go b/executor.go index 57aadf3a5..08eae1d49 100644 --- a/executor.go +++ b/executor.go @@ -2763,7 +2763,10 @@ func newGroupByIterator(rowIDs []RowIDs, children []*pql.Call, filter *Row, inde for i, call := range children { fieldName, ok := call.Args["field"].(string) if !ok { - return nil, errors.Errorf("%s call must have 'field' argument", call.Name) + return nil, errors.Errorf("%s call must have 'field' argument with valid (string) field name. Got %v of type %[2]T", call.Name, call.Args["field"]) + } + if holder.Field(index, fieldName) == nil { + return nil, ErrFieldNotFound } gbi.fields[i].Field = fieldName // Fetch fragment. diff --git a/executor_test.go b/executor_test.go index 3924657da..73872abff 100644 --- a/executor_test.go +++ b/executor_test.go @@ -2672,10 +2672,9 @@ 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) { +func TestExecutor_Execute_Query_Error(t *testing.T) { c := test.MustRunCluster(t, 3) defer c.Close() c.CreateField(t, "i", pilosa.IndexOptions{}, "general") @@ -2688,6 +2687,30 @@ func TestExecutor_Execute_Rows_Error(t *testing.T) { query: "GroupBy(Rows())", error: "Rows call must have 'field' argument", }, + { + query: "GroupBy(Rows(field=true))", + error: "Rows call must have 'field' argument", + }, + { + query: "GroupBy(Rows(field=\"true\"))", + error: "field not found", + }, + { + query: "GroupBy(Rows(field=1))", + error: "Rows call must have 'field' argument", + }, + { + query: "GroupBy(Rows(field))", + error: "parse error", + }, + { + query: "GroupBy(Rows(field=general, limit=-1))", + error: "must be positive, but got", + }, + { + query: "GroupBy(Rows(field=general), limit=-1)", + error: "must be positive, but got", + }, } for i, test := range tests { diff --git a/pql/ast.go b/pql/ast.go index 32255b6a8..03b360099 100644 --- a/pql/ast.go +++ b/pql/ast.go @@ -291,6 +291,9 @@ func (c *Call) UintArg(key string) (uint64, bool, error) { } switch tval := val.(type) { case int64: + if tval < 0 { + return 0, true, fmt.Errorf("value for '%s' must be positive, but got %v", key, tval) + } return uint64(tval), true, nil case uint64: return tval, true, nil