RE: Conflict detection and logging in logical replication

On Wednesday, July 31, 2024 6:53 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> 
> On Wed, Jul 31, 2024 at 7:40 AM Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.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