From 76948166413c514c05a6dc44041ada5b88bce07f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ad=C3=A1mek?= Date: Sun, 9 Feb 2025 12:48:17 +0100 Subject: [PATCH] feat: Allow read-only transactions in Postgres and MySQL (#1342) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: Allow read-only transactions in Postgres and MySQL Closes #1341 * use `accessMode` instead * add tests * access mode type and validation, rearranging stuff a bit. Co-authored-by: Martin Adámek <615580+B4nan@users.noreply.github.com> * rearranging stuff a bit. Co-authored-by: Martin Adámek <615580+B4nan@users.noreply.github.com> * align/simplify tests a bit Co-authored-by: Martin Adámek <615580+B4nan@users.noreply.github.com> --------- Co-authored-by: igalklebanov Co-authored-by: Martin Adámek <615580+B4nan@users.noreply.github.com> --- src/dialect/mysql/mysql-driver.ts | 18 ++- src/dialect/postgres/postgres-driver.ts | 18 ++- src/driver/driver.ts | 25 ++++ src/kysely.ts | 38 +++--- test/node/src/controlled-transaction.test.ts | 100 +++++++++++++-- test/node/src/transaction.test.ts | 128 +++++++++++-------- 6 files changed, 235 insertions(+), 92 deletions(-) diff --git a/src/dialect/mysql/mysql-driver.ts b/src/dialect/mysql/mysql-driver.ts index 4b89c8812..80cc1d44a 100644 --- a/src/dialect/mysql/mysql-driver.ts +++ b/src/dialect/mysql/mysql-driver.ts @@ -73,13 +73,19 @@ export class MysqlDriver implements Driver { connection: DatabaseConnection, settings: TransactionSettings, ): Promise { - if (settings.isolationLevel) { + if (settings.isolationLevel || settings.accessMode) { + let sql = 'set transaction' + + if (settings.isolationLevel) { + sql += ` isolation level ${settings.isolationLevel}` + } + + if (settings.accessMode) { + sql += ` ${settings.accessMode}` + } + // On MySQL this sets the isolation level of the next transaction. - await connection.executeQuery( - CompiledQuery.raw( - `set transaction isolation level ${settings.isolationLevel}`, - ), - ) + await connection.executeQuery(CompiledQuery.raw(sql)) } await connection.executeQuery(CompiledQuery.raw('begin')) diff --git a/src/dialect/postgres/postgres-driver.ts b/src/dialect/postgres/postgres-driver.ts index e91e558a5..5fe16648f 100644 --- a/src/dialect/postgres/postgres-driver.ts +++ b/src/dialect/postgres/postgres-driver.ts @@ -62,12 +62,18 @@ export class PostgresDriver implements Driver { connection: DatabaseConnection, settings: TransactionSettings, ): Promise { - if (settings.isolationLevel) { - await connection.executeQuery( - CompiledQuery.raw( - `start transaction isolation level ${settings.isolationLevel}`, - ), - ) + if (settings.isolationLevel || settings.accessMode) { + let sql = 'start transaction' + + if (settings.isolationLevel) { + sql += ` isolation level ${settings.isolationLevel}` + } + + if (settings.accessMode) { + sql += ` ${settings.accessMode}` + } + + await connection.executeQuery(CompiledQuery.raw(sql)) } else { await connection.executeQuery(CompiledQuery.raw('begin')) } diff --git a/src/driver/driver.ts b/src/driver/driver.ts index 849cd0129..372dc46d5 100644 --- a/src/driver/driver.ts +++ b/src/driver/driver.ts @@ -77,9 +77,14 @@ export interface Driver { } export interface TransactionSettings { + readonly accessMode?: AccessMode readonly isolationLevel?: IsolationLevel } +export const TRANSACTION_ACCESS_MODES = ['read only', 'read write'] as const + +export type AccessMode = ArrayItemType + export const TRANSACTION_ISOLATION_LEVELS = [ 'read uncommitted', 'read committed', @@ -89,3 +94,23 @@ export const TRANSACTION_ISOLATION_LEVELS = [ ] as const export type IsolationLevel = ArrayItemType + +export function validateTransactionSettings( + settings: TransactionSettings, +): void { + if ( + settings.accessMode && + !TRANSACTION_ACCESS_MODES.includes(settings.accessMode) + ) { + throw new Error(`invalid transaction access mode ${settings.accessMode}`) + } + + if ( + settings.isolationLevel && + !TRANSACTION_ISOLATION_LEVELS.includes(settings.isolationLevel) + ) { + throw new Error( + `invalid transaction isolation level ${settings.isolationLevel}`, + ) + } +} diff --git a/src/kysely.ts b/src/kysely.ts index 6739cc00b..cb47ea5a5 100644 --- a/src/kysely.ts +++ b/src/kysely.ts @@ -13,8 +13,8 @@ import { SingleConnectionProvider } from './driver/single-connection-provider.js import { Driver, IsolationLevel, - TransactionSettings, - TRANSACTION_ISOLATION_LEVELS, + AccessMode, + validateTransactionSettings, } from './driver/driver.js' import { createFunctionModule, @@ -706,6 +706,13 @@ export class TransactionBuilder { this.#props = freeze(props) } + setAccessMode(accessMode: AccessMode): TransactionBuilder { + return new TransactionBuilder({ + ...this.#props, + accessMode, + }) + } + setIsolationLevel(isolationLevel: IsolationLevel): TransactionBuilder { return new TransactionBuilder({ ...this.#props, @@ -714,8 +721,8 @@ export class TransactionBuilder { } async execute(callback: (trx: Transaction) => Promise): Promise { - const { isolationLevel, ...kyselyProps } = this.#props - const settings = { isolationLevel } + const { isolationLevel, accessMode, ...kyselyProps } = this.#props + const settings = { isolationLevel, accessMode } validateTransactionSettings(settings) @@ -744,20 +751,10 @@ export class TransactionBuilder { } interface TransactionBuilderProps extends KyselyProps { + readonly accessMode?: AccessMode readonly isolationLevel?: IsolationLevel } -function validateTransactionSettings(settings: TransactionSettings): void { - if ( - settings.isolationLevel && - !TRANSACTION_ISOLATION_LEVELS.includes(settings.isolationLevel) - ) { - throw new Error( - `invalid transaction isolation level ${settings.isolationLevel}`, - ) - } -} - export class ControlledTransactionBuilder { readonly #props: ControlledTransactionBuilderProps @@ -765,6 +762,13 @@ export class ControlledTransactionBuilder { this.#props = freeze(props) } + setAccessMode(accessMode: AccessMode): ControlledTransactionBuilder { + return new ControlledTransactionBuilder({ + ...this.#props, + accessMode, + }) + } + setIsolationLevel( isolationLevel: IsolationLevel, ): ControlledTransactionBuilder { @@ -775,8 +779,8 @@ export class ControlledTransactionBuilder { } async execute(): Promise> { - const { isolationLevel, ...props } = this.#props - const settings = { isolationLevel } + const { isolationLevel, accessMode, ...props } = this.#props + const settings = { isolationLevel, accessMode } validateTransactionSettings(settings) diff --git a/test/node/src/controlled-transaction.test.ts b/test/node/src/controlled-transaction.test.ts index 48ed2a75e..9196c3bbc 100644 --- a/test/node/src/controlled-transaction.test.ts +++ b/test/node/src/controlled-transaction.test.ts @@ -1,13 +1,13 @@ import * as sinon from 'sinon' -import { Connection } from 'tedious' +import { Connection, ISOLATION_LEVEL } from 'tedious' import { CompiledQuery, ControlledTransaction, Driver, DummyDriver, - IsolationLevel, Kysely, SqliteDialect, + TRANSACTION_ACCESS_MODES, } from '../../../' import { DIALECTS, @@ -249,6 +249,42 @@ for (const dialect of DIALECTS) { expect(person).to.be.undefined }) + if (dialect === 'postgres' || dialect === 'mysql') { + for (const accessMode of TRANSACTION_ACCESS_MODES) { + it(`should set the transaction access mode as "${accessMode}"`, async () => { + const trx = await ctx.db + .startTransaction() + .setAccessMode(accessMode) + .execute() + + await trx.selectFrom('person').selectAll().execute() + + await trx.commit().execute() + + expect( + executedQueries.map((it) => ({ + sql: it.sql, + parameters: it.parameters, + })), + ).to.eql( + { + postgres: [ + { sql: `start transaction ${accessMode}`, parameters: [] }, + { sql: 'select * from "person"', parameters: [] }, + { sql: 'commit', parameters: [] }, + ], + mysql: [ + { sql: `set transaction ${accessMode}`, parameters: [] }, + { sql: 'begin', parameters: [] }, + { sql: 'select * from `person`', parameters: [] }, + { sql: 'commit', parameters: [] }, + ], + }[dialect], + ) + }) + } + } + if (dialect === 'postgres' || dialect === 'mysql' || dialect === 'mssql') { for (const isolationLevel of [ 'read uncommitted', @@ -263,16 +299,60 @@ for (const dialect of DIALECTS) { .setIsolationLevel(isolationLevel) .execute() - await trx - .insertInto('person') - .values({ - first_name: 'Foo', - last_name: 'Barson', - gender: 'male', - }) - .execute() + await insertSomething(trx) await trx.commit().execute() + + if (dialect === 'mssql') { + expect(tediousBeginTransactionSpy.calledOnce).to.be.true + expect(tediousBeginTransactionSpy.getCall(0).args[1]).to.not.be + .undefined + expect(tediousBeginTransactionSpy.getCall(0).args[2]).to.equal( + ISOLATION_LEVEL[ + isolationLevel.replace(' ', '_').toUpperCase() as any + ], + ) + expect(tediousCommitTransactionSpy.calledOnce).to.be.true + } + + expect( + executedQueries.map((it) => ({ + sql: it.sql, + parameters: it.parameters, + })), + ).to.eql( + { + postgres: [ + { + sql: `start transaction isolation level ${isolationLevel}`, + parameters: [], + }, + { + sql: 'insert into "person" ("first_name", "last_name", "gender") values ($1, $2, $3)', + parameters: ['Foo', 'Barson', 'male'], + }, + { sql: 'commit', parameters: [] }, + ], + mysql: [ + { + sql: `set transaction isolation level ${isolationLevel}`, + parameters: [], + }, + { sql: 'begin', parameters: [] }, + { + sql: 'insert into `person` (`first_name`, `last_name`, `gender`) values (?, ?, ?)', + parameters: ['Foo', 'Barson', 'male'], + }, + { sql: 'commit', parameters: [] }, + ], + mssql: [ + { + sql: 'insert into "person" ("first_name", "last_name", "gender") values (@1, @2, @3)', + parameters: ['Foo', 'Barson', 'male'], + }, + ], + }[dialect], + ) }) } } diff --git a/test/node/src/transaction.test.ts b/test/node/src/transaction.test.ts index 1c3f09498..ee7059c8f 100644 --- a/test/node/src/transaction.test.ts +++ b/test/node/src/transaction.test.ts @@ -1,6 +1,6 @@ import * as sinon from 'sinon' import { Connection, ISOLATION_LEVEL } from 'tedious' -import { CompiledQuery, IsolationLevel, Transaction } from '../../../' +import { CompiledQuery, Transaction, TRANSACTION_ACCESS_MODES } from '../../../' import { clearDatabase, @@ -84,45 +84,7 @@ for (const dialect of DIALECTS) { .execute() }) - if (dialect == 'postgres') { - expect( - executedQueries.map((it) => ({ - sql: it.sql, - parameters: it.parameters, - })), - ).to.eql([ - { - sql: `start transaction isolation level ${isolationLevel}`, - parameters: [], - }, - { - sql: 'insert into "person" ("first_name", "last_name", "gender") values ($1, $2, $3)', - parameters: ['Foo', 'Barson', 'male'], - }, - { sql: 'commit', parameters: [] }, - ]) - } else if (dialect === 'mysql') { - expect( - executedQueries.map((it) => ({ - sql: it.sql, - parameters: it.parameters, - })), - ).to.eql([ - { - sql: `set transaction isolation level ${isolationLevel}`, - parameters: [], - }, - { - sql: 'begin', - parameters: [], - }, - { - sql: 'insert into `person` (`first_name`, `last_name`, `gender`) values (?, ?, ?)', - parameters: ['Foo', 'Barson', 'male'], - }, - { sql: 'commit', parameters: [] }, - ]) - } else if (dialect === 'mssql') { + if (dialect === 'mssql') { expect(tediousBeginTransactionSpy.calledOnce).to.be.true expect(tediousBeginTransactionSpy.getCall(0).args[1]).to.not.be .undefined @@ -131,21 +93,81 @@ for (const dialect of DIALECTS) { isolationLevel.replace(' ', '_').toUpperCase() as any ], ) - - expect( - executedQueries.map((it) => ({ - sql: it.sql, - parameters: it.parameters, - })), - ).to.eql([ - { - sql: 'insert into "person" ("first_name", "last_name", "gender") values (@1, @2, @3)', - parameters: ['Foo', 'Barson', 'male'], - }, - ]) - expect(tediousCommitTransactionSpy.calledOnce).to.be.true } + + expect( + executedQueries.map((it) => ({ + sql: it.sql, + parameters: it.parameters, + })), + ).to.eql( + { + postgres: [ + { + sql: `start transaction isolation level ${isolationLevel}`, + parameters: [], + }, + { + sql: 'insert into "person" ("first_name", "last_name", "gender") values ($1, $2, $3)', + parameters: ['Foo', 'Barson', 'male'], + }, + { sql: 'commit', parameters: [] }, + ], + mysql: [ + { + sql: `set transaction isolation level ${isolationLevel}`, + parameters: [], + }, + { sql: 'begin', parameters: [] }, + { + sql: 'insert into `person` (`first_name`, `last_name`, `gender`) values (?, ?, ?)', + parameters: ['Foo', 'Barson', 'male'], + }, + { sql: 'commit', parameters: [] }, + ], + mssql: [ + { + sql: 'insert into "person" ("first_name", "last_name", "gender") values (@1, @2, @3)', + parameters: ['Foo', 'Barson', 'male'], + }, + ], + }[dialect], + ) + }) + } + } + + if (dialect === 'postgres' || dialect === 'mysql') { + for (const accessMode of TRANSACTION_ACCESS_MODES) { + it(`should set the transaction access mode as "${accessMode}"`, async () => { + await ctx.db + .transaction() + .setAccessMode(accessMode) + .execute(async (trx) => { + await trx.selectFrom('person').selectAll().execute() + }) + + expect( + executedQueries.map((it) => ({ + sql: it.sql, + parameters: it.parameters, + })), + ).to.eql( + { + postgres: [ + { sql: `start transaction ${accessMode}`, parameters: [] }, + { sql: 'select * from "person"', parameters: [] }, + { sql: 'commit', parameters: [] }, + ], + mysql: [ + { sql: `set transaction ${accessMode}`, parameters: [] }, + { sql: 'begin', parameters: [] }, + { sql: 'select * from `person`', parameters: [] }, + { sql: 'commit', parameters: [] }, + ], + }[dialect], + ) }) } }