On Wed, Jul 22, 2026 at 11:37 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Jul 22, 2026 at 8:22 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > While researching this bug, I found another one that is related and
> > probably should be fixed first: even in PG17 and earlier, logical
> > decoding calls the commit_prepared callback without first calling the
> > prepare callback:
> >
> > BEGIN;
> > SELECT * FROM test WHERE id = 1 FOR SHARE;
> > PREPARE TRANSACTION 'p1';
> > COMMIT PREPARED 'p1';
> >
> > In logical replication, the subscriber ends up with an error because
> > the prepared transaction doesn't exist on it.
> >
> > In summary, with the above scenario, logical decoding calls:
> >
> > - the prepare and commit_prepared callbacks (PG18+)
> > - the commit_prepared callback (PG17-)
> >
> > Neither is correct.
> >
> > I think we shouldn't call the commit_prepared callback for an empty
> > transaction (one that has no base snapshot), so in this case we
> > shouldn't call any of the begin_prepare, prepare, or commit_prepared
> > callbacks. Thoughts?
> >
>
> I agree that when base_snapshot is not set, we shouldn't send these
> transaction commands. I checked HEAD and it seems below change in
> commit 072ee847ad lead to sending prepare:
> - if (txn->concurrent_abort && !rbtxn_is_streamed(txn))
> + if (!rbtxn_sent_prepare(txn))
> + {
> rb->prepare(rb, txn, txn->final_lsn);
> + txn->txn_flags |= RBTXN_SENT_PREPARE;
> + }
>
> Why did we remove the check of the aborted xact?
>
> I'll check PG17 and share my findings with you.
>
For PG17 and before, I think we can skip replaying commit if
base_snapshot is not set similar to ReorderBufferReplay(). See
attached. The other possibility is to update ReorderBufferReplay() to
retrun a special value so that callers can skip sending commit or
prepare.
--
With Regards,
Amit Kapila.
