Re: FOR KEY LOCK foreign keys

From: Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: FOR KEY LOCK foreign keys
Date: 2011-01-22 21:25:26
Message-ID: m21v44y6k9.fsf@2ndQuadrant.fr
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

This is a first level of review for the patch. I finally didn't get as
much time as I hoped I would, so couldn't get familiar with the locking
internals and machinery… as a result, I can't much comment on the code.

The patch applies cleanly (patch moves one hunk all by itself) and
compiles with no warning. It includes no docs, and I think it will be
required to document the user visible SELECT … FOR KEY LOCK OF x new
feature.

Code wise, very few comments here. It looks like the new code had been
there from the beginning by the reading of the patch. I only have one
question about a variable naming:

! COPY_SCALAR_FIELD(forUpdate);
! COPY_SCALAR_FIELD(strength);

forUpdate used to be a boolean, strength is now one of LCS_FORUPDATE,
LCS_FORSHARE or LCS_FORKEYLOCK. I wonder if that's a fortunate naming
here, but IANANS (I Am Not A Native Speaker).

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> As previously commented, here's a proposal with patch to turn foreign
> key checks into something less intrusive.
>
> The basic idea, as proposed by Simon Riggs, was discussed in a previous
> pgsql-hackers thread here:
> http://archives.postgresql.org/message-id/AANLkTimo9XVcEzfiBR-ut3KVNDkjm2Vxh+t8kAmWjPuv@mail.gmail.com

This link here provides a test case that will issue a deadlock, and

> Something else that might be of interest: the patch as presented here
> does NOT solve the deadlock problem originally presented by Joel

Indeed, that's the first thing I tried… I'm not sure about why fixing
the deadlock issue wouldn't be in this patch scope?

The thing that I'm able to confirm by running this test case is that the
RI trigger check is done with the new code from the patch:

CONTEXT: SQL statement "SELECT 1 FROM ONLY "public"."a" x WHERE "aid" OPERATOR(pg_catalog.=) $1 FOR KEY LOCK OF x"

Sorry for not posting more tests yet, but seeing how late I am to find
the time for the first level review I figured I might as well send that
already. I will try some other test cases, but sure enough, that should
be part of the user level documentation…

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2011-01-22 21:36:50 Re: pgsql: Get rid of the global variable holding the error state
Previous Message Peter Eisentraut 2011-01-22 20:53:41 Re: pl/python refactoring