Repository: embulk/embulk
Issue: Embulk v0.9.7 can't parse milliseconds part in `%s.%N` and `%s.%L` format.


Reported by [ntrgiang](https://github.com/embulk/embulk/issues/467#issuecomment-416636390)

Embulk v0.9.7 can't parse milliseconds part in `%s.%N` and `%s.%L` format.
Embulk v0.8.39 can parse it. 

[Reporduce code](https://github.com/hiroyuki-sato/embulk-support/tree/master/core_467_timestamp_format/ntrgiang)

```
2018-08-29 19:05:16.852 +0900: Embulk v0.8.39
+-----------------+-----------------------------+------------+-------------+
| key_name:string |               day:timestamp | value:long | name:string |
+-----------------+-----------------------------+------------+-------------+
|   2 new_clients | 2018-04-06 11:06:02.001 UTC |          1 |           a |
|   3 new_clients | 2018-04-06 11:06:03.001 UTC |          2 |           b |
|   4 new_clients | 2018-04-06 11:06:04.001 UTC |          3 |           c |
+-----------------+-----------------------------+------------+-------------+
```

```
2018-08-29 19:05:50.172 +0900: Embulk v0.9.7
+-----------------+-------------------------+------------+-------------+
| key_name:string |           day:timestamp | value:long | name:string |
+-----------------+-------------------------+------------+-------------+
|   2 new_clients | 2018-04-06 11:06:02 UTC |          1 |           a |
|   3 new_clients | 2018-04-06 11:06:03 UTC |          2 |           b |
|   4 new_clients | 2018-04-06 11:06:04 UTC |          3 |           c |
+-----------------+-------------------------+------------+-------------+
```

Changed files:
embulk-core/src/test/java/org/embulk/spi/time/TestTimestampParser.java, embulk-core/src/main/java/org/embulk/spi/time/RubyTimeParsed.java

diff --git a/embulk-core/src/main/java/org/embulk/spi/time/RubyTimeParsed.java b/embulk-core/src/main/java/org/embulk/spi/time/RubyTimeParsed.java
index 8f88293d..c486254e 100644
--- a/embulk-core/src/main/java/org/embulk/spi/time/RubyTimeParsed.java
+++ b/embulk-core/src/main/java/org/embulk/spi/time/RubyTimeParsed.java
@@ -586,20 +586,37 @@ class RubyTimeParsed extends TimeParsed {
         }
 
         if (this.instantSeconds != null) {
-            // Fractions by %Q are prioritized over fractions by %N.
-            // irb(main):002:0> Time.strptime("123456789 12.345", "%Q %S.%N").nsec
-            // => 789000000
-            // irb(main):003:0> Time.strptime("12.345 123456789", "%S.%N %Q").nsec
-            // => 789000000
-            // irb(main):004:0> Time.strptime("12.345", "%S.%N").nsec
-            // => 345000000
             if (!defaultZoneOffset.equals(ZoneOffset.UTC)) {
                 // TODO: Warn that a default time zone is specified for epoch seconds.
             }
             if (this.timeZoneName != null) {
                 // TODO: Warn that the epoch second has a time zone.
             }
-            return this.instantSeconds;
+
+            // The fraction part is "added" to the epoch second in case both are specified.
+            // irb(main):002:0> Time.strptime("1500000000.123456789", "%s.%N").nsec
+            // => 123456789
+            // irb(main):003:0> Time.strptime("1500000000456.111111111", "%Q.%N").nsec
+            // => 567111111
+            //
+            // If "sec_fraction" is specified, the value is used like |Time.at(seconds, sec_fraction * 1000000)|.
+            // https://svn.ruby-lang.org/cgi-bin/viewvc.cgi/tags/v2_3_1/lib/time.rb?view=markup#l427
+            //
+            // |Time.at| adds "seconds" (the epoch) and "sec_fraction" (the fraction part) with scaling.
+            // https://svn.ruby-lang.org/cgi-bin/viewvc.cgi/tags/v2_3_1/time.c?view=markup#l2528
+            //
+            // It behaves the same even if "seconds" is specified as a Rational, not an Integer.
+            // irb(main):004:0> Time.at(Rational(1500000000789, 1000), 100123).nsec
+            // => 889123000
+            if (this.nanoOfSecond != Integer.MIN_VALUE) {
+                if (this.instantSeconds.getEpochSecond() >= 0) {
+                    return this.instantSeconds.plusNanos(this.nanoOfSecond);
+                } else {
+                    return this.instantSeconds.minusNanos(this.nanoOfSecond);
+                }
+            } else {
+                return this.instantSeconds;
+            }
         }
 
         // Day of the year (yday: DAY_OF_YEAR) is not considered in Time.strptime, not like DateTime.strptime.
@@ -643,20 +660,37 @@ class RubyTimeParsed extends TimeParsed {
                                   final int defaultDayOfMonth,
                                   final ZoneId defaultZoneId) {
         if (this.instantSeconds != null) {
-            // Fractions by %Q are prioritized over fractions by %N.
-            // irb(main):002:0> Time.strptime("123456789 12.345", "%Q %S.%N").nsec
-            // => 789000000
-            // irb(main):003:0> Time.strptime("12.345 123456789", "%S.%N %Q").nsec
-            // => 789000000
-            // irb(main):004:0> Time.strptime("12.345", "%S.%N").nsec
-            // => 345000000
             if (!defaultZoneId.equals(ZoneOffset.UTC)) {
                 // TODO: Warn that a default time zone is specified for epoch seconds.
             }
             if (this.timeZoneName != null) {
                 // TODO: Warn that the epoch second has a time zone.
             }
-            return this.instantSeconds;
+
+            // The fraction part is "added" to the epoch second in case both are specified.
+            // irb(main):002:0> Time.strptime("1500000000.123456789", "%s.%N").nsec
+            // => 123456789
+            // irb(main):003:0> Time.strptime("1500000000456.111111111", "%Q.%N").nsec
+            // => 567111111
+            //
+            // If "sec_fraction" is specified, the value is used like |Time.at(seconds, sec_fraction * 1000000)|.
+            // https://svn.ruby-lang.org/cgi-bin/viewvc.cgi/tags/v2_3_1/lib/time.rb?view=markup#l427
+            //
+            // |Time.at| adds "seconds" (the epoch) and "sec_fraction" (the fraction part) with scaling.
+            // https://svn.ruby-lang.org/cgi-bin/viewvc.cgi/tags/v2_3_1/time.c?view=markup#l2528
+            //
+            // It behaves the same even if "seconds" is specified as a Rational, not an Integer.
+            // irb(main):004:0> Time.at(Rational(1500000000789, 1000), 100123).nsec
+            // => 889123000
+            if (this.nanoOfSecond != Integer.MIN_VALUE) {
+                if (this.instantSeconds.getEpochSecond() >= 0) {
+                    return this.instantSeconds.plusNanos(this.nanoOfSecond);
+                } else {
+                    return this.instantSeconds.minusNanos(this.nanoOfSecond);
+                }
+            } else {
+                return this.instantSeconds;
+            }
         }
 
         final ZoneId zoneId;
diff --git a/embulk-core/src/test/java/org/embulk/spi/time/TestTimestampParser.java b/embulk-core/src/test/java/org/embulk/spi/time/TestTimestampParser.java
index db3e8f6c..1cca67bf 100644
--- a/embulk-core/src/test/java/org/embulk/spi/time/TestTimestampParser.java
+++ b/embulk-core/src/test/java/org/embulk/spi/time/TestTimestampParser.java
@@ -624,6 +624,32 @@ public class TestTimestampParser {
         testToParse("-1000", "%Q", -1L);
     }
 
+    @Test
+    public void testEpochWithFraction() {
+        testToParse("1500000000.123456789", "%s.%N", 1500000000L, 123456789);
+        testToParse("1500000000456.111111111", "%Q.%N", 1500000000L, 567111111);
+        testToParse("1500000000.123", "%s.%L", 1500000000L, 123000000);
+        testToParse("1500000000456.111", "%Q.%L", 1500000000L, 567000000);
+
+        testToParse("1.5", "%s.%N", 1L, 500000000);
+        testToParse("-1.5", "%s.%N", -2L, 500000000);
+        testToParse("1.000000001", "%s.%N", 1L, 1);
+        testToParse("-1.000000001", "%s.%N", -2L, 999999999);
+    }
+
+    @Test
+    public void testRubyEpochWithFraction() {
+        testRubyToParse("1500000000.123456789", "%s.%N", 1500000000L, 123456789);
+        testRubyToParse("1500000000456.111111111", "%Q.%N", 1500000000L, 567111111);
+        testRubyToParse("1500000000.123", "%s.%L", 1500000000L, 123000000);
+        testRubyToParse("1500000000456.111", "%Q.%L", 1500000000L, 567000000);
+
+        testRubyToParse("1.5", "%s.%N", 1L, 500000000);
+        testRubyToParse("-1.5", "%s.%N", -2L, 500000000);
+        testRubyToParse("1.000000001", "%s.%N", 1L, 1);
+        testRubyToParse("-1.000000001", "%s.%N", -2L, 999999999);
+    }
+
     private void testJavaToParse(final String string, final String format, final long second, final int nanoOfSecond) {
         final TimestampParser parser = TimestampParser.of("java:" + format, "UTC");
         final Timestamp timestamp = parser.parse(string);