Re: UPDATE modifies more rows that it should

On Fri, 16 Feb 2024 at 22:15, hubert depesz lubaczewski
<depesz@depesz.com> wrote:
> I have a case where update modifies more rows than it should.
> Unfortunately, I can't seem to make smaller example. So let me show you
> what I can.

Here's a smaller example.

drop table if exists t1;
create table t1 (a int primary key, b int);
insert into t1 values(1,0),(2,0),(3,0);
set enable_hashagg=0;
set enable_hashjoin=0;
set enable_mergejoin=0;
set enable_material=0;
set enable_sort=0;

update t1 set b = b+1 where a in(select a from t1 order by a for
update of t1 limit 1);
-- UPDATE 3

-- same again
update t1 set b = b+1 where a in(select a from t1 order by a for
update of t1 limit 1);
-- UPDATE 3

-- see plan
explain analyze update t1 set b = b+1 where a in(select a from t1
order by a for update of t1 limit 1);

-- this time without FOR UPDATE
update t1 set b = b+1 where a in(select a from t1 order by a limit 1);
-- update 1 (!)

-- with FOR UPDATE again
update t1 set b = b+1 where a in(select a from t1 order by a for
update of t1 limit 1);
-- update 1 (!)

cluster t1 using t1_pkey;
update t1 set b = b+1 where a in(select a from t1 order by a for
update of t1 limit 1);
-- update 3 (!)

It seems to be caused by the FOR UPDATE.  The first time through the
Nested Loop finds the a=1 row, but on subsequent looks, the a=1 row is
locked by the FOR UPDATE and hits the TM_SelfModified case in
nodeLockRows.c which causes the goto lnext to trigger.

There is a comment there that offers an explanation of why we skip:

/*
* The target tuple was already updated or deleted by the
* current command, or by a later command in the current
* transaction.  We *must* ignore the tuple in the former
* case, so as to avoid the "Halloween problem" of repeated
* update attempts.  In the latter case it might be sensible
* to fetch the updated tuple instead, but doing so would
* require changing heap_update and heap_delete to not
* complain about updating "invisible" tuples, which seems
* pretty scary (table_tuple_lock will not complain, but few
* callers expect TM_Invisible, and we're not one of them). So
* for now, treat the tuple as deleted and do not process.
*/

but I do admit it is quite strange that the results are so dependent
on the chosen query plan.

I'd recommend not using FOR UPDATE.  It might be useful to use that if
you were executing multiple statements in a transaction to ensure they
remain locked between a SELECT and an UPDATE.  I don't see the need in
a single statement.

I can't offer you an explanation of *if* this should happen, but I do
agree that it seems strange.

David