To: Stephan Szabo <sszabo@megazone.bigpanda.com>
Cc: Simon Riggs <simon@2ndquadrant.com>,
 Karim A Nassar <Karim.Nassar@NAU.EDU>,
	Christopher Kings-Lynne <chriskl@familyhealth.com.au>,
	pgsql-performance@postgresql.org
Subject: Re: Delete query takes exorbitant amount of time 
In-reply-to: <20050329054714.V50612@megazone.bigpanda.com> 
References: <Pine.SOL.4.21.0503290120550.5642-100000@coruscant.cet.nau.edu>
	<1112095776.11750.958.camel@localhost.localdomain>
	<20050329054714.V50612@megazone.bigpanda.com>
Comments: In-reply-to Stephan Szabo <sszabo@megazone.bigpanda.com>
	message dated "Tue, 29 Mar 2005 05:50:42 -0800"
Date: Tue, 29 Mar 2005 09:29:20 -0500
Message-ID: <21384.1112106560@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>

Stephan Szabo <sszabo@megazone.bigpanda.com> writes:
> On Tue, 29 Mar 2005, Simon Riggs wrote:
>> The SQL generated for RI checking by the RI triggers currently applies a
>> limit at execution time, not at prepare time. i.e. there is no LIMIT
>> clause in the SQL.
>> 
>> We know whether the check will be limit 1 or limit 0 at prepare time, so
>> why not add a LIMIT clause to the SQL so it changes the plan, not just
>> the number of rows returned when the check query executes?

> Because IIRC, FOR UPDATE and LIMIT at least historically didn't play
> nicely together, so you could sometimes get a result where if the first
> row was locked, the FOR UPDATE would wait on it, but if it was deleted by
> the other transaction you could get 0 rows back in the trigger.

Yeah, this is still true.  It would probably be a good idea to change it
but I haven't looked into exactly what would be involved.  The basic
problem is that the FOR UPDATE filter needs to execute before LIMIT
instead of after, so presumably the FOR UPDATE shenanigans in execMain.c
would need to be pushed into a separate plan node that could go
underneath the LIMIT node.

Originally this would have led to even more broken behavior --- locks
taken on rows that weren't returned --- because the original coding of
the LIMIT node tended to pull one more row from the lower plan than it
would actually return.  But we fixed that.

I think having such a node might allow us to support FOR UPDATE in
subqueries, as well, but I haven't looked at the details.  (Whether that
is a good idea is another question --- the problem of pulling rows that
aren't nominally necessary, and thereby locking them, would apply in
spades.)

			regards, tom lane