Skip to content
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

Support Nested Transactions / Savepoints #19346

Open
mollerhoj opened this issue May 19, 2023 · 2 comments
Open

Support Nested Transactions / Savepoints #19346

mollerhoj opened this issue May 19, 2023 · 2 comments
Labels
domain/client Issue in the "Client" domain: Prisma Client, Prisma Studio etc. kind/feature A request for a new feature. tech/engines Issue for tech Engines. tech/typescript Issue for tech TypeScript. topic: interactiveTransactions topic: nested transactions topic: savepoint

Comments

@mollerhoj
Copy link

mollerhoj commented May 19, 2023

Problem

Nested Transactions are useful in many cases. They are implemented in all other serious database query systems: (TypeORM, Rail's ActiveRecord, Django ORM, Laravel's Eloquent ORM, Elixir Phoenixs Ecto, Spring etc.)

A common use case is using nested transactions to run tests within their own transaction. (Rails, Django, Phoenix and possibly others default to this behaviour).

Other use cases include more complex business logic, where partial rollback is necessary.

Many users of Prisma have run into this problem (1 2 3 4 5), and it has inspired a number of workarounds (1 2 3 4 5).

Suggested solution

Most databases implement nested transactions with "Save points". For example, see the PostgresSQL API:

BEGIN; # Outer Transaction starts
    INSERT INTO table1 VALUES (3); #
    SAVEPOINT my_savepoint; Inner Transaction starts
    INSERT INTO table1 VALUES (4);
    RELEASE SAVEPOINT my_savepoint; Inner Transaction ends
COMMIT; # Outer Transaction ends

Application code usually do not care about whether or not the transaction it's about to start is nested within another transaction. In the case of running transactional tests, all application code transactions should actually be nested transactions, but it should not need to require changes to the application code.

Thus, I propose either that prisma.$transaction is changed to run nested transactions if a transaction has already been started, or to crease a new function prisma.$possibly_nested_transaction that will nest transactions automatically.

As far as I can tell it's not possible to implement this via the prisma extension system, as we have to proxy all calls to the prisma clients to make sure no new outer transactions are started.

Here is an (untested, possibly buggy) "extension" to the prismaClient to implement this feature for PostgreSQL:

function dbClient() {
  let prismaClient = new PrismaClient({ log: ['query'] });
  let proxy = new Proxy(prismaClient, {
    get: (target: any, prop, receiver: unknown) => {
      // We need a track the nesting level
      if (target.transactionNestedLevel === undefined) {
        target.transactionNestedLevel = 0;
      };

      // On all method calls to the prismaClient, we need to check if we are currently within a transaction,
      // and if the Prisma Transaction Object has that method. If it has, call the method on the Prisma Transaction Object
      // to avoid starting and committing new transactions.
      if (prop !== '$possibly_nested_transaction') {
        if (target['currentTransaction'] && prop in target['currentTransaction']) {
          return Reflect.get(target['currentTransaction'], prop, receiver);
        } else {
          return Reflect.get(target, prop, receiver);
        }
      }
      
      // When $possibly_nested_transaction is called, we should check if we are currently within a (outer) transaction or not.
      // If we are not inside a transaction, we should start one, and save the Prisma Transaction Object for the usage described above.
      // If we are inside an (outer) transaction, we should use Savepoints instead of Transactions.
      // In both cases we must increase the nesting level, call the callback, and then decrease the nesting level again.
      return async function(callback: () => Promise<void>) {
        if (target['transactionNestedLevel'] === 0) {
          return target.$transaction(async (tx: any) => {
            target['currentTransaction'] = tx;
            target['transactionNestedLevel'] = 1;

            try {
              await callback();
            } finally {
              target['transactionNestedLevel'] = 0;
              target['currentTransaction'] = null;
            }
          });
        }

        target.$executeRaw`SAVEPOINT ${target['transactionNestedLevel']}`;
        target['transactionNestedLevel'] += 1;

        try {
          await callback();
        } catch (e) {
          target.$executeRaw`ROLLBACK TO SAVEPOINT ${target['transactionNestedLevel']}`;
          target['transactionNestedLevel'] -= 1;
          throw e;
        }

        target['transactionNestedLevel'] -= 1;
        target.$executeRaw`RELEASE SAVEPOINT ${target['transactionNestedLevel']}`;
      }
    }
  });

  return proxy;
}

Alternatives

The Prisma extension callback-free-itx implements an imperative API to start and stop transactions without wrapping them in a function. This an attempt at allowing transactional tests, however, it will not work with tests relying on transactions in the application code. It's mostly a workaround for the fact that some test runners does have the feature to wrap each test within a function. Eg:

decorateEach((runTest) => {
  prisma.$transaction(() => {
    await runTest();
  });
});
// Now all tests should be run within the `prisma.$transaction`.

Thus I believe it should be implemented in the test runners, not by Prisma.

Additional context

The issue of running transactional tests have been mentioned numerous times in this repo. Mocking database calls (as it proposed here) makes tests faster, but often, its the actual database calls that are the most important thing to test. There's a reason why serious application frameworks default to not mock the database. I've tried to do so in the past, but in most cases it simply leads to tests that don't actually test anything).

As I began investigating this, I realised that another team within my organisation had run into exactly this issue a year ago in another project that uses Prisma. Speaking with the programmers on the team, they advised me not to use Prisma for future projects.

@aseemk
Copy link

aseemk commented Mar 14, 2024

Such a good writeup (thank you). Strongly +1. Just hit & discovered this limitation, and it's a major bummer. Hard to have modular application code (e.g. helper functions that can do things atomically, creating and committing their own transactions to do so) and decouple higher-level transactions from lower-level ones.

Besides the testing use case, another use case for higher-level transactions is for robust idempotency across your whole application. Brandur Leach describes this technique really well in this article:

https://brandur.org/idempotency-keys

And ironically, I was thinking of going this way in part because Prisma also doesn't support SELECT ... FOR UPDATE! (#8580) 😕

Please consider supporting nested transactions via savepoints. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain/client Issue in the "Client" domain: Prisma Client, Prisma Studio etc. kind/feature A request for a new feature. tech/engines Issue for tech Engines. tech/typescript Issue for tech TypeScript. topic: interactiveTransactions topic: nested transactions topic: savepoint
Projects
None yet
Development

No branches or pull requests

5 participants