On Wed, Jun 23, 2021 at 9:31 AM Simon Riggs
<simon.riggs@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