batchInsert with existing transaction

[alexwhitman]

batchInsert automatically starts/commits/rollsback a transaction but it would be useful if it could be used with an existing transaction which then isn't automatically completed.

[rhys-vdw]

@alexwhitman Does this not work?

knex.transaction(trx => {
  return trx.batchInsert(/* ... */);
});

[alexwhitman]

I'm not at work to test it now but in theory it should. However, from looking at the code wouldn't it start a new transaction within the existing? If so that would seem redundant.

[wubzz]

@alexwhitman This has been added to knex 0.11.1, and the documentation has been updated as to how this works.

[alexwhitman]

@wubzz Thanks, just tested and it works fine.

Just a thought though, the example that @rhys-vdw gave was to call batchInsert on the transaction itself but that still starts a nested transaction. Instead it would have to be called as

trx.batchInsert(data).transacting(trx);

which looks slightly redundant. Would it be possible to modify so that both

knex.batchInsert(data).transacting(trx);

trx.batchInsert(data);

act in the same way?

[alexwhitman]

The following patch appears to work but I'm not sure if it's the correct way to implement it:

diff --git a/src/util/batchInsert.js b/src/util/batchInsert.js
index 4aaaa86..5594c7f 100644
--- a/src/util/batchInsert.js
+++ b/src/util/batchInsert.js
@@ -19,7 +19,10 @@ export default class BatchInsert {
     this._returning       = void 0;
     this._transaction     = null;
     this._autoTransaction = true;
-    
+
+    if (client.transacting) {
+      this.transacting(client);
+    }
   }

   /**

[wubzz]

@alexwhitman I can see the convenience in that, and I think your diff makes perfect sense. Feel free to make a PR on that!