On Fri, Jul 25, 2025 at 4:38 PM Zhijie Hou (Fujitsu) <houzj.f...@fujitsu.com> wrote: > > > The V53-0001 also includes Shveta's comments in [1]. >
Thanks, I have not yet completed the review, but please find a few comments on 001: 1) IsIndexUsableForFindingDeletedTuple() We first have: + /* + * A frozen transaction ID indicates that it must fall behind the conflict + * detection slot.xmin. + */ + if (HeapTupleHeaderXminFrozen(index_tuple->t_data)) + return true; thent his: + index_xmin = HeapTupleHeaderGetRawXmin(index_tuple->t_data); Shall we use HeapTupleHeaderGetXmin() instead of above 2? We can check if xid returned by HeapTupleHeaderGetXmin() is FrozenTransactionId or normal one and then do further processing. 2) Both FindRecentlyDeletedTupleInfoByIndex and FindRecentlyDeletedTupleInfoSeq() has: + /* Exit early if the commit timestamp data is not available */ + if (!track_commit_timestamp) + return false; We shall either move this check to FindDeletedTupleInLocalRel() or in the caller of FindDeletedTupleInLocalRel() where we check 'MySubscription->retaindeadtuples'. Moving to the caller of FindDeletedTupleInLocalRel() looks better as there is no need to call FindDeletedTupleInLocalRel itself if pre-conditions are not met. 3) FindRecentlyDeletedTupleInfoSeq(): + * during change applications. While this approach may be slow on large tables, + * it is considered acceptable because it is only used in rare conflict cases + * where the target row for an update cannot be found. + */ Shall we extend the last line to: where the target row for an update cannot be found and index scan can not be used. 4) catalogs.sgml: + If true, he detection of <xref linkend="conflict-update-deleted"/> is he --> the thanks Shveta
