Throw an error when time is compared to an invalid literal

A bigger refactor of these functions is needed to support #3290, but
this will work for the more common case that someone uses double quotes
instead of single quotes when surrounding a time literal.

Fixes #3932.

CHANGELOG.md

@@ -45,6 +45,7 @@
 - [#6129](https://github.com/influxdata/influxdb/pull/6129): Fix default continuous query lease host
 - [#6121](https://github.com/influxdata/influxdb/issues/6121): Fix panic: slice index out of bounds in TSM index
 - [#6168](https://github.com/influxdata/influxdb/pull/6168): Remove per measurement statsitics
+- [#3932](https://github.com/influxdata/influxdb/issues/3932): Invalid timestamp format should throw an error.
 
 ## v0.11.0 [2016-03-22]
 

cluster/query_executor.go

@@ -434,7 +434,12 @@ func (e *QueryExecutor) executeSelectStatement(stmt *influxql.SelectStatement, c
 
 	// Replace instances of "now()" with the current time, and check the resultant times.
 	stmt.Condition = influxql.Reduce(stmt.Condition, &influxql.NowValuer{Now: now})
-	opt.MinTime, opt.MaxTime = influxql.TimeRange(stmt.Condition)
+	var err error
+	opt.MinTime, opt.MaxTime, err = influxql.TimeRange(stmt.Condition)
+	if err != nil {
+		return err
+	}
+
 	if opt.MaxTime.IsZero() {
 		opt.MaxTime = now
 	}

influxql/ast.go

@@ -3383,15 +3383,23 @@ func OnlyTimeExpr(expr Expr) bool {
 
 // TimeRange returns the minimum and maximum times specified by an expression.
 // Returns zero times if there is no bound.
-func TimeRange(expr Expr) (min, max time.Time) {
+func TimeRange(expr Expr) (min, max time.Time, err error) {
 	WalkFunc(expr, func(n Node) {
+		if err != nil {
+			return
+		}
+
 		if n, ok := n.(*BinaryExpr); ok {
 			// Extract literal expression & operator on LHS.
 			// Check for "time" on the left-hand side first.
 			// Otherwise check for for the right-hand side and flip the operator.
-			value, op := timeExprValue(n.LHS, n.RHS), n.Op
-			if value.IsZero() {
-				if value = timeExprValue(n.RHS, n.LHS); value.IsZero() {
+			op := n.Op
+			var value time.Time
+			value, err = timeExprValue(n.LHS, n.RHS)
+			if err != nil {
+				return
+			} else if value.IsZero() {
+				if value, err = timeExprValue(n.RHS, n.LHS); value.IsZero() || err != nil {
 					return
 				} else if op == LT {
 					op = GT
@@ -3439,8 +3447,12 @@ func TimeRange(expr Expr) (min, max time.Time) {
 // TimeRangeAsEpochNano returns the minimum and maximum times, as epoch nano, specified by
 // and expression. If there is no lower bound, the start of the epoch is returned
 // for minimum. If there is no higher bound, now is returned for maximum.
-func TimeRangeAsEpochNano(expr Expr) (min, max int64) {
-	tmin, tmax := TimeRange(expr)
+func TimeRangeAsEpochNano(expr Expr) (min, max int64, err error) {
+	tmin, tmax, err := TimeRange(expr)
+	if err != nil {
+		return 0, 0, err
+	}
+
 	if tmin.IsZero() {
 		min = time.Unix(0, 0).UnixNano()
 	} else {
@@ -3456,20 +3468,22 @@ func TimeRangeAsEpochNano(expr Expr) (min, max int64) {
 
 // timeExprValue returns the time literal value of a "time == <TimeLiteral>" expression.
 // Returns zero time if the expression is not a time expression.
-func timeExprValue(ref Expr, lit Expr) time.Time {
+func timeExprValue(ref Expr, lit Expr) (t time.Time, err error) {
 	if ref, ok := ref.(*VarRef); ok && strings.ToLower(ref.Val) == "time" {
 		switch lit := lit.(type) {
 		case *TimeLiteral:
-			return lit.Val
+			return lit.Val, nil
 		case *DurationLiteral:
-			return time.Unix(0, int64(lit.Val)).UTC()
+			return time.Unix(0, int64(lit.Val)).UTC(), nil
 		case *NumberLiteral:
-			return time.Unix(0, int64(lit.Val)).UTC()
+			return time.Unix(0, int64(lit.Val)).UTC(), nil
 		case *IntegerLiteral:
-			return time.Unix(0, lit.Val).UTC()
+			return time.Unix(0, lit.Val).UTC(), nil
+		default:
+			return time.Time{}, fmt.Errorf("invalid operation: time and %T are not compatible", lit)
 		}
 	}
-	return time.Time{}
+	return time.Time{}, nil
 }
 
 // Visitor can be called by Walk to traverse an AST hierarchy.

influxql/ast_test.go

@@ -150,11 +150,10 @@ func TestSelectStatement_SetTimeRange(t *testing.T) {
 	}
 
 	s := stmt.(*influxql.SelectStatement)
-	min, max := influxql.TimeRange(s.Condition)
 	start := time.Now().Add(-20 * time.Hour).Round(time.Second).UTC()
 	end := time.Now().Add(10 * time.Hour).Round(time.Second).UTC()
 	s.SetTimeRange(start, end)
-	min, max = influxql.TimeRange(s.Condition)
+	min, max := MustTimeRange(s.Condition)
 
 	if min != start {
 		t.Fatalf("start time wasn't set properly.\n  exp: %s\n  got: %s", start, min)
@@ -175,7 +174,7 @@ func TestSelectStatement_SetTimeRange(t *testing.T) {
 	}
 
 	s = stmt.(*influxql.SelectStatement)
-	min, max = influxql.TimeRange(s.Condition)
+	min, max = MustTimeRange(s.Condition)
 	if start != min || end != max {
 		t.Fatalf("start and end times weren't equal:\n  exp: %s\n  got: %s\n  exp: %s\n  got:%s\n", start, min, end, max)
 	}
@@ -184,7 +183,7 @@ func TestSelectStatement_SetTimeRange(t *testing.T) {
 	start = time.Now().Add(-40 * time.Hour).Round(time.Second).UTC()
 	end = time.Now().Add(20 * time.Hour).Round(time.Second).UTC()
 	s.SetTimeRange(start, end)
-	min, max = influxql.TimeRange(s.Condition)
+	min, max = MustTimeRange(s.Condition)
 
 	// TODO: right now the SetTimeRange can't override the start time if it's more recent than what they're trying to set it to.
 	//       shouldn't matter for our purposes with continuous queries, but fix this later
@@ -211,7 +210,7 @@ func TestSelectStatement_SetTimeRange(t *testing.T) {
 	start = time.Now().Add(-40 * time.Hour).Round(time.Second).UTC()
 	end = time.Now().Add(20 * time.Hour).Round(time.Second).UTC()
 	s.SetTimeRange(start, end)
-	min, max = influxql.TimeRange(s.Condition)
+	min, max = MustTimeRange(s.Condition)
 
 	if min != start {
 		t.Fatalf("start time wasn't set properly.\n  exp: %s\n  got: %s", start, min)
@@ -749,8 +748,8 @@ func TestBinaryExprName(t *testing.T) {
 // Ensure the time range of an expression can be extracted.
 func TestTimeRange(t *testing.T) {
 	for i, tt := range []struct {
-		expr     string
-		min, max string
+		expr          string
+		min, max, err string
 	}{
 		// LHS VarRef
 		{expr: `time > '2000-01-01 00:00:00'`, min: `2000-01-01T00:00:00.000000001Z`, max: `0001-01-01T00:00:00Z`},
@@ -785,10 +784,13 @@ func TestTimeRange(t *testing.T) {
 		{expr: `time + 2`, min: `0001-01-01T00:00:00Z`, max: `0001-01-01T00:00:00Z`},
 		{expr: `time - '2000-01-01 00:00:00'`, min: `0001-01-01T00:00:00Z`, max: `0001-01-01T00:00:00Z`},
 		{expr: `time AND '2000-01-01 00:00:00'`, min: `0001-01-01T00:00:00Z`, max: `0001-01-01T00:00:00Z`},
+
+		// Invalid time expressions.
+		{expr: `time > "2000-01-01 00:00:00"`, min: `0001-01-01T00:00:00Z`, max: `0001-01-01T00:00:00Z`, err: `invalid operation: time and *influxql.VarRef are not compatible`},
 	} {
 		// Extract time range.
 		expr := MustParseExpr(tt.expr)
-		min, max := influxql.TimeRange(expr)
+		min, max, err := influxql.TimeRange(expr)
 
 		// Compare with expected min/max.
 		if min := min.Format(time.RFC3339Nano); tt.min != min {
@@ -799,6 +801,9 @@ func TestTimeRange(t *testing.T) {
 			t.Errorf("%d. %s: unexpected max:\n\nexp=%s\n\ngot=%s\n\n", i, tt.expr, tt.max, max)
 			continue
 		}
+		if (err != nil && err.Error() != tt.err) || (err == nil && tt.err != "") {
+			t.Errorf("%d. %s: unexpected error:\n\nexp=%s\n\ngot=%s\n\n", i, tt.expr, tt.err, err)
+		}
 	}
 }
 
@@ -1303,6 +1308,15 @@ func (o Valuer) Value(key string) (v interface{}, ok bool) {
 	return
 }
 
+// MustTimeRange will parse a time range. Panic on error.
+func MustTimeRange(expr influxql.Expr) (min, max time.Time) {
+	min, max, err := influxql.TimeRange(expr)
+	if err != nil {
+		panic(err)
+	}
+	return min, max
+}
+
 // mustParseTime parses an IS0-8601 string. Panic on error.
 func mustParseTime(s string) time.Time {
 	t, err := time.Parse(time.RFC3339, s)

influxql/iterator.go

@@ -649,7 +649,11 @@ type IteratorOptions struct {
 // newIteratorOptionsStmt creates the iterator options from stmt.
 func newIteratorOptionsStmt(stmt *SelectStatement, sopt *SelectOptions) (opt IteratorOptions, err error) {
 	// Determine time range from the condition.
-	startTime, endTime := TimeRange(stmt.Condition)
+	startTime, endTime, err := TimeRange(stmt.Condition)
+	if err != nil {
+		return IteratorOptions{}, err
+	}
+
 	if !startTime.IsZero() {
 		opt.StartTime = startTime.UnixNano()
 	} else {