Skip to content

Commit

Permalink
refactor(query): update transpiler to reflect signature change to `gr…
Browse files Browse the repository at this point in the history
…oup()` (#1689)

update other modules that used old syntax as well
  • Loading branch information
Christopher M. Wolff authored Dec 4, 2018
1 parent 62b840b commit f1d21b8
Show file tree
Hide file tree
Showing 31 changed files with 90 additions and 49 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ require (
github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d // indirect
github.com/imdario/mergo v0.3.6 // indirect
github.com/inconshreveable/mousetrap v1.0.0 // indirect
github.com/influxdata/flux v0.7.1
github.com/influxdata/flux v0.7.2-0.20181204152616-3d067c85d2ca
github.com/influxdata/influxql v0.0.0-20180925231337-1cbfca8e56b6
github.com/influxdata/usage-client v0.0.0-20160829180054-6d3895376368
github.com/jefferai/jsonx v0.0.0-20160721235117-9cc31c3135ee // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,8 @@ github.com/imdario/mergo v0.3.6 h1:xTNEAn+kxVO7dTZGu0CegyqKZmoWFI0rF8UxjlB2d28=
github.com/imdario/mergo v0.3.6/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM=
github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
github.com/influxdata/flux v0.7.1 h1:eEQ/AfF8ToqXpNAg2ktS+YfRmOA+ilEMywbH8bqmmKM=
github.com/influxdata/flux v0.7.1/go.mod h1:MIjvKpiQLRad9/ilY4jYwpIpMAhiOycJfK9YctCUGUM=
github.com/influxdata/flux v0.7.2-0.20181204152616-3d067c85d2ca h1:4h4v5CFdoCvqp1KEPOL2yXc86vCUzT6DCg9ibnEaNuY=
github.com/influxdata/flux v0.7.2-0.20181204152616-3d067c85d2ca/go.mod h1:MIjvKpiQLRad9/ilY4jYwpIpMAhiOycJfK9YctCUGUM=
github.com/influxdata/goreleaser v0.86.2-0.20181010170531-0fd209ba67f5/go.mod h1:aVuBpDAT5VtjtUxzvBt8HOd0buzvvk7OX3H2iaviixg=
github.com/influxdata/influxql v0.0.0-20180925231337-1cbfca8e56b6 h1:CFx+pP90q/qg3spoiZjf8donE4WpAdjeJfPOcoNqkWo=
github.com/influxdata/influxql v0.0.0-20180925231337-1cbfca8e56b6/go.mod h1:KpVI7okXjK6PRi3Z5B+mtKZli+R1DnZgb3N+tzevNgo=
Expand Down
6 changes: 3 additions & 3 deletions http/query_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,9 +299,9 @@ func TestFluxHandler_postFluxPlan(t *testing.T) {
{
name: "get plan from()",
w: httptest.NewRecorder(),
r: httptest.NewRequest("POST", "/api/v2/query/plan", bytes.NewBufferString(`{"query": "from(bucket:\"telegraf\")|> range(start: -5000h)|> filter(fn: (r) => r._measurement == \"mem\" AND r._field == \"used_percent\")|> group(by:[\"host\"])|> mean()"}`)),
r: httptest.NewRequest("POST", "/api/v2/query/plan", bytes.NewBufferString(`{"query": "from(bucket:\"telegraf\")|> range(start: -5000h)|> filter(fn: (r) => r._measurement == \"mem\" AND r._field == \"used_percent\")|> group(columns:[\"host\"])|> mean()"}`)),
now: func() time.Time { return time.Unix(0, 0).UTC() },
want: `{"spec":{"operations":[{"kind":"from","id":"from0","spec":{"bucket":"telegraf"}},{"kind":"range","id":"range1","spec":{"start":"-5000h0m0s","stop":"now","timeColumn":"_time","startColumn":"_start","stopColumn":"_stop"}},{"kind":"filter","id":"filter2","spec":{"fn":{"type":"FunctionExpression","block":{"type":"FunctionBlock","parameters":{"type":"FunctionParameters","list":[{"type":"FunctionParameter","key":{"type":"Identifier","name":"r"}}],"pipe":null},"body":{"type":"LogicalExpression","operator":"and","left":{"type":"BinaryExpression","operator":"==","left":{"type":"MemberExpression","object":{"type":"IdentifierExpression","name":"r"},"property":"_measurement"},"right":{"type":"StringLiteral","value":"mem"}},"right":{"type":"BinaryExpression","operator":"==","left":{"type":"MemberExpression","object":{"type":"IdentifierExpression","name":"r"},"property":"_field"},"right":{"type":"StringLiteral","value":"used_percent"}}}}}}},{"kind":"group","id":"group3","spec":{"by":["host"],"except":null,"all":false,"none":false}},{"kind":"mean","id":"mean4","spec":{"columns":["_value"]}}],"edges":[{"parent":"from0","child":"range1"},{"parent":"range1","child":"filter2"},{"parent":"filter2","child":"group3"},{"parent":"group3","child":"mean4"}],"resources":{"priority":"high","concurrency_quota":0,"memory_bytes_quota":0},"now":"1970-01-01T00:00:00Z"},"logical":{"roots":[{"Spec":{"Name":"_result"}}],"resources":{"priority":"high","concurrency_quota":1,"memory_bytes_quota":9223372036854775807},"now":"1970-01-01T00:00:00Z"},"physical":{"roots":[{"Spec":{"Name":"_result"}}],"resources":{"priority":"high","concurrency_quota":1,"memory_bytes_quota":9223372036854775807},"now":"1970-01-01T00:00:00Z"}}
want: `{"spec":{"operations":[{"kind":"from","id":"from0","spec":{"bucket":"telegraf"}},{"kind":"range","id":"range1","spec":{"start":"-5000h0m0s","stop":"now","timeColumn":"_time","startColumn":"_start","stopColumn":"_stop"}},{"kind":"filter","id":"filter2","spec":{"fn":{"type":"FunctionExpression","block":{"type":"FunctionBlock","parameters":{"type":"FunctionParameters","list":[{"type":"FunctionParameter","key":{"type":"Identifier","name":"r"}}],"pipe":null},"body":{"type":"LogicalExpression","operator":"and","left":{"type":"BinaryExpression","operator":"==","left":{"type":"MemberExpression","object":{"type":"IdentifierExpression","name":"r"},"property":"_measurement"},"right":{"type":"StringLiteral","value":"mem"}},"right":{"type":"BinaryExpression","operator":"==","left":{"type":"MemberExpression","object":{"type":"IdentifierExpression","name":"r"},"property":"_field"},"right":{"type":"StringLiteral","value":"used_percent"}}}}}}},{"kind":"group","id":"group3","spec":{"mode":"by","columns":["host"]}},{"kind":"mean","id":"mean4","spec":{"columns":["_value"]}}],"edges":[{"parent":"from0","child":"range1"},{"parent":"range1","child":"filter2"},{"parent":"filter2","child":"group3"},{"parent":"group3","child":"mean4"}],"resources":{"priority":"high","concurrency_quota":0,"memory_bytes_quota":0},"now":"1970-01-01T00:00:00Z"},"logical":{"roots":[{"Spec":{"Name":"_result"}}],"resources":{"priority":"high","concurrency_quota":1,"memory_bytes_quota":9223372036854775807},"now":"1970-01-01T00:00:00Z"},"physical":{"roots":[{"Spec":{"Name":"_result"}}],"resources":{"priority":"high","concurrency_quota":1,"memory_bytes_quota":9223372036854775807},"now":"1970-01-01T00:00:00Z"}}
`,
status: http.StatusOK,
},
Expand Down Expand Up @@ -370,7 +370,7 @@ func Test_postPlanRequest_Valid(t *testing.T) {

name: "request with query is valid",
fields: fields{
Query: `from(bucket:"telegraf")|> range(start: -5000h)|> filter(fn: (r) => r._measurement == "mem" AND r._field == "used_percent")|> group(by:["host"])|> mean()`,
Query: `from(bucket:"telegraf")|> range(start: -5000h)|> filter(fn: (r) => r._measurement == "mem" AND r._field == "used_percent")|> group(columns:["host"])|> mean()`,
},
},
{
Expand Down
4 changes: 2 additions & 2 deletions query/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ Since flux will group each series into its own table, we sometimes need to modif
```
from(bucket: "telegraf/autogen") |> range(start: -5m)
|> filter(fn: (r) => r._measurement == "cpu" and r._field == "usage_user")
|> group(by: ["region"])
|> group(columns: ["region"])
|> mean()
```

Expand All @@ -171,7 +171,7 @@ Similarly, if we wanted to group points into buckets of time, the `window` funct
```
from(bucket: "telegraf/autogen") |> range(start: -5m)
|> filter(fn: (r) => r._measurement == "cpu" and r._field == "usage_user")
|> group(by: ["region"])
|> group(columns: ["region"])
|> window(every: 1m)
|> mean()
```
Expand Down
2 changes: 1 addition & 1 deletion query/functions/inputs/from.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func createFromSource(prSpec plan.ProcedureSpec, dsid execute.DatasetID, a execu
SeriesOffset: spec.SeriesOffset,
Descending: spec.Descending,
OrderByTime: spec.OrderByTime,
GroupMode: storage.GroupMode(spec.GroupMode),
GroupMode: storage.ToGroupMode(spec.GroupMode),
GroupKeys: spec.GroupKeys,
AggregateMethod: spec.AggregateMethod,
},
Expand Down
16 changes: 16 additions & 0 deletions query/functions/inputs/storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ package storage

import (
"context"
"fmt"
"log"
"math"

"github.com/influxdata/flux"
"github.com/influxdata/flux/execute"
"github.com/influxdata/flux/functions"
"github.com/influxdata/flux/semantic"
"github.com/influxdata/platform"
"github.com/pkg/errors"
Expand Down Expand Up @@ -182,6 +184,20 @@ const (
GroupModeExcept
)

// ToGroupMode accepts the group mode from Flux and produces the appropriate storage group mode.
func ToGroupMode(fluxMode functions.GroupMode) GroupMode {
switch fluxMode {
case functions.GroupModeNone:
return GroupModeDefault
case functions.GroupModeBy:
return GroupModeBy
case functions.GroupModeExcept:
return GroupModeExcept
default:
panic(fmt.Sprint("unknown group mode: ", fluxMode))
}
}

type ReadSpec struct {
OrganizationID platform.ID
BucketID platform.ID
Expand Down
8 changes: 4 additions & 4 deletions query/influxql/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ We group together the streams based on the `GROUP BY` clause. As an example:

```
> SELECT mean(usage_user) FROM telegraf..cpu WHERE time >= now() - 5m GROUP BY time(5m), host
... |> group(by: ["_measurement", "_start", "host"]) |> window(every: 5m)
... |> group(columns: ["_measurement", "_start", "host"]) |> window(every: 5m)
```

If the `GROUP BY time(...)` doesn't exist, `window()` is skipped. Grouping will have a default of [`_measurement`, `_start`], regardless of whether a GROUP BY clause is present. If there are keys in the group by clause, they are concatenated with the default list. If a wildcard is used for grouping, then this step is skipped.
Expand All @@ -141,7 +141,7 @@ For an aggregate, the following is used instead:
```
> SELECT mean(usage_user) FROM telegraf..cpu
create_cursor(bucket: "telegraf/autogen", start: -5m, m: "cpu", f: "usage_user")
|> group(except: ["_field"])
|> group(columns: ["_field"], mode: "except")
|> mean(timeSrc: "_start", columns: ["_value"])
```

Expand Down Expand Up @@ -316,9 +316,9 @@ At this point, we have a table with the partition key that is organized by the k
We group by the measurement and the key and then use `distinct` on the values. After we find the distinct values, we group these values back by their measurements again so all of the tag values for a measurement are grouped together. We then rename the columns to the expected names.

```
... |> group(by: ["_measurement", "_key"])
... |> group(columns: ["_measurement", "_key"])
|> distinct(column: "_value")
|> group(by: ["_measurement"])
|> group(columns: ["_measurement"])
|> rename(columns: {_key: "key", _value: "value"})
```

Expand Down
3 changes: 2 additions & 1 deletion query/influxql/group.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,8 @@ func (gr *groupInfo) group(t *transpilerState, in cursor) (cursor, error) {
// there is always something to group in influxql.
// TODO(jsternberg): A wildcard will skip this step.
id := t.op("group", &transformations.GroupOpSpec{
By: tags,
Columns: tags,
Mode: "by",
}, in.ID())

if windowEvery > 0 {
Expand Down
3 changes: 2 additions & 1 deletion query/influxql/spectests/aggregates.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ func init() {
{
ID: "group0",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement", "_start"},
Columns: []string{"_measurement", "_start"},
Mode: "by",
},
},
&aggregate,
Expand Down
3 changes: 2 additions & 1 deletion query/influxql/spectests/aggregates_with_condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ func init() {
{
ID: "group0",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement", "_start"},
Columns: []string{"_measurement", "_start"},
Mode: "by",
},
},
&aggregate,
Expand Down
3 changes: 2 additions & 1 deletion query/influxql/spectests/aggregates_with_groupby.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ func init() {
{
ID: "group0",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement", "_start", "host"},
Columns: []string{"_measurement", "_start", "host"},
Mode: "by",
},
},
&aggregate,
Expand Down
3 changes: 2 additions & 1 deletion query/influxql/spectests/aggregates_with_window.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ func init() {
{
ID: "group0",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement", "_start"},
Columns: []string{"_measurement", "_start"},
Mode: "by",
},
},
{
Expand Down
3 changes: 2 additions & 1 deletion query/influxql/spectests/aggregates_with_window_offset.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ func init() {
{
ID: "group0",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement", "_start"},
Columns: []string{"_measurement", "_start"},
Mode: "by",
},
},
{
Expand Down
6 changes: 4 additions & 2 deletions query/influxql/spectests/multiple_aggregates.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ func init() {
{
ID: "group0",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement", "_start"},
Columns: []string{"_measurement", "_start"},
Mode: "by",
},
},
{
Expand Down Expand Up @@ -158,7 +159,8 @@ func init() {
{
ID: "group1",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement", "_start"},
Columns: []string{"_measurement", "_start"},
Mode: "by",
},
},
{
Expand Down
6 changes: 4 additions & 2 deletions query/influxql/spectests/multiple_statements.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ func init() {
{
ID: "group0",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement", "_start"},
Columns: []string{"_measurement", "_start"},
Mode: "by",
},
},
{
Expand Down Expand Up @@ -201,7 +202,8 @@ func init() {
{
ID: "group1",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement", "_start"},
Columns: []string{"_measurement", "_start"},
Mode: "by",
},
},
{
Expand Down
3 changes: 2 additions & 1 deletion query/influxql/spectests/raw.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ func init() {
{
ID: "group0",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement", "_start"},
Columns: []string{"_measurement", "_start"},
Mode: "by",
},
},
{
Expand Down
3 changes: 2 additions & 1 deletion query/influxql/spectests/raw_with_condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ func init() {
{
ID: "group0",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement", "_start"},
Columns: []string{"_measurement", "_start"},
Mode: "by",
},
},
{
Expand Down
6 changes: 4 additions & 2 deletions query/influxql/spectests/raw_with_regex_condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ func init() {
{
ID: "group0",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement", "_start"},
Columns: []string{"_measurement", "_start"},
Mode: "by",
},
},
{
Expand Down Expand Up @@ -258,7 +259,8 @@ func init() {
{
ID: "group0",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement", "_start"},
Columns: []string{"_measurement", "_start"},
Mode: "by",
},
},
{
Expand Down
3 changes: 2 additions & 1 deletion query/influxql/spectests/retention_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ func init() {
{
ID: "group0",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement", "_start"},
Columns: []string{"_measurement", "_start"},
Mode: "by",
},
},
{
Expand Down
3 changes: 2 additions & 1 deletion query/influxql/spectests/selectors.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ func init() {
{
ID: "group0",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement", "_start"},
Columns: []string{"_measurement", "_start"},
Mode: "by",
},
},
&selector,
Expand Down
6 changes: 4 additions & 2 deletions query/influxql/spectests/show_tag_values.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ func init() {
{
ID: "group0",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement", "_key"},
Columns: []string{"_measurement", "_key"},
Mode: "by",
},
},
{
Expand All @@ -52,7 +53,8 @@ func init() {
{
ID: "group1",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement"},
Columns: []string{"_measurement"},
Mode: "by",
},
},
{
Expand Down
6 changes: 4 additions & 2 deletions query/influxql/spectests/show_tag_values_in_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ func init() {
{
ID: "group0",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement", "_key"},
Columns: []string{"_measurement", "_key"},
Mode: "by",
},
},
{
Expand All @@ -52,7 +53,8 @@ func init() {
{
ID: "group1",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement"},
Columns: []string{"_measurement"},
Mode: "by",
},
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ func init() {
{
ID: "group0",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement", "_key"},
Columns: []string{"_measurement", "_key"},
Mode: "by",
},
},
{
Expand All @@ -98,7 +99,8 @@ func init() {
{
ID: "group1",
Spec: &transformations.GroupOpSpec{
By: []string{"_measurement"},
Columns: []string{"_measurement"},
Mode: "by",
},
},
{
Expand Down
6 changes: 4 additions & 2 deletions query/influxql/transpiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,13 @@ func (t *transpilerState) transpileShowTagValues(ctx context.Context, stmt *infl
"_value": "value",
},
}, t.op("group", &transformations.GroupOpSpec{
By: []string{"_measurement"},
Columns: []string{"_measurement"},
Mode: "by",
}, t.op("distinct", &transformations.DistinctOpSpec{
Column: execute.DefaultValueColLabel,
}, t.op("group", &transformations.GroupOpSpec{
By: []string{"_measurement", "_key"},
Columns: []string{"_measurement", "_key"},
Mode: "by",
}, op)))), nil
}

Expand Down
3 changes: 2 additions & 1 deletion query/promql/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,8 @@ func (a *Aggregate) QuerySpec() (*flux.Operation, error) {
return &flux.Operation{
ID: "merge",
Spec: &transformations.GroupOpSpec{
By: keys,
Columns: keys,
Mode: "by",
},
}, nil
}
Expand Down
Loading

0 comments on commit f1d21b8

Please sign in to comment.