Always Require a Transaction

[cliedeman]

Hello, firstly thanks for the excellent project.

Would you accept a PR that causes queries that do not contain a transaction to error out? I find its too easy to forget to pass in the transaction object.

Example:

await transaction(knex, async (trx) => {
    return Person
      .query(/*  missing transction */)
      .insert({firstName: 'Jennifer', lastName: 'Lawrence'});
  });

I expect this to be opt-in, although an argument could be made to make this the default.

Ciaran

[koskimas]

I'm not sure if such an option is useful. When enabled, it would require every query to have a transaction which makes no sense for single queries. Have you considered not installing the default knex connection and always passing either a transaction or a knex instance? Isn't that exactly the same thing? Simply leave out the Model.knex(knex) and there you have it: an error is throw if no transaction or knex instance is given to query (or the other query methods).

[cliedeman]

Hi @koskimas

Thanks for the reply, If I pass around a knex connection that means there is still no transaction?

From the docs

upsertGraph operation is not atomic by default! You need to start a transaction and pass it to the query using any of the supported ways.

I think a single query (patch, insert, delete) is probably the only place where you don't need a transaction or the overhead but all other methods (updateAndFetch, upsertGraph, insertGraph, etc.). Should have one?

I'm just exploring ways to try and catch this somehow, if this is not feasible I also want to investigate an eslint-plugin.

Ciaran

[koskimas]

No there's still no transaction of course, but it solves the problem you originally presented: forgetting to pass a transaction to the query method. There will never be implicit transactions in objection. You will always need to start them explicitly. If you need that level of magic, objection is not the right tool for you.

You don't necessarily need a transaction with updateAndFetch or similar. You only need to use a transaction when you want the guarantee that either all operations succeed, or none of them. For example updateAndFetch creates two queries: an update and a find. If the update fails, the find is never executed (even without a transaction). If the find fails, do you really want to cancel the update?

[joepie91]

When enabled, it would require every query to have a transaction which makes no sense for single queries.

It's worth noting that, AFAIK, this is already the case in PostgreSQL; each single query internally operates within a transaction. Other databases might do the same.