Thread: pull_up_simple_subquery

While working on KaiGai's "leaky views" patch, I found myself
scrutinizing the behavior of the function named in the subject line;
and specifically the retest of is_simple_subquery().  I've been unable
to make that fail.  For example, the following patch fails to break
the regression tests:

--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -718,6 +718,7 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, Ran       }       else       {
+               elog(ERROR, "croak and die");               /*                * Give up, return unmodified RangeTblRef.
              *
 

This logic was originally introduced by the following commit:

commit e439fef6fc3e81aeb865f2c5a77c6faa2ee2a931
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date:   Sat Jan 10 00:30:21 2004 +0000
   Fix subquery pullup logic to not be fooled when a view that appears   'simple' references another view that is not
simple. Must recheck   conditions after performing recursive pullup.  Per example from   Laurent Perez, 9-Jan-04.
 

However, despite my best efforts, I can't figure out what scenario
it's protecting us against, at least not on current sources.  The
original bug report is here:

http://archives.postgresql.org/pgsql-general/2004-01/msg00375.php

Tom's reply indicates that the v4 example shouldn't get flattened, but
it looks to me like current sources do flatten it and I really don't
see why they shouldn't.  Poking around with git bisect and the patch
shown above, I see that the test case stopped tickling this code with
commit e6ae3b5dbf2c07bceb737c5a0ff199b1156051d1, which introduced
PlaceHolderVars, apparently for the precise purpose of allowing joins
of this type to be flattened.  But this code survived that commit,
leaving the question of whether there are still cases where it's
needed (in which case we should probably add a comment or regression
test case, since it's not at all obvious) or whether we can rip it out
and save a few cycles.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> While working on KaiGai's "leaky views" patch, I found myself
> scrutinizing the behavior of the function named in the subject line;
> and specifically the retest of is_simple_subquery().  I've been unable
> to make that fail.

It might be that the is_simple_subquery conditions can't currently fail,
though that has been possible in the past and could be again someday.
The is_safe_append_member conditions can trivially fail after pullup,
however.  An example in the regression database:

create or replace function foo1() returns setof int8 as
' select q2 from int8_tbl, tenk1 where q1 = unique1 '
language sql stable;

select * from foo1() union all select q1 from int8_tbl;

Like the comment says, I'd rather just retest the conditions than try to
work out exactly what might be possible or impossible to happen.

> However, despite my best efforts, I can't figure out what scenario
> it's protecting us against, at least not on current sources.  The
> original bug report is here:

> http://archives.postgresql.org/pgsql-general/2004-01/msg00375.php

> Tom's reply indicates that the v4 example shouldn't get flattened, but
> it looks to me like current sources do flatten it and I really don't
> see why they shouldn't.  Poking around with git bisect and the patch
> shown above, I see that the test case stopped tickling this code with
> commit e6ae3b5dbf2c07bceb737c5a0ff199b1156051d1, which introduced
> PlaceHolderVars, apparently for the precise purpose of allowing joins
> of this type to be flattened.

Yes, that was the point of PlaceHolderVars: we used to not be able to
flatten subqueries underneath outer joins, if they had any non-nullable
output expressions.  Adding a PHV ensures that the expression will go to
null if it's supposed to.
        regards, tom lane