RE: Conflict detection and logging in logical replication

On Wednesday, July 31, 2024 6:53 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Wed, Jul 31, 2024 at 7:40 AM Zhijie Hou (Fujitsu) <houzj(dot)fnst(at)fujitsu(dot)com>
> wrote:
> >
> > Here is the V8 patch set. It includes the following changes:
> >
>
> A few more comments:
> 1. I think in FindConflictTuple() the patch is locking the tuple so that after
> finding a conflict if there is a concurrent delete, it can retry to find the tuple. If
> there is no concurrent delete then we can successfully report the conflict. Is
> that correct? If so, it is better to explain this somewhere in the comments.
>
> 2.
> * Note that this doesn't lock the values in any way, so it's
> * possible that a conflicting tuple is inserted immediately
> * after this returns. But this can be used for a pre-check
> * before insertion.
> ..
> ExecCheckIndexConstraints()
>
> These comments indicate that this function can be used before inserting the
> tuple, however, this patch uses it after inserting the tuple as well. So, I think the
> comments should be updated accordingly.
>
> 3.
> * For unique indexes, we usually don't want to add info to the IndexInfo for
> * checking uniqueness, since the B-Tree AM handles that directly. However,
> * in the case of speculative insertion, additional support is required.
> ...
> BuildSpeculativeIndexInfo(){...}
>
> This additional support is now required even for logical replication to detect
> conflicts. So the comments atop this function should reflect the same.

Thanks for the comments.

Here is the V9 patch set which addressed above and Shveta's comments.

Best Regards,
Hou zj