Re: Writeable CTE patch
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi>
Cc: Alex Hunsaker <badalex@gmail.com>, pgsql-hackers@postgresql.org
Date: 2009-11-27T17:42:02Z
Lists: pgsql-hackers
Attachments
- no_es_result_relation_info.patch (text/x-patch) patch
Marko Tiikkaja <marko.tiikkaja@cs.helsinki.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