From 4391d0532339f00b8138c997b624be2fa3cfbff6 Mon Sep 17 00:00:00 2001 From: Taras Kornichuk Date: Sun, 25 Oct 2026 17:07:32 +0200 Subject: [PATCH 1/2] feat(database): put transaction settings in BEGIN through SqlClient.BeginTransaction effect 4.0.6 adds SqlClient.BeginTransaction (Effect-TS/effect#9576), a reference that replaces the client's BEGIN for one top-level transaction. When the installed effect has it and the dialect gives the new optional DialectTransactions.beginTransaction hook, transaction() starts with BEGIN ISOLATION LEVEL ..., READ ONLY, DEFERRABLE and sends no SET TRANSACTION. On effect 4.0.0 to 4.0.5 nothing changes. The reference is read by name, so the peer range stays ^4.0.0, and it is cleared inside the body so a transaction there on another client keeps its own BEGIN. Co-Authored-By: Claude Opus 5.5 --- .changeset/begin-transaction-settings.md | 5 ++ design/transactions.md | 21 ++++++++ docs/database.md | 12 +++-- docs/reference.md | 4 +- src/ch/dialect.ts | 8 +++ src/database/begin-transaction.ts | 22 +++++++++ src/database/database.test.ts | 62 ++++++++++++++++++++++-- src/database/database.ts | 53 ++++++++++++++++---- src/pg/dialect.ts | 24 ++++++--- 9 files changed, 188 insertions(+), 23 deletions(-) create mode 100644 .changeset/begin-transaction-settings.md create mode 100644 src/database/begin-transaction.ts diff --git a/.changeset/begin-transaction-settings.md b/.changeset/begin-transaction-settings.md new file mode 100644 index 0000000..b9fab65 --- /dev/null +++ b/.changeset/begin-transaction-settings.md @@ -0,0 +1,5 @@ +--- +"@maple-dev/effect-orm": minor +--- + +With effect 4.0.6 or later, `Db.transaction({ isolationLevel, accessMode, deferrable })` puts the settings in the statement that starts the transaction (`BEGIN ISOLATION LEVEL SERIALIZABLE, READ ONLY`) through Effect's `SqlClient.BeginTransaction`, one round trip less than the `SET TRANSACTION` it sent before. On effect 4.0.0 to 4.0.5, which do not have `BeginTransaction`, it still sends `SET TRANSACTION`. A dialect gives the BEGIN statement with the new optional `DialectTransactions.beginTransaction`; `postgresDialect` has it. `observe` sees the BEGIN statement. diff --git a/design/transactions.md b/design/transactions.md index b8ffda5..106afc4 100644 --- a/design/transactions.md +++ b/design/transactions.md @@ -68,6 +68,11 @@ client (`beginTransaction`, default `"BEGIN"`; ClickHouse passes `"BEGIN TRANSAC a `SET TRANSACTION ...` statement as the first statement inside the transaction, which Postgres accepts until the first query of the transaction. +effect 4.0.6 adds `SqlClient.BeginTransaction` (Effect-TS/effect#9576): a +`Context.Reference` that, when set, replaces the client's BEGIN for a +top-level transaction (sent unprepared; savepoints ignore it). `transaction` uses it when the +installed effect has it; see section 9. + **Tracing.** A `sql.transaction` span with `db.transaction.commit`, `db.transaction.savepoint` and `db.transaction.rollback` events; statements inside are children of it. @@ -452,6 +457,10 @@ export interface Dialect extends SqlSyntax { `withTransaction` cannot do for a pooled client. That is the point to revisit, not now. - SQLite later: `full`, savepoints, no isolation levels (it is serializable), access mode via `BEGIN DEFERRED/IMMEDIATE` rather than options. +- With `SqlClient.BeginTransaction` (section 9) a dialect can also give `beginTransaction`, + the whole BEGIN statement. That is the hook for SQLite's `BEGIN IMMEDIATE` and MySQL's + `START TRANSACTION READ ONLY`. MySQL's isolation level still needs a `SET TRANSACTION` + before BEGIN, which neither hook can do on a pooled client. Capability is runtime, checked before any statement. A type-level split (no `transaction` method on a ClickHouse database) was considered and deferred: the `Dialect` value is not a @@ -646,3 +655,15 @@ What landed differently from the sections above: - **Not yet done from phase 2:** the over-the-wire `@effect/sql-pg` suite (two connections, real serialization failures), and filing the upstream Effect issue for orDie'd COMMIT and ROLLBACK. The PGlite suite simulates contention with `RAISE ... USING ERRCODE`. +- **Settings in BEGIN (`SqlClient.BeginTransaction`).** `DialectTransactions.beginTransaction` + (optional) writes the BEGIN that carries the settings; Postgres writes + `BEGIN ISOLATION LEVEL ..., READ ONLY, DEFERRABLE`. The peer range starts at effect 4.0.0, + which has no `BeginTransaction`, so `src/database/begin-transaction.ts` reads it by name + (`Reflect.get`, so bundlers do not flag a missing export). When it is there and the dialect + has the hook, `transaction` provides the statement around `withTransaction` and sends no + `SET TRANSACTION`; otherwise it sends `SET TRANSACTION` as before. The reference is cleared + inside the body: Effect applies it to every top-level transaction started while it is + provided, so a transaction the body starts on a second client would take it too. `observe` + sees the BEGIN statement, so a statement collector still sees the settings. When the peer + range moves to `^4.0.6`, the seam becomes a plain `SqlClient.BeginTransaction` and the + `SET TRANSACTION` path stays only for dialects without `beginTransaction`. diff --git a/docs/database.md b/docs/database.md index 2e0cccd..41d089c 100644 --- a/docs/database.md +++ b/docs/database.md @@ -143,7 +143,7 @@ For SQL inside a builder query rather than a whole statement, use | --- | --- | | `dialect` | `postgresDialect` or `clickhouseDialect` | | `command` | Wraps `execute`. Pass ClickHouse's `asCommand`: DDL has no JSON result | -| `observe` | Called with every statement before it runs, including the `SET TRANSACTION` a transaction's settings become | +| `observe` | Called with every statement before it runs, including the `BEGIN ...` or `SET TRANSACTION` a transaction's settings become | `run` refuses a query compiled for another dialect (`CompiledQuery.dialect`), as a defect: a query compiled with the `/clickhouse` entry's `compile` would run on Postgres with the wrong @@ -205,7 +205,12 @@ and settings. | `deferrable` | With `serializable` and `read only`, wait for a safe snapshot | | `retry` | `"contention"`, or `{ times, schedule }` | -Settings become one `SET TRANSACTION ...` statement, run first. A nested transaction cannot +With effect 4.0.6 or later, settings go into the statement that starts the transaction, +`BEGIN ISOLATION LEVEL SERIALIZABLE, READ ONLY`, through Effect's `SqlClient.BeginTransaction`. +With effect 4.0.0 to 4.0.5, which do not have it, they become one `SET TRANSACTION ...` +statement, run first. The result is the same; BEGIN saves one round trip. Inside the body, +`BeginTransaction` is cleared, so a transaction the body starts on another client keeps that +client's own BEGIN. A nested transaction cannot set any of them, or `retry`, and fails with `TransactionOptionsRejected`: Postgres fixes the isolation level at the first statement, and a retry has to start the outermost transaction again. `read uncommitted` is not offered because Postgres runs it as `read committed`. @@ -221,7 +226,8 @@ again. `read uncommitted` is not offered because Postgres runs it as `read commi | `TransactionUnsupported` | The dialect has no transactions (ClickHouse) | Effect 4.0.0 turns a failed COMMIT or ROLLBACK into a defect and, when ROLLBACK fails, drops -the body's own failure. `transaction` turns both back into the typed errors above. Defects +the body's own failure. +`transaction` turns both back into the typed errors above. Defects from `body` stay defects. `TransactionClosed` is a defect, not an error to handle. A fiber forked inside a transaction diff --git a/docs/reference.md b/docs/reference.md index abc2d46..e6b600a 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -88,7 +88,9 @@ to its own dialect; `options.dialect` overrides it. `Dialect`, `DialectClauses` and `ParamStyle` describe a database: how identifiers and literals are written, how params reach the server, and which clauses exist. Pass one as `options.dialect`. `DialectTransactions` (with `IsolationLevel` and `TransactionSettings`) says -which transactions the database supports; see [Database](./database.md). See [Params and compilation](./params-and-compilation.md#dialects) and +which transactions the database supports, and how settings are written: `beginTransaction` +(optional) for a BEGIN that carries them, `setTransaction` for a statement run after BEGIN; +see [Database](./database.md#settings). See [Params and compilation](./params-and-compilation.md#dialects) and [Postgres](./postgres.md). ### Params diff --git a/src/ch/dialect.ts b/src/ch/dialect.ts index 17b6c97..6180842 100644 --- a/src/ch/dialect.ts +++ b/src/ch/dialect.ts @@ -110,6 +110,14 @@ export interface DialectTransactions { * transaction, or `undefined` when there is nothing to apply. */ readonly setTransaction: (settings: TransactionSettings) => string | undefined + /** + * The statement that starts a transaction with `settings` applied, such as + * `BEGIN ISOLATION LEVEL SERIALIZABLE`, or `undefined` to keep the client's + * BEGIN. Used in place of `setTransaction` when the installed effect has + * `SqlClient.BeginTransaction` (4.0.6 and later); without this hook, or on an + * older effect, `setTransaction` is used. + */ + readonly beginTransaction?: (settings: TransactionSettings) => string | undefined } /** No transactions. What a dialect without a `transactions` entry gets. */ diff --git a/src/database/begin-transaction.ts b/src/database/begin-transaction.ts new file mode 100644 index 0000000..c2fbb80 --- /dev/null +++ b/src/database/begin-transaction.ts @@ -0,0 +1,22 @@ +// Effect's `SqlClient.BeginTransaction` (effect 4.0.6, Effect-TS/effect#9576): +// a reference whose value replaces the client's BEGIN for one top-level +// transaction. With it, a transaction's settings travel in BEGIN itself, one +// round trip less than a separate `SET TRANSACTION`. +// +// The peer range starts at effect 4.0.0, which has no such export, so it is read +// by name at run time: `undefined` there, and `transaction` keeps sending `SET +// TRANSACTION`. `Reflect.get` keeps bundlers from flagging a missing export +// on an older effect. When the peer range moves to ^4.0.6, this module becomes +// a plain `SqlClient.BeginTransaction`. + +import { Context } from "effect" +import * as SqlClient from "effect/sql/SqlClient" + +/** The statement that starts a top-level transaction; `undefined` keeps the client's. */ +export type BeginTransactionReference = Context.Reference + +const found: unknown = Reflect.get(SqlClient, "BeginTransaction") + +/** `SqlClient.BeginTransaction` when the installed effect has it. */ +export const coreBeginTransaction: BeginTransactionReference | undefined = + Context.isKey(found) && Context.isReference(found) ? (found as BeginTransactionReference) : undefined diff --git a/src/database/database.test.ts b/src/database/database.test.ts index 072673f..1c1cc55 100644 --- a/src/database/database.test.ts +++ b/src/database/database.test.ts @@ -5,13 +5,15 @@ import { PgliteClient } from "@effect/sql-pglite" import { assert, describe, expect, it, layer } from "@effect/vitest" -import { Cause, DateTime, Deferred, Effect, Exit, Fiber, Layer, Ref, Schedule, Schema } from "effect" +import { Cause, Context, DateTime, Deferred, Effect, Exit, Fiber, Layer, Ref, Schedule, Schema } from "effect" import * as SqlClient from "effect/sql/SqlClient" import * as CH from "../ch/index" import * as PG from "../postgres" import { clickhouseDialect } from "../ch/dialect" import { postgresDialect } from "../pg/dialect" import * as Db from "../database" +import { coreBeginTransaction } from "./begin-transaction" +import { makeDatabase } from "./database" import { renderTemplate } from "./sql" const statements: Array = [] @@ -44,6 +46,19 @@ const ids = (table: string) => const raiseSqlState = (code: "40001" | "40P01") => Db.execute({ sql: `DO $$ BEGIN RAISE EXCEPTION 'forced' USING ERRCODE = '${code}'; END $$` }) +// Effect 4.0.6 reads `SqlClient.BeginTransaction` when a transaction starts. This +// client does the same with a test reference on any Effect version: it records +// the statement it would begin with, then runs the real transaction. +const FakeBegin = Context.Reference("test/BeginTransaction", { defaultValue: () => undefined }) +const recordingBegin = (sql: SqlClient.SqlClient, begins: Array): SqlClient.SqlClient => + Object.assign(Object.create(sql) as SqlClient.SqlClient, { + withTransaction: (effect: Effect.Effect) => + Effect.flatMap(Effect.service(FakeBegin), (begin) => { + begins.push(begin) + return sql.withTransaction(effect) + }), + }) + class Domain extends Error { readonly _tag = "Domain" } @@ -214,7 +229,7 @@ layer(Live, { excludeTestServices: true })("Database on PGlite", (it) => { }), ) - it.effect("applies settings as the first statement of the transaction", () => + it.effect("applies settings when the transaction starts", () => Effect.gen(function* () { statements.length = 0 const settings = yield* Db.transaction( @@ -227,12 +242,53 @@ layer(Live, { excludeTestServices: true })("Database on PGlite", (it) => { }), { isolationLevel: "serializable", accessMode: "read only", deferrable: true }, ) - expect(statements[0]).toBe("SET TRANSACTION ISOLATION LEVEL SERIALIZABLE, READ ONLY, DEFERRABLE") + // effect 4.0.6 and later carry the settings in BEGIN; 4.0.0 sends them as SET TRANSACTION. + expect(statements[0]).toBe( + `${coreBeginTransaction === undefined ? "SET TRANSACTION" : "BEGIN"} ISOLATION LEVEL SERIALIZABLE, READ ONLY, DEFERRABLE`, + ) expect(settings.row).toEqual({ isolation: "serializable", read_only: "on" }) expect(settings.info).toEqual({ depth: 0, isolationLevel: "serializable", accessMode: "read only" }) }), ) + it.effect("with Effect's BeginTransaction, settings go into the BEGIN statement and no SET TRANSACTION is sent", () => + Effect.gen(function* () { + const begins: Array = [] + const sent: Array = [] + const db = makeDatabase( + recordingBegin(yield* SqlClient.SqlClient, begins), + { dialect: postgresDialect, observe: (statement) => Effect.sync(() => void sent.push(statement.sql)) }, + FakeBegin, + ) + const inBody = yield* db.transaction( + Effect.andThen(db.execute(Db.sql`SELECT 1`), Effect.service(FakeBegin)), + { isolationLevel: "serializable", accessMode: "read only", deferrable: true }, + ) + yield* db.transaction(db.execute(Db.sql`SELECT 2`)) + expect(begins).toEqual(["BEGIN ISOLATION LEVEL SERIALIZABLE, READ ONLY, DEFERRABLE", undefined]) + expect(sent).toEqual(["BEGIN ISOLATION LEVEL SERIALIZABLE, READ ONLY, DEFERRABLE", "SELECT 1", "SELECT 2"]) + // Not seen inside the body, so a transaction it starts on another client keeps that client's BEGIN. + expect(inBody).toBeUndefined() + }), + ) + + it.effect("without Effect's BeginTransaction, settings are a SET TRANSACTION run first", () => + Effect.gen(function* () { + const sent: Array = [] + const db = makeDatabase( + yield* SqlClient.SqlClient, + { dialect: postgresDialect, observe: (statement) => Effect.sync(() => void sent.push(statement.sql)) }, + undefined, + ) + const row = yield* db.transaction( + db.query(Db.sql`SELECT current_setting('transaction_isolation') AS isolation`), + { isolationLevel: "repeatable read" }, + ) + expect(sent[0]).toBe("SET TRANSACTION ISOLATION LEVEL REPEATABLE READ") + expect(row).toEqual([{ isolation: "repeatable read" }]) + }), + ) + it.effect("a write under read only fails as DatabaseError with its SQLSTATE", () => Effect.gen(function* () { const table = yield* freshTable diff --git a/src/database/database.ts b/src/database/database.ts index aaf89d4..f64258b 100644 --- a/src/database/database.ts +++ b/src/database/database.ts @@ -29,6 +29,7 @@ import { TransactionUnsupported, type TransactionError, } from "./errors" +import { coreBeginTransaction, type BeginTransactionReference } from "./begin-transaction" import { isSqlTemplate, renderTemplate, type SqlTemplate } from "./sql" import { firstLine, reasonOf, sqlStateOf, toDatabaseError } from "./sql-error" import { traceStatement, type TracedStatement } from "./tracing" @@ -149,9 +150,9 @@ export interface FromSqlClientOptions { */ readonly command?: (effect: Effect.Effect) => Effect.Effect /** - * Called with every statement before it runs, including the `SET - * TRANSACTION` a transaction's settings become. For collecting statements - * into one span or log line per request. + * Called with every statement before it runs, including the `BEGIN ...` + * (effect 4.0.6 and later) or `SET TRANSACTION` a transaction's settings + * become. For collecting statements into one span or log line per request. */ readonly observe?: (statement: Statement) => Effect.Effect /** @@ -203,7 +204,20 @@ const committed = (state: OpenState) => }) /** A `Database` over an Effect `SqlClient`. */ -export const fromSqlClient = (sql: SqlClient.SqlClient, options: FromSqlClientOptions): DatabaseApi => { +export const fromSqlClient = (sql: SqlClient.SqlClient, options: FromSqlClientOptions): DatabaseApi => + makeDatabase(sql, options, coreBeginTransaction) + +/** + * `fromSqlClient` with the `SqlClient.BeginTransaction` reference given, so + * tests can drive both paths on any effect. `undefined` means the installed + * effect has none: settings are sent as `SET TRANSACTION`. Not exported from + * the package. + */ +export const makeDatabase = ( + sql: SqlClient.SqlClient, + options: FromSqlClientOptions, + beginReference: BeginTransactionReference | undefined, +): DatabaseApi => { const { dialect } = options const capabilities = dialect.transactions ?? noTransactions const command = options.command ?? ((effect) => effect) @@ -384,7 +398,16 @@ export const fromSqlClient = (sql: SqlClient.SqlClient, options: FromSqlClientOp depth === 0 || Option.isNone(outer) ? { depth, isolationLevel: settings.isolationLevel, accessMode: settings.accessMode } : { ...outer.value, depth } - const setTransaction = depth === 0 ? capabilities.setTransaction(settings) : undefined + // Settings travel in BEGIN when both effect and the dialect can do it, + // else as a `SET TRANSACTION` run first. Top level only: a savepoint + // takes no settings. + const beginHook = beginReference === undefined ? undefined : capabilities.beginTransaction + const beginStatement = depth === 0 && beginHook !== undefined ? beginHook(settings) : undefined + const begin = + beginReference !== undefined && beginStatement !== undefined + ? { reference: beginReference, sql: beginStatement } + : undefined + const setTransaction = depth === 0 && beginHook === undefined ? capabilities.setTransaction(settings) : undefined const state: OpenState = { open: true, client: sql } let bodyExit: Exit.Exit | undefined @@ -392,6 +415,10 @@ export const fromSqlClient = (sql: SqlClient.SqlClient, options: FromSqlClientOp if (setTransaction !== undefined) yield* execute({ sql: setTransaction }) return yield* body }).pipe( + // The reference applies to every top-level transaction started while it + // is provided; clear it for the body, so a transaction on another client + // keeps that client's BEGIN. + (effect) => (begin === undefined ? effect : Effect.provideService(effect, begin.reference, undefined)), Effect.provideService(Transaction, info), Effect.provideService(TransactionOpen, state), Effect.onExit((exit) => @@ -404,9 +431,16 @@ export const fromSqlClient = (sql: SqlClient.SqlClient, options: FromSqlClientOp ), ) - const exit = yield* Effect.exit(sql.withTransaction(inner).pipe(Effect.ensuring(committed(state)))) + const transaction = + begin === undefined + ? sql.withTransaction(inner) + : Effect.andThen( + observe({ sql: begin.sql }), + sql.withTransaction(inner).pipe(Effect.provideService(begin.reference, begin.sql)), + ) + const exit = yield* Effect.exit(transaction.pipe(Effect.ensuring(committed(state)))) if (Exit.isSuccess(exit)) return exit.value - return yield* restoreCause(exit.cause, bodyExit, depth) + return yield* restoreCause(exit.cause, bodyExit, depth, begin?.sql) }).pipe( Effect.withSpan("effect_orm.transaction", { // Semconv defines no transaction attributes, and `db.*` is reserved for it. @@ -435,6 +469,7 @@ const restoreCause = ( cause: Cause.Cause, bodyExit: Exit.Exit | undefined, depth: number, + begin: string | undefined, ): Effect.Effect => { const failure = Cause.findError(cause) if (failure._tag === "Success" && failure.success instanceof BodyDefect) { @@ -455,7 +490,7 @@ const restoreCause = ( ) } // The body never ran: BEGIN or SAVEPOINT died, not ROLLBACK. - if (bodyExit === undefined) return Effect.fail(toDatabaseError(depth === 0 ? "BEGIN" : "SAVEPOINT")(error)) + if (bodyExit === undefined) return Effect.fail(toDatabaseError(depth === 0 ? (begin ?? "BEGIN") : "SAVEPOINT")(error)) // An interrupted transaction whose ROLLBACK died stays interrupted. if (Cause.hasInterrupts(bodyExit.cause)) return Effect.failCause(bodyExit.cause as Cause.Cause) return Effect.fail( @@ -468,7 +503,7 @@ const restoreCause = ( } // BEGIN or SAVEPOINT failed before the body ran. if (bodyExit === undefined && failure._tag === "Success" && isSqlError(failure.success)) { - return Effect.fail(toDatabaseError(depth === 0 ? "BEGIN" : "SAVEPOINT")(failure.success)) + return Effect.fail(toDatabaseError(depth === 0 ? (begin ?? "BEGIN") : "SAVEPOINT")(failure.success)) } return Effect.failCause(cause as Cause.Cause) } diff --git a/src/pg/dialect.ts b/src/pg/dialect.ts index e76a414..ef4bcf4 100644 --- a/src/pg/dialect.ts +++ b/src/pg/dialect.ts @@ -1,7 +1,7 @@ // The Postgres dialect. import { DateTime, Schema } from "effect" -import type { Dialect } from "../ch/dialect" +import type { Dialect, TransactionSettings } from "../ch/dialect" import { QueryBuilderError } from "../ch/errors" import { PARAM_MARKER_PREFIX } from "../ch/param" import { PgTimestampLiteral, timestampLiteral } from "./types" @@ -85,6 +85,16 @@ const placeholderCasts: Readonly> = { * rather than ClickHouse's `1`/`0`, and `param.dateTime` an ISO-8601 instant * rather than a zoneless string a session time zone could reinterpret. */ +// Postgres takes the same mode list after `SET TRANSACTION` and after `BEGIN`. +const transactionModes = (settings: TransactionSettings): string | undefined => { + const modes = [ + settings.isolationLevel === undefined ? undefined : `ISOLATION LEVEL ${settings.isolationLevel.toUpperCase()}`, + settings.accessMode?.toUpperCase(), + settings.deferrable === undefined ? undefined : settings.deferrable ? "DEFERRABLE" : "NOT DEFERRABLE", + ].filter((mode) => mode !== undefined) + return modes.length === 0 ? undefined : modes.join(", ") +} + export const postgresDialect: Dialect = { name: "postgres", functions: "postgres", @@ -120,12 +130,12 @@ export const postgresDialect: Dialect = { accessModes: true, deferrable: true, setTransaction: (settings) => { - const modes = [ - settings.isolationLevel === undefined ? undefined : `ISOLATION LEVEL ${settings.isolationLevel.toUpperCase()}`, - settings.accessMode?.toUpperCase(), - settings.deferrable === undefined ? undefined : settings.deferrable ? "DEFERRABLE" : "NOT DEFERRABLE", - ].filter((mode) => mode !== undefined) - return modes.length === 0 ? undefined : `SET TRANSACTION ${modes.join(", ")}` + const modes = transactionModes(settings) + return modes === undefined ? undefined : `SET TRANSACTION ${modes}` + }, + beginTransaction: (settings) => { + const modes = transactionModes(settings) + return modes === undefined ? undefined : `BEGIN ${modes}` }, }, } From 9a661d42b94b9b94491b1d16362fc5873f105c1b Mon Sep 17 00:00:00 2001 From: Taras Kornichuk Date: Sun, 25 Oct 2026 17:07:36 +0200 Subject: [PATCH 2/2] fix(database): keep a failed COMMIT typed when effect reports it as a failure effect 4.0.6 fails with the SqlError on a failed COMMIT, where 4.0.0 died. A SqlError failure after the body succeeded can only be COMMIT's, so it is TransactionCommitFailed on both versions. Co-Authored-By: Claude Opus 5.5 --- .changeset/begin-transaction-settings.md | 2 +- design/transactions.md | 3 ++ docs/database.md | 4 +-- src/database/database.test.ts | 24 ++++++++++++++++ src/database/database.ts | 36 ++++++++++++++---------- 5 files changed, 51 insertions(+), 18 deletions(-) diff --git a/.changeset/begin-transaction-settings.md b/.changeset/begin-transaction-settings.md index b9fab65..3179896 100644 --- a/.changeset/begin-transaction-settings.md +++ b/.changeset/begin-transaction-settings.md @@ -2,4 +2,4 @@ "@maple-dev/effect-orm": minor --- -With effect 4.0.6 or later, `Db.transaction({ isolationLevel, accessMode, deferrable })` puts the settings in the statement that starts the transaction (`BEGIN ISOLATION LEVEL SERIALIZABLE, READ ONLY`) through Effect's `SqlClient.BeginTransaction`, one round trip less than the `SET TRANSACTION` it sent before. On effect 4.0.0 to 4.0.5, which do not have `BeginTransaction`, it still sends `SET TRANSACTION`. A dialect gives the BEGIN statement with the new optional `DialectTransactions.beginTransaction`; `postgresDialect` has it. `observe` sees the BEGIN statement. +With effect 4.0.6 or later, `Db.transaction({ isolationLevel, accessMode, deferrable })` puts the settings in the statement that starts the transaction (`BEGIN ISOLATION LEVEL SERIALIZABLE, READ ONLY`) through Effect's `SqlClient.BeginTransaction`, one round trip less than the `SET TRANSACTION` it sent before. On effect 4.0.0 to 4.0.5, which do not have `BeginTransaction`, it still sends `SET TRANSACTION`. A dialect gives the BEGIN statement with the new optional `DialectTransactions.beginTransaction`; `postgresDialect` has it. `observe` sees the BEGIN statement. A failed COMMIT, which effect 4.0.6 reports as a `SqlError` failure instead of a defect, is still `TransactionCommitFailed`. diff --git a/design/transactions.md b/design/transactions.md index 106afc4..4b8e2a7 100644 --- a/design/transactions.md +++ b/design/transactions.md @@ -667,3 +667,6 @@ What landed differently from the sections above: sees the BEGIN statement, so a statement collector still sees the settings. When the peer range moves to `^4.0.6`, the seam becomes a plain `SqlClient.BeginTransaction` and the `SET TRANSACTION` path stays only for dialects without `beginTransaction`. +- **COMMIT failure on effect 4.0.6.** Effect 4.0.6 reports a failed COMMIT as a `SqlError` + failure, not a defect. A `SqlError` failure after the body succeeded is COMMIT's, so it is + `TransactionCommitFailed` on both versions. ROLLBACK still dies on 4.0.6. diff --git a/docs/database.md b/docs/database.md index 41d089c..8b1ea2e 100644 --- a/docs/database.md +++ b/docs/database.md @@ -226,8 +226,8 @@ again. `read uncommitted` is not offered because Postgres runs it as `read commi | `TransactionUnsupported` | The dialect has no transactions (ClickHouse) | Effect 4.0.0 turns a failed COMMIT or ROLLBACK into a defect and, when ROLLBACK fails, drops -the body's own failure. -`transaction` turns both back into the typed errors above. Defects +the body's own failure. From 4.0.6, a failed COMMIT is a `SqlError` failure instead. +`transaction` turns all of these back into the typed errors above. Defects from `body` stay defects. `TransactionClosed` is a defect, not an error to handle. A fiber forked inside a transaction diff --git a/src/database/database.test.ts b/src/database/database.test.ts index 1c1cc55..80dfe3c 100644 --- a/src/database/database.test.ts +++ b/src/database/database.test.ts @@ -7,6 +7,7 @@ import { PgliteClient } from "@effect/sql-pglite" import { assert, describe, expect, it, layer } from "@effect/vitest" import { Cause, Context, DateTime, Deferred, Effect, Exit, Fiber, Layer, Ref, Schedule, Schema } from "effect" import * as SqlClient from "effect/sql/SqlClient" +import { isSqlError } from "effect/sql/SqlError" import * as CH from "../ch/index" import * as PG from "../postgres" import { clickhouseDialect } from "../ch/dialect" @@ -313,6 +314,29 @@ layer(Live, { excludeTestServices: true })("Database on PGlite", (it) => { }), ) + it.effect("a failed COMMIT in the error channel, as effect 4.0.6 reports it, is TransactionCommitFailed too", () => + Effect.gen(function* () { + const sql = yield* SqlClient.SqlClient + // effect 4.0.0 dies on a failed COMMIT; 4.0.6 fails with the SqlError. + const failingCommit = Object.assign(Object.create(sql) as SqlClient.SqlClient, { + withTransaction: (effect: Effect.Effect) => + sql.withTransaction(effect).pipe( + Effect.catchDefect((defect) => (isSqlError(defect) ? Effect.fail(defect) : Effect.die(defect))), + ), + }) + const db = makeDatabase(failingCommit, { dialect: postgresDialect }, undefined) + const parent = `t_${++tables}` + yield* db.execute(Db.sql`CREATE TABLE ${Db.sql.identifier(parent)} (id int4 PRIMARY KEY)`) + const child = Db.sql.identifier(`${parent}_child`) + yield* db.execute( + Db.sql`CREATE TABLE ${child} (id int4, parent int4 REFERENCES ${Db.sql.identifier(parent)}(id) DEFERRABLE INITIALLY DEFERRED)`, + ) + const error = yield* Effect.flip(db.transaction(db.execute(Db.sql`INSERT INTO ${child} VALUES (1, 999)`))) + assert(error instanceof Db.TransactionCommitFailed) + expect(error.sqlState).toBe("23503") + }), + ) + it.effect("retry: contention re-runs the whole transaction", () => Effect.gen(function* () { const table = yield* freshTable diff --git a/src/database/database.ts b/src/database/database.ts index f64258b..f471495 100644 --- a/src/database/database.ts +++ b/src/database/database.ts @@ -10,7 +10,7 @@ import { Cause, Context, Effect, Exit, Layer, Option, Schedule, Schema } from "effect" import { dual } from "effect/Function" import * as SqlClient from "effect/sql/SqlClient" -import { isSqlError } from "effect/sql/SqlError" +import { isSqlError, type SqlError } from "effect/sql/SqlError" import { compileCH, compileUnion, CompiledQueryDecodeError, type CompiledQuery } from "../ch/compile" import { noTransactions, type Dialect, type IsolationLevel, type TransactionSettings } from "../ch/dialect" import type { QueryBuilderError } from "../ch/errors" @@ -459,11 +459,23 @@ export const makeDatabase = ( return { dialect, run, query, execute, transaction, retryContention } } +const commitFailed = (error: SqlError, depth: number): TransactionCommitFailed => { + const sqlState = sqlStateOf(error) + return new TransactionCommitFailed({ + message: `${depth === 0 ? "COMMIT" : "releasing the savepoint"} failed: ${firstLine(error)}`, + reason: reasonOf(error), + ...(sqlState === undefined ? undefined : { sqlState }), + cause: error, + }) +} + /** * Turn what came out of `withTransaction` back into what the caller should see. - * Effect dies on a failed COMMIT or ROLLBACK, and a dying ROLLBACK replaces the - * body's own exit. Body defects were carried through as `BodyDefect`, so any - * other defect that is a `SqlError` came from transaction control. + * Effect dies on a failed ROLLBACK, and a dying ROLLBACK replaces the body's own + * exit. A failed COMMIT dies on effect 4.0.0 and fails with the `SqlError` from + * 4.0.6. Body defects were carried through as `BodyDefect`, so any other defect + * that is a `SqlError` came from transaction control, and so did a `SqlError` + * failure after the body succeeded. */ const restoreCause = ( cause: Cause.Cause, @@ -478,17 +490,7 @@ const restoreCause = ( const defect = Cause.findDefect(cause) if (defect._tag === "Success" && isSqlError(defect.success)) { const error = defect.success - if (bodyExit !== undefined && Exit.isSuccess(bodyExit)) { - const sqlState = sqlStateOf(error) - return Effect.fail( - new TransactionCommitFailed({ - message: `${depth === 0 ? "COMMIT" : "releasing the savepoint"} failed: ${firstLine(error)}`, - reason: reasonOf(error), - ...(sqlState === undefined ? undefined : { sqlState }), - cause: error, - }), - ) - } + if (bodyExit !== undefined && Exit.isSuccess(bodyExit)) return Effect.fail(commitFailed(error, depth)) // The body never ran: BEGIN or SAVEPOINT died, not ROLLBACK. if (bodyExit === undefined) return Effect.fail(toDatabaseError(depth === 0 ? (begin ?? "BEGIN") : "SAVEPOINT")(error)) // An interrupted transaction whose ROLLBACK died stays interrupted. @@ -501,6 +503,10 @@ const restoreCause = ( }), ) } + // The body succeeded, so this failure is COMMIT's (effect 4.0.6 and later). + if (bodyExit !== undefined && Exit.isSuccess(bodyExit) && failure._tag === "Success" && isSqlError(failure.success)) { + return Effect.fail(commitFailed(failure.success, depth)) + } // BEGIN or SAVEPOINT failed before the body ran. if (bodyExit === undefined && failure._tag === "Success" && isSqlError(failure.success)) { return Effect.fail(toDatabaseError(depth === 0 ? (begin ?? "BEGIN") : "SAVEPOINT")(failure.success))