-
Notifications
You must be signed in to change notification settings - Fork 161
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
NumberFormatException when parsing OID values exceeding 32-bit signed integer range #1068
Comments
I'm also hitting this issue. Postgres OID docs specify "The oid type is currently implemented as an unsigned four-byte integer. " The relevant Skunk code is here. Oid's sadly isn't representable by an
Actually, I hit this when trying out @rolang's Dumbo, a (hopefully faster) alternative to Flyway. AFAICT, because it makes use of |
I did more investigation and thinking on how to solve this.
The saving grace is, we have enough bits in an Int to store an unsigned Int without losing info. We'll just have to accept the caveat, that for the rare case of oids > Int.MaxInt, they will be represented in Scala as a negative Int:
With this encoding, the fix can be contained entire within the 1-line oid codec. If maintainers are on board with this solution (which will be backwards compatible with all Postgres oids seen to date, that are less than Int.MaxInt), I'll send a PR? |
@benhutchison This seems reasonable to me. Happy to review a PR. |
Offtopic, but since it was mentioned, from some of my testing locally Flyway was always way slower with
Same migration scripts with the same dockerized single node instance and empty database with latest Flyway as of now 10.12.0. But don't take it as proper benchmarking 😅. |
Actually Flyway's performance when a database doesn't require any migration was what drove me nuts and led me to stumble on Dumbo. I have a CLI tool that extracts reports from a database, but includes a shared kernel that also checks migrations. With Dumbo takes 10868ms to extract a report , with Flyway 19887ms |
should We close this as We merged #1086 ? |
Fortunately we have
Seems like it to me. Thanks Thanh! |
Description:
Currently,
Typer
usesInteger.parseInt()
for parsing PostgreSQL OIDs. However, PostgreSQL OIDs are 32-bit unsigned integers, and usingInteger.parseInt()
can lead to aNumberFormatException
for values that exceed the range of a 32-bit signed integer.Stack Trace
Expected behavior:
Typer should parse PostgreSQL OIDs correctly, including values within the 32-bit unsigned integer range.
Actual behavior:
When encountering a PostgreSQL OID value that exceeds the range of a 32-bit signed integer,
Integer.parseInt()
throws a NumberFormatException, as it cannot handle values outside the signed integer range.Proposed solution:
Instead of using
Integer.parseInt()
, use a method that can handle 32-bit unsigned integers, such asLong.parseLong()
or explicitly convert the value to an unsigned integer representation.The text was updated successfully, but these errors were encountered: