duckdb / duckdb/duckdb-java

Timestamp handling is broken for BCE dates

Open
#200 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
127
Forks
80
Avg merge
13h 49m
Merged PRs (30d)
48

Description

Noticed some discrepancies when using negative year values in tests and tried to look deeper:

- historically Java has 2 different date-time APIs - [java.util.Date + java.util.Calendar](https://docs.oracle.com/javase/8/docs/api/java/util/Calendar.html) from Java 1.1 and [java.time](https://docs.oracle.com/javase/8/docs/api/java/time/package-summary.html) from Java 8
- JDBC API mandates the usage of `java.sql.Timestamp` (that is thin wrapper over `java.util.Date`) for passing dates to DB and reading dates from DB
- in DuckDB JDBC `java.sql.Timestamp` is currently used for parsing dates, formatting dates and converting it to-from `java.time.LocalDateTime`
- apparently `java.sql.Timestamp` is completely broken for BCE dates and cannot be used neither for parsing/formatting, nor for converting to/from `java.time.LocalDateTime`, the following behaviour cannot be correct:

```java
java.time.LocalDateTime ldt = java.time.LocalDateTime.of(-490, 9, 12, 9, 0, 0);
System.out.println("Original LocalDateTime: " + ldt);
// convert to timestamp field-by-field
java.sql.Timestamp ts1 = java.sql.Timestamp.valueOf(ldt);
System.out.println("Timestamp converted field-by-field: " + ts1);
// convert to timestamp using UNIX time offset in milliseconds
java.sql.Timestamp ts2 = new java.sql.Timestamp(ldt.toInstant(ZoneOffset.UTC).toEpochMilli());
System.out.println("Timestamp converted using UNIX time offset: " + ts2);
// convert back to LocalDateTime - year value is in CE now
java.time.LocalDateTime ldt1 = ts1.toLocalDateTime();
System.out.println("LocalDateTime converted back: " + ldt1);
System.out.println(ldt.getYear() == ldt1.getYear());
```
```
Original LocalDateTime: -0490-09-12T09:00
Timestamp converted field-by-field: 0491-09-12 09:00:00.0
Timestamp converted using UNIX time offset: 0491-09-17 09:00:00.0
LocalDateTime converted back: 0491-09-12T09:00
false
```

Apparently nobody uses `java.sql.Timestamp` in practice (except JDBC drivers) and this problem is [a known bug in JDK](https://bugs.openjdk.org/browse/JDK-8249280).

This may be a low priority (CE dates work correctly), but users can realistically load some datasets with historic dates.

To fix this in DuckDB driver, all internal usage of `java.sql.Timestamp` (and `java.sql.Date`) functionality needs to be replaced with `java.time` usage. And `java.sql.Timestamp` values (coming from client app, or returned to client app) need to be treated only as UNIX time milliseconds offset holders.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.