amazon-ion / ion-java

Java streaming parser/serializer for Ion.
https://amazon-ion.github.io/ion-docs/
Apache License 2.0
866 stars 110 forks source link

[feature request] Please add support for Timestamp nanoseconds #301

Closed wsargent closed 4 years ago

wsargent commented 4 years ago

Currently most methods in amazon.ion.Timestamp are created with millisecond precision, i.e.

    public static Timestamp now()
    {
        long millis = System.currentTimeMillis();
        return new Timestamp(millis, UNKNOWN_OFFSET);
    }

It doesn't look like there's an easy way to specify nanosecond precision on a timestamp, or expand out the _fraction field. The only thing that mentions nanoseconds is the SQL timestamp conversion:

public static Timestamp forSqlTimestampZ(java.sql.Timestamp sqlTimestamp)
    {
        if (sqlTimestamp == null) return null;

        long millis = sqlTimestamp.getTime();
        Timestamp ts = new Timestamp(millis, UTC_OFFSET);
        int nanos = sqlTimestamp.getNanos();
        BigDecimal frac = BigDecimal.valueOf(nanos).movePointLeft(9);
        ts._fraction = frac;
        return ts;
    }

which is nice but seems seems a bit arbitrary. At the very least I should be able to pass in the components of an Instant and be able to construct a matching timestamp that way.

Likewise, the timestamp tests are only accurate to the millisecond, with the nanosecond test involving an SQL timestamp:

https://github.com/amzn/ion-java/blob/d5939190808b815ec940ef5d45db0d39e9c3fb63/test/com/amazon/ion/TimestampTest.java#L1374

wsargent commented 4 years ago

This is related to https://github.com/amzn/ion-java/issues/266 which is about converting a Timestamp to an Instant.

wsargent commented 4 years ago

It does seem like it's possible to do using Timestamp.valueOf:

Timestamp expectedTsMillisWithNanosPrecision = Timestamp.valueOf("1677-09-21T00:12:43.145000000Z");

But that still involves converting an Instant to a String and then back again.

wsargent commented 4 years ago

And it's also possible for

Timestamp.forMillis(new BigDecimal("-9223372036854.775808"), UTC_OFFSET);

So technically I can do this:

Instant now = Instant.now();
BigDecimal millis= BigDecimal.valueOf(now.getEpochSecond()).scaleByPowerOfTen(3);
BigDecimal nanos = BigDecimal.valueOf(now.getNano()).movePointLeft(6).setScale(3, RoundingMode.FLOOR);
writer.writeTimestamp(Timestamp.forMillis(millis.add(nanos), 0));      

It's not very convenient though, because there's no direct access to the fraction.

dlurton commented 4 years ago

The following overload of Timestamp.forSecond() can be invoked to instantiate a Timestamp with arbitrary precision.

https://github.com/amzn/ion-java/blob/b6842d54245fdef9366236856130d9cb086b6f37/src/com/amazon/ion/Timestamp.java#L1313-L1315

Is there a reason this won't work?

wsargent commented 4 years ago

It's the same issue as Timestamp.forMillis -- there's no facility to pass in an arbitrary fraction of a second in there, so the BigDecimal has to be cobbled together out of Instant.

 public static Timestamp forSecond(int year, int month, int day, 
                                   int hour, int minute, int second, 
                                   BigDecimal fraction,
                                   Integer offset) 

would work...

dlurton commented 4 years ago

What's the precision of the "fraction" in your example? I assume the precision is "second" and that the value would have to be constrained to be less than 1 and greater than or equal to 0.

wsargent commented 4 years ago

Ideally, arbitrary precision using BigDecimal#getScale(). You could specify millis, micros or nanos.

wsargent commented 4 years ago

Just tried it and got an exception, because Instant doesn't have chrono values. It literally doesn't know what the year is, only what the seconds and nanos are.

Instant now = Instant.now();
Timestamp fromSeconds = Timestamp.forSecond(now.get(ChronoField.YEAR),
                now.get(ChronoField.MONTH_OF_YEAR),
                now.get(ChronoField.DAY_OF_MONTH),
                now.get(ChronoField.HOUR_OF_DAY),
                now.get(ChronoField.MINUTE_OF_HOUR),
                now.get(ChronoField.NANO_OF_SECOND) / 1000000,
                Timestamp.UTC_OFFSET);

yields

Exception in thread "main" java.time.temporal.UnsupportedTemporalTypeException: Unsupported field: Year
    at java.base/java.time.Instant.get(Instant.java:566)
    at example.Main.writeObject(Main.java:47)
    at example.Main.main(Main.java:29)
wsargent commented 4 years ago

The ideal situation is if ion-java had a timestamp implementation that added https://github.com/caplin/nanotime/ and then you could run with nanosec precision in Java 5 if you felt like.

Some interesting reading...

dlurton commented 4 years ago

I'm pretty sure we can't add additional dependencies due to downstream impacts. I'm looking at how we might create a Timestamp.forEpochSecond function similar to Instant.ofEpochSecond. I'm not yet certain if this is the right approach tho.

wsargent commented 4 years ago

The integration between JNI and Instant is neat tho:

    @Override
    public Instant instant() {
        long time = clock_gettime();

        // Decode the long, the first 32 bits are the seconds since epoch,
        // and the other 32 bits are the nanoseconds.
        return Instant.ofEpochSecond( time >> 32, time & 0xFFFFFFFFL);
    }
dlurton commented 4 years ago

@wsargent can you look at https://github.com/amzn/ion-java/pull/302 and see if it meets your needs?

dlurton commented 4 years ago

302 merged