Disallow derivative call with non-duration 2nd arg

Previously, calling derivative with a non-duration second argument was
allowed during parsing but would panic during execution due to a failed
type conversion. This change ensures the second argument is a duration
literal.
pull/7475/head
Mark Rushakoff 2016-10-17 16:20:53 -07:00
parent 40c75fcd44
commit 0ddb7ad842
2 changed files with 25 additions and 6 deletions

View File

@ -1667,11 +1667,7 @@ func (s *SelectStatement) validateAggregates(tr targetRequirement) error {
return err
}
switch expr.Name {
case "derivative", "non_negative_derivative":
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)
}
case "elapsed":
case "derivative", "non_negative_derivative", "elapsed":
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)
}
@ -1679,7 +1675,7 @@ func (s *SelectStatement) validateAggregates(tr targetRequirement) error {
if len(expr.Args) == 2 {
// Second must be a duration .e.g (1h)
if _, ok := expr.Args[1].(*DurationLiteral); !ok {
return errors.New("elapsed requires a duration argument")
return fmt.Errorf("second argument to %s must be a duration, got %T", expr.Name, expr.Args[1])
}
}
case "difference", "cumulative_sum":

View File

@ -1313,6 +1313,29 @@ func TestSources_HasSystemSource(t *testing.T) {
}
}
// Parse statements that might appear valid but should return an error.
// If allowed to execute, at least some of these statements would result in a panic.
func TestParse_Errors(t *testing.T) {
for _, tt := range []struct {
tmpl string
good string
bad string
}{
// Second argument to derivative must be duration
{tmpl: `SELECT derivative(f, %s) FROM m`, good: "1h", bad: "true"},
} {
good := fmt.Sprintf(tt.tmpl, tt.good)
if _, err := influxql.ParseStatement(good); err != nil {
t.Fatalf("statement %q should have parsed correctly but returned error: %s", good, err)
}
bad := fmt.Sprintf(tt.tmpl, tt.bad)
if _, err := influxql.ParseStatement(bad); err == nil {
t.Fatalf("statement %q should have resulted in a parse error but did not", bad)
}
}
}
// Valuer represents a simple wrapper around a map to implement the influxql.Valuer interface.
type Valuer map[string]interface{}