On Monday, November 7, 2022 4:17 PM Peter Smith <smithpb2...@gmail.com> > > Here are my review comments for v42-0001
Thanks for the comments. > ====== > > 28. handle_streamed_transaction > > static bool > handle_streamed_transaction(LogicalRepMsgType action, StringInfo s) { > - TransactionId xid; > + TransactionId current_xid; > + ParallelApplyWorkerInfo *winfo; > + TransApplyAction apply_action; > + StringInfoData origin_msg; > + > + apply_action = get_transaction_apply_action(stream_xid, &winfo); > > /* not in streaming mode */ > - if (!in_streamed_transaction) > + if (apply_action == TRANS_LEADER_APPLY) > return false; > > - Assert(stream_fd != NULL); > Assert(TransactionIdIsValid(stream_xid)); > > + origin_msg = *s; > > ~ > > 28b. > Why not assign it at the declaration, the same as apply_handle_stream_prepare > does? The assignment is unnecessary for non-streaming transaction, so I delayed it. > ~ > > 44b. > If this is always written to a file, then wouldn't a better function name be > something including the word "serialize" - e.g. > serialize_message()? I feel it would be better to be consistent with the existing style stream_xxx_xx(). I think I have addressed all the comments, but since quite a few logics are changed in the new version so I might missed something. And dome code wrapping need to be adjusted, I plan to run pg_indent for next version. Best regards, Hou zj