-
-
Notifications
You must be signed in to change notification settings - Fork 230
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
Fix Transactions in sql-libsql
#3720
Conversation
🦋 Changeset detectedLatest commit: e9727ca The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
I've just hit the same problem when using a libSQL Server (i.e. Turso); it works fine with a local SQLite (hence the tests passing). I've been looking at |
Oh very interesting. I didn't realize overrides were already an established pattern here. I mean, that provides some hope of a solution. Do you have any example code you've tried so far? The other change I want to make to this package is to allow a live client to be provided instead of config. The reason being is that the Turso client is doing replication frame tracking as well, so if you want to use the /**
* @category models
* @since 1.0.0
*/
export type LibsqlClientConfig =
& {
readonly spanAttributes?: Record<string, unknown> | undefined
readonly transformResultNames?: ((str: string) => string) | undefined
readonly transformQueryNames?: ((str: string) => string) | undefined
}
& ({
_tag: "Config"
/** The database URL.
*
* The client supports `libsql:`, `http:`/`https:`, `ws:`/`wss:` and `file:` URL. For more infomation,
* please refer to the project README:
*
* https://github.com/libsql/libsql-client-ts#supported-urls
*/
readonly url: string
/** Authentication token for the database. */
readonly authToken?: string | undefined
/** etc... */
} | {
_tag: "LiveClient"
liveClient: Libsql.Client
}) Oh, and FYI I added the live libsql docker image to the tests now via |
Not yet, but it's a blocker for us at the moment so I'll be looking shortly to see if I can figure it out. (I'm new to LibSQL too!)
👍 Could make the LibSQL client available as a service too; probably outside the scope of this package though so maybe a separate
Fantastic! Might be worth running them against both LibSQL and SQLite as there seem to be more differences than I originally anticipated. |
I suspect there is a way to get it to work, but it's beyond my Effect knowledge. It seems that |
I've hacked something together where transactions seem to work against both LibSQL Server and SQLite: thewilkybarkid@047ca25. There's probably plenty of gotchas though, and it doesn't support nested transactions/savepoints. |
Awesome work! I am out of the office today but will take a deeper look tomorrow. Feel free to hit me up on discord as well - |
How does libsql handle nested transactions? I didn't see any mention of savepoints. |
I'm not sure the JS client does at the moment, can't see any references to it. I've opened tursodatabase/libsql-client-ts#272. |
Re savepoints - once you have initiated a transaction with the client, you proceed as normal and can execute import { createClient } from "@libsql/client";
const client = createClient({ url: 'http://localhost:8080' });
const tx = await client.transaction('write');
await tx.execute(`
-- Set a savepoint
SAVEPOINT sp1;
`);
await tx.execute(`
-- Insert a new user
INSERT INTO users (name) VALUES ('Alice');
`)
await tx.execute(`
-- Set another savepoint
SAVEPOINT sp2;
`)
await tx.execute(`-- Insert another user
INSERT INTO users (name) VALUES ('Bob');
`)
await tx.execute(`-- Rollback to the second savepoint (sp2) to undo the last insert
ROLLBACK TO sp2;
`)
await tx.commit();
|
This looks like a great start to me. Do you want to make a PR into my branch, have me copy it over, or open a new PR? |
Happy for you to copy it over if you have time @jkonowitch! |
Just did - added you as I will add tests / implementation for savepoints. |
sql-libsql
sql-libsql
@tim-smart @thewilkybarkid I think this is now ready for review. |
… database can be cleaned/reused between runs
This reverts commit 72eadfd.
…now fixed This reverts commit 2654eb4.
2afd6f7
to
bedc00a
Compare
bedc00a
to
e9727ca
Compare
Thanks! I ended up adding SqlClient.makeWithTransaction to ensure consistency with tracing etc. |
Thanks both. 🙂 |
Type
Description
The
libsql
sdk uses a non-standard method of initiating transaction connections that is not compatible with@effect/sql
. It expects to fully manage sendingBEGIN
,COMMIT
,ROLLBACK
statements as well as initiating new connections because under the hood it uses a custom websocket protocol. SeeTherefore, I believe we should treatlibsql
as we doD1
, as a package that does not support transactions. Users can batch multiple statements together using thebatch
API if needed: https://docs.turso.tech/sdk/ts/reference#batch-transactions@thewilkybarkid realized that
withTransaction
can be easily overriden, so we are taking that approach.Related
sql-libsql
do not work #3709