duckdb / duckdb/duckdb-java

Timestamp handling is broken for BCE dates

Aperta
#200 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
C++
Stelle
127
Fork
80
Merge medio
13h 49m
PR unite (30g)
48

Descrizione

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.

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Direzione di ricerca

Inizia tracciando gli usi interni di java.sql.Timestamp e java.sql.Date nel driver JDBC di DuckDB per l’analisi, la formattazione e la conversione da e verso java.time.LocalDateTime. Riproduci l’esempio BCE e verifica che i valori Timestamp forniti dal client e restituiti utilizzino offset in millisecondi UNIX e che le date BCE vengano convertite correttamente in entrambe le direzioni.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
java
Ambito
databases
Tipo di issue
Bug
Difficoltà
5/5
Tempo stimato
Più di una settimana
Stato di attività
Ferma
Chiarezza
Abbastanza chiara
Idoneità per principianti
35/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.