Skip to content

Commit

Permalink
duration: fix some small duration formatting issues
Browse files Browse the repository at this point in the history
Release note (bug fix): Postgres style intervals now print a `+` sign
for day units if the year/month unit preceding was negative
(e.g. `-1 year -2 months 2 days` will now print as
`-1 year -2 months +2 days`).

Release note (bug fix): SQL Standard intervals will omit the day value
if the day value is 0
  • Loading branch information
otan committed Jul 13, 2021
1 parent 21d36a3 commit 8057ef7
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 24 deletions.
2 changes: 1 addition & 1 deletion pkg/cli/sql_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ func (c *sqlConn) getLastQueryStatistics() (
}

// This should really be the same as the session's IntervalStyle
// but that only effect negative intervals in the magnitude
// but that only effects negative intervals in the magnitude
// of days - and all these latencies should be positive.
parsedExecLatency, _ := tree.ParseDInterval(duration.IntervalStyle_POSTGRES, execLatencyRaw)
parsedServiceLatency, _ := tree.ParseDInterval(duration.IntervalStyle_POSTGRES, serviceLatencyRaw)
Expand Down
18 changes: 8 additions & 10 deletions pkg/sql/logictest/testdata/logic_test/interval
Original file line number Diff line number Diff line change
Expand Up @@ -417,48 +417,46 @@ INSERT INTO interval_parsing VALUES
query T
SELECT i FROM intervals ORDER BY pk
----
-2 years -11 mons 1 day +04:05:06.123
-2 years -11 mons +1 day 04:05:06.123
1 day 04:06:08.123
2 years 11 mons -2 days +03:25:45.678

query T
SELECT array_to_string(array_agg(i ORDER BY pk), ' ') FROM intervals
----
-2 years -11 mons 1 day +04:05:06.123
1 day 04:06:08.123
2 years 11 mons -2 days +03:25:45.678
-2 years -11 mons +1 day 04:05:06.123 1 day 04:06:08.123 2 years 11 mons -2 days +03:25:45.678

query T
SELECT (array_agg(i ORDER BY pk))::string FROM intervals
----
{"-2 years -11 mons 1 day +04:05:06.123","1 day 04:06:08.123","2 years 11 mons -2 days +03:25:45.678"}
{"-2 years -11 mons +1 day 04:05:06.123","1 day 04:06:08.123","2 years 11 mons -2 days +03:25:45.678"}

query T
SELECT i::string FROM intervals ORDER BY pk
----
-2 years -11 mons 1 day +04:05:06.123
-2 years -11 mons +1 day 04:05:06.123
1 day 04:06:08.123
2 years 11 mons -2 days +03:25:45.678

query T
SELECT (i,) FROM intervals ORDER BY pk
----
("-2 years -11 mons 1 day +04:05:06.123")
("-2 years -11 mons +1 day 04:05:06.123")
("1 day 04:06:08.123")
("2 years 11 mons -2 days +03:25:45.678")

query T
SELECT row_to_json(intervals) FROM intervals ORDER BY pk
----
{"i": "-2 years -11 mons 1 day +04:05:06.123", "pk": 1}
{"i": "-2 years -11 mons +1 day 04:05:06.123", "pk": 1}
{"i": "1 day 04:06:08.123", "pk": 2}
{"i": "2 years 11 mons -2 days +03:25:45.678", "pk": 3}

query TT
SELECT i, i::INTERVAL FROM interval_parsing ORDER BY pk
----
-10 years 22 months 1 day 01:02:03 -8 years -2 mons 1 day +01:02:03
-10 years -22 months 1 day 01:02:03 -11 years -10 mons 1 day +01:02:03
-10 years 22 months 1 day 01:02:03 -8 years -2 mons +1 day 01:02:03
-10 years -22 months 1 day 01:02:03 -11 years -10 mons +1 day 01:02:03
-10 years 22 months -1 day 01:02:03 -8 years -2 mons -1 days +01:02:03
-10 years 22 months -1 day -01:02:03 -8 years -2 mons -1 days -01:02:03

Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/sem/tree/interval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func TestValidSQLIntervalSyntax(t *testing.T) {
{input: `1- 4:08:23`, output: `1 year 04:08:23`},
{input: `0-2 3 4:08`, output: `2 mons 3 days 04:08:00`},
{input: `1- 3 4:08:`, output: `1 year 3 days 04:08:00`},
{input: `-1- 3 4:08:`, output: `-1 years 3 days +04:08:00`},
{input: `-1- 3 4:08:`, output: `-1 years +3 days 04:08:00`},
{input: `0- 3 4:08`, output: `3 days 04:08:00`},
{input: `-0- 3 4:08`, output: `3 days 04:08:00`},
{input: `-0- -0 4:08`, output: `04:08:00`},
Expand Down Expand Up @@ -406,7 +406,7 @@ func TestPGIntervalSyntax(t *testing.T) {
{input: `1 day -12:30`, output: `1 day -12:30:00`},
{input: `1 day -00:30`, output: `1 day -00:30:00`},
{input: `1 day -00:00:30`, output: `1 day -00:00:30`},
{input: `-1 day +00:00:30`, output: `-1 days +00:00:30`, outputSQLStandard: `-1 days -00:00:30`},
{input: `-1 day +00:00:30`, output: `-1 days +00:00:30`},
{input: `2 days -4:08.1234`, output: `2 days -00:04:08.1234`},
{input: `2 days -4:08`, itm: minuteToSecondITM, output: `2 days -00:04:08`},
{input: `1 day 12:30.5`, output: `1 day 00:12:30.5`},
Expand Down
49 changes: 39 additions & 10 deletions pkg/util/duration/duration.go
Original file line number Diff line number Diff line change
Expand Up @@ -410,20 +410,28 @@ func (d Duration) encodePostgres(buf *bytes.Buffer) {
return true
}

negDays := d.Months < 0 || d.Days < 0
if absGE(d.Months, 12) {
years := d.Months / 12
months := d.Months
if absGE(months, 12) {
years := months / 12
wrote = wrotePrev(wrote, buf)
fmt.Fprintf(buf, "%d year%s", years, isPlural(years))
d.Months %= 12
months %= 12
}
if d.Months != 0 {
if months != 0 {
wrote = wrotePrev(wrote, buf)
fmt.Fprintf(buf, "%d mon%s", d.Months, isPlural(d.Months))
fmt.Fprintf(buf, "%d mon%s", months, isPlural(months))
}

// Keep track of whether the previous unit was negative.
// If so, and the next one is positive, add `+` in front of the positive unit.
wasNegative := d.Months < 0
if d.Days != 0 {
wrote = wrotePrev(wrote, buf)
if wasNegative && d.Days > 0 {
buf.WriteString("+")
}
fmt.Fprintf(buf, "%d day%s", d.Days, isPlural(d.Days))
wasNegative = d.Days < 0
}

if d.nanos == 0 {
Expand All @@ -434,7 +442,7 @@ func (d Duration) encodePostgres(buf *bytes.Buffer) {

if d.nanos/nanosInMicro < 0 {
buf.WriteString("-")
} else if negDays {
} else if wasNegative {
buf.WriteString("+")
}

Expand All @@ -453,6 +461,9 @@ func (d Duration) encodeSQLStandard(buf *bytes.Buffer) {
hasYearMonth := d.Months != 0
hasDayTime := d.Days != 0 || d.nanos != 0
hasDay := d.Days != 0

// A SQL Standard value always has the same sign, and must only
// have Year-Month XOR (Days and/or Time) values.
sqlStandardValue := !(hasNegative && hasPositive) &&
!(hasYearMonth && hasDayTime)

Expand Down Expand Up @@ -499,9 +510,27 @@ func (d Duration) encodeSQLStandard(buf *bytes.Buffer) {
seconds = -seconds
micros = -micros
}
fmt.Fprintf(buf, "%c%d-%d %c%d %c%d:%02d:",
yearSign, years, months, daySign, days, secSign, hours, minutes)
writeSecondsMicroseconds(buf, seconds, micros)
needSpace := false
if hasYearMonth {
fmt.Fprintf(
buf,
"%c%d-%d",
yearSign, years, months,
)
needSpace = true
}
if hasDayTime {
if needSpace {
buf.WriteString(" ")
}
fmt.Fprintf(
buf,
"%c%d %c%d:%02d:",
daySign, days,
secSign, hours, minutes,
)
writeSecondsMicroseconds(buf, seconds, micros)
}
case hasYearMonth:
fmt.Fprintf(buf, "%d-%d", years, months)
case hasDay:
Expand Down
45 changes: 44 additions & 1 deletion pkg/util/duration/duration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,7 @@ func TestFormat(t *testing.T) {
sqlStandard: "0",
},
// all negative

// plural
{
duration: Duration{Months: -35, Days: -2, nanos: -12345678000000},
Expand Down Expand Up @@ -714,7 +715,9 @@ func TestFormat(t *testing.T) {
iso8601: "P-2Y-11MT-0.001S",
sqlStandard: "-2-11 +0 -0:00:00.001",
},

// all positive

// plural
{
duration: Duration{Months: 35, Days: 2, nanos: 12345678000000},
Expand Down Expand Up @@ -764,12 +767,14 @@ func TestFormat(t *testing.T) {
iso8601: "P2Y11MT0.001S",
sqlStandard: "+2-11 +0 +0:00:00.001",
},

// mixed positive and negative units

// PG prints '+' when a time unit changes the sign compared to the previous
// unit, i.e. below CRDB should print +2 days (in 'postgres' style).
{
duration: Duration{Months: -35, Days: 2, nanos: -12345678000000},
postgres: "-2 years -11 mons 2 days -03:25:45.678",
postgres: "-2 years -11 mons +2 days -03:25:45.678",
iso8601: "P-2Y-11M2DT-3H-25M-45.678S",
sqlStandard: "-2-11 +2 -3:25:45.678",
},
Expand All @@ -779,12 +784,50 @@ func TestFormat(t *testing.T) {
iso8601: "P2Y11M-2DT3H25M45.678S",
sqlStandard: "+2-11 -2 +3:25:45.678",
},
{
duration: Duration{Months: -35, Days: 2, nanos: 12345678000000},
postgres: "-2 years -11 mons +2 days 03:25:45.678",
iso8601: "P-2Y-11M2DT3H25M45.678S",
sqlStandard: "-2-11 +2 +3:25:45.678",
},
// no year/month
{
duration: Duration{Days: -2, nanos: 12345678000000},
postgres: "-2 days +03:25:45.678",
iso8601: "P-2DT3H25M45.678S",
sqlStandard: "-2 +3:25:45.678",
},
// no days
{
duration: Duration{Months: -35, nanos: 12345678000000},
postgres: "-2 years -11 mons +03:25:45.678",
iso8601: "P-2Y-11MT3H25M45.678S",
sqlStandard: "-2-11 +0 +3:25:45.678",
},
{
duration: Duration{Days: -1, nanos: -123456789000000},
postgres: "-1 days -34:17:36.789",
iso8601: "P-1DT-34H-17M-36.789S",
sqlStandard: "-1 34:17:36.789",
},
{
duration: Duration{Months: -12, Days: 3, nanos: -12345678000000},
postgres: "-1 years +3 days -03:25:45.678",
iso8601: "P-1Y3DT-3H-25M-45.678S",
sqlStandard: "-1-0 +3 -3:25:45.678",
},
{
duration: Duration{Months: -12, Days: -3, nanos: 12345678000000},
postgres: "-1 years -3 days +03:25:45.678",
iso8601: "P-1Y-3DT3H25M45.678S",
sqlStandard: "-1-0 -3 +3:25:45.678",
},
{
duration: Duration{Months: -12, Days: -3, nanos: 12345678000000},
postgres: "-1 years -3 days +03:25:45.678",
iso8601: "P-1Y-3DT3H25M45.678S",
sqlStandard: "-1-0 -3 +3:25:45.678",
},
}

for _, tt := range tests {
Expand Down

0 comments on commit 8057ef7

Please sign in to comment.