Skip to content

Commit

Permalink
Merge #65063
Browse files Browse the repository at this point in the history 
65063: pgwire: only show TZ minutes offset if non-zero r=otan a=rafiss

Similar to #57265, this commit matches the PG behavior to
only show the minutes offset if it is non-zero.

Release note (bug fix): Minute timezone offsets are only displayed in
the wire protocol if they are non-zero for TimestampTZ and TimeTZ values.
Previously, they would always display.

Release note (bug fix): Binary TimeTZ values were not being decoded
correctly when being sent as a parameter in the wire protocol. This
is now fixed.

Release note (cli change): The SQL shell now formats times with time
zones so that the minutes and seconds offsets are only shown if they are
non-zero. Also, infinite floating point values are formatted as
`Infinity` rather than `Inf` now.

Co-authored-by: Rafi Shamim <[email protected]>
  • Loading branch information
@rafiss
craig[bot] and rafiss committed May 17, 2021
2 parents c8a62f2 + c08ee66 commit adb98c2
Show file tree
Hide file tree
Showing 8 changed files with 237 additions and 23 deletions.
17 changes: 11 additions & 6 deletions pkg/cli/cli_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,9 @@ func Example_sql_format() {
c.RunWithArgs([]string{"sql", "-e", "create database t; create table t.times (bare timestamp, withtz timestamptz)"})
c.RunWithArgs([]string{"sql", "-e", "insert into t.times values ('2016-01-25 10:10:10', '2016-01-25 10:10:10-05:00')"})
c.RunWithArgs([]string{"sql", "-e", "select bare from t.times; select withtz from t.times"})
c.RunWithArgs([]string{"sql", "-e", "select '2021-03-20'::date; select '01:01'::time; select '01:01'::timetz"})
c.RunWithArgs([]string{"sql", "-e", "select (1/3.0)::real; select (1/3.0)::double precision"})
c.RunWithArgs([]string{"sql", "-e",
"select '2021-03-20'::date; select '01:01'::time; select '01:01'::timetz; select '01:01+02:02'::timetz"})
c.RunWithArgs([]string{"sql", "-e", "select (1/3.0)::real; select (1/3.0)::double precision; select '-inf'::float8"})

// Output:
// sql -e create database t; create table t.times (bare timestamp, withtz timestamptz)
Expand All @@ -299,19 +300,23 @@ func Example_sql_format() {
// bare
// 2016-01-25 10:10:10
// withtz
// 2016-01-25 15:10:10+00:00:00
// sql -e select '2021-03-20'::date; select '01:01'::time; select '01:01'::timetz
// 2016-01-25 15:10:10+00
// sql -e select '2021-03-20'::date; select '01:01'::time; select '01:01'::timetz; select '01:01+02:02'::timetz
// date
// 2021-03-20
// time
// 01:01:00
// timetz
// 01:01:00+00:00:00
// sql -e select (1/3.0)::real; select (1/3.0)::double precision
// 01:01:00+00
// timetz
// 01:01:00+02:02
// sql -e select (1/3.0)::real; select (1/3.0)::double precision; select '-inf'::float8
// float4
// 0.33333334
// float8
// 0.3333333333333333
// float8
// -Infinity
}

func Example_sql_column_labels() {
Expand Down
15 changes: 14 additions & 1 deletion pkg/cli/sql_util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"database/sql/driver"
"fmt"
"io"
"math"
"net/url"
"reflect"
"strconv"
Expand Down Expand Up @@ -1134,6 +1135,11 @@ func formatVal(
if colType == "FLOAT4" {
width = 32
}
if math.IsInf(t, 1) {
return "Infinity"
} else if math.IsInf(t, -1) {
return "-Infinity"
}
return strconv.FormatFloat(t, 'g', -1, width)

case string:
Expand Down Expand Up @@ -1178,6 +1184,13 @@ func formatVal(
// Some unknown/new time-like format.
tfmt = timeutil.FullTimeFormat
}
if tfmt == timeutil.TimestampWithTZFormat || tfmt == timeutil.TimeWithTZFormat {
if _, offsetSeconds := t.Zone(); offsetSeconds%60 != 0 {
tfmt += ":00:00"
} else if offsetSeconds%3600 != 0 {
tfmt += ":00"
}
}
return t.Format(tfmt)
}

Expand All @@ -1186,7 +1199,7 @@ func formatVal(

var timeOutputFormats = map[string]string{
"TIMESTAMP": timeutil.TimestampWithoutTZFormat,
"TIMESTAMPTZ": timeutil.FullTimeFormat,
"TIMESTAMPTZ": timeutil.TimestampWithTZFormat,
"TIME": timeutil.TimeWithoutTZFormat,
"TIMETZ": timeutil.TimeWithTZFormat,
"DATE": timeutil.DateFormat,
Expand Down
18 changes: 16 additions & 2 deletions pkg/cmd/generate-binary/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,6 @@ var inputs = map[string][]string{
"9004-10-19 10:23:54",
},

/* TODO(mjibson): fix these; there's a slight timezone display difference
"'%s'::timestamptz": {
"1999-01-08 04:05:06+00",
"1999-01-08 04:05:06+00:00",
Expand All @@ -317,7 +316,22 @@ var inputs = map[string][]string{
"4004-10-19 10:23:54",
"9004-10-19 10:23:54",
},
*/

"'%s'::timetz": {
"04:05:06+00",
"04:05:06+00:00",
"04:05:06+10",
"04:05:06+10:00",
"04:05:06+10:30",
"04:05:06",
"10:23:54",
"00:00:00",
"10:23:54",
"10:23:54 BC",
"10:23:54",
"10:23:54+1:2:3",
"10:23:54+1:2",
},

"'%s'::date": {
"1999-01-08",
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/pgwire/pgwirebase/encoding.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ func DecodeDatum(
return nil, pgerror.Newf(pgcode.Syntax, "timetz requires 12 bytes for binary format")
}
timeOfDayMicros := int64(binary.BigEndian.Uint64(b))
offsetSecs := int32(binary.BigEndian.Uint32(b))
offsetSecs := int32(binary.BigEndian.Uint32(b[8:]))
return tree.NewDTimeTZFromOffset(timeofday.TimeOfDay(timeOfDayMicros), offsetSecs), nil
case oid.T_interval:
if len(b) < 16 {
Expand Down
175 changes: 175 additions & 0 deletions pkg/sql/pgwire/testdata/encodings.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1700,6 +1700,181 @@
"TextAsBinary": [57, 48, 48, 52, 45, 49, 48, 45, 49, 57, 32, 49, 48, 58, 50, 51, 58, 53, 52],
"Binary": [3, 17, 83, 233, 31, 54, 66, 128]
},
{
"SQL": "'1999-01-08 04:05:06+00'::timestamptz",
"Oid": 1184,
"Text": "1999-01-08 04:05:06+00",
"TextAsBinary": [49, 57, 57, 57, 45, 48, 49, 45, 48, 56, 32, 48, 52, 58, 48, 53, 58, 48, 54, 43, 48, 48],
"Binary": [255, 255, 227, 225, 177, 91, 128, 128]
},
{
"SQL": "'1999-01-08 04:05:06+00:00'::timestamptz",
"Oid": 1184,
"Text": "1999-01-08 04:05:06+00",
"TextAsBinary": [49, 57, 57, 57, 45, 48, 49, 45, 48, 56, 32, 48, 52, 58, 48, 53, 58, 48, 54, 43, 48, 48],
"Binary": [255, 255, 227, 225, 177, 91, 128, 128]
},
{
"SQL": "'1999-01-08 04:05:06+10'::timestamptz",
"Oid": 1184,
"Text": "1999-01-07 18:05:06+00",
"TextAsBinary": [49, 57, 57, 57, 45, 48, 49, 45, 48, 55, 32, 49, 56, 58, 48, 53, 58, 48, 54, 43, 48, 48],
"Binary": [255, 255, 227, 217, 79, 151, 24, 128]
},
{
"SQL": "'1999-01-08 04:05:06+10:00'::timestamptz",
"Oid": 1184,
"Text": "1999-01-07 18:05:06+00",
"TextAsBinary": [49, 57, 57, 57, 45, 48, 49, 45, 48, 55, 32, 49, 56, 58, 48, 53, 58, 48, 54, 43, 48, 48],
"Binary": [255, 255, 227, 217, 79, 151, 24, 128]
},
{
"SQL": "'1999-01-08 04:05:06+10:30'::timestamptz",
"Oid": 1184,
"Text": "1999-01-07 17:35:06+00",
"TextAsBinary": [49, 57, 57, 57, 45, 48, 49, 45, 48, 55, 32, 49, 55, 58, 51, 53, 58, 48, 54, 43, 48, 48],
"Binary": [255, 255, 227, 216, 228, 77, 70, 128]
},
{
"SQL": "'1999-01-08 04:05:06'::timestamptz",
"Oid": 1184,
"Text": "1999-01-08 04:05:06+00",
"TextAsBinary": [49, 57, 57, 57, 45, 48, 49, 45, 48, 56, 32, 48, 52, 58, 48, 53, 58, 48, 54, 43, 48, 48],
"Binary": [255, 255, 227, 225, 177, 91, 128, 128]
},
{
"SQL": "'2004-10-19 10:23:54'::timestamptz",
"Oid": 1184,
"Text": "2004-10-19 10:23:54+00",
"TextAsBinary": [50, 48, 48, 52, 45, 49, 48, 45, 49, 57, 32, 49, 48, 58, 50, 51, 58, 53, 52, 43, 48, 48],
"Binary": [0, 0, 137, 201, 15, 13, 226, 128]
},
{
"SQL": "'0001-01-01 00:00:00'::timestamptz",
"Oid": 1184,
"Text": "0001-01-01 00:00:00+00",
"TextAsBinary": [48, 48, 48, 49, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 48, 48, 43, 48, 48],
"Binary": [255, 31, 226, 255, 197, 156, 96, 0]
},
{
"SQL": "'0004-10-19 10:23:54'::timestamptz",
"Oid": 1184,
"Text": "0004-10-19 10:23:54+00",
"TextAsBinary": [48, 48, 48, 52, 45, 49, 48, 45, 49, 57, 32, 49, 48, 58, 50, 51, 58, 53, 52, 43, 48, 48],
"Binary": [255, 32, 80, 6, 42, 191, 2, 128]
},
{
"SQL": "'0004-10-19 10:23:54 BC'::timestamptz",
"Oid": 1184,
"Text": "0004-10-19 10:23:54+00 BC",
"TextAsBinary": [48, 48, 48, 52, 45, 49, 48, 45, 49, 57, 32, 49, 48, 58, 50, 51, 58, 53, 52, 43, 48, 48, 32, 66, 67],
"Binary": [255, 31, 135, 24, 26, 133, 34, 128]
},
{
"SQL": "'4004-10-19 10:23:54'::timestamptz",
"Oid": 1184,
"Text": "4004-10-19 10:23:54+00",
"TextAsBinary": [52, 48, 48, 52, 45, 49, 48, 45, 49, 57, 32, 49, 48, 58, 50, 51, 58, 53, 52, 43, 48, 48],
"Binary": [0, 224, 195, 139, 243, 92, 194, 128]
},
{
"SQL": "'9004-10-19 10:23:54'::timestamptz",
"Oid": 1184,
"Text": "9004-10-19 10:23:54+00",
"TextAsBinary": [57, 48, 48, 52, 45, 49, 48, 45, 49, 57, 32, 49, 48, 58, 50, 51, 58, 53, 52, 43, 48, 48],
"Binary": [3, 17, 83, 233, 31, 54, 66, 128]
},
{
"SQL": "'04:05:06+00'::timetz",
"Oid": 1266,
"Text": "04:05:06+00",
"TextAsBinary": [48, 52, 58, 48, 53, 58, 48, 54, 43, 48, 48],
"Binary": [0, 0, 0, 3, 108, 139, 192, 128, 0, 0, 0, 0]
},
{
"SQL": "'04:05:06+00:00'::timetz",
"Oid": 1266,
"Text": "04:05:06+00",
"TextAsBinary": [48, 52, 58, 48, 53, 58, 48, 54, 43, 48, 48],
"Binary": [0, 0, 0, 3, 108, 139, 192, 128, 0, 0, 0, 0]
},
{
"SQL": "'04:05:06+10'::timetz",
"Oid": 1266,
"Text": "04:05:06+10",
"TextAsBinary": [48, 52, 58, 48, 53, 58, 48, 54, 43, 49, 48],
"Binary": [0, 0, 0, 3, 108, 139, 192, 128, 255, 255, 115, 96]
},
{
"SQL": "'04:05:06+10:00'::timetz",
"Oid": 1266,
"Text": "04:05:06+10",
"TextAsBinary": [48, 52, 58, 48, 53, 58, 48, 54, 43, 49, 48],
"Binary": [0, 0, 0, 3, 108, 139, 192, 128, 255, 255, 115, 96]
},
{
"SQL": "'04:05:06+10:30'::timetz",
"Oid": 1266,
"Text": "04:05:06+10:30",
"TextAsBinary": [48, 52, 58, 48, 53, 58, 48, 54, 43, 49, 48, 58, 51, 48],
"Binary": [0, 0, 0, 3, 108, 139, 192, 128, 255, 255, 108, 88]
},
{
"SQL": "'04:05:06'::timetz",
"Oid": 1266,
"Text": "04:05:06+00",
"TextAsBinary": [48, 52, 58, 48, 53, 58, 48, 54, 43, 48, 48],
"Binary": [0, 0, 0, 3, 108, 139, 192, 128, 0, 0, 0, 0]
},
{
"SQL": "'10:23:54'::timetz",
"Oid": 1266,
"Text": "10:23:54+00",
"TextAsBinary": [49, 48, 58, 50, 51, 58, 53, 52, 43, 48, 48],
"Binary": [0, 0, 0, 8, 183, 61, 130, 128, 0, 0, 0, 0]
},
{
"SQL": "'00:00:00'::timetz",
"Oid": 1266,
"Text": "00:00:00+00",
"TextAsBinary": [48, 48, 58, 48, 48, 58, 48, 48, 43, 48, 48],
"Binary": [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
},
{
"SQL": "'10:23:54'::timetz",
"Oid": 1266,
"Text": "10:23:54+00",
"TextAsBinary": [49, 48, 58, 50, 51, 58, 53, 52, 43, 48, 48],
"Binary": [0, 0, 0, 8, 183, 61, 130, 128, 0, 0, 0, 0]
},
{
"SQL": "'10:23:54 BC'::timetz",
"Oid": 1266,
"Text": "10:23:54+00",
"TextAsBinary": [49, 48, 58, 50, 51, 58, 53, 52, 43, 48, 48],
"Binary": [0, 0, 0, 8, 183, 61, 130, 128, 0, 0, 0, 0]
},
{
"SQL": "'10:23:54'::timetz",
"Oid": 1266,
"Text": "10:23:54+00",
"TextAsBinary": [49, 48, 58, 50, 51, 58, 53, 52, 43, 48, 48],
"Binary": [0, 0, 0, 8, 183, 61, 130, 128, 0, 0, 0, 0]
},
{
"SQL": "'10:23:54+1:2:3'::timetz",
"Oid": 1266,
"Text": "10:23:54+01:02:03",
"TextAsBinary": [49, 48, 58, 50, 51, 58, 53, 52, 43, 48, 49, 58, 48, 50, 58, 48, 51],
"Binary": [0, 0, 0, 8, 183, 61, 130, 128, 255, 255, 241, 117]
},
{
"SQL": "'10:23:54+1:2'::timetz",
"Oid": 1266,
"Text": "10:23:54+01:02",
"TextAsBinary": [49, 48, 58, 50, 51, 58, 53, 52, 43, 48, 49, 58, 48, 50],
"Binary": [0, 0, 0, 8, 183, 61, 130, 128, 255, 255, 241, 120]
},
{
"SQL": "'{00000000-0000-0000-0000-000000000000}'::uuid[]",
"Oid": 2951,
Expand Down
7 changes: 3 additions & 4 deletions pkg/sql/pgwire/testdata/pgtest/timezone
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,15 @@ ReadyForQuery
{"Type":"CommandComplete","CommandTag":"SELECT 1"}
{"Type":"ReadyForQuery","TxStatus":"I"}

# PostgreSQL does not display seconds offset here, but CockroachDB does.
send crdb_only
send
Query {"String": "SELECT '1882-05-23T00:00:00'::\"timestamptz\""}
----

until crdb_only ignore_data_type_sizes
until ignore_data_type_sizes
ReadyForQuery
----
{"Type":"RowDescription","Fields":[{"Name":"timestamptz","TableOID":0,"TableAttributeNumber":0,"DataTypeOID":1184,"DataTypeSize":0,"TypeModifier":-1,"Format":0}]}
{"Type":"DataRow","Values":[{"text":"1882-05-23 00:00:00+00:00"}]}
{"Type":"DataRow","Values":[{"text":"1882-05-23 00:00:00+00"}]}
{"Type":"CommandComplete","CommandTag":"SELECT 1"}
{"Type":"ReadyForQuery","TxStatus":"I"}

Expand Down
12 changes: 7 additions & 5 deletions pkg/sql/pgwire/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -533,10 +533,10 @@ func (b *writeBuffer) writeBinaryDatum(

const (
pgTimeFormat = "15:04:05.999999"
pgTimeTZFormat = pgTimeFormat + "-07:00"
pgTimeTZFormat = pgTimeFormat + "-07"
pgDateFormat = "2006-01-02"
pgTimeStampFormatNoOffset = pgDateFormat + " " + pgTimeFormat
pgTimeStampFormat = pgTimeStampFormatNoOffset + "-07:00"
pgTimeStampFormat = pgTimeStampFormatNoOffset + "-07"
pgTime2400Format = "24:00:00"
)

Expand All @@ -554,11 +554,11 @@ func formatTime(t timeofday.TimeOfDay, tmp []byte) []byte {
// formatTimeTZ formats t into a format lib/pq understands, appending to the
// provided tmp buffer and reallocating if needed. The function will then return
// the resulting buffer.
// Note it does not understand the "second" component of the offset as lib/pq
// cannot parse it.
func formatTimeTZ(t timetz.TimeTZ, tmp []byte) []byte {
format := pgTimeTZFormat
if t.OffsetSecs%60 != 0 {
format += ":00:00"
} else if t.OffsetSecs%3600 != 0 {
format += ":00"
}
ret := t.ToTime().AppendFormat(tmp, format)
Expand All @@ -577,7 +577,9 @@ func formatTs(t time.Time, offset *time.Location, tmp []byte) (b []byte) {
var format string
if offset != nil {
format = pgTimeStampFormat
if _, offset := t.In(offset).Zone(); offset%60 != 0 {
if _, offsetSeconds := t.In(offset).Zone(); offsetSeconds%60 != 0 {
format += ":00:00"
} else if offsetSeconds%3600 != 0 {
format += ":00"
}
} else {
Expand Down
14 changes: 10 additions & 4 deletions pkg/util/timeutil/timeutil.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,23 @@

package timeutil

// FullTimeFormat is the time format used to display any timestamp
// with date, time and time zone data.
// FullTimeFormat is the time format used to display any unknown timestamp
// type, and always shows the full time zone offset.
const FullTimeFormat = "2006-01-02 15:04:05.999999-07:00:00"

// TimestampWithTZFormat is the time format used to display
// timestamps with a time zone offset. The minutes and seconds
// offsets are only added if they are non-zero.
const TimestampWithTZFormat = "2006-01-02 15:04:05.999999-07"

// TimestampWithoutTZFormat is the time format used to display
// timestamps without a time zone offset.
// timestamps without a time zone offset. The minutes and seconds
// offsets are only added if they are non-zero.
const TimestampWithoutTZFormat = "2006-01-02 15:04:05.999999"

// TimeWithTZFormat is the time format used to display a time
// with a time zone offset.
const TimeWithTZFormat = "15:04:05.999999-07:00:00"
const TimeWithTZFormat = "15:04:05.999999-07"

// TimeWithoutTZFormat is the time format used to display a time
// without a time zone offset.
Expand Down

0 comments on commit adb98c2

Please sign in to comment.