You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@orc.apache.org by "Gang Wu (Jira)" <ji...@apache.org> on 2021/03/22 04:04:00 UTC
[jira] [Comment Edited] (ORC-763) ORC timestamp inconsistencies
before UNIX epoch
[ https://issues.apache.org/jira/browse/ORC-763?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17305875#comment-17305875 ]
Gang Wu edited comment on ORC-763 at 3/22/21, 4:03 AM:
-------------------------------------------------------
The ORC timestamp issue is a long story. The root cause is that java.sql.Timestamp has a bug which adds one second to any timestamp value with negative second and nanosecond greater than 999999 in silence. To workaround this, the Java ORC reader takes one second off if any timestamp value has negative second and nanosecond greater than 999999. The C++ ORC writer and reader follow same logic so that they can share the same behavior (ideally).
Unfortunately, this behavior introduces a new issue that both 1969-12-31 23.59.59.001 and 1970-01-01 00.00.00.001 are mapped to the same value. Because 1969-12-31 23.59.59.001 means second = -1 and nanosecond > 999999, it is altered by JDK to be second = 0 and written to an ORC file. The Java ORC reader does not takes 1 second off since the value read is second = 0.
The inconsistency between Java and C++ is something we can fix easily. The Java ORC Reader used to check timestamp with negative second and non-zero nanosecond to take one second off from it (same as the C++ reader today). But this behavior of Java reader was changed in this commit: [ORC-306|[https://github.com/apache/orc/commit/9c105b92a0c2ab9c624b7bffd3c8b3a91d892175#|https://github.com/apache/orc/commit/9c105b92a0c2ab9c624b7bffd3c8b3a91d892175]]. So we can fix the C++ reader/writer to use same logic.
was (Author: wgtmac):
The ORC timestamp issue is a long story. The root cause is that java.sql.Timestamp has a bug which adds one second to any timestamp value with negative second and nanosecond greater than 999999 in silence. To workaround this, the Java ORC reader takes one second off if any timestamp value has negative second and nanosecond greater than 999999. The C++ ORC writer and reader follow same logic so that they can share the same behavior (ideally).
Unfortunately, this behavior introduces a new issue that both 1969-12-31 23.59.59.001 and 1970-01-01 00.00.00.001 are mapped to the same value. Because 1969-12-31 23.59.59.001 means second = -1 and nanosecond > 999999, it is altered by JDK to be second = 0 and written to an ORC file. The Java ORC reader does not takes 1 second off since the value read is second = 0.
The inconsistency between Java and C++ is something we can fix easily. The Java ORC Reader used to check timestamp with negative second and non-zero nanosecond to take one second off from it (same as the C++ reader today). But this behavior of Java reader was changed in this commit: [ORC-306|[https://github.com/apache/orc/commit/9c105b92a0c2ab9c624b7bffd3c8b3a91d892175#diff-93bbe966778037431d62855f3e63b7ba36803c08a3dfd4e28482c7192cc9dc48L982].] So we can fix the C++ reader/writer to use same logic.
> ORC timestamp inconsistencies before UNIX epoch
> -----------------------------------------------
>
> Key: ORC-763
> URL: https://issues.apache.org/jira/browse/ORC-763
> Project: ORC
> Issue Type: Bug
> Reporter: Zoltán Borók-Nagy
> Priority: Critical
>
> I did some experiments with Hive (Java ORC 1.5.1) and Impala (C++ ORC 1.6.2) and found some bugs related to timestamps before the UNIX epoch.
> From Hive:
> 0: jdbc:hive2://localhost:11050/default> create table orc_ts (ts timestamp) stored as orc;
> 0: jdbc:hive2://localhost:11050/default> insert into orc_ts values ('1969-12-31 23:59:59'), ('1969-12-31 23:59:59.0001'), ('1969-12-31 23:59:59.001');
> 0: jdbc:hive2://localhost:11050/default> insert into orc_ts values ('1969-12-31 23:59:58'), ('1969-12-31 23:59:58.0001'), ('1969-12-31 23:59:58.001');
> 0: jdbc:hive2://localhost:11050/default> select * from orc_ts;
> +---------------------------+
> |orc_ts.ts|
> +---------------------------+
> |1969-12-31 23:59:59.0|
> |1969-12-31 23:59:59.0001|
> |1970-01-01 00:00:00.001|
> |1969-12-31 23:59:58.0|
> |1969-12-31 23:59:58.0001|
> |1969-12-31 23:59:58.001|
> +---------------------------+
> Please note that we inserted '1969-12-31 23:59:59.001' and we got '1970-01-01 00:00:00.001'. So Java ORC read/writes are inconsistent in themselves.
> From Impala:
>
> {noformat}
> [localhost:21050] default> select * from orc_ts;
> +-------------------------------+
> | ts |
> +-------------------------------+
> | 1969-12-31 23:59:59 |
> | 1969-12-31 23:59:58.000100000 |
> | 1970-01-01 00:00:00.001000000 |
> | 1969-12-31 23:59:58 |
> | 1969-12-31 23:59:57.000100000 |
> | 1969-12-31 23:59:58.001000000 |
> +-------------------------------+{noformat}
> From Impala the second and fifth timestamps are also off by one second. The third timestamp is also off by one second, but consistent with Java.
> https://issues.apache.org/jira/browse/ORC-306 mentions a Java bug that it ORC tries to workaround. Seems like data files store values in a way to workaround the Java issue which is unnecessary in C++.
> Looking at the code the Java and C++ code they construct timestamp values differently.
> C++:
> [https://github.com/apache/orc/blob/654f777bc34841eed3b340047308f8dac7f554db/c%2B%2B/src/ColumnReader.cc#L377-L379]
> {noformat}
> if (secsBuffer[i] < 0 && nanoBuffer[i] != 0) {
> secsBuffer[i] -= 1;
> }
> {noformat}
> Java:
> [https://github.com/apache/orc/blob/master/java/core/src/java/org/apache/orc/impl/TreeReaderFactory.java#L1227-L1229]
>
> {noformat}
> if (millis < 0 && newNanos > 999_999) {
> millis -= TimestampTreeWriter.MILLIS_PER_SECOND;
> }
> {noformat}
> C++ 'checks for nanoBuffer[i] != 0' while Java checks for 'newNanos > 999_999'. Both only for timestamps before the epoch.
> This gives us a pattern when C++ and Java is inconsistent:
> * timestamps before the UNIX epoch, AND
> * have the format YYYY-MM-DD HH:MM:ss.000XXX
> I checked the actual values in the data files, written by ORC Java, read by ORC C++ lib:
> {noformat}
> (gdb) print secsBuffer[0] + epochOffset // 1969-12-31 23:59:59 => correct
> $1 = -1
> (gdb) print secsBuffer[1] + epochOffset // 1969-12-31 23:59:59.0001 (orc C++)=> 1969-12-31 23:59:58.000100000
> $2 = -1
> (gdb) print secsBuffer[2] + epochOffset // 1969-12-31 23:59:59.001 (orc C++)=> 1970-01-01 00:00:00.001000000
> $3 = 0{noformat}
> {noformat}
> (gdb) print secsBuffer[0] + epochOffset // 1969-12-31 23:59:58 => correct
> $9 = -2
> (gdb) print secsBuffer[1] + epochOffset // 1969-12-31 23:59:58.0001 (orc c++)=> 1969-12-31 23:59:57.000100000
> $10 = -2
> (gdb) print secsBuffer[2] + epochOffset // 1969-12-31 23:59:58.001 (orc c++)=> 1969-12-31 23:59:58.001000000
> $11 = -1{noformat}
> The seconds are the same for '1969-12-31 23:59:58' and '1969-12-31 23:59:58.0001', but differ for '1969-12-31 23:59:58.001'. I think this is a bug in the Java writer. The workaround for the Java bug (ORC-306) shouldn't have any effect on the written data files.
> So the values for the seconds are off by one when the timestamp is before the epoch and milliseconds are not zero.
> I think the data files should always store the values corresponding to the spec, i.e. number of seconds since the ORC epoch, plus additional nanoseconds that we need to add to the timestamp. If that'd be true then we wouldn't need the above 'if' statement in the c++ code.
>
>
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)