Model's preloads do not use transaction

[enixsoft]

Prerequisites

When querying a model which uses transaction, shouldn't its preloads also automatically use this transaction?

Package version

18.1.0

Node.js and npm version

Node.js v18.17.1 npm 9.6.7

Sample Code (to reproduce the issue)

const post = await Database.transaction(async (trx) => {
    const post = await Post.query()
      .preload('user')
      .useTransaction(trx)
      .forUpdate()
      .first()

    return post
  })

[thetutlage]

How do you confirm they are not using the transaction?

[enixsoft]

@thetutlage

For local development I use single database connection to easily debug transaction problems, and the code I posted times out with: KnexTimeoutError: Knex: Timeout acquiring a connection. The pool is probably full. Are you missing a .transacting(trx) call?

When I run the application with DEBUG=knex:* npm run dev, the output with single database connection is this, then it times out:

  knex:tx trx2: Starting top level transaction +0ms
  knex:client acquired connection from pool: __knexUid1 +10s
  knex:query BEGIN; trx2 +10s
  knex:bindings undefined trx2 +10s
  knex:query select "id", "userId" from "Posts" limit $1 for update trx2 +3ms
  knex:bindings [ 1 ] trx2 +3ms

When I increase maximum database connections to 2, the output is:

  knex:tx trx3: Starting top level transaction +0ms
  knex:client acquired connection from pool: __knexUid2 +6s
  knex:query BEGIN; trx3 +6s
  knex:bindings undefined trx3 +6s
  knex:query select "id", "userId" from "Posts" limit $1 for update trx3 +1ms
  knex:bindings [ 1 ] trx3 +1ms
  knex:client acquired connection from pool: __knexUid1 +4ms
  knex:query select * from "Users" where "id" in ($1) undefined +3ms
  knex:bindings [ 2 ] undefined +3ms
  knex:client releasing connection to pool: __knexUid1 +1ms
  knex:query COMMIT; trx3 +2ms
  knex:bindings undefined trx3 +2ms
  knex:tx trx3: releasing connection +18ms
  knex:client releasing connection to pool: __knexUid2 +10ms

It appears that the preload query to Users table does not have the reference to transaction (trx3) and uses a different connection.

[pokedpeter]

I'm curious if this PR resolves it? https://github.com/adonisjs/lucid/pull/963/files This was to fix another preload issue, but looking at some of the relevant changes in the PR it might also pass in the existing transaction?

[thetutlage]

Alright, can you please create a fresh application with this code to help me reproduce the issue?

[pokedpeter]

I created this one to test my fix in https://github.com/adonisjs/lucid/pull/963/files if you want to use it: https://github.com/pokedpeter/adonis-preload

It runs the same code as the OP with a User and Post models and seeder data using the default route.

(My fix doesn't resolve this transaction issue)

As mentioned by the OP, two connections open up, with the preload one running separately from the transaction one:

[thetutlage]

Alright, I see why that happens. A quick fix will be to use the following code.

const post = await Database.transaction(async (trx) => {
    const post = await Post.query({ client: trx })
      .preload('user')
      .forUpdate()
      .first()

    return post
  })

Instead of using useTransaction method you pass the transaction client as the client option to the query.

[thetutlage]

I will see if we should get rid of useTransaction method altogether in the next major release and rely on client option only. Until then, I will put this issue on hold.

The reason for removing useTransaction is because it internally calls the knex.transacting method, but knex does not expose any property to know if a knex query is using a transaction.

Therefore, we will have to create additional state in Lucid query builders to check if knex is using a transaction or not + the our query builder directly is using a transaction or not.

In the end, it just complicates everything. So, better have a single API to use transactions.

[thetutlage]

Well, I went ahead and deprecated the useTransaction method on the query builder. The commit includes code examples in docblock on how to use the transaction object effectively to create different query builders.

https://github.com/adonisjs/lucid/commit/223db3907b247a996b663f6c75961eecc6a9eb18