bug: JsonArray#getInstant() made by postgres client throws DateTimeParseException #53

Closed
wants to merge 1 commit into
from

Projects

None yet

4 participants

@KengoTODA

Currently I cannot use JsonArray#getInstant(int) to get record, because it throws exception like below.

java.time.format.DateTimeParseException: Text '2016-07-24T14:32:24.363' could not be parsed at index 23
at java.time.format.DateTimeFormatter.parseResolved0(DateTimeFormatter.java:1949)
at java.time.format.DateTimeFormatter.parse(DateTimeFormatter.java:1777)
at io.vertx.core.json.JsonArray.getInstant(JsonArray.java:233)
at io.vertx.ext.asyncsql.PostgreSQLTest.lambda$21(PostgreSQLTest.java:149)
at io.vertx.ext.asyncsql.impl.AsyncSQLConnectionImpl.lambda$handleAsyncQueryResultToResultSet$10(AsyncSQLConnectionImpl.java:260)
at io.vertx.core.impl.FutureImpl.checkCallHandler(FutureImpl.java:158)
at io.vertx.core.impl.FutureImpl.complete(FutureImpl.java:111)
at io.vertx.ext.asyncsql.impl.ScalaUtils$1.apply(ScalaUtils.java:41)
at io.vertx.ext.asyncsql.impl.ScalaUtils$1.apply(ScalaUtils.java:37)
at scala.concurrent.impl.CallbackRunnable.run(Promise.scala:32)
at io.vertx.ext.asyncsql.impl.VertxEventLoopExecutionContext.lambda$execute$1(VertxEventLoopExecutionContext.java:70)
at io.vertx.core.impl.ContextImpl.lambda$wrapTask$3(ContextImpl.java:359)
at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:339)
at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:393)
at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:742)
at java.lang.Thread.run(Thread.java:745)

It seems that the string data generated by this client lacks the last 'Z'. I mean, text data should be '2016-07-24T14:32:24.363Z' but it is '2016-07-24T14:32:24.363' for now.

This PR contains a test which reproduces problem. I haven't found which code we should fix yet.
To test, I used postgres database which is installed by following docker command (same with the on in README).

docker run --rm --name vertx-postgres -e POSTGRES_USER=vertx -e POSTGRES_PASSWORD=password -e POSTGRES_DB=testdb -p 5432:5432 postgres
@KengoTODA KengoTODA reproduce bug with JUnit test case
java.time.format.DateTimeParseException: Text '2016-07-24T14:32:24.363' could not be parsed at index 23
at java.time.format.DateTimeFormatter.parseResolved0(DateTimeFormatter.java:1949)
at java.time.format.DateTimeFormatter.parse(DateTimeFormatter.java:1777)
at io.vertx.core.json.JsonArray.getInstant(JsonArray.java:233)
at io.vertx.ext.asyncsql.PostgreSQLTest.lambda$21(PostgreSQLTest.java:149)
at io.vertx.ext.asyncsql.impl.AsyncSQLConnectionImpl.lambda$handleAsyncQueryResultToResultSet$10(AsyncSQLConnectionImpl.java:260)
at io.vertx.core.impl.FutureImpl.checkCallHandler(FutureImpl.java:158)
at io.vertx.core.impl.FutureImpl.complete(FutureImpl.java:111)
at io.vertx.ext.asyncsql.impl.ScalaUtils$1.apply(ScalaUtils.java:41)
at io.vertx.ext.asyncsql.impl.ScalaUtils$1.apply(ScalaUtils.java:37)
at scala.concurrent.impl.CallbackRunnable.run(Promise.scala:32)
at io.vertx.ext.asyncsql.impl.VertxEventLoopExecutionContext.lambda$execute$1(VertxEventLoopExecutionContext.java:70)
at io.vertx.core.impl.ContextImpl.lambda$wrapTask$3(ContextImpl.java:359)
at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:339)
at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:393)
at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:742)
at java.lang.Thread.run(Thread.java:745)
2660d2f
@vietj vietj added the to review label Jul 24, 2016
@pmlopes
Member
pmlopes commented Aug 3, 2016

There are a couple of things here, postgres recognizes 2 kinds of timestamps:

  • with TZ
  • without TZ

And they are correct, what is happening is that in your DDL is that you define it without TZ. Now the implications of this is that this timestamp would make only sense if you're assuming UTC, in order to correct it to the client locale in that specific case you must suffix the client local offset.

The tricky part here is that if the server or the connection string specify other locale all conversions might get messed up.

@pmlopes pmlopes self-assigned this Aug 3, 2016
@pmlopes pmlopes added the bug label Aug 3, 2016
@pmlopes pmlopes added a commit that referenced this pull request Aug 3, 2016
@pmlopes pmlopes Fixes #53: TS WITHOUT TZ are returned as String, TS WITH TZ are conve…
…rted to Instants properly JSON formatted
4cba230
@pmlopes
Member
pmlopes commented Aug 3, 2016

In order to use TS WITHOUT TIME ZONE you must handle them as String. To handle them as Instant your data should contain TZ info.

This is required because the JSON RFC for encoding temporal types always has TZ data. If we were to define the TZ for types without TZ then would would break the behaviour of the current code.

@KengoTODA

Thanks, I proposed another MR to add one more test, could you check #55 later?

@KengoTODA

One thing I feel strange, is that, why I need to use WITH TIME ZONE to use getInstant() even though Instant class should have no timezone related information...

@Narigo Narigo closed this in #54 Aug 4, 2016
@Narigo Narigo removed the to review label Aug 4, 2016
@KengoTODA KengoTODA deleted the KengoTODA:bug-instant-postgres branch Aug 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment