Back out prior patch and instead just suppress SubqueryScan elimination

when there are extra resjunk columns in the child node.  I found some
additional cases involving Append nodes that weren't handled by the
prior patch, and it's not clear how to fix them in the same way without
breaking inheritance cases.  So the prudent path seems to be to narrow
the scope of the optimization.

diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 0cd41d6f1e69b6095da9500fc05afb2bc58b5295..fe01555a3c40963a44570eb92454ee171f945276 100644 (file)
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/optimizer/plan/setrefs.c,v 1.113 2005/09/05 17:25:01 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/optimizer/plan/setrefs.c,v 1.114 2005/09/05 18:59:38 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -130,7 +130,6 @@ static void set_sa_opfuncid(ScalarArrayOpExpr *opexpr);
 Plan *
 set_plan_references(Plan *plan, List *rtable)
 {
-       bool            copy_lefttree_tlist = false;
        ListCell   *l;
 
        if (plan == NULL)
@@ -219,13 +218,14 @@ set_plan_references(Plan *plan, List *rtable)
 
                        /*
                         * These plan types don't actually bother to evaluate their
-                        * targetlists, because they just return their unmodified
-                        * input tuples; so their targetlists should just be copies
-                        * of their input plan nodes' targetlists.  The actual copying
-                        * has to be done after we've finalized the input node.
-                        */
-                       copy_lefttree_tlist = true;
-                       /*
+                        * targetlists (because they just return their unmodified
+                        * input tuples).  The optimizer is lazy about creating really
+                        * valid targetlists for them --- it tends to just put in a
+                        * pointer to the child plan node's tlist.  Hence, we leave
+                        * the tlist alone.  In particular, we do not want to process
+                        * subplans in the tlist, since we will likely end up reprocessing
+                        * subplans that also appear in lower levels of the plan tree!
+                        *
                         * Since these plan types don't check quals either, we should
                         * not find any qual expression attached to them.
                         */
@@ -238,7 +238,6 @@ set_plan_references(Plan *plan, List *rtable)
                         * or quals.  It does have live expressions for limit/offset,
                         * however.
                         */
-                       copy_lefttree_tlist = true;
                        Assert(plan->qual == NIL);
                        fix_expr_references(plan, ((Limit *) plan)->limitOffset);
                        fix_expr_references(plan, ((Limit *) plan)->limitCount);
@@ -267,10 +266,9 @@ set_plan_references(Plan *plan, List *rtable)
 
                        /*
                         * Append, like Sort et al, doesn't actually evaluate its
-                        * targetlist or check quals, so don't fix targetlist/qual.
-                        * But do recurse into child plans.  (Unlike Sort et al, the
-                        * correct tlist was made by createplan.c and we shouldn't
-                        * replace it.)
+                        * targetlist or check quals, and we haven't bothered to give it
+                        * its own tlist copy. So, don't fix targetlist/qual. But do
+                        * recurse into child plans.
                         */
                        Assert(plan->qual == NIL);
                        foreach(l, ((Append *) plan)->appendplans)
@@ -317,20 +315,6 @@ set_plan_references(Plan *plan, List *rtable)
                sp->plan = set_plan_references(sp->plan, sp->rtable);
        }
 
-       /*
-        * If this is a non-projecting plan node, create a minimally valid
-        * targetlist for it.  Someday we might need to make this look really
-        * real, with Vars referencing the input node's outputs, but for now
-        * the executor only cares that the tlist has the right TargetEntry
-        * fields (resname, resjunk etc) and exprType results.  So we can
-        * get away with just copying the input node's tlist.  (Note:
-        * createplan.c already did copy the input, but we have to do it
-        * over in case we removed a SubqueryScan node: the new input plan
-        * node might have extra resjunk fields.)
-        */
-       if (copy_lefttree_tlist)
-               plan->targetlist = copyObject(plan->lefttree->targetlist);
-
        return plan;
 }
 
@@ -430,6 +414,10 @@ trivial_subqueryscan(SubqueryScan *plan)
        if (plan->scan.plan.qual != NIL)
                return false;
 
+       if (list_length(plan->scan.plan.targetlist) !=
+               list_length(plan->subplan->targetlist))
+               return false;                   /* tlists not same length */
+
        attrno = 1;
        forboth(lp, plan->scan.plan.targetlist, lc, plan->subplan->targetlist)
        {
@@ -448,18 +436,6 @@ trivial_subqueryscan(SubqueryScan *plan)
                attrno++;
        }
 
-       if (lp)
-               return false;                   /* parent tlist longer than child */
-
-       /* extra child items are OK only if all are resjunk */
-       for_each_cell(lc, lc)
-       {
-               TargetEntry *ctle = (TargetEntry *) lfirst(lc);
-
-               if (!ctle->resjunk)
-                       return false;
-       }
-
        return true;
 }