On Wed, Jun 23, 2021 at 9:31 AM Simon Riggs <simon.ri...@enterprisedb.com> wrote: > You're right that skipping the check might also skip killing a prior > row version, but it doesn't prevent later scans from killing them, so > there is no correctness aspect to that.
Probably not, no. I'll assume for now that there is no correctness issue. > In the case of a non-HOT UPDATE the backend will see the index entry > for the old row version and then check it, pointlessly. Since that has > just been modified, that won't ever be killed, so skipping the check > makes sense in those cases. I agree that the check itself is pointless here. But that in itself doesn't make the call to _bt_check_unique() useless. It might still manage to set LP_DEAD bits when nothing else will. I realize that the original reason for setting LP_DEAD bits in _bt_check_unique() was something like "well, might as well do this here too". But I believe that LP_DEAD bit setting inside _bt_check_unique() is nevertheless often more valuable than the better known kill_prior_tuple mechanism. I have seen clear and convincing examples of this in the past. Might not really be true anymore. Another thing is _bt_findinsertloc() and _bt_delete_or_dedup_one_page(), which themselves use the checkingunique flag that you're changing the value of. There could also be unintended side-effects there. OTOH they also use indexUnchanged too, so even if there is a problem it might be fixable. -- Peter Geoghegan
