Stop wrapping single changes in transactions where possible

[roji]

@AndriySvyryd here's a draft removing the unnecessary transaction where possible.

I got stuck in a thorny chicken and egg issue. To know whether a transaction needs to be started, I need to know if there's more than one batch. So the code as of now always peeks one batch forward; but this fails Can_insert_TPT_dependents_with_identity since GenerateColumnModifications gets called on ModificationCommand in the later batch, and the earlier batch hasn't yet been executed (and so hasn't propagated server-generated keys into the shared InternalEntityEntry).

Hopefully I'm understanding the situation right (am new here). One workaround could be to restart the enumeration if I see more than one batch; this would be optimized for the one-batch case, but would cause multiple enumerations (and so multiple TopologicalSort etc.) for multiple batches - not ideal.

Ideally, while preparing the second batch, we'd already know that the value will be propagated by the time we execute it, even if it hasn't yet been at the time we generate it; this way batch generation would no longer depend on execution order etc. But I have no idea how feasible/reasonable that is.

NOTE: Solved this, see below.

Closes #27439

[roji]

@AndriySvyryd FYI I ended up solving the above by changing CommandBatchPreparer to allow checking if there is another batch up ahead, but without actually generating it; this makes CommandBatchPreparer itself behave like an enumerator, and unfortunately means we can't use yield.

Technically, this still isn't as good as being able to generate all the batches without executing any, since the current logic may return that there are batches, whereas when we actually get around to generate them, we find that there are no modifications to be done (i.e. update without write operations). In that case we'd get a transaction though we don't need, but that seems like an acceptable edge case.

Let me know what you think - this PR is close to ready, mostly test cleanup remains.

[AndriySvyryd]

I think we can avoid adding the peek API (and perhaps go back to using yield) if we signal that there are still more commands afterwards by adding a parameter to ModificationCommandBatch.Complete that will affect the value of ModificationCommandBatch.RequiresTransaction.

Note that ModificationCommandBatch.Complete will be removed by #20664 since ModificationCommandBatch.AddCommand will always be called on the current batch before starting the next one.

[roji]

I think we can avoid adding the peek API (and perhaps go back to using yield) if we signal that there are still more commands afterwards by adding a parameter to ModificationCommandBatch.Complete that will affect the value of ModificationCommandBatch.RequiresTransaction.

Thanks for this suggestion... I ended up doing something which may be even better: BatchCommands now simply returns a tuple of batches and "HasMore" flags. This avoids putting information in the batch itself on whether more batches are coming, and also avoids depending on Complete in case it goes away.

Note that ModificationCommandBatch.Complete will be removed by #20664 since ModificationCommandBatch.AddCommand will always be called on the current batch before starting the next one.

OK, yeah, we should discuss so I understand what you have in mind.

[roji]

Login issues to SQL Server on Helix...