Add transactor.parentTransaction

[caseywebdev]

Recently our app has had the need to know when the base transaction has committed, regardless of nested transaction levels. This works with executionPromise when you're one transaction deep, but if you have nested transactions the executionPromise only refers to the inner-most transaction. This change adds a baseExecutionPromise property on the transactor that will always reference the outer-most executionPromise.

Our specific use case is that we have functions that receive transactions and may open up nested transactions which can open up more nested transactions. Sometimes in those transactions, offline work records are inserted into a queue table to be processed out-of-band of the current transaction. We want to start that work as soon as the transaction is committed, but if we attempt to process the work when executionPromise is resolved in a nested transaction, the outer transaction is still not committed and the records aren't visible to the work processors yet, so no work is consumed.

I've monkey-patched this for our purposes for now, but I think it's a valuable addition to the core. For anyone interested in the patch:

import Transaction from 'knex/lib/execution/transaction.js';
const { _evaluateContainer } = Transaction.prototype;
Object.assign(Transaction.prototype, {
  async _evaluateContainer(config, container) {
    return await _evaluateContainer.call(this, config, tx => {
      this.baseExecutionPromise = tx.baseExecutionPromise =
        this.outerTx?.baseExecutionPromise ?? tx.executionPromise;
      return container(tx);
    });
  }
});

[coveralls]

coverage: 92.792% (+0.006%) from 92.786%
when pulling 7c318f7 on caseywebdev:base-execution-promise
into 770b2f2 on knex:master.

[caseywebdev]

Any feedback here?

[caseywebdev]

@OlivierCavadenti would you mind taking a look?

[caseywebdev]

@OlivierCavadenti friendly ping

[rluvaton]

@kibertoad what if instead of having another property for the top transaction promise

we would have one of those to allow full traversal of the transactions:

1. an array of transactions promises
2. property that point to the parent transaction

I prefer option 2 as it's:

1. simpler to implement IMO
2. cleaner
3. Less prone to leaks

[kibertoad]

@rluvaton yeah, I agree!
@caseywebdev does that sound reasonable to you?

[caseywebdev]

Fine with me. I've updated the implementation.

[rluvaton]

Great job!

[caseywebdev]

executionPromise is not available on outerTx directly. Need a reference to it somewhere.

[rluvaton]

Thanks, yeah after I wrote I saw

[rluvaton]

Please define this and the transactor property in the constructor (with default undefined value) to take advantage of v8 hidden classes

[caseywebdev]

This seems unprecedented? For example context, isTransaction, userParams, savepoint, commit, rollback, isCompleted, and executionPromise are all not defined on the QueryBuilder constructor.

[caseywebdev]

executionPromise is not available on outerTx directly. Need a reference to it somewhere.

[rluvaton]

@kibertoad LGTM

[kibertoad]

@caseywebdev I assume TS types need to be updated as well?

[caseywebdev]

@caseywebdev I assume TS types need to be updated as well?

Just pushed

[kibertoad]

Thank you! Can you also add a note about this feature to https://github.com/knex/documentation?

[kibertoad]

Released in 3.1.0