Disallow pullup of a subquery with a subquery in its targetlist?
Back at
/messages/by-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 ...
The problem is that the planner decides it can "pull up" the subquery s,
or flatten it into the outer query. This entails substituting the
subqury's targetlist expressions for outer-query Vars referencing s,
and there's more than one reference to s.color_id. So we get multiple
copies of the inner subquery, and they will produce different results
at runtime due to the use of random(). This results in inconsistent
behavior.
We decided long ago that we should forbid pullup of subqueries that
contain volatile functions in their targetlists, because of what's
basically the same hazard: you might get more evaluations of the
volatile functions than you expected, yielding inconsistent results
and/or unwanted side-effects.
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.
Thoughts? If we do change this, should we back-patch it?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I wrote:
Back at
/messages/by-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