Sqlite nested transactions #114
Conversation
…ion handle the try catch. Added decrement for rollback so it only gets call at the very end
There was a problem hiding this comment.
Looks good, I added some suggestions around code organisation and test/ coverage. After fixing/confirming them I’ll be happy to merge it 🙂
Thanks @davecosec !
| @@ -55,6 +57,8 @@ export type SQLiteClientConnectionOptions< | |||
| ConnectorType extends SQLiteConnectorType = SQLiteConnectorType, | |||
| > = { | |||
| connector: ConnectorType; | |||
| transactionCounter: TransactionNestingCounter; | |||
There was a problem hiding this comment.
Do we need to have transactionCounter as part of options? Shouldn’t be that created internally in the connection and kept there?🤔
|
|
||
| const testCases = [ | ||
| { testName: 'in-memory', fileName: inMemoryfileName }, | ||
| // { testName: 'file', fileName: fileName }, |
There was a problem hiding this comment.
FIX: I think that we need to test it with real file also as it has different consistency behaviour.
src/packages/dumbo/src/storage/sqlite/core/transactions/transactions.int.spec.ts
Show resolved
Hide resolved
| initTransaction: (connection) => | ||
| sqliteTransaction(options.connector, connection), | ||
| initTransaction: (connection) => { | ||
| return sqliteTransaction( |
There was a problem hiding this comment.
SUGGESTION: I think that we can simplify that by keeping transaction a singleton inside the client as SQLIte cannot have more than one open transaction. Then we could setup counter internally inside transaction without the need to pass it from the connection.
…ansaction pass through
|
@davecosec I ensured that there's only one active transaction within the connection in da677ee. That allowed the transaction nesting counter to move into the transaction. |
oskardudycz
added
enhancement
Added capability to use nested transactions using Savepoints. Thanks to that, we can more easily manage a typical unit of work case, where multiple transactions are handling it.
For now, it supports only SQLite, but we can also emulate that for PostgreSQL in the future.
Added also a singleton for an open transaction within a connection.