Re: Remembering bug #6123

From: Florian Pflug <fgp(at)phlo(dot)org>
To: Kevin Grittner <Kevin(dot)Grittner(at)wicourts(dot)gov>
Cc: <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Remembering bug #6123
Date: 2012-01-12 15:48:37
Message-ID: 0D6B34D5-9933-4B03-A47E-1B42EDD74E2F@phlo.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Jan12, 2012, at 00:32 , Kevin Grittner wrote:
> Going back through the patches we had to make to 9.0 to move to
> PostgreSQL triggers, I noticed that I let the issues raised as bug
> #6123 lie untouched during the 9.2 development cycle. In my view,
> the best suggestion for a solution was proposed by Florian here:
>
> http://archives.postgresql.org/pgsql-hackers/2011-08/msg00388.php

I've recently encountered a related issue, this time not related to
triggers but to FOR UPDATE.

My code declared a cursor which iterates over pairs of parent and
child rows, and updated the parent which values from the child. I
declared the cursor "FOR UPDATE OF parent", and used "WHERE CURRENT OF"
to update the parent row while iterating over the cursor. The code
looked something like this

DECLARE
c_parent_child CURSOR FOR
SELECT ... FROM parent JOIN child ON ...
FOR UPDATE OF parent;

BEGIN
FOR v_row IN c_parent_child LOOP
...
UPDATE parent SET ... WHERE CURRENT OF c_parent_child
END LOOP

I figured that was all safe and sound, since with the cursor's results
should be unaffected by the later UPDATES - after all, it's cmax is
smaller than any of the UPDATEs command ids. What I didn't take into
account, however, is that "FOR UPDATE OF" will silently skip rows which
have been updated (by the same transaction) after the cursor's snapshot
was established.

Thus, what happened was that things worked fine for parents with only
one child, but for parents with multiple children only the first child
got be processed. Once I realized that source of that problem, the fix was
easy - simply using the parent table's primary key to do the update, and
dropping the "FOR UPDATE OF" clause fixed the problem.

So, I guess my question is, if we add safeguards against these sorts of
bugs for triggers, should we also add them to FOR UPDATE? Historically,
we seem to have taken the stand that modifications of self-updated tuples
should be ignored. If we're going to reverse that decision (which I think
Kevin has argued for quite convincingly), it seems consistent to complain
about all modifications to self-updated tuples, not only to those involving
triggers.

OTOH, the more cases we complain, the larger the chance that we cause serious
issues for people who upgrade to 9.2. (or 9.3, whatever).

best regards,
Florian Pflug

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2012-01-12 15:49:16 Re: pgbench post-connection command
Previous Message Pavel Stehule 2012-01-12 15:44:25 Re: JSON for PG 9.2