Skip to content

Commit

Permalink
Corrects the coefficient value of timestamp nanoseconds when writing …
Browse files Browse the repository at this point in the history
…to Ion binary (#177)

Writing a timestamp's nanosecond into Ion binary will write bytes of two values: Coefficient and exponent. These two values are used to calculate the nanoseconds using the equation: coefficient * 10 ^ -exponent.

For example a timestamp:  `2021-05-27 00:19:05.000050000` has coefficient: `50000` and exponent: `9`.

Inside marshal binary function, we get the correct coefficient by calling `time.Nanosecond` which returns `50000` however the exponent is calculated by converting the coefficient to string and getting the length which returns `5` and is incorrect.

We can set the exponent to `9` if there are nanoseconds because it will always be `9` precision units.
  • Loading branch information
byronlin13 authored Jun 3, 2021
1 parent 4e5ade8 commit 3971047
Show file tree
Hide file tree
Showing 7 changed files with 247 additions and 82 deletions.
2 changes: 1 addition & 1 deletion ion/binarywriter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ func TestWriteBinaryTimestamp(t *testing.T) {
nowish, _ := NewTimestampFromStr("2019-08-04T18:15:43.863494+10:00", TimestampPrecisionNanosecond, TimezoneLocal)

testBinaryWriter(t, eval, func(w Writer) {
assert.NoError(t, w.WriteTimestamp(NewTimestamp(time.Time{}, TimestampPrecisionNanosecond, TimezoneUTC)))
assert.NoError(t, w.WriteTimestamp(NewTimestamp(time.Time{}, TimestampPrecisionSecond, TimezoneUTC)))
assert.NoError(t, w.WriteTimestamp(nowish))
})
}
Expand Down
38 changes: 11 additions & 27 deletions ion/bitstream.go
Original file line number Diff line number Diff line change
Expand Up @@ -652,30 +652,12 @@ func (b *bitstream) ReadTimestamp() (Timestamp, error) {

// Check the fractional seconds part of the timestamp.
if length > 0 {
// First byte indicates number of precision units in fractional seconds.
fracSecsBytes, err := b.in.Peek(1)
nsecs, overflow, fractionPrecision, err = b.readNsecs(length)
if err != nil {
return Timestamp{}, err
}

nsecs, overflow, err = b.readNsecs(length)
if err != nil {
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 {
fractionPrecision = fracSecsBytes[0] ^ 0xC0
if fractionPrecision > 0 {
precision = TimestampPrecisionNanosecond
}
}
Expand All @@ -692,32 +674,34 @@ func (b *bitstream) ReadTimestamp() (Timestamp, 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
// This function returns the nanoseconds as an int, overflow as a bool, exponent as an uint8, and an error
// if there was a problem executing this function.
func (b *bitstream) readNsecs(length uint64) (int, bool, error) {
func (b *bitstream) readNsecs(length uint64) (int, bool, uint8, error) {
d, err := b.readDecimal(length)
if err != nil {
return 0, false, err
return 0, false, 0, err
}

nsec, err := d.ShiftL(9).trunc()
if err != nil || nsec < 0 || nsec > 999999999 {
msg := fmt.Sprintf("invalid timestamp fraction: %v", d)
return 0, false, &SyntaxError{msg, b.pos}
return 0, false, 0, &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}
return 0, false, 0, &SyntaxError{msg, b.pos}
}

exponent := uint8(d.scale)

// Overflow to second.
if nsec == 1000000000 {
return 0, true, nil
return 0, true, exponent, nil
}

return int(nsec), false, nil
return int(nsec), false, exponent, nil
}

// ReadDecimal reads a decimal value of the given length: an exponent encoded as a
Expand Down
10 changes: 1 addition & 9 deletions ion/marshal.go
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 @@ -443,15 +442,8 @@ func (m *Encoder) encodeTimeDate(v reflect.Value) error {
kind = TimezoneUnspecified
}

// Get number of fractional seconds precisions
ns := t.Nanosecond()
numFractionalSeconds := 0
if ns > 0 {
numFractionalSeconds = len(strconv.Itoa(ns))
}

// Time.Date has nano second component
timestamp := NewTimestampWithFractionalSeconds(t, TimestampPrecisionNanosecond, kind, uint8(numFractionalSeconds))
timestamp := NewTimestampWithFractionalSeconds(t, TimestampPrecisionNanosecond, kind, maxFractionalPrecision)
return m.w.WriteTimestamp(timestamp)
}

Expand Down
19 changes: 16 additions & 3 deletions ion/marshal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,12 @@ 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")
test(time.Date(2010, 1, 1, 0, 0, 0, 0, loc), "2010-01-01T00:00:00.000000000-05:00")
loc = time.FixedZone("UTC+8", 8*60*60)
test(time.Date(2010, 1, 1, 0, 0, 0, 0, loc), "2010-01-01T00:00:00+08:00")
test(time.Date(2010, 1, 1, 0, 0, 0, 0, loc), "2010-01-01T00:00:00.000000000+08:00")

test("hello\tworld", "\"hello\\tworld\"")

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
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
79 changes: 47 additions & 32 deletions ion/timestamp.go
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 @@ -131,23 +133,34 @@ type Timestamp struct {

// NewDateTimestamp constructor meant for timestamps that only have a date portion (ie. no time portion).
func NewDateTimestamp(dateTime time.Time, precision TimestampPrecision) Timestamp {
return Timestamp{dateTime, precision, TimezoneUnspecified, 0}
numDecimalPlacesOfFractionalSeconds := uint8(0)
if precision >= TimestampPrecisionNanosecond {
numDecimalPlacesOfFractionalSeconds = maxFractionalPrecision
}
return Timestamp{dateTime, precision, TimezoneUnspecified, numDecimalPlacesOfFractionalSeconds}
}

// NewTimestamp constructor
func NewTimestamp(dateTime time.Time, precision TimestampPrecision, kind TimezoneKind) Timestamp {
numDecimalPlacesOfFractionalSeconds := uint8(0)

if precision <= TimestampPrecisionDay {
// Timestamps with Year, Month, or Day precision necessarily have TimezoneUnspecified timezone.
kind = TimezoneUnspecified
} else if precision >= TimestampPrecisionNanosecond {
numDecimalPlacesOfFractionalSeconds = maxFractionalPrecision
}
return Timestamp{dateTime, precision, kind, 0}
return Timestamp{dateTime, precision, kind, numDecimalPlacesOfFractionalSeconds}
}

// NewTimestampWithFractionalSeconds constructor
func NewTimestampWithFractionalSeconds(dateTime time.Time, precision TimestampPrecision, kind TimezoneKind, fractionPrecision uint8) Timestamp {
if fractionPrecision > 9 {
if fractionPrecision > maxFractionalPrecision {
// 9 is the max precision supported
fractionPrecision = 9
fractionPrecision = maxFractionalPrecision
}
if precision < TimestampPrecisionNanosecond {
fractionPrecision = 0
}
return Timestamp{dateTime, precision, kind, fractionPrecision}
}
Expand All @@ -159,30 +172,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 +459,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 +469,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 +529,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

0 comments on commit 3971047

Please sign in to comment.