diff --git a/docs/query-language.md b/docs/query-language.md index babcc390c..4b93b6311 100644 --- a/docs/query-language.md +++ b/docs/query-language.md @@ -781,3 +781,111 @@ Options(Row(f1=10), shards=[0, 2]) ```response {"attrs":{},"columns":[100, 2097152]} ``` + +**Spec:** + +``` +Rows(field=, previous=, limit=, column=) +``` + +**Description:** + +Rows returns a list of row IDs in the given field which have at least one bit +set. The field argument is mandatory, the others are optional. + +If `previous` is given, rows prior to and including the specified row ID or +key will not be returned. If `column` is given, only rows which have a set bit +in the given column will be returned. `previous` or `column` must be strings if +and only if the field or index respectively is using key translation. If `limit` +is given, the number of rowIDs returned will be less than or equal to +`limit`. The combination of `limit` and `previous` allows for paging over large +result sets. Results are always ordered, so setting `previous` as the last +result of the previous request will start from the next available row. + + +**Result Type:** Object with `"rows" or "keys" and an array of integers or strings respectively.` + +**Examples:** + +Without keys: +```request +Rows(field=blah) +``` +```response +{"rows":[1,9,39]} +``` + +With keys: +```request +Rows(field=blahk) +``` +```response +{"rows":null,"keys":["haha","zaaa","traa"]} +``` + + +**Spec:** + +``` +GroupBy(, [RowsCall...], limit=, filter=) +``` + +**Description:** + +GroupBy returns the count of the intersection of every combination of rows +taking one row each from the specified `Rows` calls. It returns only those +combinations for which the count is greater than 0. + +The optional `filter` argument takes any type of `Row` query (e.g. Row, Union, + Intersect, etc.) which will be intersected with each result prior to returning + the count. This is analagous to a WHERE clause applied to a relational GROUP BY + query. + +The optional `limit` argument limits the number of results returned. The results +are ordered, so as long as the data isn't changing, the same query will return +the same result set. + +Paging through results is supported by passing the `previous` argument to each +of the `Rows` calls in the GroupBy. Take the last result from your previous +`GroupBy` query, and pass each row ID in that result as the `previous` argument +to each of the respective `Rows` queries in your next `GroupBy` query. + +**Result Type:** Array of "groups". Each group is an object with a group key and +a count key. The count is an integer, and the group is an array of objects which +specify the field and row for each row that was intersected to get that result. + +**Examples:** + +A single `Rows` query. +```request +GroupBy(Rows(field=blah)) +``` +```response +[{"group":[{"field":"blah","rowID":1}],"count":1}, +{"group":[{"field":"blah","rowID":9}],"count":1}, +{"group":[{"field":"blah","rowID":39}],"count":1}] +``` + +With two `Rows` queries - one with IDs and one with keys. +```request +GroupBy(Rows(field=blah), Rows(field=blahk), limit=7) +``` +```response +[{"group":[{"field":"blah","rowID":1},{"field":"blahk","rowKey":"haha"}],"count":1}, + {"group":[{"field":"blah","rowID":1},{"field":"blahk","rowKey":"zaaa"}],"count":1}, + {"group":[{"field":"blah","rowID":1},{"field":"blahk","rowKey":"traa"}],"count":1}, + {"group":[{"field":"blah","rowID":9},{"field":"blahk","rowKey":"haha"}],"count":1}, + {"group":[{"field":"blah","rowID":9},{"field":"blahk","rowKey":"zaaa"}],"count":1}, + {"group":[{"field":"blah","rowID":9},{"field":"blahk","rowKey":"traa"}],"count":1}, + {"group":[{"field":"blah","rowID":39},{"field":"blahk","rowKey":"haha"}],"count":1}] +``` + +Getting the rest of the results from the previous example (paging). +```request +GroupBy(Rows(field=blah, previous=39), Rows(field=blahk, previous="haha"), limit=7) +``` + +```response +[{"group":[{"field":"blah","rowID":39},{"field":"blahk","rowKey":"zaaa"}],"count":1}, + {"group":[{"field":"blah","rowID":39},{"field":"blahk","rowKey":"traa"}],"count":1}] +``` diff --git a/executor.go b/executor.go index bbb8c2297..08eae1d49 100644 --- a/executor.go +++ b/executor.go @@ -2761,7 +2761,13 @@ 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 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. frag := holder.fragment(index, fieldName, viewStandard, shard) diff --git a/executor_test.go b/executor_test.go index 141533d7f..8ac7008ab 100644 --- a/executor_test.go +++ b/executor_test.go @@ -2674,6 +2674,66 @@ func TestExecutor_Execute_Rows(t *testing.T) { } } +func TestExecutor_Execute_Query_Error(t *testing.T) { + c := test.MustRunCluster(t, 1) + 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", + }, + { + 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", + }, + { + query: "GroupBy(Rows(field=general), filter=Rows(field=general))", + error: "unknown call: Rows", + }, + } + + 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) { c := test.MustRunCluster(t, 1) defer c.Close() 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