diff options
| author | Tom Lane <tgl@sss.pgh.pa.us> | 2022-08-17 15:52:53 -0400 |
|---|---|---|
| committer | Tom Lane <tgl@sss.pgh.pa.us> | 2022-08-17 15:52:53 -0400 |
| commit | b3ff6c742f6c7f750e9f74476576839cb039e1ab (patch) | |
| tree | 958a48667e1d9172c496ecb100162c9e3876c1c6 /src/backend/optimizer/plan/createplan.c | |
| parent | 6569ca43973b754e8213072c8ddcae9e7baf2aaa (diff) | |
| download | postgresql-b3ff6c742f6c7f750e9f74476576839cb039e1ab.tar.gz postgresql-b3ff6c742f6c7f750e9f74476576839cb039e1ab.zip | |
Use an explicit state flag to control PlaceHolderInfo creation.
Up to now, callers of find_placeholder_info() were required to pass
a flag indicating if it's OK to make a new PlaceHolderInfo. That'd
be fine if the callers had free choice, but they do not. Once we
begin deconstruct_jointree() it's no longer OK to make more PHIs;
while callers before that always want to create a PHI if it's not
there already. So there's no freedom of action, only the opportunity
to cause bugs by creating PHIs too late. Let's get rid of that in
favor of adding a state flag PlannerInfo.placeholdersFrozen, which
we can set at the point where it's no longer OK to make more PHIs.
This patch also simplifies a couple of call sites that were using
complicated logic to avoid calling find_placeholder_info() as much
as possible. Now that that lookup is O(1) thanks to the previous
commit, the extra bitmap manipulations are probably a net negative.
Discussion: https://postgr.es/m/1405792.1660677844@sss.pgh.pa.us
Diffstat (limited to 'src/backend/optimizer/plan/createplan.c')
| -rw-r--r-- | src/backend/optimizer/plan/createplan.c | 9 |
1 files changed, 2 insertions, 7 deletions
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index aa9d4e51b9e..cd8a3ef7cb7 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -4915,13 +4915,8 @@ replace_nestloop_params_mutator(Node *node, PlannerInfo *root) /* Upper-level PlaceHolderVars should be long gone at this point */ Assert(phv->phlevelsup == 0); - /* - * Check whether we need to replace the PHV. We use bms_overlap as a - * cheap/quick test to see if the PHV might be evaluated in the outer - * rels, and then grab its PlaceHolderInfo to tell for sure. - */ - if (!bms_overlap(phv->phrels, root->curOuterRels) || - !bms_is_subset(find_placeholder_info(root, phv, false)->ph_eval_at, + /* Check whether we need to replace the PHV */ + if (!bms_is_subset(find_placeholder_info(root, phv)->ph_eval_at, root->curOuterRels)) { /* |