On Wed, Jul 31, 2024 at 7:40 AM Zhijie Hou (Fujitsu) <houzj.f...@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. -- With Regards, Amit Kapila.