Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse time literals using the time zone in the select statement #8642

Merged
merged 1 commit into from
Jul 28, 2017
Merged

Parse time literals using the time zone in the select statement #8642

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
- [#8601](https://github.com/influxdata/influxdb/pull/8601): Fixed time boundaries for continuous queries with time zones.
- [#8097](https://github.com/influxdata/influxdb/pull/8097): Return query parsing errors in CSV formats.
- [#8607](https://github.com/influxdata/influxdb/issues/8607): Fix time zone shifts when the shift happens on a time zone boundary.
- [#8639](https://github.com/influxdata/influxdb/issues/8639): Parse time literals using the time zone in the select statement.

## v1.3.1 [unreleased]

Expand Down
4 changes: 2 additions & 2 deletions coordinator/statement_executor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,11 +534,11 @@ func (e *StatementExecutor) createIterators(stmt *influxql.SelectStatement, ctx
}

// Replace instances of "now()" with the current time, and check the resultant times.
nowValuer := influxql.NowValuer{Now: now}
nowValuer := influxql.NowValuer{Now: now, Location: stmt.Location}
stmt = stmt.Reduce(&nowValuer)

var err error
opt.MinTime, opt.MaxTime, err = influxql.TimeRange(stmt.Condition)
opt.MinTime, opt.MaxTime, err = influxql.TimeRange(stmt.Condition, stmt.Location)
if err != nil {
return nil, stmt, err
}
Expand Down
108 changes: 66 additions & 42 deletions influxql/ast.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1482,7 +1482,7 @@ func (s *SelectStatement) RewriteTimeCondition(now time.Time) error {
if err != nil {
return err
} else if interval > 0 && s.Condition != nil {
_, tmax, err := TimeRange(s.Condition)
_, tmax, err := TimeRange(s.Condition, s.Location)
if err != nil {
return err
}
Expand Down Expand Up @@ -3778,19 +3778,23 @@ func (l *StringLiteral) IsTimeLiteral() bool {
}

// ToTimeLiteral returns a time literal if this string can be converted to a time literal.
func (l *StringLiteral) ToTimeLiteral() (*TimeLiteral, error) {
func (l *StringLiteral) ToTimeLiteral(loc *time.Location) (*TimeLiteral, error) {
if loc == nil {
loc = time.UTC
}

if isDateTimeString(l.Val) {
t, err := time.Parse(DateTimeFormat, l.Val)
t, err := time.ParseInLocation(DateTimeFormat, l.Val, loc)
if err != nil {
// try to parse it as an RFCNano time
t, err = time.Parse(time.RFC3339Nano, l.Val)
t, err = time.ParseInLocation(time.RFC3339Nano, l.Val, loc)
if err != nil {
return nil, ErrInvalidTime
}
}
return &TimeLiteral{Val: t}, nil
} else if isDateString(l.Val) {
t, err := time.Parse(DateFormat, l.Val)
t, err := time.ParseInLocation(DateFormat, l.Val, loc)
if err != nil {
return nil, ErrInvalidTime
}
Expand Down Expand Up @@ -4040,7 +4044,7 @@ func OnlyTimeExpr(expr Expr) bool {

// TimeRange returns the minimum and maximum times specified by an expression.
// It returns zero times if there is no bound.
func TimeRange(expr Expr) (min, max time.Time, err error) {
func TimeRange(expr Expr, loc *time.Location) (min, max time.Time, err error) {
WalkFunc(expr, func(n Node) {
if err != nil {
return
Expand All @@ -4052,11 +4056,11 @@ func TimeRange(expr Expr) (min, max time.Time, err error) {
// Otherwise check for for the right-hand side and flip the operator.
op := n.Op
var value time.Time
value, err = timeExprValue(n.LHS, n.RHS)
value, err = timeExprValue(n.LHS, n.RHS, loc)
if err != nil {
return
} else if value.IsZero() {
if value, err = timeExprValue(n.RHS, n.LHS); value.IsZero() || err != nil {
if value, err = timeExprValue(n.RHS, n.LHS, loc); value.IsZero() || err != nil {
return
} else if op == LT {
op = GT
Expand Down Expand Up @@ -4105,7 +4109,7 @@ func TimeRange(expr Expr) (min, max time.Time, err error) {
// an expression. If there is no lower bound, the minimum time is returned
// for minimum. If there is no higher bound, the maximum time is returned.
func TimeRangeAsEpochNano(expr Expr) (min, max int64, err error) {
tmin, tmax, err := TimeRange(expr)
tmin, tmax, err := TimeRange(expr, nil)
if err != nil {
return 0, 0, err
}
Expand All @@ -4125,12 +4129,12 @@ func TimeRangeAsEpochNano(expr Expr) (min, max int64, err error) {

// 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) (t time.Time, err error) {
func timeExprValue(ref Expr, lit Expr, loc *time.Location) (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 strlit.IsTimeLiteral() {
t, err := strlit.ToTimeLiteral()
t, err := strlit.ToTimeLiteral(loc)
if err != nil {
return time.Time{}, err
}
Expand Down Expand Up @@ -4797,6 +4801,11 @@ func reduceBinaryExpr(expr *BinaryExpr, valuer Valuer) Expr {
lhs := reduce(expr.LHS, valuer)
rhs := reduce(expr.RHS, valuer)

loc := time.UTC
if v, ok := valuer.(ZoneValuer); ok {
loc = v.Zone()
}

// Do not evaluate if one side is nil.
if lhs == nil || rhs == nil {
return &BinaryExpr{LHS: lhs, RHS: rhs, Op: expr.Op}
Expand Down Expand Up @@ -4827,17 +4836,17 @@ func reduceBinaryExpr(expr *BinaryExpr, valuer Valuer) Expr {
case *BooleanLiteral:
return reduceBinaryExprBooleanLHS(op, lhs, rhs)
case *DurationLiteral:
return reduceBinaryExprDurationLHS(op, lhs, rhs)
return reduceBinaryExprDurationLHS(op, lhs, rhs, loc)
case *IntegerLiteral:
return reduceBinaryExprIntegerLHS(op, lhs, rhs)
return reduceBinaryExprIntegerLHS(op, lhs, rhs, loc)
case *nilLiteral:
return reduceBinaryExprNilLHS(op, lhs, rhs)
case *NumberLiteral:
return reduceBinaryExprNumberLHS(op, lhs, rhs)
case *StringLiteral:
return reduceBinaryExprStringLHS(op, lhs, rhs)
return reduceBinaryExprStringLHS(op, lhs, rhs, loc)
case *TimeLiteral:
return reduceBinaryExprTimeLHS(op, lhs, rhs)
return reduceBinaryExprTimeLHS(op, lhs, rhs, loc)
default:
return &BinaryExpr{Op: op, LHS: lhs, RHS: rhs}
}
Expand Down Expand Up @@ -4868,7 +4877,7 @@ func reduceBinaryExprBooleanLHS(op Token, lhs *BooleanLiteral, rhs Expr) Expr {
return &BinaryExpr{Op: op, LHS: lhs, RHS: rhs}
}

func reduceBinaryExprDurationLHS(op Token, lhs *DurationLiteral, rhs Expr) Expr {
func reduceBinaryExprDurationLHS(op Token, lhs *DurationLiteral, rhs Expr, loc *time.Location) Expr {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind of sucks that this bleeds into the some of the binary expression reducing. Guess it has to though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I didn't like this. Might be improved in the future with some better parsing of the expression.

switch rhs := rhs.(type) {
case *DurationLiteral:
switch op {
Expand Down Expand Up @@ -4915,11 +4924,11 @@ func reduceBinaryExprDurationLHS(op Token, lhs *DurationLiteral, rhs Expr) Expr
return &TimeLiteral{Val: rhs.Val.Add(lhs.Val)}
}
case *StringLiteral:
t, err := rhs.ToTimeLiteral()
t, err := rhs.ToTimeLiteral(loc)
if err != nil {
break
}
expr := reduceBinaryExprDurationLHS(op, lhs, t)
expr := reduceBinaryExprDurationLHS(op, lhs, t, loc)

// If the returned expression is still a binary expr, that means
// we couldn't reduce it so this wasn't used in a time literal context.
Expand All @@ -4932,7 +4941,7 @@ func reduceBinaryExprDurationLHS(op Token, lhs *DurationLiteral, rhs Expr) Expr
return &BinaryExpr{Op: op, LHS: lhs, RHS: rhs}
}

func reduceBinaryExprIntegerLHS(op Token, lhs *IntegerLiteral, rhs Expr) Expr {
func reduceBinaryExprIntegerLHS(op Token, lhs *IntegerLiteral, rhs Expr, loc *time.Location) Expr {
switch rhs := rhs.(type) {
case *NumberLiteral:
return reduceBinaryExprNumberLHS(op, &NumberLiteral{Val: float64(lhs.Val)}, rhs)
Expand Down Expand Up @@ -4983,17 +4992,17 @@ func reduceBinaryExprIntegerLHS(op Token, lhs *IntegerLiteral, rhs Expr) Expr {
}
case *TimeLiteral:
d := &DurationLiteral{Val: time.Duration(lhs.Val)}
expr := reduceBinaryExprDurationLHS(op, d, rhs)
expr := reduceBinaryExprDurationLHS(op, d, rhs, loc)
if _, ok := expr.(*BinaryExpr); !ok {
return expr
}
case *StringLiteral:
t, err := rhs.ToTimeLiteral()
t, err := rhs.ToTimeLiteral(loc)
if err != nil {
break
}
d := &DurationLiteral{Val: time.Duration(lhs.Val)}
expr := reduceBinaryExprDurationLHS(op, d, t)
expr := reduceBinaryExprDurationLHS(op, d, t, loc)
if _, ok := expr.(*BinaryExpr); !ok {
return expr
}
Expand Down Expand Up @@ -5075,7 +5084,7 @@ func reduceBinaryExprNumberLHS(op Token, lhs *NumberLiteral, rhs Expr) Expr {
return &BinaryExpr{Op: op, LHS: lhs, RHS: rhs}
}

func reduceBinaryExprStringLHS(op Token, lhs *StringLiteral, rhs Expr) Expr {
func reduceBinaryExprStringLHS(op Token, lhs *StringLiteral, rhs Expr, loc *time.Location) Expr {
switch rhs := rhs.(type) {
case *StringLiteral:
switch op {
Expand All @@ -5086,17 +5095,17 @@ func reduceBinaryExprStringLHS(op Token, lhs *StringLiteral, rhs Expr) Expr {
// could be a different result if they use different formats
// for the same time.
if lhs.IsTimeLiteral() && rhs.IsTimeLiteral() {
tlhs, err := lhs.ToTimeLiteral()
tlhs, err := lhs.ToTimeLiteral(loc)
if err != nil {
return expr
}

trhs, err := rhs.ToTimeLiteral()
trhs, err := rhs.ToTimeLiteral(loc)
if err != nil {
return expr
}

t := reduceBinaryExprTimeLHS(op, tlhs, trhs)
t := reduceBinaryExprTimeLHS(op, tlhs, trhs, loc)
if _, ok := t.(*BinaryExpr); !ok {
expr = t
}
Expand All @@ -5109,17 +5118,17 @@ func reduceBinaryExprStringLHS(op Token, lhs *StringLiteral, rhs Expr) Expr {
// could be a different result if they use different formats
// for the same time.
if lhs.IsTimeLiteral() && rhs.IsTimeLiteral() {
tlhs, err := lhs.ToTimeLiteral()
tlhs, err := lhs.ToTimeLiteral(loc)
if err != nil {
return expr
}

trhs, err := rhs.ToTimeLiteral()
trhs, err := rhs.ToTimeLiteral(loc)
if err != nil {
return expr
}

t := reduceBinaryExprTimeLHS(op, tlhs, trhs)
t := reduceBinaryExprTimeLHS(op, tlhs, trhs, loc)
if _, ok := t.(*BinaryExpr); !ok {
expr = t
}
Expand All @@ -5129,11 +5138,11 @@ func reduceBinaryExprStringLHS(op Token, lhs *StringLiteral, rhs Expr) Expr {
return &StringLiteral{Val: lhs.Val + rhs.Val}
default:
// Attempt to convert the string literal to a time literal.
t, err := lhs.ToTimeLiteral()
t, err := lhs.ToTimeLiteral(loc)
if err != nil {
break
}
expr := reduceBinaryExprTimeLHS(op, t, rhs)
expr := reduceBinaryExprTimeLHS(op, t, rhs, loc)

// If the returned expression is still a binary expr, that means
// we couldn't reduce it so this wasn't used in a time literal context.
Expand All @@ -5143,11 +5152,11 @@ func reduceBinaryExprStringLHS(op Token, lhs *StringLiteral, rhs Expr) Expr {
}
case *DurationLiteral:
// Attempt to convert the string literal to a time literal.
t, err := lhs.ToTimeLiteral()
t, err := lhs.ToTimeLiteral(loc)
if err != nil {
break
}
expr := reduceBinaryExprTimeLHS(op, t, rhs)
expr := reduceBinaryExprTimeLHS(op, t, rhs, loc)

// If the returned expression is still a binary expr, that means
// we couldn't reduce it so this wasn't used in a time literal context.
Expand All @@ -5156,11 +5165,11 @@ func reduceBinaryExprStringLHS(op Token, lhs *StringLiteral, rhs Expr) Expr {
}
case *TimeLiteral:
// Attempt to convert the string literal to a time literal.
t, err := lhs.ToTimeLiteral()
t, err := lhs.ToTimeLiteral(loc)
if err != nil {
break
}
expr := reduceBinaryExprTimeLHS(op, t, rhs)
expr := reduceBinaryExprTimeLHS(op, t, rhs, loc)

// If the returned expression is still a binary expr, that means
// we couldn't reduce it so this wasn't used in a time literal context.
Expand All @@ -5169,11 +5178,11 @@ func reduceBinaryExprStringLHS(op Token, lhs *StringLiteral, rhs Expr) Expr {
}
case *IntegerLiteral:
// Attempt to convert the string literal to a time literal.
t, err := lhs.ToTimeLiteral()
t, err := lhs.ToTimeLiteral(loc)
if err != nil {
break
}
expr := reduceBinaryExprTimeLHS(op, t, rhs)
expr := reduceBinaryExprTimeLHS(op, t, rhs, loc)

// If the returned expression is still a binary expr, that means
// we couldn't reduce it so this wasn't used in a time literal context.
Expand All @@ -5189,7 +5198,7 @@ func reduceBinaryExprStringLHS(op Token, lhs *StringLiteral, rhs Expr) Expr {
return &BinaryExpr{Op: op, LHS: lhs, RHS: rhs}
}

func reduceBinaryExprTimeLHS(op Token, lhs *TimeLiteral, rhs Expr) Expr {
func reduceBinaryExprTimeLHS(op Token, lhs *TimeLiteral, rhs Expr, loc *time.Location) Expr {
switch rhs := rhs.(type) {
case *DurationLiteral:
switch op {
Expand All @@ -5200,7 +5209,7 @@ func reduceBinaryExprTimeLHS(op Token, lhs *TimeLiteral, rhs Expr) Expr {
}
case *IntegerLiteral:
d := &DurationLiteral{Val: time.Duration(rhs.Val)}
expr := reduceBinaryExprTimeLHS(op, lhs, d)
expr := reduceBinaryExprTimeLHS(op, lhs, d, loc)
if _, ok := expr.(*BinaryExpr); !ok {
return expr
}
Expand All @@ -5222,11 +5231,11 @@ func reduceBinaryExprTimeLHS(op Token, lhs *TimeLiteral, rhs Expr) Expr {
return &BooleanLiteral{Val: lhs.Val.Before(rhs.Val) || lhs.Val.Equal(rhs.Val)}
}
case *StringLiteral:
t, err := rhs.ToTimeLiteral()
t, err := rhs.ToTimeLiteral(loc)
if err != nil {
break
}
expr := reduceBinaryExprTimeLHS(op, lhs, t)
expr := reduceBinaryExprTimeLHS(op, lhs, t, loc)

// If the returned expression is still a binary expr, that means
// we couldn't reduce it so this wasn't used in a time literal context.
Expand Down Expand Up @@ -5300,9 +5309,16 @@ type Valuer interface {
Value(key string) (interface{}, bool)
}

// ZoneValuer is the interface that specifies the current time zone.
type ZoneValuer interface {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need this interface? Seems like the only place this gets used is

+	loc := time.UTC
 +	if v, ok := valuer.(ZoneValuer); ok {
 +		loc = v.Zone()
 +	}

Is there any anticipation that there will be anything else that implements ZoneValuer?

I guess I've never quite understood why people use create an interface (it seems pretty common) for something where a concrete type would suffice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because this is an extension of the Valuer interface and not all Valuers can have a Zone method.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But you could just as easily casted to a NowValuer

// Zone returns the time zone location.
Zone() *time.Location
}

// NowValuer returns only the value for "now()".
type NowValuer struct {
Now time.Time
Now time.Time
Location *time.Location
}

// Value is a method that returns the value and existence flag for a given key.
Expand All @@ -5313,6 +5329,14 @@ func (v *NowValuer) Value(key string) (interface{}, bool) {
return nil, false
}

// Zone is a method that returns the time.Location.
func (v *NowValuer) Zone() *time.Location {
if v.Location != nil {
return v.Location
}
return time.UTC
}

// ContainsVarRef returns true if expr is a VarRef or contains one.
func ContainsVarRef(expr Expr) bool {
var v containsVarRefVisitor
Expand Down
Loading