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 a7156320b182b75821a9d172e920b2ec0fecff10..b388e2325415db39ed5744f0aa221a3db98d2671 100644 (file)
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -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;
 }