[bug] Nested transaction with batch-mode do not flush batch on commit

[jonasPoehler]

Hi Rob,

we found an issue with nested transactions utilizing the batch mode.
We are using an outer transaction with a nested transaction for which we turn on batch mode, since we first do a lot of inserts of the same type and afterwards run some updates on the same type. Between the inserts and the updates we do an explicit flush, but after the updates we "just commit" the transaction and close the nested one to return to the parent without batching (see previous issue #3538 ). Our expectation would be, that the commit while not actually commiting the underlying transaction due to nesting, would at least flush the batch control since batching is turned off for the outer transaction, but this doesn't happen.
We discovered this, because we did multiple of these nested transactions with batch mode in sequence and statments in the following transactions conflicted with unflushed statements once the actual flush occured.

I'm currently not completely sure how to fix this. A simple solution is to just call txn.flush() in ScopeTrans#commitTransaction, but maybe this should only be done, whe the batch mode is active for the inner transaction but not the outer one. Additionally there might be other factors in play (e. g. PersistListeners, maybe more) which might need to be handled as well (PersistListeners would already be called on flush).

What is your opinion on this issue? Would my suggested fix be enough or could this cause different problems?

All the best
Jonas

[rbygrave]

Our expectation would be, that the commit while not actually commiting the underlying transaction due to nesting, would at least flush the batch

I agree, this is a fair expectation.

just call txn.flush() in ScopeTrans#commitTransaction

Yes. Ultimately is this the "right thing to do" and we are asking the question - Is there a case where that isn't what we want?

The flush() is called for 2 reasons:

• (A) Performance and
• (B) Such that DML changes [insert, update, delete] are visible to following statements. This is why there is the "flushOnQuery" and "flushOnMixed" [UpdateSql, CallableSql].

So in the context of calling txn.flush() in ScopeTrans#commitTransaction

Q: Would we not want to flush due to (B)?

I'm going to say no. That is, because we are an nested transaction and performing a "commit" the expectation is that all the changes in this nested transaction should be "visible" to statements executed after the commit. [Noting that this is generally not an issue in practice due to the outer transaction flushOnQuery flushOnMixed settings]

Test case wise, I've adjusted the test such that after the nested transaction commit() a select query [using the outer transaction] would expect the EBasicVersion name to be "Other" and for this to fail we also need the outer transaction to use flushOnQuery false.

Q: What about (A) Performance? Would there be a case where this is a significant negative impact?

It's possible but I don't think this trumps (B) and I can't see it being significant in that the effect would be "1 extra batch prepared statement execution per statement", so for this to be significant we'd need a relatively large number of different statements to flush.

Side Effect:

With this change a flush() can occur earlier and that means that exceptions thrown at flush time like DataIntegrityException could now occur earlier. For example, if the nested transaction is in it's own method, that might now throw a DataIntegrityException where as before that might have been thrown later in the calling method instead.

I don't think this is an "issue" per say but there is potential for this to be a confusing behavior change.

[rbygrave]

Closed by #3565