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()?

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. Andres put a tremendous amount of work into
ensuring that logical decoding could always safely resume from the
last-confirmed point, and I don't see why logging of conflicts should
be held to a lesser standard. If this goes in like this, we'll have a
never-ending series of bug reports due to low-probability failures to
insert into the conflict table, and we'll never be able to fix all of
those, because this is broken by design.

IMHO, this preparatory commit should be reverted and this entire patch
set needs to be sent back to the drawing board for a total redesign.
I'm open to the idea that I've misunderstood the situation but from
where I'm sitting it seems pretty shocking that any part of this would
have gotten committed with things as they are.

-- 
Robert Haas
EDB: http://www.enterprisedb.com


Reply via email to