Unnecessary FOR UPDATE lock instead of possible FOR NO KEY UPDATE lock in an UPSERT statement

Hi,

Since Postgres 9.3, UPDATE statements generally try to grab FOR NO KEY
UPDATE locks,
unless columns participating in UNIQUE indexes also were modified:

>commit 0ac5ad5134f2769ccbaefec73844f8504c4d6182
>Author: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
>Date: Wed Jan 23 12:04:59 2013 -0300
>...
>UPDATE commands that do not modify the values stored in the columns that
are
>part of the key of the tuple now grab a SELECT FOR NO KEY UPDATE lock on
the tuple,
>allowing them to proceed concurrently with tuple locks of the FOR KEY
SHARE variety.

If we do an UPDATE statement and rewrite part of the key like "set a = a"
Postgres still grabs a FOR NO KEY UPDATE lock.

But if we do an UPSERT statement and rewrite part of the tuple key with the
same value
in the ON CONFLICT clause, Postgres grabs a more aggressive FOR UPDATE
lock.

(I couldn't provide a self-contained script, because we need to monitor row
locks
in a separate connection). See below:

create extension if not exists pgrowlocks ;
create sequence if not exists books_id_seq ;

create table if not exists books (
id integer primary key default nextval('books_id_seq'),
provider_id int not null,
external_id text not null,
some_data text,
unique(provider_id, external_id) -- this is important
);

truncate books;
insert into books (provider_id, external_id) values (1, 'ABC');

-- Case 1 (passed): update uniq key to the same value;
-- expect No Key Update lock, because they key hasn't changed
begin;
update books set (provider_id, external_id) = (provider_id, external_id);

-- (run this query in a different connection)
SELECT p.* FROM books AS t, pgrowlocks('books') AS p WHERE p.locked_row =
t.ctid;
/*
locked_row | locker | multi | xids | modes | pids
------------+---------+-------+-----------+-------------------+------
(0,1) | 2103114 | f | {2103114} | {"No Key Update"} | {0}
*/
rollback;

-- Case 2 (passed): update uniq key to a different value;
-- expect For Update lock, because the uniq key has changed
begin;
update books set (provider_id, external_id) = (2, 'ABC');

-- (run this query in a different connection)
SELECT p.* FROM books AS t, pgrowlocks('books') AS p WHERE p.locked_row =
t.ctid;
/*
locked_row | locker | multi | xids | modes | pids
------------+---------+-------+-----------+----------+------
(0,1) | 2103117 | f | {2103117} | {Update} | {0}
*/
rollback;

-- Case 3 (failed): update uniq key to the same value, but use the UPSERT
query,
-- expect No Key Update lock because the key hasn't changed,
-- but we actually get For Update lock
begin;
insert into books (provider_id, external_id)
select 1, 'ABC'
on conflict (provider_id, external_id)
do update set (provider_id, external_id) = (excluded.provider_id,
excluded.external_id);

-- (run this query in a different connection)
SELECT p.* FROM books AS t, pgrowlocks('books') AS p WHERE p.locked_row =
t.ctid;
/*
locked_row | locker | multi | xids | modes | pids
------------+---------+-------+-----------+----------+------
(0,1) | 2103119 | f | {2103119} | {Update} | {0}
*/
rollback;

-- Case 4, same UPSERT, but don't update uniq key in the ON CONFLICT
clause,
-- expect No Key Update lock because the key hasn't changed
begin;
insert into books (provider_id, external_id, some_data)
select 1, 'ABC', 'some data'
on conflict (provider_id, external_id)
do update set some_data = excluded.some_data;

-- (run this query in a different connection)
SELECT p.* FROM books AS t, pgrowlocks('books') AS p WHERE p.locked_row =
t.ctid;
/*
locked_row | locker | multi | xids | modes | pids
------------+---------+-------+-----------+-------------------+------
(0,1) | 2103122 | f | {2103122} | {"No Key Update"} | {0}
*/
rollback;

So in case 3 we rewrite the key part of the tuple with the same value
and get the stronger FOR UPDATE lock.
While in case 1 we do the same but acquire a softer FOR NO KEY UPDATE lock
which is the correct behavior since we haven't actually changed the key.

This is inconsistent.
I expect UPSERT query in case 3 to grab a FOR NO KEY UPDATE lock
because SET (a, b) = (excluded.a, excluded.b) is not actually modifying the
key.

Unnecessarily stronger locks increase lock contentions,
for example blocking FK constraint checks.

Postgres version: PostgreSQL 12.4 on x86_64-apple-darwin16.7.0, compiled by
Apple LLVM version 8.1.0 (clang-802.0.42), 64-bit
Also reproduced with Postgresql 12.2 on a Linux box.

Best regards,
Ivan