"Robert Haas" <robertmhaas@gmail.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. 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.
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,
that is you can sayDECLARE c CURSOR ... FROM parent_table ...UPDATE parent_table ... WHERE CURRENT OF c
and the right things will happen even if the cursor is currently showing
a row from some child table. SELECT FOR UPDATE does not presently
support inheritance, so we'd really have to fix that in order to not
have a loss of functionality. This is something that was on my private
to-do list for 8.4 but I hadn't thought of an easy way to do it. But,
revisiting the issue just now, I have an idea: when a FOR UPDATE target
is an inheritance tree, make the plan return not only ctid as a junk
column, but also tableoid. The executor top level could easily use that
to determine which table to actually try to do heap_lock_tuple in.
I haven't looked at the planner code for this lately, but I'm thinking
it is probably less than a day's work to make it happen.
Comments?
regards, tom lane