Re: Conflict detection and logging in logical replication

On Mon, Jul 29, 2024 at 11:44 AM Zhijie Hou (Fujitsu)
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> Here is the V7 patch set that addressed all the comments so far[1][2][3].

Thanks for the patch, few comments:

1)
build_index_value_desc()
/* Assume the index has been locked */
indexDesc = index_open(indexoid, NoLock);

-- Comment is not very informative. Can we explain in the header if
the caller is supposed to lock it?

2)
apply_handle_delete_internal()

--Do we need to check "(!edata->mtstate || edata->mtstate->operation
!= CMD_UPDATE)" in the else part as well? Can there be a scenario
where during update flow, it is trying to delete from a partition and
comes here, but till then that row is deleted already and we end up
raising 'delete_missing' additionally instead of 'update_missing'
alone?

3)
errdetail_apply_conflict(): Bulid the index value string.
-- Bulid->Build

4)
patch003: create_subscription.sgml
the conflict statistics are collected(displayed in the

-- collected (displayed in the -->space before '(' is needed.

thanks
Shveta