Re: Writeable CTE patch

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Marko Tiikkaja <marko(dot)tiikkaja(at)cs(dot)helsinki(dot)fi>
Cc: Alex Hunsaker <badalex(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Writeable CTE patch
Date: 2009-11-27 19:44:50
Message-ID: 5503.1259351090@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> 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.

On further review there's a really serious stumbling block here.
Consider
INSERT INTO t1 SELECT * FROM t2 UNION ALL SELECT * FROM t3
where the three tables all have the same user columns but t2 has
OIDs and t3 not (or vice versa). Without ExecContextForcesOids
or something very much like it, both scan nodes will think they
can return physical tuples. The output of the Append node will
therefore contain some tuples with OIDs and some without. Append
itself can't fix that since it doesn't project. In many queries
this would not matter --- but if we are inserting them directly
into t1 without any further filtering, it does matter.

I can imagine various ways around this, but it's not clear that
any of them are much less grotty than the code is now. In any
case this was just a marginal code cleanup idea and it doesn't
seem worth spending so much time on right now.

I'm going to go back to plan A: drop the es_result_relation_info
changes from the patch.

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2009-11-27 20:05:07 Re: Re: [COMMITTERS] pgsql: Rewrite GEQO's gimme_tree function so that it always finds a
Previous Message Tom Lane 2009-11-27 17:42:02 Re: Writeable CTE patch