Robert Haas <robertmhaas@gmail.com> writes:
> I agree with the general direction. A few comments:
> - Isn't it redundant to test if IsInParallelMode() ||
> IsParallelWorker()? We can't be in a parallel worker without also
> being in parallel mode, except during the worker startup sequence.
Hmm. The existing code in AssignTransactionId and
CommandCounterIncrement tests both, so I figured that the conservative
course was to make DefineSavepoint and friends test both. Are you
saying AssignTransactionId and CommandCounterIncrement are wrong?
If you're saying you don't believe that these routines are reachable
during parallel worker start, that could be true, but I'm not sure
I want to make that assumption. In any case, surely the xxxSavepoint
routines are not hot enough to make it an interesting
micro-optimization. (Perhaps it is worthwhile in AssignTransactionId
and CCI, but changing those seems like a job for another patch.)
regards, tom lane