On Wed, Jun 9, 2021 at 8:44 PM Mark Dilger <mark.dil...@enterprisedb.com> wrote: > > > On Jun 9, 2021, at 7:52 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: > > > > Here's a draft patch for that. I decided the most sensible way to > > organize this is to pair the existing ensure_transaction() subroutine > > with a cleanup subroutine. Rather unimaginatively, perhaps, I renamed > > it to begin_transaction_step and named the cleanup end_transaction_step. > > (Better ideas welcome.) > > Thanks! The regression test I posted earlier passes with this patch applied. >
I have also read the patch and it looks good to me. > > Somewhat unrelated, but ... am I reading the code correctly that > > apply_handle_stream_start and related routines are using Asserts > > to check that the remote sent stream-control messages in the correct > > order? > > Yes. I think you are talking about Assert(!in_streamed_transaction). There is no particular reason that such Asserts are required, so we can change to test-and-elog as you suggested later in your email. > That seems many degrees short of acceptable. > > Even if you weren't reading that correctly, this bit: > > xid = pq_getmsgint(s, 4); > > Assert(TransactionIdIsValid(xid)); > > simply asserts that the sending server didn't send an invalid subtransaction > id. > This also needs to be changed to test-and-elog. -- With Regards, Amit Kapila.
