Delay parsing of date/time strings until needed

The current code would compare every string literal it crossed and tried to coerce them to time literals if the _looked_ like date/time strings. The only time the TimeLiteral was used is when comparing to the the 'time' value in a where clause. This change moves the string parsing code until we attempt to compare 'time' to a string, at which point we know we need/want a TimeLiteral, and not just an ordinary string. Fixes #6727
influxdata · May 27, 2016 · f2fd988 · f2fd988
1 parent da6a5ec
commit f2fd988
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 37 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -34,7 +34,7 @@
 - [#6235](https://github.com/influxdata/influxdb/issues/6235): Fix measurement field panic in tsm1 engine.
 - [#6663](https://github.com/influxdata/influxdb/issues/6663): Fixing panic in SHOW FIELD KEYS.
 - [#6624](https://github.com/influxdata/influxdb/issues/6624): Ensure clients requesting gzip encoded bodies don't receive empty body
-- [#6652](https://github.com/influxdata/influxdb/issues/6652): Fix panic: interface conversion: tsm1.Value is *tsm1.StringValue, not *tsm1.FloatValue
+- [#6652](https://github.com/influxdata/influxdb/issues/6652): Fix panic: interface conversion: tsm1.Value is \*tsm1.StringValue, not \*tsm1.FloatValue
 - [#6406](https://github.com/influxdata/influxdb/issues/6406): Max index entries exceeded
 - [#6557](https://github.com/influxdata/influxdb/issues/6557): Overwriting points on large series can cause memory spikes during compactions
 - [#6611](https://github.com/influxdata/influxdb/issues/6611): Queries slow down hundreds times after overwriting points
@@ -49,6 +49,7 @@
 - [#6672](https://github.com/influxdata/influxdb/issues/6672): Accept points with trailing whitespace.
 - [#6599](https://github.com/influxdata/influxdb/issues/6599): Ensure that future points considered in SHOW queries.
 - [#6720](https://github.com/influxdata/influxdb/issues/6720): Concurrent map read write panic. Thanks @arussellsaw
+- [#6727](https://github.com/influxdata/influxdb/issues/6727): queries with strings that look like dates end up with date types, not string types
 
 ## v0.13.0 [2016-05-12]
 

diff --git a/influxql/ast.go b/influxql/ast.go
@@ -38,6 +38,11 @@ const (
 	AnyField = 8
 )
 
+var (
+	// Invalid timestamp string used to compare against time field
+	ErrInvalidTime = errors.New("invalid timestamp string")
+)
+
 // InspectDataType returns the data type of a given value.
 func InspectDataType(v interface{}) DataType {
 	switch v.(type) {
@@ -3566,6 +3571,27 @@ func TimeRangeAsEpochNano(expr Expr) (min, max int64, err error) {
 // Returns zero time if the expression is not a time expression.
 func timeExprValue(ref Expr, lit Expr) (t time.Time, err error) {
 	if ref, ok := ref.(*VarRef); ok && strings.ToLower(ref.Val) == "time" {
+		// If literal looks like a date time then parse it as a time literal.
+		if strlit, ok := lit.(*StringLiteral); ok {
+			if isDateTimeString(strlit.Val) {
+				t, err := time.Parse(DateTimeFormat, strlit.Val)
+				if err != nil {
+					// try to parse it as an RFCNano time
+					t, err = time.Parse(time.RFC3339Nano, strlit.Val)
+					if err != nil {
+						return time.Time{}, ErrInvalidTime
+					}
+				}
+				lit = &TimeLiteral{Val: t}
+			} else if isDateString(strlit.Val) {
+				t, err := time.Parse(DateFormat, strlit.Val)
+				if err != nil {
+					return time.Time{}, ErrInvalidTime
+				}
+				lit = &TimeLiteral{Val: t}
+			}
+		}
+
 		switch lit := lit.(type) {
 		case *TimeLiteral:
 			if lit.Val.After(time.Unix(0, MaxTime)) {

diff --git a/influxql/parser.go b/influxql/parser.go
@@ -2378,25 +2378,6 @@ func (p *Parser) parseUnaryExpr() (Expr, error) {
 
 		return nil, newParseError(tokstr(tok0, lit), []string{"(", "identifier"}, pos)
 	case STRING:
-		// If literal looks like a date time then parse it as a time literal.
-		if isDateTimeString(lit) {
-			t, err := time.Parse(DateTimeFormat, lit)
-			if err != nil {
-				// try to parse it as an RFCNano time
-				t, err := time.Parse(time.RFC3339Nano, lit)
-				if err != nil {
-					return nil, &ParseError{Message: "unable to parse datetime", Pos: pos}
-				}
-				return &TimeLiteral{Val: t}, nil
-			}
-			return &TimeLiteral{Val: t}, nil
-		} else if isDateString(lit) {
-			t, err := time.Parse(DateFormat, lit)
-			if err != nil {
-				return nil, &ParseError{Message: "unable to parse date", Pos: pos}
-			}
-			return &TimeLiteral{Val: t}, nil
-		}
 		return &StringLiteral{Val: lit}, nil
 	case NUMBER:
 		v, err := strconv.ParseFloat(lit, 64)

diff --git a/influxql/parser_test.go b/influxql/parser_test.go
@@ -138,7 +138,7 @@ func TestParser_ParseStatement(t *testing.T) {
 					RHS: &influxql.BinaryExpr{
 						Op:  influxql.GT,
 						LHS: &influxql.VarRef{Val: "time"},
-						RHS: &influxql.TimeLiteral{Val: now.UTC()},
+						RHS: &influxql.StringLiteral{Val: now.UTC().Format(time.RFC3339Nano)},
 					},
 				},
 				Dimensions: []*influxql.Dimension{{Expr: &influxql.Call{Name: "time", Args: []influxql.Expr{&influxql.DurationLiteral{Val: 10 * time.Hour}}}}},
@@ -193,7 +193,7 @@ func TestParser_ParseStatement(t *testing.T) {
 				Condition: &influxql.BinaryExpr{
 					Op:  influxql.GT,
 					LHS: &influxql.VarRef{Val: "time"},
-					RHS: &influxql.TimeLiteral{Val: now.UTC()},
+					RHS: &influxql.StringLiteral{Val: now.UTC().Format(time.RFC3339Nano)},
 				},
 			},
 		},
@@ -274,7 +274,7 @@ func TestParser_ParseStatement(t *testing.T) {
 				Condition: &influxql.BinaryExpr{
 					Op:  influxql.GT,
 					LHS: &influxql.VarRef{Val: "time"},
-					RHS: &influxql.TimeLiteral{Val: now.UTC()},
+					RHS: &influxql.StringLiteral{Val: now.UTC().Format(time.RFC3339Nano)},
 				},
 			},
 		},
@@ -325,7 +325,7 @@ func TestParser_ParseStatement(t *testing.T) {
 				Condition: &influxql.BinaryExpr{
 					Op:  influxql.GT,
 					LHS: &influxql.VarRef{Val: "time"},
-					RHS: &influxql.TimeLiteral{Val: now.UTC()},
+					RHS: &influxql.StringLiteral{Val: now.UTC().Format(time.RFC3339Nano)},
 				},
 			},
 		},
@@ -363,7 +363,7 @@ func TestParser_ParseStatement(t *testing.T) {
 				Condition: &influxql.BinaryExpr{
 					Op:  influxql.GT,
 					LHS: &influxql.VarRef{Val: "time"},
-					RHS: &influxql.TimeLiteral{Val: now.UTC()},
+					RHS: &influxql.StringLiteral{Val: now.UTC().Format(time.RFC3339Nano)},
 				},
 			},
 		},
@@ -400,7 +400,7 @@ func TestParser_ParseStatement(t *testing.T) {
 				Condition: &influxql.BinaryExpr{
 					Op:  influxql.GT,
 					LHS: &influxql.VarRef{Val: "time"},
-					RHS: &influxql.TimeLiteral{Val: now.UTC()},
+					RHS: &influxql.StringLiteral{Val: now.UTC().Format(time.RFC3339Nano)},
 				},
 			},
 		},
@@ -439,7 +439,7 @@ func TestParser_ParseStatement(t *testing.T) {
 				Condition: &influxql.BinaryExpr{
 					Op:  influxql.GT,
 					LHS: &influxql.VarRef{Val: "time"},
-					RHS: &influxql.TimeLiteral{Val: now.UTC()},
+					RHS: &influxql.StringLiteral{Val: now.UTC().Format(time.RFC3339Nano)},
 				},
 			},
 		},
@@ -479,7 +479,7 @@ func TestParser_ParseStatement(t *testing.T) {
 				Condition: &influxql.BinaryExpr{
 					Op:  influxql.GT,
 					LHS: &influxql.VarRef{Val: "time"},
-					RHS: &influxql.TimeLiteral{Val: now.UTC()},
+					RHS: &influxql.StringLiteral{Val: now.UTC().Format(time.RFC3339Nano)},
 				},
 			},
 		},
@@ -695,7 +695,7 @@ func TestParser_ParseStatement(t *testing.T) {
 				Condition: &influxql.BinaryExpr{
 					Op:  influxql.GT,
 					LHS: &influxql.VarRef{Val: "time"},
-					RHS: &influxql.TimeLiteral{Val: now.UTC()},
+					RHS: &influxql.StringLiteral{Val: now.UTC().Format(time.RFC3339Nano)},
 				},
 			},
 		},
@@ -882,7 +882,7 @@ func TestParser_ParseStatement(t *testing.T) {
 				Condition: &influxql.BinaryExpr{
 					Op:  influxql.LT,
 					LHS: &influxql.VarRef{Val: "time"},
-					RHS: &influxql.TimeLiteral{Val: now.UTC()},
+					RHS: &influxql.StringLiteral{Val: now.UTC().Format(time.RFC3339Nano)},
 				},
 				Dimensions: []*influxql.Dimension{{Expr: &influxql.Call{Name: "time", Args: []influxql.Expr{&influxql.DurationLiteral{Val: 5 * time.Minute}}}}},
 				Fill:       influxql.NumberFill,
@@ -902,7 +902,7 @@ func TestParser_ParseStatement(t *testing.T) {
 				Condition: &influxql.BinaryExpr{
 					Op:  influxql.LT,
 					LHS: &influxql.VarRef{Val: "time"},
-					RHS: &influxql.TimeLiteral{Val: now.UTC()},
+					RHS: &influxql.StringLiteral{Val: now.UTC().Format(time.RFC3339Nano)},
 				},
 				Dimensions: []*influxql.Dimension{{Expr: &influxql.Call{Name: "time", Args: []influxql.Expr{&influxql.DurationLiteral{Val: 5 * time.Minute}}}}},
 				Fill:       influxql.NoFill,
@@ -921,7 +921,7 @@ func TestParser_ParseStatement(t *testing.T) {
 				Condition: &influxql.BinaryExpr{
 					Op:  influxql.LT,
 					LHS: &influxql.VarRef{Val: "time"},
-					RHS: &influxql.TimeLiteral{Val: now.UTC()},
+					RHS: &influxql.StringLiteral{Val: now.UTC().Format(time.RFC3339Nano)},
 				},
 				Dimensions: []*influxql.Dimension{{Expr: &influxql.Call{Name: "time", Args: []influxql.Expr{&influxql.DurationLiteral{Val: 5 * time.Minute}}}}},
 				Fill:       influxql.PreviousFill,
@@ -2418,11 +2418,8 @@ func TestParser_ParseExpr(t *testing.T) {
 		{s: `true`, expr: &influxql.BooleanLiteral{Val: true}},
 		{s: `false`, expr: &influxql.BooleanLiteral{Val: false}},
 		{s: `my_ident`, expr: &influxql.VarRef{Val: "my_ident"}},
-		{s: `'2000-01-01 00:00:00'`, expr: &influxql.TimeLiteral{Val: mustParseTime("2000-01-01T00:00:00Z")}},
-		{s: `'2000-01-01 00:00:00.232'`, expr: &influxql.TimeLiteral{Val: mustParseTime("2000-01-01T00:00:00.232Z")}},
-		{s: `'2000-01-32 00:00:00'`, err: `unable to parse datetime at line 1, char 1`},
-		{s: `'2000-01-01'`, expr: &influxql.TimeLiteral{Val: mustParseTime("2000-01-01T00:00:00Z")}},
-		{s: `'2000-01-99'`, err: `unable to parse date at line 1, char 1`},
+		{s: `'2000-01-01 00:00:00'`, expr: &influxql.StringLiteral{Val: "2000-01-01 00:00:00"}},
+		{s: `'2000-01-01'`, expr: &influxql.StringLiteral{Val: "2000-01-01"}},
 
 		// Simple binary expression
 		{