Re: Conflict detection and logging in logical replication

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.

--
With Regards,
Amit Kapila.