Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct the coefficient value of timestamp nanoseconds when writing to Ion binary #177

Merged
merged 10 commits into from
Jun 3, 2021
Merged

Correct the coefficient value of timestamp nanoseconds when writing to Ion binary #177

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4c4b12e
Correct the coefficient value of timestamp nanoseconds
May 27, 2021
968633c
Fix writing and reading of binary/text timestamp values to get tests …
May 28, 2021
23f7953
Add unit tests
May 28, 2021
2e6dcd9
Address PR comments
May 28, 2021
0600361
Rename variables to be more clear
May 28, 2021
3c5587e
Apply go fmt formatting
May 28, 2021
5acde3b
Address PR comments
May 31, 2021
02a9c36
Address PR comments
May 31, 2021
44e0b20
Parse VarInt to get the timestamp precision
Jun 2, 2021
b3422b0
Set fractional precision to 9 if timestamp precision is TimestampPrec…
Jun 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 1 addition & 12 deletions ion/bitstream.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,18 +663,7 @@ func (b *bitstream) ReadTimestamp() (Timestamp, error) {
return Timestamp{}, err
}

if nsecs > 0 {
fractionPrecision = 9

// Adjust fractionPrecision for each trailing zero.
// ie. .123456000 should have 6 fractionPrecision instead of 9
ns := nsecs
for ns > 0 && (ns%10) == 0 {
ns /= 10
fractionPrecision--
}
precision = TimestampPrecisionNanosecond
} else if len(fracSecsBytes) > 0 && fracSecsBytes[0] > 0xC0 && (fracSecsBytes[0]^0xC0) > 0 {
if len(fracSecsBytes) > 0 && fracSecsBytes[0] > 0xC0 && (fracSecsBytes[0]^0xC0) > 0 {
fractionPrecision = fracSecsBytes[0] ^ 0xC0
precision = TimestampPrecisionNanosecond
}
Expand Down
4 changes: 2 additions & 2 deletions ion/marshal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"math/big"
"reflect"
"sort"
"strconv"
"time"
)

Expand Down Expand Up @@ -446,8 +445,9 @@ func (m *Encoder) encodeTimeDate(v reflect.Value) error {
// Get number of fractional seconds precisions
ns := t.Nanosecond()
numFractionalSeconds := 0
// Go native time object will always have 9 precision units when nanoseconds > 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the precision change when nanoseconds > 0? Why isn't it a 0 of the same precision?

Copy link
Contributor Author

@byronlin13 byronlin13 Jun 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For time.time objects, if the nanosecond is 0 or not specified, it is treated as no nanoseconds.

timeObject, err := time.Parse(time.RFC3339, "2000-01-01T01:01:01.00Z") will have timeObject.nanoseconds = 0

  • The string representation of this object is: 2000-01-01 01:01:01 +0000 UTC

timeObject, err := time.Parse(time.RFC3339, "2000-01-01T01:01:01.01Z") will have timeObject.nanoseconds = 10000000

  • The string representation of this object is: 2000-01-01 01:01:01.01 +0000 UTC

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Time type documentation says:

A Time represents an instant in time with nanosecond precision.

Unlike Ion's Timestamp type, which supports variable precision, it seems like time.Time always has nanosecond precision. If that's true, I would expect a time.Time with zero nanoseconds to be written out as a Timestamp with a fractional seconds precision of 9 decimal places.

For time.time objects, if the nanosecond is 0 or not specified, it is treated as no nanoseconds.

Could you please point me to the relevant docs for this? How do we distinguish between 0 or not specified in a time.Time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct, I was basing my interpretation on the fact that time.nanoseconds = 0 and the string representation was not showing fractional seconds 🤦 .

I removed the nanoseconds > 0 check in encodeTimeDate().

I also added guards to NewDateTimestamp(), NewTimestamp() and set fractionalPrecision = 9 if the timestamp precision is TimestampPrecisionNanosecond

if ns > 0 {
numFractionalSeconds = len(strconv.Itoa(ns))
numFractionalSeconds = maxFractionalPrecision
}

// Time.Date has nano second component
Expand Down
15 changes: 14 additions & 1 deletion ion/marshal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ func TestMarshalText(t *testing.T) {
test(MustParseDecimal("1.20"), "1.20")
test(NewTimestamp(time.Date(2010, 1, 1, 0, 0, 0, 0, time.UTC), TimestampPrecisionSecond, TimezoneUTC),
"2010-01-01T00:00:00Z")
test(time.Date(2010, 1, 1, 0, 0, 0, 770000000, time.UTC), "2010-01-01T00:00:00.77Z")
test(time.Date(2010, 1, 1, 0, 0, 0, 1, time.UTC), "2010-01-01T00:00:00.000000001Z")
test(time.Date(2010, 1, 1, 0, 0, 0, 770000000, time.UTC), "2010-01-01T00:00:00.770000000Z")
loc, _ := time.LoadLocation("EST")
test(time.Date(2010, 1, 1, 0, 0, 0, 0, loc), "2010-01-01T00:00:00-05:00")
loc = time.FixedZone("UTC+8", 8*60*60)
Expand Down Expand Up @@ -138,6 +139,18 @@ func TestMarshalBinary(t *testing.T) {
0x8A, 0x21, 0x2A,
0x8B, 0x20,
}))

test(time.Date(2010, 1, 1, 0, 0, 0, 1, time.UTC), "time with 1 nanosecond", prefixIVM([]byte{
0x6A, 0x80, 0x0F, 0xDA, 0x81, 0x81, 0x80, 0x80, 0x80,
0xC9, // exponent: 9
0x01, // coefficient: 1
}))

test(time.Date(2010, 1, 1, 0, 0, 0, 5000, time.UTC), "time with 5000 nanoseconds", prefixIVM([]byte{
0x6B, 0x80, 0x0F, 0xDA, 0x81, 0x81, 0x80, 0x80, 0x80,
0xC9, // exponent: 9
0x13, 0x88, // coefficient: 5000
}))
}

func prefixIVM(data []byte) []byte {
Expand Down
17 changes: 8 additions & 9 deletions ion/textutils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,21 +45,20 @@ func TestParseTimestamp(t *testing.T) {
test("1234-05-06T", "1234-05-06T00:00:00Z", TimestampPrecisionDay, TimezoneUnspecified, 0)
test("1234-05-06T07:08Z", "1234-05-06T07:08:00Z", TimestampPrecisionMinute, TimezoneUTC, 0)
test("1234-05-06T07:08:09Z", "1234-05-06T07:08:09Z", TimestampPrecisionSecond, TimezoneUTC, 0)
test("1234-05-06T07:08:09.100Z", "1234-05-06T07:08:09.100Z", TimestampPrecisionNanosecond, TimezoneUTC, 1)
test("1234-05-06T07:08:09.100100Z", "1234-05-06T07:08:09.100100Z", TimestampPrecisionNanosecond, TimezoneUTC, 4)
test("1234-05-06T07:08:09.100Z", "1234-05-06T07:08:09.100Z", TimestampPrecisionNanosecond, TimezoneUTC, 3)
test("1234-05-06T07:08:09.100100Z", "1234-05-06T07:08:09.100100Z", TimestampPrecisionNanosecond, TimezoneUTC, 6)

// Test rounding of >=9 fractional seconds.
test("1234-05-06T07:08:09.000100100Z", "1234-05-06T07:08:09.000100100Z", TimestampPrecisionNanosecond, TimezoneUTC, 7)
test("1234-05-06T07:08:09.100100100Z", "1234-05-06T07:08:09.100100100Z", TimestampPrecisionNanosecond, TimezoneUTC, 7)
test("1234-05-06T07:08:09.00010010044Z", "1234-05-06T07:08:09.000100100Z", TimestampPrecisionNanosecond, TimezoneUTC, 7)
test("1234-05-06T07:08:09.00010010044Z", "1234-05-06T07:08:09.000100100Z", TimestampPrecisionNanosecond, TimezoneUTC, 7)
test("1234-05-06T07:08:09.000100100Z", "1234-05-06T07:08:09.000100100Z", TimestampPrecisionNanosecond, TimezoneUTC, 9)
test("1234-05-06T07:08:09.100100100Z", "1234-05-06T07:08:09.100100100Z", TimestampPrecisionNanosecond, TimezoneUTC, 9)
test("1234-05-06T07:08:09.00010010044Z", "1234-05-06T07:08:09.000100100Z", TimestampPrecisionNanosecond, TimezoneUTC, 9)
test("1234-05-06T07:08:09.00010010055Z", "1234-05-06T07:08:09.000100101Z", TimestampPrecisionNanosecond, TimezoneUTC, 9)
test("1234-05-06T07:08:09.00010010099Z", "1234-05-06T07:08:09.000100101Z", TimestampPrecisionNanosecond, TimezoneUTC, 9)
test("1234-05-06T07:08:09.99999999999Z", "1234-05-06T07:08:10.000000000Z", TimestampPrecisionNanosecond, TimezoneUTC, 9)
test("1234-12-31T23:59:59.99999999999Z", "1235-01-01T00:00:00.000000000Z", TimestampPrecisionNanosecond, TimezoneUTC, 9)
test("1234-05-06T07:08:09.000100100+09:10", "1234-05-06T07:08:09.000100100+09:10", TimestampPrecisionNanosecond, TimezoneLocal, 7)
test("1234-05-06T07:08:09.100100100-10:11", "1234-05-06T07:08:09.100100100-10:11", TimestampPrecisionNanosecond, TimezoneLocal, 7)
test("1234-05-06T07:08:09.00010010044+09:10", "1234-05-06T07:08:09.000100100+09:10", TimestampPrecisionNanosecond, TimezoneLocal, 7)
test("1234-05-06T07:08:09.000100100+09:10", "1234-05-06T07:08:09.000100100+09:10", TimestampPrecisionNanosecond, TimezoneLocal, 9)
test("1234-05-06T07:08:09.100100100-10:11", "1234-05-06T07:08:09.100100100-10:11", TimestampPrecisionNanosecond, TimezoneLocal, 9)
test("1234-05-06T07:08:09.00010010044+09:10", "1234-05-06T07:08:09.000100100+09:10", TimestampPrecisionNanosecond, TimezoneLocal, 9)
test("1234-05-06T07:08:09.00010010055-10:11", "1234-05-06T07:08:09.000100101-10:11", TimestampPrecisionNanosecond, TimezoneLocal, 9)
test("1234-05-06T07:08:09.00010010099+09:10", "1234-05-06T07:08:09.000100101+09:10", TimestampPrecisionNanosecond, TimezoneLocal, 9)
test("1234-05-06T07:08:09.99999999999-10:11", "1234-05-06T07:08:10.000000000-10:11", TimestampPrecisionNanosecond, TimezoneLocal, 9)
Expand Down
60 changes: 32 additions & 28 deletions ion/timestamp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ const (
TimestampPrecisionNanosecond
)

const maxFractionalPrecision = 9

func (tp TimestampPrecision) String() string {
switch tp {
case TimestampNoPrecision:
Expand Down Expand Up @@ -159,30 +161,15 @@ func NewTimestampFromStr(dateStr string, precision TimestampPrecision, kind Time
if precision >= TimestampPrecisionNanosecond {
pointIdx := strings.LastIndex(dateStr, ".")
if pointIdx != -1 {
nonZeroFraction := false

idx := pointIdx + 1
for idx < len(dateStr) && isDigit(int(dateStr[idx])) {
if dateStr[idx] != '0' {
nonZeroFraction = true
}
fractionUnits++
idx++
}

if idx == len(dateStr) {
return Timestamp{}, fmt.Errorf("ion: invalid date string '%v'", dateStr)
}

// We do not want to include trailing zeros for a non-zero fraction (ie. .1234000 -> .1234)
// So we adjust fractionUnits accordingly.
if nonZeroFraction {
idx--
for idx > pointIdx && dateStr[idx] == '0' {
fractionUnits--
idx--
}
}
}
}

Expand Down Expand Up @@ -461,7 +448,7 @@ func (ts Timestamp) String() string {
// So we may need to make some adjustments.

// Add back removed trailing zeros from fractional seconds (ie. ".000")
if ts.precision >= TimestampPrecisionNanosecond && ts.dateTime.Nanosecond() == 0 && ts.numFractionalSeconds > 0 {
if ts.precision >= TimestampPrecisionNanosecond && ts.numFractionalSeconds > 0 {
// Find the position of 'T'
tIndex := strings.Index(format, "T")
if tIndex == -1 {
Expand All @@ -471,23 +458,39 @@ func (ts Timestamp) String() string {
}
}

index := strings.LastIndex(format, "Z")
if index == -1 || index < tIndex {
index = strings.LastIndex(format, "+")
if index == -1 || index < tIndex {
index = strings.LastIndex(format, "-")
timeZoneIndex := strings.LastIndex(format, "Z")
if timeZoneIndex == -1 || timeZoneIndex < tIndex {
timeZoneIndex = strings.LastIndex(format, "+")
if timeZoneIndex == -1 || timeZoneIndex < tIndex {
timeZoneIndex = strings.LastIndex(format, "-")
}
}

// This position better be right of 'T'
if index != -1 && tIndex < index {
if timeZoneIndex != -1 && tIndex < timeZoneIndex {
zeros := strings.Builder{}
zeros.WriteByte('.')
for i := uint8(0); i < ts.numFractionalSeconds; i++ {
numZerosNeeded := 0

// Specify trailing zeros if fractional precision is less than the nanoseconds.
// e.g. A timestamp: 2021-05-25T13:41:31.00001234 with fractional precision: 2 will return "2021-05-25 13:41:31.00"
ns := ts.dateTime.Nanosecond()
if ns == 0 || maxFractionalPrecision-len(strconv.Itoa(ns)) >= int(ts.numFractionalSeconds) {
zeros.WriteByte('.')
numZerosNeeded = int(ts.numFractionalSeconds)
} else {
decimalPlaceIndex := strings.LastIndex(format, ".")
if decimalPlaceIndex != -1 {
decimalPlacesOccupied := timeZoneIndex - decimalPlaceIndex - 1
numZerosNeeded = int(ts.numFractionalSeconds) - decimalPlacesOccupied
}
}

// Add trailing zeros until the fractional seconds component is the correct length
for i := 0; i < numZerosNeeded; i++ {
zeros.WriteByte('0')
}

format = format[0:index] + zeros.String() + format[index:]
format = format[0:timeZoneIndex] + zeros.String() + format[timeZoneIndex:]
}
}

Expand Down Expand Up @@ -515,12 +518,13 @@ func (ts Timestamp) Equal(ts1 Timestamp) bool {
ts.numFractionalSeconds == ts1.numFractionalSeconds
}

// TruncatedNanoseconds returns nanoseconds with trailing zeros removed (ie. 123456000 gets truncated to 123456).
// TruncatedNanoseconds returns nanoseconds with trailing values removed up to the difference of max fractional precision - time stamp's fractional precision
// e.g. 123456000 with fractional precision: 3 will get truncated to 123.
func (ts Timestamp) TruncatedNanoseconds() int {
nsecs := ts.dateTime.Nanosecond()
for i := uint8(0); i < (9-ts.numFractionalSeconds) && nsecs > 0 && (nsecs%10) == 0; i++ {

for i := uint8(0); i < (maxFractionalPrecision-ts.numFractionalSeconds) && nsecs > 0; i++ {
nsecs /= 10
}

return nsecs
}
Loading