This pr has some implications, so we should discuss a little further:

• The experience on Citus MX is different than coordinator for local tables. Do we really want this?
• The root of the problem is that DistributedExecutionRequiresRollback () decides on remote transaction's rollback behavior. But, at that point we really don't know whether any remote transaction would happen or not.
  • Especially with our "failover to local" logic, moving DecideTransactionPropertiesForTaskList () after deciding on local vs remote tasks (e.g., CreateDistributedExecution()) is not enough. We have to defer this decision to after running remote execution (e.g., RunDistributedExecution()), which also complicates the code quite a bit.
  • That's why, I ended up relying on ShouldExecuteTasksLocally in DecideTransactionPropertiesForTaskList which is not great

May fix #4509

Codecov Report

Merging #4595 (442449c) into master (07d3b4f) will decrease coverage by 0.00%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4595      +/-   ##
==========================================
- Coverage   92.64%   92.64%   -0.01%     
==========================================
  Files         212      212              
  Lines       42485    42492       +7     
==========================================
+ Hits        39361    39366       +5     
- Misses       3124     3126       +2     

I'll think a little more

Ì think part of the issue is that we consider almost anything a coordinated transaction, and coordinated transaction is the check we used in transaction_maangement.c on PREPARE TRANSACTION.

We should probably fix this, but this draft PR is not useful for now, closing it