On Tue, Jul 7, 2026 at 12:10 AM Robert Haas <[email protected]> wrote: > > On Thu, Jul 2, 2026 at 4:09 AM Dilip Kumar <[email protected]> wrote: > > I rebased the remaining patches on top of HEAD. So far, I have run > > pgindent and completed the doc merge for 0001. The 0002 and 0003 are > > just rebased. > > I just saw that a preparatory commit for this feature went in and it > triggered me to look at this thread. What v63-0001 does is: when an > error is thrown, the try/catch block sets aside the error, calls > AbortOutOfAnyTransaction(), tries to insert the conflict tuple, and > then re-throws the error. > > I think this is an entirely unacceptable design, for two reasons: > > 1. Setting aside the error in the middle of a try/catch block like > this seems completely unsafe. What if there's another try/catch block > on the stack that needs to run before AbortOutOfAnyTransaction > fires()? >
If I understand you correctly, the concern is that AbortOutOfAnyTransaction() tears down the entire transaction stack, so if it runs from a nested PG_CATCH, any outer catch block still queued on the stack that expects a live (sub)transaction would then execute against already-aborted state. If that's the concern, I don't think it applies here, because this isn't a nested catch. The AbortOutOfAnyTransaction() and the deferred insert run in start_apply()'s PG_CATCH (and ParallelApplyWorkerMain() for the parallel worker), which is the outermost catch wrapping the whole apply loop. There is no apply-level catch outside it. In fact, AbortOutOfAnyTransaction() was already being called at this exact spot before this patch; the patch only adds the deferred conflict-tuple insert between that abort and the re-throw, so we are not aborting any earlier than the code already did. Having said that, if the worry is that this might not remain the outermost catch in the future, we could convert this outer block to a sigsetjmp-based handler, which would tie the abort unambiguously to the top-level teardown and remove any dependence on its position in the try/catch stack. Please let me know if you had a different scenario in mind. > 2. If inserting into the conflict table itself errors out, then the > conflict log table will be permanently out-of-sync with the status of > the replicated table. > The conflict log table can't permanently diverge from the replicated table. On the error path we call replorigin_xact_clear() before inserting the conflict row, so the CLT-insert transaction commits without advancing the replication origin. The origin therefore stays at the last-confirmed LSN, and the errored transaction is re-streamed and re-applied on the next attempt; which also re-attempts the conflict logging. If the CLT insert itself fails, nothing is committed and the same replay re-does both. Is there a specific path where you see the origin advancing without the row being applied that could lead to divergence? -- With Regards, Amit Kapila.
