Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tryparse Inexact Error #265

Closed
Arkoniak opened this issue Mar 2, 2023 · 5 comments
Closed

tryparse Inexact Error #265

Arkoniak opened this issue Mar 2, 2023 · 5 comments

Comments

@Arkoniak
Copy link

Arkoniak commented Mar 2, 2023

It seems that this commit: b82a753 introduced regression in processing of tryparse function in function pqparse(::Type{DateTime}, str::AbstractString), similar to the issue that was resolved in #248

So, currently in some 1.6 and 1.7 versions of Julia, selecting from the postgres table which has timestamp column results in InexactError from tryparse function.

@iamed2
Copy link
Collaborator

iamed2 commented Mar 2, 2023

Can you give an example please?

I cannot seem to replicate this on Julia 1.6.7 with LibPQ 1.15.1.

@Arkoniak
Copy link
Author

Arkoniak commented Mar 3, 2023

Sure.

Julia 1.7.1, LibPQ 1.15.1:

using LibPQ
using Dates

LibPQ.pqparse(DateTime, "2023-03-03 12:34:56.789012")

yields output

ERROR: InexactError: convert(Dates.Decimal3, 789012)
Stacktrace:
 [1] tryparsenext
   @ ~/Languages/julia-1.7.1/share/julia/stdlib/v1.7/Dates/src/io.jl:153 [inlined]
 [2] tryparsenext
   @ ~/Languages/julia-1.7.1/share/julia/stdlib/v1.7/Dates/src/io.jl:41 [inlined]
 [3] macro expansion
   @ ~/Languages/julia-1.7.1/share/julia/stdlib/v1.7/Dates/src/parse.jl:64 [inlined]
 [4] tryparsenext_core(str::String, pos::Int64, len::Int64, df::DateFormat{Symbol("y-m-d HH:MM:SS.s"), Tuple{Dates.DatePart{'y'}, Dates.Delim{Char, 1}, Dates.DatePart{'m'}, Dates.Delim{Char, 1}, Dates.DatePart{'d'}, Dates.Delim{Char, 1}, Dates.DatePart{'H'}, Dates.Delim{Char, 1}, Dates.DatePart{'M'}, Dates.Delim{Char, 1}, Dates.DatePart{'S'}, Dates.Delim{Char, 1}, Dates.DatePart{'s'}}}, raise::Bool)
   @ Dates ~/Languages/julia-1.7.1/share/julia/stdlib/v1.7/Dates/src/parse.jl:38
 [5] macro expansion
   @ ~/Languages/julia-1.7.1/share/julia/stdlib/v1.7/Dates/src/parse.jl:150 [inlined]
 [6] tryparsenext_internal
   @ ~/Languages/julia-1.7.1/share/julia/stdlib/v1.7/Dates/src/parse.jl:125 [inlined]
 [7] tryparse(::Type{DateTime}, str::String, df::DateFormat{Symbol("y-m-d HH:MM:SS.s"), Tuple{Dates.DatePart{'y'}, Dates.Delim{Char, 1}, Dates.DatePart{'m'}, Dates.Delim{Char, 1}, Dates.DatePart{'d'}, Dates.Delim{Char, 1}, Dates.DatePart{'H'}, Dates.Delim{Char, 1}, Dates.DatePart{'M'}, Dates.Delim{Char, 1}, Dates.DatePart{'S'}, Dates.Delim{Char, 1}, Dates.DatePart{'s'}}})
   @ Dates ~/Languages/julia-1.7.1/share/julia/stdlib/v1.7/Dates/src/parse.jl:290
 [8] pqparse(#unused#::Type{DateTime}, str::String)
   @ LibPQ ~/.julia/packages/LibPQ/gYEIG/src/parsing.jl:262
 [9] top-level scope
   @ REPL[8]:1

In Julia 1.8.2, LibPQ 1.15.1

using LibPQ
using Dates

LibPQ.pqparse(DateTime, "2023-03-03 12:34:56.789012")

yields

2023-03-03T12:34:56.789

Also, Julia 1.7.1, LibPQ 1.14.1

julia> using LibPQ
julia> using Dates
julia> LibPQ.pqparse(DateTime, "2023-03-03 12:34:56.789123")
2023-03-03T12:34:56.789

@iamed2
Copy link
Collaborator

iamed2 commented Mar 3, 2023

This works properly with LibPQ.jl 1.15.1 on Julia 1.7.3, Julia 1.6.7, and Julia 1.8.2+. Unfortunately my suggestion is to upgrade Julia to a later patch version, since the above error is a bug in Julia that was fixed.

@iamed2 iamed2 closed this as completed Mar 3, 2023
@Arkoniak
Copy link
Author

Arkoniak commented Mar 3, 2023

Sorry, I do not quite understand...
Why can't we do the same as in #248? It seems simple enough.

Or, even better, just change https://github.com/iamed2/LibPQ.jl/blob/master/src/parsing.jl#L262 from

parsed = tryparse(DateTime, str, TIMESTAMP_FORMAT)

to

parsed = tryparse(DateTime, _trunc_seconds(str), TIMESTAMP_FORMAT)

as it basically was in 1.14.1?

I can make a PR if you do not have time for these changes, no big deal.

@iamed2
Copy link
Collaborator

iamed2 commented Mar 6, 2023

I would accept a PR for a static version-guarded fix for old patch versions. Typically, old patch versions of Julia are not supported, as they are neither tested in CI nor recommended for installation by the Julia developers. And I do not want to impact performance for everyone by adding regex replacement to every parse call (which is why it is written as it is now). But a different version guarded by @static would have no performance impact and is clearly marked.

I cannot guarantee it will survive in perpetuity, especially if there is a refactor. Testing on old patch versions is wasteful. However, if you reference this issue, I will ping you before removing your changes in the future to check if you've moved to a patched version of Julia.

Arkoniak pushed a commit to Arkoniak/LibPQ2.jl that referenced this issue Mar 8, 2023
Arkoniak pushed a commit to Arkoniak/LibPQ2.jl that referenced this issue Mar 13, 2023
Arkoniak pushed a commit to Arkoniak/LibPQ2.jl that referenced this issue Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants