From cff60f2dfa470d5736a19d36eb910844e31db764 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 9 Aug 2011 11:33:46 -0400 Subject: [PATCH] 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. --- src/backend/optimizer/prep/prepjointree.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index ac622a34d9d..63a52f2d976 100644 --- 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))