search

kysely-org / kysely-postgres-js

Kysely dialect for PostgreSQL using the Postgres.js client.
MIT License
58 stars 3 forks source link

Fix handling of transactions with explicit isolation level #3

Closed sds closed 1 year ago

sds commented 1 year ago

First of all, super excited for this plugin! Been wanting to use Kysely with Postgres.js for a while. Thanks for getting it started!

Looks like the current implementation doesn't support transactions with specific isolation levels. Fix that by mirroring what the vanilla Postgres driver does here: https://github.com/kysely-org/kysely/blob/a826cede4007211ec670bfe094de8acdddbbe1cb/src/dialect/postgres/postgres-driver.ts#L58-L66

igalklebanov commented 1 year ago

Hey đź‘‹

Thanks!

Could we add a test case for isolation level?

sds commented 1 year ago

I tried adding the tests from the upstream Kysely project, but it looks like the start transaction .../commit statements don't trigger an event that we can tap into here.

You're more familiar with the nuts and bolts of Kysely itself—is there a simple workaround? More specifically: is there a reason we're performing the transaction handling logic in the PostgresJSConnection rather than in the PostgresJSDriver, like what is done upstream?

igalklebanov commented 1 year ago

I tried adding the tests from the upstream Kysely project, but it looks like the start transaction .../commit statements don't trigger an event that we can tap into here.

You're more familiar with the nuts and bolts of Kysely itself—is there a simple workaround? More specifically: is there a reason we're performing the transaction handling logic in the PostgresJSConnection rather than in the PostgresJSDriver, like what is done upstream?

Its a hack around Postgres.js's limitations (no single connection getter, transaction API not suitable for Kysely - we have no control over commit/rollback) and how Kysely is built around these parts.

The lifecycle of a transaction in Kysely ~is: acquireConnection: connection -> beginTransaction(connection) -> userFunctionality(connection) -> commitTransaction(connection) OR rollbackTransaction(connection) -> releaseConnection(connection).

acquireConnection is used regardless of transaction scenario, returns a new Connection that has the pool. If that connection is then used to begin a transaction, we create a new pool of 1 that is used in this Connection until commit/rollback.

I wonder if we could spy on Postgres.js' .unsafe function and assert it was called with isolation level. I'll dig around Kysely logging, maybe I could emit the query's details.

sds commented 1 year ago

Tests pass! Let me know if there's anything else you'd like addressed as part of this PR.