Re: "ORDER BY" clause prevents "UPDATE WHERE CURRENT OF"

From: Gregory Stark <stark(at)enterprisedb(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "Robert Haas" <robertmhaas(at)gmail(dot)com>, Postgres <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: "ORDER BY" clause prevents "UPDATE WHERE CURRENT OF"
Date: 2008-11-15 05:56:23
Message-ID: 873ahtmt14.fsf@oxford.xeocode.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> "Robert Haas" <robertmhaas(at)gmail(dot)com> writes:
>> What's particularly unfortunate is Greg's comment that this would work
>> if the planner chose an index scan. Had I defined an index on the
>> columns in question, my code might have worked and been deployed to
>> production - and then broken if the planner changed its mind and
>> decided to use a seqscan after all.
>
>> ISTM any cursor that's going to be updated should specify WHERE UPDATE
>> in the query, but maybe that's not a hard requirement as of now.
>
> Well, it's contrary to SQL spec, which says that sufficiently simple
> cursors should be updatable even if they don't say FOR UPDATE.
>
> However ... the more I think about it, the more I wonder if we shouldn't
> rip out the current search-the-plan-tree hack and allow WHERE CURRENT OF
> *only* for cursors that say FOR UPDATE.

It is tempting since it would make application code which looped updating
WHERE CURRENT OF semantically equivalent to UPDATE FROM. It seems better to
have one set of functionality rather than two similar but subtly different
sets of functionality available depending on the coding style.

> Aside from the above issue, there's an already known and documented risk if
> you omit FOR UPDATE, which is that your WHERE CURRENT OF update silently
> becomes a no-op if someone else has already updated the target row since
> your query started. It seems like not using FOR UPDATE is sufficiently
> dangerous practice that requiring it wouldn't be doing our users a
> disservice.

Could we implicitly add FOR UPDATE when planning and executing a cursor of a
sufficiently simple query? How simple does the spec say the query needs to be?

> There is one thing we lack in order to go that far, though: the current
> implementation of WHERE CURRENT OF can cope with inheritance queries,

How would this implementation relate to the issues described in
inheritance_planner (which always seemed strange):

* inheritance_planner
* Generate a plan in the case where the result relation is an
* inheritance set.
*
* We have to handle this case differently from cases where a source relation
* is an inheritance set. Source inheritance is expanded at the bottom of the
* plan tree (see allpaths.c), but target inheritance has to be expanded at
* the top. The reason is that for UPDATE, each target relation needs a
* different targetlist matching its own column set. Also, for both UPDATE
* and DELETE, the executor needs the Append plan node at the top, else it
* can't keep track of which table is the current target table. Fortunately,
* the UPDATE/DELETE target can never be the nullable side of an outer join,
* so it's OK to generate the plan this way.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's 24x7 Postgres support!

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Smith 2008-11-15 07:45:28 Re: Updated posix fadvise patch v19
Previous Message Andrew Chernow 2008-11-15 03:44:37 Re: libpq-events windows gotcha