RE: Conflict detection for update_deleted in logical replication

On Thursday, July 31, 2025 5:29 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Tue, Jul 29, 2025 at 10:51 AM Zhijie Hou (Fujitsu) <houzj(dot)fnst(at)fujitsu(dot)com>
> wrote:
> >
> > This is the V54 patch set, with only patch 0001 updated to address the
> > latest comments.
> >
>
> Few minor comments:

Thanks for the comments.

> 1.
> /* The row to be updated was deleted by a different origin */
> CT_UPDATE_DELETED,
> /* The row to be updated was modified by a different origin */
> CT_UPDATE_ORIGIN_DIFFERS,
> /* The updated row value violates unique constraint */ CT_UPDATE_EXISTS,
> /* The row to be updated is missing */
> CT_UPDATE_MISSING,
>
> Is there a reason to keep CT_UPDATE_DELETED before
> CT_UPDATE_ORIGIN_DIFFERS? I mean why not keep it just before
> CT_UPDATE_MISSING on the grounds that they are always handled together?

I agree that it makes more sense to put it before update_missing, and changed it.

>
> 2. Will it be better to name FindRecentlyDeletedTupleInfoByIndex as
> RelationFindDeletedTupleInfoByIndex to make it similar to existing function
> RelationFindReplTupleByIndex? If you agree then make a similar change for
> FindRecentlyDeletedTupleInfoSeq as well.

Yes, the suggested name looks better.

>
> Apart from above, please find a number of comment edits and other cosmetic
> changes in the attached.

Thanks, I have addressed above comments and merge the patch into 0001.

Here is V55 patch set.

Best Regards,
Hou zj