sql: internal executor is very confusing about managing transaction state

[rafiss]

The way the internal executor works is that when it manages the txn lifecycle (i.e. you pass it nil), then it runs the whole transaction state machine.

However, when you pass in a txn, then it moves itself to the open state with totally bogus extraTxnState and then when the statement finishes, it throws that bogus extraTxnState away.

This can lead to problems, since it bypasses the lifecycle and state management which normally happens for SQL transactions. For example: the deferred creation of jobs and the checking of the two version invariant.

See #76764 for an example where this caused real corruption.

Jira issue: CRDB-14493

Epic CRDB-14492

[ajwerner]

cross-referencing for future discoverability: #69495 #71467

[rafiss]

cross-referencing for future discoverability

+1. I'm also putting all these in the same jira epic

[ajwerner]

We should probably make the internal executor return an error if you feed it more than one statement in a batch. It has odd semantics both with and without a txn. We should also probably reject statements which perform any schema changes when providing a transaction explicitly, as we know that's not handled correctly.

[RichardJCai]

We should probably make the internal executor return an error if you feed it more than one statement in a batch. It has odd semantics both with and without a txn. We should also probably reject statements which perform any schema changes when providing a transaction explicitly, as we know that's not handled correctly.

It actually does error right now now since we call ParseOne

[ajwerner]

Okay, cool. One thing we could do is return an error for any schema change statement or non-DML that has a transaction provided by the call.