eclipse-vertx/vertx-sql-client

DataTypeCodec relies on COMPAT fallback for 'BC' era parsing

jerboaa opened this issue · 13 comments

Version

4.4.6

Context

This quarkus issue made me arrive here and has the full context:
quarkusio/quarkus#37208

Do you have a reproducer?

Yes.

Steps to reproduce

Get a JDK 22 EA version. E.g. from the Adoptium API: wget -O jdk-22-ea.tar.gz https://api.adoptium.net/v3/binary/latest/22/ea/linux/x64/jdk/hotspot/normal/eclipse, and untar it. tar -xf jdk-22-ea.tar.gz

./jdk-22+24/bin/jshell -J-showversion
openjdk version "22-beta" 2024-03-19
OpenJDK Runtime Environment Temurin-22+24-202311162331 (build 22-beta+24-ea)
OpenJDK 64-Bit Server VM Temurin-22+24-202311162331 (build 22-beta+24-ea, mixed mode, sharing)
|  Welcome to JShell -- Version 22-beta
|  For an introduction type: /help intro

jshell> java.time.LocalDateTime.parse("4714-11-24 00:00:00 BC", java.time.format.DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss G", Locale.ROOT));
|  Exception java.time.format.DateTimeParseException: Text '4714-11-24 00:00:00 BC' could not be parsed at index 20
|        at DateTimeFormatter.parseResolved0 (DateTimeFormatter.java:2108)
|        at DateTimeFormatter.parse (DateTimeFormatter.java:2010)
|        at LocalDateTime.parse (LocalDateTime.java:494)
|        at (#1:1)

Extra

I've filed this bug in upstream OpenJDK and the change is intentional as old code fell back to the COMPAT provider when the ROOT locale was being used. The workaround is to use Locale.US.

jdk-22+24/bin/jshell -J-showversion
openjdk version "22-beta" 2024-03-19
OpenJDK Runtime Environment Temurin-22+24-202311162331 (build 22-beta+24-ea)
OpenJDK 64-Bit Server VM Temurin-22+24-202311162331 (build 22-beta+24-ea, mixed mode, sharing)
|  Welcome to JShell -- Version 22-beta
|  For an introduction type: /help intro

jshell> java.time.LocalDateTime.parse("4714-11-24 00:00:00 BC", java.time.format.DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss G", Locale.US));
$1 ==> -4713-11-24T00:00

Can this line be changed to that, please? It should be compatible with older JDKs as well (tested with JDK 17).

public static final LocalDateTime LDT_MINUS_INFINITY = LocalDateTime.parse("4714-11-24 00:00:00 BC",
DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss G", Locale.ROOT));

pinging @vietj since he was one of the last persons to touch that part

can you provide a reproducer (a SQL statement would work fine) for this @jerboaa ?

No, sorry. I have no idea. Perhaps somebody in quarkusio/quarkus#37208 can help?

@vietj is the following reproducer OK?

  1. Grab an OpenJDK 22+24-EA build from https://github.com/adoptium/temurin22-binaries/releases/tag/jdk-22%2B24-ea-beta
git clone https://github.com/zakkak/vertx-pg-1379-reproducer.git
cd vertx-pg-1379-reproducer
mvn package
/path/to/jdk-22+24-ea/bin/java -jar target/vertx-pg-example-1.0-SNAPSHOT-jar-with-dependencies.jar

All it does is:

System.out.println( DataTypeCodec.LDT_MINUS_INFINITY );

Which runs fine with JDK 21 printing:

-4713-11-24T00:00

but fails with JDK 22+24-ea printing:

Exception in thread "main" java.lang.ExceptionInInitializerError
	at com.example.App.main(App.java:13)
Caused by: java.time.format.DateTimeParseException: Text '4714-11-24 00:00:00 BC' could not be parsed at index 20
	at java.base/java.time.format.DateTimeFormatter.parseResolved0(DateTimeFormatter.java:2108)
	at java.base/java.time.format.DateTimeFormatter.parse(DateTimeFormatter.java:2010)
	at java.base/java.time.LocalDateTime.parse(LocalDateTime.java:494)
	at io.vertx.pgclient.impl.codec.DataTypeCodec.<clinit>(DataTypeCodec.java:1077)
	... 1 more

ah right, I thought an interaction with the database was required :-) @zakkak

@vietj Do you have an ETA when this will be fixed? We will need the updated vertx version then in quarkus too. Thanks!

@jerboaa not yet, but for sure the issue is on our radar

OK, thanks!

Hi @vietj, I applied the change in my fork and tested it on github actions. Unfortunately I can't do any more testing since I am not familiar with the code base. I also opened #1391 in case it helps.

@zakkak I see the bug you filed has been fixed, the JDK 22-EA in which the bug appear has never been released, so I am assuming the next EA will have the bug fixed and that shall be fine ?

@zakkak I see the bug you filed has been fixed, the JDK 22-EA in which the bug appear has never been released, so I am assuming the next EA will have the bug fixed and that shall be fine ?

Which bug? This one? https://bugs.openjdk.org/browse/JDK-8320431

That got closed as Not an issue, which essentially means a fix as suggested in #1391 is needed. It's intended behaviour in the JDK which was a side-effect of it working in earlier JDKs and got finally fixed.

ok so this is not a temporary workaround as far as you can tell

ok so this is not a temporary workaround as far as you can tell

Yes.