Re: Bug in logical decoding with DDL and subtransactions

Hi,

On Tue, Oct 13, 2026 at 1:15 AM alvherre(at)kurilemu(dot)de
<alvherre(at)kurilemu(dot)de> wrote:
>
> Okay, I spent some more time trying to rewrite these comments in the new
> function so that they explain what we're doing in terms we use elsewhere
> in the code, and according to my understanding of how this works. The
> phrasing proposed by Bingshuai contains some terms that we don't use
> anywhere else, such as "a decoding pass" and other variations thereof,
> referring to the logical decoding "session". I would appreciate a
> review for correctness; if it's wrong, please propose fixes as minimal
> as possible (that is, not a complete rewrite).

Thanks for updating the comments.

They look good to me.

> By the way, the assurance that with this coding we will never call
> bsearch() with a NULL subxact array seems a bit too trusting ... I think
> we should just add the "return false" if nsubxacts == 0 in
> TransactionIdInSubxactArray() just to be on the safe side. It costs
> nothing.

+1

> (Well, nothing compared to the cost of doing a hash lookup for
> every single aborted subtransaction -- should we instead impose a cost
> on ReorderBufferBuildTupleCidHash?)

I think in most cases the hash lookup only happens once, for the primary XID -
we need to look up the other subxids only when the primary XID has no changes,
which I personally think is rare. And even in that case, we'd find the toplevel
transaction on the second lookup, for one of the subxids, unless many of the
subxids also have no changes. So the cost looks acceptable.

Best Regards,
Zhijie Hou