pgsql: Fix a number of issues around modifying a previously updated row

From: Andres Freund <andres(at)anarazel(dot)de>
To: pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: pgsql: Fix a number of issues around modifying a previously updated row
Date: 2019-04-08 05:16:32
Message-ID: E1hDMe0-0006EO-Me@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Fix a number of issues around modifying a previously updated row.

This commit fixes three, unfortunately related, issues:

1) Since 5db6df0c01, the introduction of DML via tableam, it was
possible to trigger "ERROR: unexpected table_lock_tuple status: 1"
when updating a row that was previously updated in the same
transaction - but only when the previously updated row was before
updated in a concurrent transaction (and READ COMMITTED was
used). The reason for that was that that case simply wasn't
expected. Fixing that lead to:

2) Even before the above commit, there were error checks (introduced
in 6868ed7491b7) preventing a row being updated by different
commands within the same statement (say in a function called by an
UPDATE) - but that check wasn't performed when the row was first
updated in a concurrent transaction - instead the second update was
silently skipped in that case. After this change we throw the same
error as we'd without the concurrent transaction.

3) The error messages (introduced in 6868ed7491b7) preventing such
updates emitted the same error message for both DELETE and
UPDATE ("tuple to be updated was already modified by an operation
triggered by the current command"). While that could be changed
separately, it made it hard to write tests that verify the correct
correct behavior of the code.

This commit changes heap's implementation of table_lock_tuple() to
return TM_SelfModified instead of TM_Invisible (previously loosely
modeled after EvalPlanQualFetch), and teaches nodeModifyTable.c to
handle that in response to table_lock_tuple() and not just in response
to table_(delete|update).

Additionally it fixes the wrong error message (see 3 above). The
comment for table_lock_tuple() is also adjusted to state that
TM_Deleted won't return information in TM_FailureData - it'll not
always be available.

This also adds tests to ensure that DELETE/UPDATE correctly error out
when affecting a row that concurrently was modified by another
transaction.

Author: Andres Freund
Reported-By: Tom Lane, when investigating a bug bug fix to another bug
by Amit Langote
Discussion: https://postgr.es/m/19321.1554567786@sss.pgh.pa.us

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/41f5e04aec6cf63ba8392adf70e9289e9c3706d6

Modified Files
--------------
src/backend/access/heap/heapam_handler.c | 8 ++-
src/backend/executor/nodeModifyTable.c | 44 +++++++++++++++--
src/include/access/tableam.h | 6 +--
src/test/isolation/expected/eval-plan-qual.out | 67 ++++++++++++++++++++++++++
src/test/isolation/specs/eval-plan-qual.spec | 27 +++++++++++
src/test/regress/expected/triggers.out | 2 +-
6 files changed, 144 insertions(+), 10 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Andres Freund 2019-04-08 05:52:55 pgsql: Reset memory context once per tuple in validateForeignKeyConstra
Previous Message Michael Paquier 2019-04-08 04:46:15 pgsql: Add more tests for partition tuple routing with dropped attribut