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))