NumberFormatException when parsing OID values exceeding 32-bit signed integer range about skunk HOT 7 OPEN

I did more investigation and thinking on how to solve this.

1. CockroachDB apparently uses oids in the upper half of the unsigned range. Its also an awesome database that already works with Skunk, and I'd really like to support it.

2. The surface area within Skunk of oid: Int is huge. I tried changing to oid: Long but the compile errors just kept coming in waves.

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:

scala> (Int.MaxValue.toLong + 1).toInt
val res2: Int = -2147483648

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?

from skunk.

@benhutchison This seems reasonable to me. Happy to review a PR.

from skunk.

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

from skunk.

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 Int, unless we added an encoding that oids above max int were stored as negative ints?

val oid: Codec[Int] = Codec.simple(_.toString, _.toInt.asRight, Type.oid)

Actually, I hit this when trying out @rolang's Dumbo, a (hopefully faster) alternative to Flyway.

AFAICT, because it makes use of Typer.Strategy.SearchPath which requires querying database metadata, and so hits the oid codec problem.

from skunk.

Actually, I hit this when trying out @rolang's Dumbo, a (hopefully faster) alternative to Flyway.

Offtopic, but since it was mentioned, from some of my testing locally Flyway was always way slower with CockroachDB. One of the results as example that I was running:

// flyway: 75 migrations executed in 58265ms
// dumbo: 75 migrations executed in 22780ms

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 😅.

from skunk.