Skip to content

Commit 738a0a8

Browse files
robsdeduderobsdedude
committed
Fix and unify Duration's string representation
1 parent ed3e3bc commit 738a0a8

File tree

3 files changed

+270
-27
lines changed

3 files changed

+270
-27
lines changed

‎neo4j/dbtype/temporal.go‎

Lines changed: 108 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package dbtype
1919

2020
import (
2121
"fmt"
22+
"strings"
2223
"time"
2324
)
2425

@@ -90,26 +91,121 @@ type Duration struct {
9091
Nanos int
9192
}
9293

94+
func divMod(a, b int64) (int64, int64) {
95+
return a / b, a % b
96+
}
97+
98+
// sign returns -1 for negative numbers and 1 for positive numbers (including 0)
99+
func sign(x int64) int64 {
100+
/*
101+
* x >> 63: only keep sign bit (0...01 for negative, 0...0 for positive)
102+
* -1: "copy" sign bit to everywhere: 0...0 for negative, 1...1 for positive
103+
* ^: flip all bits: 1...1 for negative, 0...0 for positive
104+
* |1: set LSB: 1...1 (=-1) for negative, 0...1 (=1) for positive
105+
*/
106+
107+
/*
108+
* x >> 63: only keep sign bit (0...01 for negative, 0...0 for positive)
109+
* -1: "copy" sign bit to everywhere: 0...0 for negative, 1...1 for positive
110+
* ^: flip all bits: 1...1 for negative, 0...0 for positive
111+
* |1: set LSB: 1...1 (=-1) for negative, 0...1 (=1) for positive
112+
*/
113+
// This bit-juggling implementation tends to be ever so slightly faster than
114+
// the equivalent branching implementation:
115+
//
116+
// if x < 0 {
117+
// return -1
118+
// }
119+
// return 1
120+
//
121+
// In the context of string formating this is likely negligible, yet a fun exercise ;)
122+
return int64(^(uint64(x)>>63 - 1) | 1)
123+
}
124+
93125
// String returns the string representation of this Duration in ISO-8601 compliant form.
94126
func (d Duration) String() string {
95-
sign := ""
96-
if d.Seconds < 0 && d.Nanos > 0 {
97-
d.Seconds++
98-
d.Nanos = int(time.Second) - d.Nanos
127+
hasTime := d.Seconds|int64(d.Nanos) != 0
128+
hasDate := d.Months|d.Days != 0
129+
if !hasTime && !hasDate {
130+
return "PT0S"
131+
}
132+
res := []byte("P")
99133

100-
if d.Seconds == 0 {
101-
sign = "-"
134+
if hasDate {
135+
years, months := divMod(d.Months, 12)
136+
if years != 0 {
137+
res = fmt.Append(res, years, "Y")
138+
}
139+
if months != 0 {
140+
res = fmt.Append(res, months, "M")
141+
}
142+
if d.Days != 0 {
143+
res = fmt.Append(res, d.Days, "D")
102144
}
103145
}
146+
if hasTime {
147+
res = fmt.Append(res, "T")
148+
149+
minutes1, seconds := divMod(d.Seconds, 60)
150+
secondsNanos, nanos := divMod(int64(d.Nanos), int64(time.Second))
151+
seconds = seconds + secondsNanos
152+
153+
// Wrapping seconds again after nanos have been folded into seconds.
154+
// If we had int128 at our disposal simple divMod operations would suffice as nanos could be
155+
// folded into seconds as the first step without having to worry about overflows.
156+
minutes2, seconds := divMod(seconds, 60)
157+
minutes := minutes1 + minutes2
158+
if minutes != 0 && seconds != 0 {
159+
signMinutes := sign(minutes)
160+
signSeconds := sign(seconds)
161+
if signMinutes != signSeconds {
162+
minutes += signSeconds
163+
seconds += 60 * signMinutes
164+
}
165+
}
166+
hours, minutes := divMod(minutes, 60)
104167

105-
timePart := ""
106-
if d.Nanos == 0 {
107-
timePart = fmt.Sprintf("%s%d", sign, d.Seconds)
108-
} else {
109-
timePart = fmt.Sprintf("%s%d.%09d", sign, d.Seconds, d.Nanos)
168+
if hours != 0 {
169+
res = fmt.Append(res, hours, "H")
170+
}
171+
if minutes != 0 {
172+
res = fmt.Append(res, minutes, "M")
173+
}
174+
if nanos < 0 {
175+
if seconds < 0 {
176+
nanosStr := strings.TrimRight(fmt.Sprintf(".%09d", -nanos), "0")
177+
res = fmt.Append(res, seconds, nanosStr, "S")
178+
} else if seconds > 0 {
179+
seconds--
180+
nanos = int64(time.Second) + nanos
181+
nanosStr := strings.TrimRight(fmt.Sprintf(".%09d", nanos), "0")
182+
res = fmt.Append(res, seconds, nanosStr, "S")
183+
} else {
184+
nanosStr := strings.TrimRight(fmt.Sprintf(".%09d", -nanos), "0")
185+
res = fmt.Append(res, "-0", nanosStr, "S")
186+
}
187+
} else if nanos > 0 {
188+
if seconds < -1 {
189+
seconds++
190+
nanos = int64(time.Second) - nanos
191+
nanosStr := strings.TrimRight(fmt.Sprintf(".%09d", nanos), "0")
192+
res = fmt.Append(res, seconds, nanosStr, "S")
193+
} else if seconds > -1 {
194+
// 0 100_000_000
195+
nanosStr := strings.TrimRight(fmt.Sprintf(".%09d", nanos), "0")
196+
res = fmt.Append(res, seconds, nanosStr, "S")
197+
} else {
198+
seconds++
199+
nanos = int64(time.Second) - nanos
200+
nanosStr := strings.TrimRight(fmt.Sprintf(".%09d", nanos), "0")
201+
res = fmt.Append(res, "-0", nanosStr, "S")
202+
}
203+
} else if seconds != 0 {
204+
res = fmt.Append(res, seconds, "S")
205+
}
110206
}
111207

112-
return fmt.Sprintf("P%dM%dDT%sS", d.Months, d.Days, timePart)
208+
return string(res)
113209
}
114210

115211
func (d1 Duration) Equal(d2 Duration) bool {

‎neo4j/dbtype/temporaltypes_test.go‎

Lines changed: 161 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,12 @@ package dbtype
1919

2020
import (
2121
"fmt"
22+
"math"
23+
"math/rand/v2"
2224
"testing"
2325
"time"
26+
27+
"github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/testutil"
2428
)
2529

2630
type stringTestCase[T interface{ String() string }] struct {
@@ -152,22 +156,119 @@ func TestLocalDateTimeString(outer *testing.T) {
152156
}
153157

154158
func TestDurationSting(outer *testing.T) {
159+
intIs64BitsWide := math.MaxInt64 == int64(math.MaxInt)
160+
161+
var maxNanosCase, minNanosCase, maxCase, minCase stringTestCase[Duration]
162+
163+
{
164+
var output string
165+
duration := Duration{Months: 0, Days: 0, Seconds: 0, Nanos: math.MaxInt}
166+
if intIs64BitsWide {
167+
output = "PT2562047H47M16.854775807S"
168+
} else {
169+
output = "PT2.147483647S"
170+
}
171+
maxNanosCase = stringTestCase[Duration]{input: duration, output: output}
172+
}
173+
174+
{
175+
var output string
176+
duration := Duration{Months: 0, Days: 0, Seconds: 0, Nanos: math.MinInt}
177+
if intIs64BitsWide {
178+
output = "PT-2562047H-47M-16.854775808S"
179+
} else {
180+
output = "PT-2.147483648S"
181+
}
182+
minNanosCase = stringTestCase[Duration]{input: duration, output: output}
183+
}
184+
185+
{
186+
var output string
187+
duration := Duration{Months: math.MaxInt64, Days: math.MaxInt64, Seconds: math.MaxInt64, Nanos: math.MaxInt}
188+
if intIs64BitsWide {
189+
output = "P768614336404564650Y7M9223372036854775807DT2562047790577263H17M23.854775807S"
190+
} else {
191+
output = "P768614336404564650Y7M9223372036854775807DT2562047788015215H30M9.147483647S"
192+
}
193+
maxCase = stringTestCase[Duration]{input: duration, output: output}
194+
}
195+
196+
{
197+
var output string
198+
duration := Duration{Months: math.MinInt64, Days: math.MinInt64, Seconds: math.MinInt64, Nanos: math.MinInt}
199+
if intIs64BitsWide {
200+
output = "P-768614336404564650Y-8M-9223372036854775808DT-2562047790577263H-17M-24.854775808S"
201+
} else {
202+
output = "P-768614336404564650Y-8M-9223372036854775808DT-2562047788015215H-30M-10.147483648S"
203+
}
204+
minCase = stringTestCase[Duration]{input: duration, output: output}
205+
}
206+
155207
outer.Parallel()
156208
testCases := []stringTestCase[Duration]{
157-
{input: Duration{Months: 15, Days: 32, Seconds: 785, Nanos: 789215800}, output: "P15M32DT785.789215800S"},
158-
{input: Duration{Months: 0, Days: 32, Seconds: 785, Nanos: 789215800}, output: "P0M32DT785.789215800S"},
159-
{input: Duration{Months: 0, Days: 0, Seconds: 785, Nanos: 789215800}, output: "P0M0DT785.789215800S"},
160-
{input: Duration{Months: 0, Days: 0, Seconds: 0, Nanos: 789215800}, output: "P0M0DT0.789215800S"},
161-
{input: Duration{Months: 0, Days: 0, Seconds: -1, Nanos: 0}, output: "P0M0DT-1S"},
162-
{input: Duration{Months: 0, Days: 0, Seconds: 0, Nanos: 999999999}, output: "P0M0DT0.999999999S"},
163-
{input: Duration{Months: 0, Days: 0, Seconds: -1, Nanos: 5}, output: "P0M0DT-0.999999995S"},
164-
{input: Duration{Months: 0, Days: 0, Seconds: -1, Nanos: 999999999}, output: "P0M0DT-0.000000001S"},
165-
{input: Duration{Months: 500, Days: 0, Seconds: 0, Nanos: 0}, output: "P500M0DT0S"},
166-
{input: Duration{Months: 0, Days: 0, Seconds: 0, Nanos: 5}, output: "P0M0DT0.000000005S"},
167-
{input: Duration{Months: 0, Days: 0, Seconds: -500, Nanos: 1}, output: "P0M0DT-499.999999999S"},
168-
{input: Duration{Months: 0, Days: 0, Seconds: -500, Nanos: 0}, output: "P0M0DT-500S"},
169-
{input: Duration{Months: -10, Days: 5, Seconds: -2, Nanos: 500}, output: "P-10M5DT-1.999999500S"},
170-
{input: Duration{Months: -10, Days: -5, Seconds: -2, Nanos: 500}, output: "P-10M-5DT-1.999999500S"},
209+
// all 0
210+
{input: Duration{Months: 0, Days: 0, Seconds: 0, Nanos: 0}, output: "PT0S"},
211+
// single positive component
212+
{input: Duration{Months: 1, Days: 0, Seconds: 0, Nanos: 0}, output: "P1M"},
213+
{input: Duration{Months: 0, Days: 1, Seconds: 0, Nanos: 0}, output: "P1D"},
214+
{input: Duration{Months: 0, Days: 0, Seconds: 1, Nanos: 0}, output: "PT1S"},
215+
{input: Duration{Months: 0, Days: 0, Seconds: 0, Nanos: 1}, output: "PT0.000000001S"},
216+
{input: Duration{Months: math.MaxInt64, Days: 0, Seconds: 0, Nanos: 0}, output: "P768614336404564650Y7M"},
217+
{input: Duration{Months: 0, Days: math.MaxInt64, Seconds: 0, Nanos: 0}, output: "P9223372036854775807D"},
218+
{input: Duration{Months: 0, Days: 0, Seconds: math.MaxInt64, Nanos: 0}, output: "PT2562047788015215H30M7S"},
219+
maxNanosCase,
220+
// single negative component
221+
{input: Duration{Months: -1, Days: 0, Seconds: 0, Nanos: 0}, output: "P-1M"},
222+
{input: Duration{Months: 0, Days: -1, Seconds: 0, Nanos: 0}, output: "P-1D"},
223+
{input: Duration{Months: 0, Days: 0, Seconds: -1, Nanos: 0}, output: "PT-1S"},
224+
{input: Duration{Months: 0, Days: 0, Seconds: 0, Nanos: -1}, output: "PT-0.000000001S"},
225+
{input: Duration{Months: math.MinInt64, Days: 0, Seconds: 0, Nanos: 0}, output: "P-768614336404564650Y-8M"},
226+
{input: Duration{Months: 0, Days: math.MinInt64, Seconds: 0, Nanos: 0}, output: "P-9223372036854775808D"},
227+
{input: Duration{Months: 0, Days: 0, Seconds: math.MinInt64, Nanos: 0}, output: "PT-2562047788015215H-30M-8S"},
228+
minNanosCase,
229+
// only time components
230+
{input: Duration{Months: 0, Days: 0, Seconds: 1, Nanos: 1}, output: "PT1.000000001S"},
231+
{input: Duration{Months: 0, Days: 0, Seconds: -1, Nanos: -1}, output: "PT-1.000000001S"},
232+
// only date components
233+
{input: Duration{Months: 1, Days: 1, Seconds: 0, Nanos: 0}, output: "P1M1D"},
234+
{input: Duration{Months: -1, Days: -1, Seconds: 0, Nanos: 0}, output: "P-1M-1D"},
235+
{input: Duration{Months: -1, Days: 1, Seconds: 0, Nanos: 0}, output: "P-1M1D"},
236+
{input: Duration{Months: 1, Days: -1, Seconds: 0, Nanos: 0}, output: "P1M-1D"},
237+
// all components
238+
{input: Duration{Months: 1, Days: 1, Seconds: 1, Nanos: 1}, output: "P1M1DT1.000000001S"},
239+
{input: Duration{Months: -1, Days: -1, Seconds: -1, Nanos: -1}, output: "P-1M-1DT-1.000000001S"},
240+
// nanos don't need trailing 0 decimals
241+
{input: Duration{Months: 0, Days: 0, Seconds: 0, Nanos: 1_000}, output: "PT0.000001S"},
242+
{input: Duration{Months: 0, Days: 0, Seconds: 0, Nanos: -1_000}, output: "PT-0.000001S"},
243+
// nanos wrap into seconds
244+
{input: Duration{Months: 0, Days: 0, Seconds: 0, Nanos: 2_100_000_000}, output: "PT2.1S"},
245+
{input: Duration{Months: 0, Days: 0, Seconds: 0, Nanos: -2_100_000_000}, output: "PT-2.1S"},
246+
{input: Duration{Months: 0, Days: 0, Seconds: 3, Nanos: 2_100_000_000}, output: "PT5.1S"},
247+
{input: Duration{Months: 0, Days: 0, Seconds: -3, Nanos: -2_100_000_000}, output: "PT-5.1S"},
248+
{input: Duration{Months: 0, Days: 0, Seconds: -3, Nanos: 2_100_000_000}, output: "PT-0.9S"},
249+
{input: Duration{Months: 0, Days: 0, Seconds: 3, Nanos: -2_100_000_000}, output: "PT0.9S"},
250+
{input: Duration{Months: 0, Days: 0, Seconds: -3, Nanos: 2_100_000_000}, output: "PT-0.9S"},
251+
{input: Duration{Months: 0, Days: 0, Seconds: 3, Nanos: -2_100_000_000}, output: "PT0.9S"},
252+
// seconds wrap into minutes and hours
253+
{input: Duration{Months: 0, Days: 0, Seconds: 60, Nanos: 0}, output: "PT1M"},
254+
{input: Duration{Months: 0, Days: 0, Seconds: -60, Nanos: 0}, output: "PT-1M"},
255+
{input: Duration{Months: 0, Days: 0, Seconds: 3600, Nanos: 0}, output: "PT1H"},
256+
{input: Duration{Months: 0, Days: 0, Seconds: -3600, Nanos: 0}, output: "PT-1H"},
257+
// all max/min components
258+
maxCase,
259+
minCase,
260+
}
261+
262+
if intIs64BitsWide {
263+
testCases = append(testCases, []stringTestCase[Duration]{
264+
// nanos wrap into seconds and hours
265+
{input: Duration{Months: 0, Days: 0, Seconds: 7_203, Nanos: 3_601_100_000_000}, output: "PT3H4.1S"},
266+
{input: Duration{Months: 0, Days: 0, Seconds: 7_263, Nanos: 3_721_100_000_000}, output: "PT3H3M4.1S"},
267+
{input: Duration{Months: 0, Days: 0, Seconds: 7_203, Nanos: -3_601_100_000_000}, output: "PT1H1.9S"},
268+
{input: Duration{Months: 0, Days: 0, Seconds: 7_383, Nanos: -3_721_100_000_000}, output: "PT1H1M1.9S"},
269+
{input: Duration{Months: 0, Days: 0, Seconds: -7_203, Nanos: 3_601_100_000_000}, output: "PT-1H-1.9S"},
270+
{input: Duration{Months: 0, Days: 0, Seconds: -7_323, Nanos: 3_661_100_000_000}, output: "PT-1H-1M-1.9S"},
271+
}...)
171272
}
172273

173274
for _, testCase := range testCases {
@@ -178,3 +279,49 @@ func TestDurationSting(outer *testing.T) {
178279
})
179280
}
180281
}
282+
283+
func TestSign(outer *testing.T) {
284+
outer.Parallel()
285+
286+
type testCaseType struct {
287+
name string
288+
input int64
289+
output int64
290+
}
291+
292+
testCases := []testCaseType{
293+
{name: "0", input: 0, output: 1},
294+
{name: "1", input: 1, output: 1},
295+
{name: "2", input: 2, output: 1},
296+
{name: "math.MaxInt64 / 2", input: math.MaxInt64 / 2, output: 1},
297+
{name: "math.MaxInt64 - 1", input: math.MaxInt64 - 1, output: 1},
298+
{name: "math.MaxInt64", input: math.MaxInt64, output: 1},
299+
{name: "-1", input: -1, output: -1},
300+
{name: "-2", input: -2, output: -1},
301+
{name: "math.MinInt64 / 2", input: math.MinInt64 / 2, output: -1},
302+
{name: "math.MinInt64 + 1", input: math.MinInt64 + 1, output: -1},
303+
{name: "math.MinInt64", input: math.MinInt64, output: -1},
304+
}
305+
306+
for _, testCase := range testCases {
307+
testCase := testCase
308+
outer.Run(testCase.name, func(inner *testing.T) {
309+
inner.Parallel()
310+
res := sign(testCase.input)
311+
testutil.AssertDeepEquals(inner, res, testCase.output)
312+
})
313+
314+
}
315+
}
316+
317+
func BenchmarkSign(b *testing.B) {
318+
numbers := make([]int64, 0, 1_000_000)
319+
for i := 0; i < 1_000_000; i++ {
320+
numbers = append(numbers, rand.Int64())
321+
}
322+
b.ResetTimer()
323+
324+
for i := 0; i < b.N; i++ {
325+
sign(numbers[i%1_000_000])
326+
}
327+
}

‎testkit/Dockerfile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
FROM ubuntu:20.04
22

33
ARG go_version_min=1.23.11
4-
ARG go_version_default=1.23.5
4+
ARG go_version_default=1.24.5
55

66
# Install all needed to build and run tests
77
# Install tzdata to make sure Go timezone info works correctly (need noninteractive to avoid

0 commit comments

Comments
 (0)