Re: Writeable CTE patch

Marko Tiikkaja <marko(dot)tiikkaja(at)cs(dot)helsinki(dot)fi> writes:
> Tom Lane wrote:
>> since OIDs in user tables have been deprecated for several versions
>> now, I'm thinking that maybe the case doesn't arise often enough to
>> justify keeping such a wart in the executor.

> Under the circumstances I'd lean towards this option.

I've been fooling around with this further and have gotten as far as
the attached patch. It passes regression tests but suffers from an
additional performance loss: the physical-tlist optimization is disabled
when scanning a relation having OIDs. (That is, we'll always use
ExecProject even if the scan is "SELECT * FROM ...".) I think this loss
is worth worrying about since it would apply to queries on system
catalogs, even if the database has no OIDs in user tables. The trick
is to make the knowledge of the required hasoid state available at
ExecAssignResultType time, so that the plan node's result tupdesc is
constructed correctly.

What seems like the best bet is to merge ExecAssignResultTypeFromTL
and ExecAssignScanProjectionInfo into a single function that should
be used by scan node types. It'll do the determination of whether
a physical-tlist optimization is possible, and then set up both the
output tupdesc and the projection info accordingly. This will make the
patch diff a good bit longer but not much more interesting, so I'm
sending it along at this stage.

I think this is worth doing since it cleans up one of the grottier
parts of executor initialization. The whole thing around
ExecContextForcesOids was never pretty, and it's been the source of
more than one bug if memory serves.

Comments?

regards, tom lane