LibPQ.jl icon indicating copy to clipboard operation
LibPQ.jl copied to clipboard

fix: datetime tryparse (#265)

Open Arkoniak opened this issue 2 years ago • 4 comments
trafficstars

I've tested it locally with Julia versions 1.7.2 and 1.8.1 and it works correctly. Not sure, whether it make sense to add any specific tests, since runtests works with postgres database as a whole and should cover this function (for latest versions of Julia at least).

Arkoniak avatar Mar 08 '23 12:03 Arkoniak

Yup no need to add specific tests.

This should be added to the ZonedDateTime and UTCDateTime parsing methods as well though, to replicate the same behaviour (I assume it also happens for those types?). For ZonedDateTime you can put the whole for-loop in the try-catch.

iamed2 avatar Mar 09 '23 18:03 iamed2

Well, in the end, I moved all @static to utility function, this allows for

  1. having clear code in main function
  2. it'll be easy to remove this fix in the future (just delete _tryparse and change all _tryparse calls to tryparse

Also, I've updated pqparse(::Type{Time}, str::AbstractString) function to use the same approach.

I am using tryparse in catch block, because for some reason for ZonedDateTime tryparse is working properly in 1.7

Arkoniak avatar Mar 13 '23 18:03 Arkoniak

Well, in the end, I moved all @static to utility function, this allows for

  1. having clear code in main function
  2. it'll be easy to remove this fix in the future (just delete _tryparse and change all _tryparse calls to tryparse

Also, I've updated pqparse(::Type{Time}, str::AbstractString) function to use the same approach.

I am using tryparse in catch block, because for some reason for ZonedDateTime tryparse is working properly in 1.7

Arkoniak avatar Mar 23 '23 08:03 Arkoniak

Sorry for bothering @iamed2 , but is there anything else that should be done in this PR?

Arkoniak avatar Mar 23 '23 08:03 Arkoniak