Avoid creating PlaceHolderVars immediately within PlaceHolderVars.

Such a construction is useless since the lower PlaceHolderVar is already
nullable; no need to make it more so.  Noted while pursuing bug #6154.

This is just a minor planner efficiency improvement, since the final plan
will come out the same anyway after PHVs are flattened.  So not worth the
risk of back-patching.

diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index ac622a34d9d96e726af679479e69e7b7d7eaa65a..63a52f2d97635f24d683e1c18561763b858b53cc 100644 (file)
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -1411,6 +1411,12 @@ pullup_replace_vars_callback(Var *var,
                /* Simple Vars always escape being wrapped */
                wrap = false;
            }
+           else if (newnode && IsA(newnode, PlaceHolderVar) &&
+                    ((PlaceHolderVar *) newnode)->phlevelsup == 0)
+           {
+               /* No need to wrap a PlaceHolderVar with another one, either */
+               wrap = false;
+           }
            else if (rcon->wrap_non_vars)
            {
                /* Wrap all non-Vars in a PlaceHolderVar */
@@ -1420,10 +1426,16 @@ pullup_replace_vars_callback(Var *var,
            {
                /*
                 * If it contains a Var of current level, and does not contain
-                * any non-strict constructs, then it's certainly nullable and
-                * we don't need to insert a PlaceHolderVar.  (Note: in future
-                * maybe we should insert PlaceHolderVars anyway, when a tlist
-                * item is expensive to evaluate?
+                * any non-strict constructs, then it's certainly nullable so
+                * we don't need to insert a PlaceHolderVar.
+                *
+                * This analysis could be tighter: in particular, a non-strict
+                * construct hidden within a lower-level PlaceHolderVar is not
+                * reason to add another PHV.  But for now it doesn't seem
+                * worth the code to be more exact.
+                *
+                * Note: in future maybe we should insert a PlaceHolderVar
+                * anyway, if the tlist item is expensive to evaluate?
                 */
                if (contain_vars_of_level((Node *) newnode, 0) &&
                    !contain_nonstrict_functions((Node *) newnode))