Skip to content

Commit

Permalink
influxql: when using derivative, check 'group by time' instead of 'wh…
Browse files Browse the repository at this point in the history
…ere time ...'

When using derivative, it is required that aggregate function is used as
sub-call when grouping by time is used. However, AST parsing used to check
if WHERE-clause contained condition with 'time'.

Fix this by changing check to see if groupByInterval is present.

Modify also related error case tests and add check for
select derivative(...) ... where time > ...
  • Loading branch information
jarisukanen committed Sep 18, 2015
1 parent 83d2df1 commit c844ec1
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 4 deletions.
5 changes: 3 additions & 2 deletions influxql/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -1136,8 +1136,9 @@ func (s *SelectStatement) validateAggregates(tr targetRequirement) error {
if min, max, got := 1, 2, len(expr.Args); got > max || got < min {
return fmt.Errorf("invalid number of arguments for %s, expected at least %d but no more than %d, got %d", expr.Name, min, max, got)
}
// Validate that if they have a time dimension, they need a sub-call like min/max, etc.
if s.hasTimeDimensions(s.Condition) {
// Validate that if they have grouping by time, they need a sub-call like min/max, etc.
groupByInterval, _ := s.GroupByInterval()
if groupByInterval > 0 {
if _, ok := expr.Args[0].(*Call); !ok {
return fmt.Errorf("aggregate function required inside the call to %s", expr.Name)
}
Expand Down
20 changes: 18 additions & 2 deletions influxql/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,22 @@ func TestParser_ParseStatement(t *testing.T) {
},
},

{
s: fmt.Sprintf(`SELECT derivative(field1, 1h) FROM myseries WHERE time > '%s'`, now.UTC().Format(time.RFC3339Nano)),
stmt: &influxql.SelectStatement{
IsRawQuery: false,
Fields: []*influxql.Field{
{Expr: &influxql.Call{Name: "derivative", Args: []influxql.Expr{&influxql.VarRef{Val: "field1"}, &influxql.DurationLiteral{Val: time.Hour}}}},
},
Sources: []influxql.Source{&influxql.Measurement{Name: "myseries"}},
Condition: &influxql.BinaryExpr{
Op: influxql.GT,
LHS: &influxql.VarRef{Val: "time"},
RHS: &influxql.TimeLiteral{Val: now.UTC()},
},
},
},

{
s: `SELECT derivative(mean(field1), 1h) FROM myseries;`,
stmt: &influxql.SelectStatement{
Expand Down Expand Up @@ -1415,11 +1431,11 @@ func TestParser_ParseStatement(t *testing.T) {
{s: `SELECT derivative(), field1 FROM myseries`, err: `mixing aggregate and non-aggregate queries is not supported`},
{s: `select derivative() from myseries`, err: `invalid number of arguments for derivative, expected at least 1 but no more than 2, got 0`},
{s: `select derivative(mean(value), 1h, 3) from myseries`, err: `invalid number of arguments for derivative, expected at least 1 but no more than 2, got 3`},
{s: `SELECT derivative(value) FROM myseries where time < now() and time > now() - 1d`, err: `aggregate function required inside the call to derivative`},
{s: `SELECT derivative(value) FROM myseries group by time(1h)`, err: `aggregate function required inside the call to derivative`},
{s: `SELECT non_negative_derivative(), field1 FROM myseries`, err: `mixing aggregate and non-aggregate queries is not supported`},
{s: `select non_negative_derivative() from myseries`, err: `invalid number of arguments for non_negative_derivative, expected at least 1 but no more than 2, got 0`},
{s: `select non_negative_derivative(mean(value), 1h, 3) from myseries`, err: `invalid number of arguments for non_negative_derivative, expected at least 1 but no more than 2, got 3`},
{s: `SELECT non_negative_derivative(value) FROM myseries where time < now() and time > now() - 1d`, err: `aggregate function required inside the call to non_negative_derivative`},
{s: `SELECT non_negative_derivative(value) FROM myseries group by time(1h)`, err: `aggregate function required inside the call to non_negative_derivative`},
{s: `SELECT field1 from myseries WHERE host =~ 'asd' LIMIT 1`, err: `found asd, expected regex at line 1, char 42`},
{s: `SELECT value > 2 FROM cpu`, err: `invalid operator > in SELECT clause at line 1, char 8; operator is intended for WHERE clause`},
{s: `SELECT value = 2 FROM cpu`, err: `invalid operator = in SELECT clause at line 1, char 8; operator is intended for WHERE clause`},
Expand Down

0 comments on commit c844ec1

Please sign in to comment.