eoin-obrien / prisma-extension-kysely

Drop down to raw SQL in Prisma without sacrificing type safety!
https://www.npmjs.com/package/prisma-extension-kysely
MIT License
207 stars 5 forks source link

Issue with prisma.$kysely transaction #71

Open MrVhek opened 8 months ago

MrVhek commented 8 months ago

Hi, I am not sure but after a few weeks using your extension, It don't seems that it support prisma transaction, is this the case ? I am expecting that when I do an interactive transaction it use the transaction that prisma created but it doesn't seems to work...

prisma.$transaction(async (tx: PrismaTransaction) => {
    await tx.$kysely...
}

Is this not supported ?

eoin-obrien commented 8 months ago

Which version of the extension are you using? I'll dive in and investigate once I know!

MrVhek commented 8 months ago

I'm using the last version: 2.1.0 What makes me think that is that we have a strange issue that I documented here: https://github.com/prisma/prisma/discussions/23200 And once I change a portion of my code from:

const variableValues = await tx.$kysely
        .insertInto("VariableValues")
        .values(variableValuesData)
        .returning(["VariableValues.uid", "VariableValues.variableId", "VariableValues.timestamp"])
        .execute()

To a simpler way:

const variableValuesQuery = tx.$kysely
        .insertInto("VariableValues")
        .values(variableValuesData)
        .returning(["VariableValues.uid", "VariableValues.variableId", "VariableValues.timestamp"])
        .compile()
    const variableValues = (await tx.$queryRawUnsafe(
        variableValuesQuery.sql,
        ...variableValuesQuery.parameters,
    )) as {uid: string; variableId: number; timestamp: Date}[]

No errors occurs during the transaction. We had an issue of transaction that were rollbacked but not those specific data which are the one where I use the extension to update. I've checked the code and I'm not a pro of prisma extension but it seems that is is taking the prisma client in the constructor and executing query with it.

EDIT: after a few investigation, I've been trying to get an object id for what is used and it in fact uses the transaction client as expected, but I always have this error: 'Transaction API error: Transaction already closed: A query cannot be executed on a committed transaction.', I don't have this error if I'm using the tx.$queryRawUnsafe myself and I have a normal constraint failure.

eoin-obrien commented 8 months ago

Thanks for the additional info! Can you create a repo with a minimal reproducible example of the issue so I can dig in and see what I can do?

MrVhek commented 8 months ago

Here you go: https://github.com/MrVhek/PrismaKyselyBug If you uncomment the working version you'll have the strange issue of a foreign not being found in the same transaction.

MrVhek commented 7 months ago

I maybe got an idea of what the problem could be. Is it possible that in case I have two transactions in parrallel, the wrong client is used for making queries ? Because I have an unique Prisma client extended but I can have multiple transaction at the same time. Is the driver attaching to each transaction client the good prisma client. I'll try to do test looking for this particular issue.

MrVhek commented 7 months ago

After investigation, I can definitely reproduce this issue with a simple example: here two kysely get that I parametrized with parameters I know. I've added a function to get an unique id for an object and you see that the first two get are corrects, but after this, the two gets are using the same transaction client.

2024-04-02T12:44:16.423Z [igonogo API] info: Starting a postgresql pool with 30 connections.
2024-04-02T12:44:16.464Z [igonogo API] debug: BEGIN: duration: 0 ms
Transaction options:  async tx=>{console.log("Executing query: ",1,"with",getUniqueIdForObject(tx));await getStudyVariablesByStudyId(1,tx);await getStudyVariablesByStudyId(1,tx)} unique_id_1
Executing query:  1 with unique_id_1
Acquiring connection unique_id_1
Executing query:  [ 1 ] with unique_id_1
2024-04-02T12:44:16.486Z [igonogo API] debug: BEGIN: duration: 0 ms
Transaction options:  async tx2=>{console.log("Executing query: ",2,"with",getUniqueIdForObject(tx2));await getStudyVariablesByStudyId(2,tx2);await getStudyVariablesByStudyId(2,tx2)} unique_id_2
Executing query:  2 with unique_id_2
Acquiring connection unique_id_2
Executing query:  [ 2 ] with unique_id_2
2024-04-02T12:44:16.502Z [igonogo API] debug: select "Variables".*, "StudyVariables".* from "StudyVariables" inner join "Variables" on "StudyVariables"."variableUid" = "Variables"."uid" where "StudyVariables"."studyId" = $1: duration: 15 ms
Acquiring connection unique_id_2
Executing query:  [ 2 ] with unique_id_2
2024-04-02T12:44:16.508Z [igonogo API] debug: select "Variables".*, "StudyVariables".* from "StudyVariables" inner join "Variables" on "StudyVariables"."variableUid" = "Variables"."uid" where "StudyVariables"."studyId" = $1: duration: 34 ms
2024-04-02T12:44:16.511Z [igonogo API] debug: select "Variables".*, "StudyVariables".* from "StudyVariables" inner join "Variables" on "StudyVariables"."variableUid" = "Variables"."uid" where "StudyVariables"."studyId" = $1: duration: 2 ms
Acquiring connection unique_id_2
Executing query:  [ 1 ] with unique_id_2
2024-04-02T12:44:16.521Z [igonogo API] debug: select "Variables".*, "StudyVariables".* from "StudyVariables" inner join "Variables" on "StudyVariables"."variableUid" = "Variables"."uid" where "StudyVariables"."studyId" = $1: duration: 3 ms
2024-04-02T12:44:16.523Z [igonogo API] debug: COMMIT: duration: 0 ms
2024-04-02T12:44:16.527Z [igonogo API] debug: COMMIT: duration: 0 ms
ssukienn commented 6 months ago

Hi, this is a really handy extension, thank you.

Unfortunately, I can confirm. Wrong transaction or no transaction is effectively used.

You can repro this by setting some constraints like CHECK/UNIQUE on the column and seed the DB so the next update/insert would violate the constraint. This can be done with a query before the tx.

Then create 2 queries inside the transaction:

If you do it all in Prisma or Kysely alone - all good. If you run the first query with Prisma and the second with Kysely, then Kysely does not see the results from the first query and will report a constraint violation. This whole example could be serial, no parallel tx, or anything like that.

So seems like Kysely sees different snapshot of the DB and is either in a different explicit transaction than Prisma query or none at all.

I advise to not mix the clients for single transaction here until it is resolved.

MrVhek commented 5 months ago

I ended up doing my functions and not using this prisma-extension-kysely at all for this You can do

const query = tx.$kysely...compile()
tx.$queryRawUnsafe(query.sql, ...query.parameters)
DaichiShirakawa commented 2 months ago

Hello guys, After having this problem myself, I finally figured out what's causing it. :)

✅ Working example

In conclusion, the following code should work as intended.

const prisma = new Prisma();
const xprisma = prisma.$extends( /* with kyselyExtension */ );

await xprisma.$transaction(async (tx) => {
  // Insert by Prisma
  await tx.model.create({ data: { name: "Jhon" } });
  // Select by Kysely -> You can find Jhon
  await tx.$kysely.selectFrom("model").selectAll().execute();
});

🤷‍♂️ Not working example

In my case, it did not work because I had implemented the following:

// I was use prisma-extension-kysely with PrismaService via NestJS
class PrismaService extends PrismaClient {
  constructor() {
    super();
  }

  get $kysely() {
    return this
      .$extends( /* with kyselyExtension */ )
      .$kysely;
  }
}

const prisma = new PrismaService();

await prisma.$transaction(async (tx) => {
  // Insert by Prisma
  await tx.model.create({ data: { name: "Jhon" } });
  // Select by Kysely -> ❗ You CAN NOT find Jhon
  await tx.$kysely.selectFrom("model").selectAll().execute();
});

In this case, tx.$kysely generates new instance with extension that refers wrong connection, each time tx.$kysely referenced. It means you should use extended prisma instance consistently.

In other words, $extends should be called only once in a process.

DaichiShirakawa commented 2 months ago

As a side note, I'll write a more concrete implementation in NestJS It works well in my case!

I would be happy If it helps you. 👍

✅ Minimum PrismaService example

@Injectable()
export class PrismaService extends PrismaClient {
  private constructor(config: ConfigService) {
    super();
    // init with config
  }

  /**
   * @see https://github.com/eoin-obrien/prisma-extension-kysely
   * @see https://github.com/eoin-obrien/prisma-extension-kysely/issues/71
   */
  static withKysely(config: ConfigService) {
    return new PrismaService(config).$extends(
      kyselyExtension({
        kysely: (driver) => {
          return new Kysely<DB>({
            dialect: {
              createDriver: () => driver,
              createAdapter: () => new PostgresAdapter(),
              createIntrospector: (db) => new PostgresIntrospector(db),
              createQueryCompiler: () => new PostgresQueryCompiler(),
            },
          });
        },
      })
    ) as unknown as PrismaService;
  }

  /** Don't forget it */
  declare $kysely: Kysely<DB>;
}

✅ PrismaModule example

@Module({
  imports: [ConfigModule],
  providers: [
    {
      provide: PrismaService,
      inject: [ConfigService],
      useFactory: (config: ConfigService) => {
        return PrismaService.withKysely(config);
      },
    },
  ],
  exports: [PrismaService],
})
export class PrismaModule {}