Re: EvalPlanQual seems a tad broken

I wrote:
> [ EvalPlanQual does not work well ]

I'm planning to go back to work on this now that we're out of
CommitFest.

> We could improve that by feeding successfully locked rows into the EPQ
> machinery as well as ones that were found to be outdated. But that
> would still leave us with two failure cases:

> 1. if some of the tables being joined are not selected FOR UPDATE.

> 2. if the select involves any set-returning functions in the targetlist.

> I think we could get around #1 by having *all* tables in the query
> marked FOR UPDATE at least in a dummy form, ie give them entries in
> the rowMarks list and create junk tlist entries to report their current
> ctid. Then we'd feed all the relevant rows into the EPQ machinery.
> We'd just not lock the ones we weren't asked to lock.

On further review it seems that a better way to do this is to make
things happen inside the EPQ machinery. We need to "freeze" the rows
returned by *all* scan nodes, not only the ones referencing real tables
--- for example, a join against a VALUES scan node would still be a
problem if we don't lock down the VALUES output, since we could end up
getting multiple join rows out. This means we can't assume that there
is a CTID associated with every scan node that EPQ needs to lock down.
What looks like it would work instead is to pass through the current
scan tuple for every scan plan node, not only the ones that are FOR
UPDATE targets. I'm tempted to try to move the responsibility for this
into execScan.c instead of having all the individual scan plan types
know about it.

> I do not see any very good way around #2. I'm tempted to propose
> that we just forbid SRFs in the targetlist of a FOR UPDATE query.
> This could be justified on the same grounds that we forbid aggregate
> functions there, ie, they destroy the one-to-one correspondence between
> table rows and SELECT output rows. If you really had to have it you
> could do something like
> select srf(...) from (select ... for update) ss;

This still seems like a necessary restriction unfortunately :-(.

regards, tom lane