On Thu, Dec 15, 2022 at 8:58 AM houzj.f...@fujitsu.com <houzj.f...@fujitsu.com> wrote: >
Few minor comments: ================= 1. + for (i = list_length(subxactlist) - 1; i >= 0; i--) + { + TransactionId xid_tmp = lfirst_xid(list_nth_cell(subxactlist, i)); + + if (xid_tmp == subxid) + { + RollbackToSavepoint(spname); + CommitTransactionCommand(); + subxactlist = list_truncate(subxactlist, i + 1); I find that there is always one element extra in the list after rollback to savepoint. Don't we need to truncate the list to 'i' as shown in the diff below? 2. * Note that If it's an empty sub-transaction then we will not find * the subxid here. If in above comment seems to be in wrong case. Anyway, I have slightly modified it as you can see in the diff below. $ git diff diff --git a/src/backend/replication/logical/applyparallelworker.c b/src/backend/replication/logical/applyparallelworker.c index 11695c75fa..c809b1fd01 100644 --- a/src/backend/replication/logical/applyparallelworker.c +++ b/src/backend/replication/logical/applyparallelworker.c @@ -1516,8 +1516,8 @@ pa_stream_abort(LogicalRepStreamAbortData *abort_data) * Search the subxactlist, determine the offset tracked for the * subxact, and truncate the list. * - * Note that If it's an empty sub-transaction then we will not find - * the subxid here. + * Note that for an empty sub-transaction we won't find the subxid + * here. */ for (i = list_length(subxactlist) - 1; i >= 0; i--) { @@ -1527,7 +1527,7 @@ pa_stream_abort(LogicalRepStreamAbortData *abort_data) { RollbackToSavepoint(spname); CommitTransactionCommand(); - subxactlist = list_truncate(subxactlist, i + 1); + subxactlist = list_truncate(subxactlist, i); break; } } -- With Regards, Amit Kapila.