Re: Disallow pullup of a subquery with a subquery in its targetlist?

I wrote:
> Back at
> http://www.postgresql.org/message-id/520D221E.2060008@gmail.com
> there was a complaint about strange behavior of a query that looks
> basically like this:

> SELECT ...
> FROM
> (SELECT ... ,
> ( SELECT ...
> ORDER BY random()
> LIMIT 1
> ) AS color_id
> FROM ...
> ) s
> LEFT JOIN ...

> ...

> I first wondered why the instance of random() didn't prevent pullup
> in this example. That's because contain_volatile_functions() does
> not recurse into SubLinks, which maybe is the wrong thing; but
> I'm hesitant to change it without detailed analysis of all the
> (many) call sites.

> However, I think that a good case could also be made for fixing this
> by deciding that we shouldn't pull up if there are SubLinks in the
> subquery targetlist, period. Even without any volatile functions,
> multiple copies of a subquery seem like a probable loser cost-wise.

I experimented with the latter behavior and decided it was too invasive;
in particular, it changes the plans chosen for some queries in the
regression tests, and I think it's actually breaking those tests, in the
sense that the queries no longer exercise the planner code paths they
were designed to test.

So I went back to the first idea of allowing contain_volatile_functions()
to recurse into sub-selects. It turns out that this is not as big a deal
as I feared, because recursing into SubLinks will only affect behavior
before the planner has converted SubLinks to SubPlans, and most of the
existing calls to contain_volatile_functions/contain_mutable_functions
are after that and so don't need individual analysis. I've pretty well
convinced myself that looking into sub-selects is a good idea in the
places that examine volatility before that. Accordingly, I propose the
attached patch, which fixes the complained-of behavior but doesn't
change any existing regression test results.

regards, tom lane