Repository: amzn/ion-go
Pull request #89 (2020-07-02 21:06:35)

Add timestamp precision rounding
Currently timestamps are truncated at nanoseconds. Add rounding up if value is 5 or greater.

Eg. 
```
"1234-05-06T07:08:09.0001001004Z" -> "1234-05-06T07:08:09.000100100Z"
"1234-05-06T07:08:09.0001001005Z" -> "1234-05-06T07:08:09.000100101Z"
```

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.




Patch:
diff --git a/ion/bitstream.go b/ion/bitstream.go
index 1885a9f..d6c0243 100644
--- a/ion/bitstream.go
+++ b/ion/bitstream.go
@@ -545,7 +545,7 @@ func (b *bitstream) ReadTimestamp() (time.Time, error) {
 		}
 	}
 
-	nsecs, err := b.readNsecs(len)
+	nsecs, overflow, err := b.readNsecs(len)
 	if err != nil {
 		return time.Time{}, err
 	}
@@ -553,33 +553,49 @@ func (b *bitstream) ReadTimestamp() (time.Time, error) {
 	b.state = b.stateAfterValue()
 	b.clear()
 
-	return tryCreateTimeWithNSecAndOffset(ts, nsecs, offset)
+	return tryCreateTimeWithNSecAndOffset(ts, nsecs, overflow, offset)
 }
 
-func tryCreateTimeWithNSecAndOffset(ts []int, nsecs int, offset int64) (time.Time, error) {
+func tryCreateTimeWithNSecAndOffset(ts []int, nsecs int, overflow bool, offset int64) (time.Time, error) {
 	date := time.Date(ts[0], time.Month(ts[1]), ts[2], ts[3], ts[4], ts[5], nsecs, time.UTC)
 	// time.Date converts 2000-01-32 input to 2000-02-01
 	if ts[0] != date.Year() || time.Month(ts[1]) != date.Month() || ts[2] != date.Day() {
 		return time.Time{}, fmt.Errorf("ion: invalid timestamp")
 	}
 
+	if overflow {
+		date = date.Add(time.Second)
+	}
 	return date.In(time.FixedZone("fixed", int(offset)*60)), nil
 }
 
-// ReadNsecs reads the fraction part of a timestamp and truncates it to nanoseconds.
-func (b *bitstream) readNsecs(len uint64) (int, error) {
+// ReadNsecs reads the fraction part of a timestamp and rounds to nanoseconds.
+// This function returns the nanoseconds as an int, overflow as a bool, and an error
+// if there was a problem executing this function.
+func (b *bitstream) readNsecs(len uint64) (int, bool, error) {
 	d, err := b.readDecimal(len)
 	if err != nil {
-		return 0, err
+		return 0, false, err
 	}
 
-	nsec, err := d.ShiftL(9).Trunc()
+	nsec, err := d.ShiftL(9).trunc()
 	if err != nil || nsec < 0 || nsec > 999999999 {
 		msg := fmt.Sprintf("invalid timestamp fraction: %v", d)
-		return 0, &SyntaxError{msg, b.pos}
+		return 0, false, &SyntaxError{msg, b.pos}
+	}
+
+	nsec, err = d.ShiftL(9).round()
+	if err != nil {
+		msg := fmt.Sprintf("invalid timestamp fraction: %v", d)
+		return 0, false, &SyntaxError{msg, b.pos}
+	}
+
+	// Overflow to second.
+	if nsec == 1000000000 {
+		return 0, true, nil
 	}
 
-	return int(nsec), nil
+	return int(nsec), false, nil
 }
 
 // ReadDecimal reads a decimal value of the given length: an exponent encoded as a
diff --git a/ion/decimal.go b/ion/decimal.go
index 1b52228..87760c2 100644
--- a/ion/decimal.go
+++ b/ion/decimal.go
@@ -231,13 +231,14 @@ func (d *Decimal) upscale(scale int32) *Decimal {
 	}
 }
 
-// Trunc attempts to truncate this decimal to an int64, dropping any fractional bits.
-func (d *Decimal) Trunc() (int64, error) {
+// Check to upscale a decimal which means to make 'n' bigger by making 'scale' smaller.
+// Makes comparisons and math easier, at the expense of more storage space.
+func (d *Decimal) checkToUpscale() (*Decimal, error) {
 	if d.scale < 0 {
 		// Don't even bother trying this with numbers that *definitely* too big to represent
 		// as an int64, because upscale(0) will consume a bunch of memory.
 		if d.scale < -20 {
-			return 0, &strconv.NumError{
+			return d, &strconv.NumError{
 				Func: "ParseInt",
 				Num:  d.String(),
 				Err:  strconv.ErrRange,
@@ -245,15 +246,37 @@ func (d *Decimal) Trunc() (int64, error) {
 		}
 		d = d.upscale(0)
 	}
+	return d, nil
+}
 
+// Trunc attempts to truncate this decimal to an int64, dropping any fractional bits.
+func (d *Decimal) trunc() (int64, error) {
+	ud, err := d.checkToUpscale()
+	if err != nil {
+		return 0, err
+	}
+	d = ud
 	str := d.n.String()
 
-	want := len(str) - int(d.scale)
-	if want <= 0 {
+	truncateTo := len(str) - int(d.scale)
+	if truncateTo <= 0 {
 		return 0, nil
 	}
 
-	return strconv.ParseInt(str[:want], 10, 64)
+	return strconv.ParseInt(str[:truncateTo], 10, 64)
+}
+
+// Round attempts to truncate this decimal to an int64, rounding any fractional bits.
+func (d *Decimal) round() (int64, error) {
+	ud, err := d.checkToUpscale()
+	if err != nil {
+		return 0, err
+	}
+	d = ud
+
+	floatValue := float64(d.n.Int64()) / math.Pow10(int(d.scale))
+	roundedValue := math.Round(floatValue)
+	return int64(roundedValue), nil
 }
 
 // Truncate returns a new decimal, truncated to the given number of
diff --git a/ion/textutils.go b/ion/textutils.go
index 7cfb872..206bc41 100644
--- a/ion/textutils.go
+++ b/ion/textutils.go
@@ -401,9 +401,8 @@ func parseTimestamp(val string) (time.Time, error) {
 
 		if val[idx] == 'z' || val[idx] == 'Z' {
 			if idx >= 29 {
-				// Too much precision for a go Time.
-				// TODO: We should probably round instead of truncating? Ah well.
-				return time.Parse(layoutNanosecondsAndOffset, val[:29]+val[idx:])
+				// Greater than 9 fractional seconds.
+				return roundFractionalSeconds(val, idx)
 			}
 			return time.Parse(layoutSecondsAndOffset, val)
 		}
@@ -411,9 +410,8 @@ func parseTimestamp(val string) (time.Time, error) {
 		if val[idx] == '+' || val[idx] == '-' {
 			if isValidOffset(val, idx) {
 				if idx >= 29 {
-					// Too much precision for a go Time.
-					// TODO: We should probably round instead of truncating? Ah well.
-					return time.Parse(layoutNanosecondsAndOffset, val[:29]+val[idx:])
+					// Greater than 9 fractional seconds.
+					return roundFractionalSeconds(val, idx)
 				}
 				return time.Parse(layoutSecondsAndOffset, val)
 			}
@@ -450,3 +448,33 @@ func isValidOffset(val string, idx int) bool {
 func invalidTimestamp(val string) (time.Time, error) {
 	return time.Time{}, fmt.Errorf("ion: invalid timestamp: %v", val)
 }
+
+func roundFractionalSeconds(val string, idx int) (time.Time, error) {
+	// Convert to float to perform rounding.
+	floatValue, err := strconv.ParseFloat(val[18:idx], 64)
+	if err != nil {
+		return invalidTimestamp(val)
+	}
+	roundedStringValue := fmt.Sprintf("%.9f", floatValue)
+
+	roundedFloatValue, err := strconv.ParseFloat(roundedStringValue, 64)
+	if err != nil {
+		return invalidTimestamp(val)
+	}
+
+	// Microsecond overflow 9.9999999999 -> 10.00000000.
+	if roundedFloatValue == 10 {
+		roundedStringValue := "9.000000000"
+		val = val[:18] + roundedStringValue + val[idx:]
+		timeValue, err := time.Parse(layoutNanosecondsAndOffset, val)
+		if err != nil {
+			return invalidTimestamp(val)
+		}
+
+		timeValue = timeValue.Add(time.Second)
+		return timeValue, err
+	}
+
+	val = val[:18] + roundedStringValue + val[idx:]
+	return time.Parse(layoutNanosecondsAndOffset, val)
+}


Test patch:
diff --git a/ion/bitstream_test.go b/ion/bitstream_test.go
index 8cbf57a..603f327 100644
--- a/ion/bitstream_test.go
+++ b/ion/bitstream_test.go
@@ -1,6 +1,9 @@
 package ion
 
-import "testing"
+import (
+	"testing"
+	"time"
+)
 
 func TestBitstream(t *testing.T) {
 	ion := []byte{
@@ -109,3 +112,188 @@ func TestBitcodeString(t *testing.T) {
 		}
 	}
 }
+
+func TestBinaryReadTimestamp(t *testing.T) {
+	test := func(ion []byte, expectedValue string) {
+		t.Run(expectedValue, func(t *testing.T) {
+
+			b := bitstream{}
+			b.InitBytes(ion)
+			b.Next()
+
+			val, err := b.ReadTimestamp()
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			expectedTime, err := time.Parse(time.RFC3339Nano, expectedValue)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if !val.Equal(expectedTime) {
+				t.Errorf("expected %v, got %v", expectedValue, val)
+			}
+		})
+	}
+
+	test([]byte{
+		0x63,
+		0x80,       // offset 0
+		0x0F, 0xD0, // year: 2000
+	}, "2000-01-01T00:00:00Z")
+
+	test([]byte{
+		0x64,
+		0x80,       // offset 0
+		0x0F, 0xD0, // year: 2000
+		0x85, // month: 5
+	}, "2000-05-01T00:00:00Z")
+
+	test([]byte{
+		0x65,
+		0x80,       // offset 0
+		0x0F, 0xD0, // year: 2000
+		0x85, // month: 5
+		0x86, // day: 6
+	}, "2000-05-06T00:00:00Z")
+
+	test([]byte{
+		0x67,
+		0x80,       // offset 0
+		0x0F, 0xD0, // year: 2000
+		0x85, // month: 5
+		0x86, // day: 6
+		0x87, // hour: 7
+		0x88, // minute: 8
+	}, "2000-05-06T07:08:00Z")
+
+	test([]byte{
+		0x68,
+		0x80,       // offset 0
+		0x0F, 0xD0, // year: 2000
+		0x85, // month: 5
+		0x86, // day: 6
+		0x87, // hour: 7
+		0x88, // minute: 8
+		0x89, // second: 9
+	}, "2000-05-06T07:08:09Z")
+
+	test([]byte{
+		0x6A,
+		0x80,       // offset 0
+		0x0F, 0xD0, // year: 2000
+		0x81, // month: 1
+		0x81, // day: 1
+		0x80, // hour: 0
+		0x80, // minute: 0
+		0x80, // second: 0
+		0x80, // 0 precision units
+		0x00, // 0
+	}, "2000-01-01T00:00:00Z")
+
+	test([]byte{
+		0x69,
+		0x80,       // offset 0
+		0x0F, 0xD0, // year: 2000
+		0x81, // month: 1
+		0x81, // day: 1
+		0x80, // hour: 0
+		0x80, // minute: 0
+		0x80, // second: 0
+		0xC2, // 2 precision units
+	}, "2000-01-01T00:00:00.00Z")
+
+	test([]byte{
+		0x6A,
+		0x80,       // offset 0
+		0x0F, 0xD0, // year: 2000
+		0x85, // month: 5
+		0x86, // day: 6
+		0x87, // hour: 7
+		0x88, // minute: 8
+		0x89, // second: 9
+		0xC3, // 3 precision units
+		0x64, // 100
+	}, "2000-05-06T07:08:09.100Z")
+
+	test([]byte{
+		0x6C,
+		0x80,       // offset 0
+		0x0F, 0xD0, // year: 2000
+		0x85,             // month: 5
+		0x86,             // day: 6
+		0x87,             // hour: 7
+		0x88,             // minute: 8
+		0x89,             // second: 9
+		0xC6,             // 6 precision units
+		0x01, 0x87, 0x04, // 100100
+	}, "2000-05-06T07:08:09.100100Z")
+
+	test([]byte{
+		0x6C,
+		0x88,       // offset +8
+		0x0F, 0xD0, // year: 2000
+		0x85,             // month: 5
+		0x86,             // day: 6
+		0x87,             // hour: 7
+		0x88,             // minute: 8
+		0x89,             // second: 9
+		0xC6,             // 6 precision units
+		0x01, 0x87, 0x04, // 100100
+	}, "2000-05-06T07:08:09.100100Z")
+
+	// Test >9 fractional seconds.
+	test([]byte{
+		0x6A,
+		0x80,       // offset 0
+		0x0F, 0xD0, // year: 2000
+		0x81, // month: 1
+		0x81, // day: 1
+		0x80, // hour: 0
+		0x80, // minute: 0
+		0x80, // second: 0
+		0xCA, // 10 precision units
+		0x2C, // 44
+	}, "2000-01-01T00:00:00.000000004Z")
+
+	test([]byte{
+		0x6A,
+		0x80,       // offset 0
+		0x0F, 0xD0, // year: 2000
+		0x81, // month: 1
+		0x81, // day: 1
+		0x80, // hour: 0
+		0x80, // minute: 0
+		0x80, // second: 0
+		0xCA, // 10 precision units
+		0x2D, // 45
+	}, "2000-01-01T00:00:00.000000005Z")
+
+	test([]byte{
+		0x6A,
+		0x80,       // offset 0
+		0x0F, 0xD0, // year: 2000
+		0x81, // month: 1
+		0x81, // day: 1
+		0x80, // hour: 0
+		0x80, // minute: 0
+		0x80, // second: 0
+		0xCA, // 10 precision units
+		0x2E, // 46
+	}, "2000-01-01T00:00:00.000000005Z")
+
+	test([]byte{
+		0x6E,
+		0x8E,
+		0x80,       // offset 0
+		0x0F, 0xD0, // year: 2000
+		0x8C,                         // month: 12
+		0x9F,                         // day: 31
+		0x97,                         // hour: 23
+		0xBB,                         // minute: 59
+		0xBB,                         // second: 59
+		0xCA,                         // 10 precision units
+		0x02, 0x54, 0x0B, 0xE3, 0xFF, // 9999999999
+	}, "2001-01-01T00:00:00.00000000Z")
+}
diff --git a/ion/decimal_test.go b/ion/decimal_test.go
index 21b72c9..3b5a1cf 100644
--- a/ion/decimal_test.go
+++ b/ion/decimal_test.go
@@ -133,7 +133,7 @@ func TestTrunc(t *testing.T) {
 	test := func(a string, eval int64) {
 		t.Run(fmt.Sprintf("trunc(%v)=%v", a, eval), func(t *testing.T) {
 			aa := MustParseDecimal(a)
-			val, err := aa.Trunc()
+			val, err := aa.trunc()
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -153,6 +153,37 @@ func TestTrunc(t *testing.T) {
 	test("1d3", 1000)
 }
 
+func TestRound(t *testing.T) {
+	test := func(a string, eval int64) {
+		t.Run(fmt.Sprintf("trunc(%v)=%v", a, eval), func(t *testing.T) {
+			aa := MustParseDecimal(a)
+			val, err := aa.round()
+			if err != nil {
+				t.Fatal(err)
+			}
+			if val != eval {
+				t.Errorf("expected %v, got %v", eval, val)
+			}
+		})
+	}
+
+	test("0.", 0)
+	test("0.01", 0)
+	test("1.", 1)
+	test("-1.", -1)
+	test("1.01", 1)
+	test("-1.01", -1)
+	test("1.4", 1)
+	test("1.5", 2)
+	test("1.6", 2)
+	test("0.4", 0)
+	test("0.5", 1)
+	test("0.9999999999", 1)
+	test("0.099", 0)
+	test("101", 101)
+	test("1d3", 1000)
+}
+
 func addF(a, b *Decimal) *Decimal { return a.Add(b) }
 func subF(a, b *Decimal) *Decimal { return a.Sub(b) }
 func mulF(a, b *Decimal) *Decimal { return a.Mul(b) }
diff --git a/ion/textreader_test.go b/ion/textreader_test.go
index 24fe3d0..9851462 100644
--- a/ion/textreader_test.go
+++ b/ion/textreader_test.go
@@ -225,7 +225,9 @@ func TestTimestamps(t *testing.T) {
 	test("2001-01-01T00:00:00.000+00:00", et)
 	test("2001-01-01T00:00:00.000000Z", et)
 	test("2001-01-01T00:00:00.000000000Z", et)
-	test("2001-01-01T00:00:00.000000000999Z", et) // We truncate, at least for now.
+
+	et2 := time.Date(2001, time.January, 1, 0, 0, 0, 1, time.UTC)
+	test("2001-01-01T00:00:00.000000000999Z", et2)
 
 	testA("foo::'bar'::2001-01-01T00:00:00.000Z", []string{"foo", "bar"}, et)
 }
diff --git a/ion/textutils_test.go b/ion/textutils_test.go
index 67144a8..6f8b1f5 100644
--- a/ion/textutils_test.go
+++ b/ion/textutils_test.go
@@ -34,6 +34,23 @@ func TestParseTimestamp(t *testing.T) {
 	test("1234-05-06T07:08:09.100Z", "1234-05-06T07:08:09.100Z")
 	test("1234-05-06T07:08:09.100100Z", "1234-05-06T07:08:09.100100Z")
 
+	// Test rounding of >=9 fractional seconds.
+	test("1234-05-06T07:08:09.000100100Z", "1234-05-06T07:08:09.000100100Z")
+	test("1234-05-06T07:08:09.100100100Z", "1234-05-06T07:08:09.100100100Z")
+	test("1234-05-06T07:08:09.00010010044Z", "1234-05-06T07:08:09.000100100Z")
+	test("1234-05-06T07:08:09.00010010044Z", "1234-05-06T07:08:09.000100100Z")
+	test("1234-05-06T07:08:09.00010010055Z", "1234-05-06T07:08:09.000100101Z")
+	test("1234-05-06T07:08:09.00010010099Z", "1234-05-06T07:08:09.000100101Z")
+	test("1234-05-06T07:08:09.99999999999Z", "1234-05-06T07:08:10.000000000Z")
+	test("1234-12-31T23:59:59.99999999999Z", "1235-01-01T00:00:00.000000000Z")
+	test("1234-05-06T07:08:09.000100100+09:10", "1234-05-06T07:08:09.000100100+09:10")
+	test("1234-05-06T07:08:09.100100100-10:11", "1234-05-06T07:08:09.100100100-10:11")
+	test("1234-05-06T07:08:09.00010010044+09:10", "1234-05-06T07:08:09.000100100+09:10")
+	test("1234-05-06T07:08:09.00010010055-10:11", "1234-05-06T07:08:09.000100101-10:11")
+	test("1234-05-06T07:08:09.00010010099+09:10", "1234-05-06T07:08:09.000100101+09:10")
+	test("1234-05-06T07:08:09.99999999999-10:11", "1234-05-06T07:08:10.000000000-10:11")
+	test("1234-12-31T23:59:59.99999999999+09:10", "1235-01-01T00:00:00.000000000+09:10")
+
 	test("1234-05-06T07:08+09:10", "1234-05-06T07:08:00+09:10")
 	test("1234-05-06T07:08:09-10:11", "1234-05-06T07:08:09-10:11")
 }