Skip to content

Commit

Permalink
Merge #42781
Browse files Browse the repository at this point in the history
42781: sql: Fix for invalid value for parameter "TimeZone": "GMT-08:00" #41776 r=otan a=droidnoob

Resolves #41776

Previously, the inputs starting with `UTC` and `GMT` would cause error.
Postgres supports inputs like this. So adding support for
these inputs was necessary.
sql: Added a check to convert the strings to offset.
timeutil: added a function to convert input to offset seconds.
Added unit tests. Added a regex to accept the above inputs.
Cleaned the code.

Release note (sql change): `SET TIME ZONE` now accepts inputs
beginning with `GMT` and `UTC`, such as `GMT+5` and
`UTC-3:59`. This was previously unsupported.


@otan 

Co-authored-by: Ananthakrishnan <[email protected]>
  • Loading branch information
craig[bot] and droidnoob committed Nov 27, 2019
2 parents f97dc13 + 3b8d21a commit f3d278e
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 3 deletions.
11 changes: 11 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/timestamp
Original file line number Diff line number Diff line change
Expand Up @@ -192,3 +192,14 @@ query R
SELECT extract(timezone FROM '2001-01-01 13:00:00+01:15'::timestamptz)
----
10800


subtest regression_41776

statement ok
SET TIME ZONE 'GMT+1'

query T
SELECT '2001-01-01 00:00:00'::TIMESTAMP::TIMESTAMPTZ
----
2001-01-01 00:00:00 +0100 +0100
8 changes: 6 additions & 2 deletions pkg/sql/set_var.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,12 @@ func timeZoneVarGetStringVal(
if err1 != nil {
loc, err1 = timeutil.LoadLocation(strings.ToTitle(location))
if err1 != nil {
return "", wrapSetVarError("timezone", values[0].String(),
"cannot find time zone %q: %v", location, err)
var ok bool
offset, ok = timeutil.TimeZoneOffsetStringConversion(location)
if !ok {
return "", wrapSetVarError("timezone", values[0].String(),
"cannot find time zone %q: %v", location, err)
}
}
}
}
Expand Down
49 changes: 48 additions & 1 deletion pkg/util/timeutil/time_zone_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,18 @@ package timeutil

import (
"fmt"
"regexp"
"strconv"
"strings"
"time"
)

const fixedOffsetPrefix string = "fixed offset:"
const (
fixedOffsetPrefix = "fixed offset:"
offsetBoundSecs = 167*60*60 + 59*60
)

var timezoneOffsetRegex = regexp.MustCompile(`(?i)(GMT|UTC)([+-])(\d{1,3}(:[0-5]?\d){0,2})\b$`)

// FixedOffsetTimeZoneToLocation creates a time.Location with a set offset and
// with a name that can be marshaled by crdb between nodes.
Expand Down Expand Up @@ -68,3 +74,44 @@ func ParseFixedOffsetTimeZone(location string) (offset int, origRepr string, suc
}
return offset, strings.TrimSuffix(strings.TrimPrefix(origRepr, "("), ")"), true
}

// TimeZoneOffsetStringConversion converts a time string to offset seconds.
// Supported time zone strings: GMT/UTC±[00:00:00 - 169:59:00].
// Seconds/minutes omittable and is case insensitive.
func TimeZoneOffsetStringConversion(s string) (offset int64, ok bool) {
submatch := timezoneOffsetRegex.FindStringSubmatch(strings.ReplaceAll(s, " ", ""))
if len(submatch) == 0 {
return 0, false
}
prefix := string(submatch[0][3])
timeString := string(submatch[3])

var (
hoursString = "0"
minutesString = "0"
secondsString = "0"
)
if strings.Contains(timeString, ":") {
offsets := strings.Split(timeString, ":")
hoursString, minutesString = offsets[0], offsets[1]
if len(offsets) == 3 {
secondsString = offsets[2]
}
} else {
hoursString = timeString
}

hours, _ := strconv.ParseInt(hoursString, 10, 64)
minutes, _ := strconv.ParseInt(minutesString, 10, 64)
seconds, _ := strconv.ParseInt(secondsString, 10, 64)
offset = (hours * 60 * 60) + (minutes * 60) + seconds

if prefix == "-" {
offset *= -1
}

if offset > offsetBoundSecs || offset < -offsetBoundSecs {
return 0, false
}
return offset, true
}
48 changes: 48 additions & 0 deletions pkg/util/timeutil/time_zone_util_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright 2019 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package timeutil

import "testing"

func TestTimeZoneOffsetStringConversion(t *testing.T) {
testCases := []struct {
timezone string
offsetSecs int64
ok bool
}{
{`GMT+00:00`, 0, true},
{`UTC-1:00:00`, -3600, true},
{`UTC-1:0:00`, -3600, true},
{`UTC+15:59:0`, 57540, true},
{` GMT +167:59:00 `, 604740, true},
{`GMT-15:59:59`, -57599, true},
{`GMT-06:59`, -25140, true},
{`GMT+167:59:00`, 604740, true},
{`GMT+ 167: 59:0`, 604740, true},
{`GMT+5`, 18000, true},
{`UTC+5:9`, 5*60*60 + 9*60, true},
{`UTC-5:9:1`, -(5*60*60 + 9*60 + 1), true},
{`GMT-15:59:5Z`, 0, false},
{`GMT-15:99:1`, 0, false},
{`GMT+6:`, 0, false},
{`GMT-6:00:`, 0, false},
{`GMT+168:00:00`, 0, false},
{`GMT-170:00:59`, 0, false},
}

for i, testCase := range testCases {
offset, ok := TimeZoneOffsetStringConversion(testCase.timezone)
if offset != testCase.offsetSecs || ok != testCase.ok {
t.Errorf("%d: Expected offset: %d, success: %v for time %s, but got offset: %d, success: %v",
i, testCase.offsetSecs, testCase.ok, testCase.timezone, offset, ok)
}
}
}

0 comments on commit f3d278e

Please sign in to comment.