References: <18280-4c8060178cb41750@postgresql.org>
 <tencent_0726471CA7087C4AC6EF1F19409DF11FCF08@qq.com>
User-agent: mu4e 1.10.7; emacs 29.1
From: Andy Fan <zhihuifan1213@163.com>
To: feichanghong <feichanghong@qq.com>
Cc: pgsql-bugs@lists.postgresql.org,Masahiko Sawada
 <sawada.mshk@gmail.com>,Amit Kapila <amit.kapila16@gmail.com>
Subject: Re: BUG #18280: logical decoding build wrong snapshot for
 subtransactions
Date: Fri, 12 Jan 2024 10:00:41 +0800
In-reply-to: <tencent_0726471CA7087C4AC6EF1F19409DF11FCF08@qq.com>
Message-ID: <878r4vmho2.fsf@163.com>
MIME-Version: 1.0
Content-Type: text/plain
Archived-At: <https://www.postgresql.org/message-id/878r4vmho2.fsf%40163.com>
Precedence: bulk


Hi,

Thanks for the steps for reproducing this bug and the fix!

"feichanghong" <feichanghong@qq.com> writes:

>> SnapBuildXidSetCatalogChanges (introduced in 272248a) skipping the check for
>> the sub-transactions when the parent transaction has been marked as
>> containing catalog changes should be the root cause of the problem.
>
> My fix is to remove the ReorderBufferXidHasCatalogChanges condition from the SnapBuildXidSetCatalogChanges function. This
> approach has a false positive; the subtransactions that didn't change catalog to the snapshot are also added to the
> commit list. But that won't be a problem since we use snapshot built during decoding only to read system catalogs, as
> 272248a's commit message says.

The patch looks good to me.

Added Masahiko who is the author of commit 272248a and Amit who is alway
good at this area as well. Hope this can get more attention from them.

-- 
Best Regards
Andy Fan