Calculation of relids (pull_varnos result) for PlaceHolderVars - Mailing list pgsql-hackers
| From | Tom Lane |
|---|---|
| Subject | Calculation of relids (pull_varnos result) for PlaceHolderVars |
| Date | |
| Msg-id | 171041.1610849523@sss.pgh.pa.us Whole thread Raw |
| Responses |
Re: Calculation of relids (pull_varnos result) for PlaceHolderVars
(Tom Lane <tgl@sss.pgh.pa.us>)
|
| List | pgsql-hackers |
I've been looking into the planner failure reported at [1].
The given test case is comparable to this query in the
regression database:
regression=# select i8.*, ss.v, t.unique2
from int8_tbl i8
left join int4_tbl i4 on i4.f1 = 1
left join lateral (select i4.f1 + 1 as v) as ss on true
left join tenk1 t on t.unique2 = ss.v
where q2 = 456;
ERROR: failed to assign all NestLoopParams to plan nodes
The core of the problem turns out to be that pull_varnos() returns
an incorrect result for the PlaceHolderVar that represents the ss.v
output. Because the only Var within the PHV's expression is i4.f1,
pull_varnos() returns just the relid set (2), implying that the
value can be calculated after having scanned only the i4 relation.
But that's wrong: i4.f1 here represents an outer-join output, so it
can only be computed after forming the join (1 2) of i8 and i4.
In this example, the erroneous calculation leads the planner to
construct a plan with an invalid join order, which triggers a
sanity-check failure in createplan.c.
The relid set (2) is the minimum possible join level at which such
a PHV could be evaluated; (1 2) is the maximum level, corresponding
to the PHV's syntactic position above the i8/i4 outer join. After
thinking about this I've realized that what pull_varnos() ideally
ought to use is the PHV's ph_eval_at level, which is the join level
we actually intend to evaluate it at. There are a couple of
problems, one that's not too awful and one that's a pain in the
rear:
1. pull_varnos() can be used before we've calculated ph_eval_at, as
well as during deconstruct_jointree() which can change ph_eval_at.
This doesn't seem fatal. We can fall back to the conservative
assumption of using the syntactic level if the PlaceHolderInfo isn't
there yet. Once it is (i.e., within deconstruct_jointree()) I think
we are okay, because any given PHV's ph_eval_at should have reached
its final value before we consider any qual involving that PHV.
2. pull_varnos() is not passed the planner "root" data structure,
so it can't get at the PlaceHolderInfo list. We can change its
API of course, but that propagates to dozens of places.
The 0001 patch attached goes ahead and makes those API changes.
I think this is perfectly reasonable to do in HEAD, but it most
likely is an unacceptable API/ABI break for the back branches.
There's one change needed in contrib/postgres_fdw, and other
extensions likely call one or more of the affected functions too.
As an alternative back-branch fix, we could consider the 0002
patch attached, which simply changes pull_varnos() to make the
most conservative assumption that ph_eval_at could wind up as
the PHV's syntactic level (phrels). The trouble with this is
that we'll lose some valid optimizations. There is only one
visible plan change in the regression tests, but it's kind of
unpleasant: we fail to remove a join that we did remove before.
So I'm not sure how much of a problem this'd be in the field.
A third way is to preserve the existing pull_varnos() API in
the back branches, changing all the internal calls to use a
new function that has the additional "root" parameter. This
seems feasible but I've not attempted to code it yet.
(We might be able to get rid of a lot of this mess if I ever
finish the changes I have in mind to represent outer-join outputs
more explicitly. That seems unlikely to happen for v14 at this
point, and it'd certainly never be back-patchable.)
One loose end is that I'm not sure how far to back-patch. The
given test case only fails back to v12. I've not bisected, but
I suspect that the difference is the v12-era changes to collapse out
trivial Result RTEs (4be058fe9 and follow-ons), such as the lateral
sub-select in this test case. The pull_varnos() calculation is
surely just as wrong for a long time before that, but perhaps it's
only a latent bug before that? I've not managed to construct a test
case that fails in v11, but I don't have a lot of confidence that
there isn't one.
Thoughts?
regards, tom lane
[1] https://www.postgresql.org/message-id/237d2b72-6dd0-7b24-3a6f-94288cd44b9c@bfw-online.de
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 2f2d4d171c..48c2f23ae0 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -5709,7 +5709,8 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel,
* RestrictInfos, so we must make our own.
*/
Assert(!IsA(expr, RestrictInfo));
- rinfo = make_restrictinfo(expr,
+ rinfo = make_restrictinfo(root,
+ expr,
true,
false,
false,
diff --git a/src/backend/optimizer/path/clausesel.c b/src/backend/optimizer/path/clausesel.c
index 4f5b870d1b..d263ecf082 100644
--- a/src/backend/optimizer/path/clausesel.c
+++ b/src/backend/optimizer/path/clausesel.c
@@ -227,7 +227,7 @@ clauselist_selectivity_ext(PlannerInfo *root,
}
else
{
- ok = (NumRelids(clause) == 1) &&
+ ok = (NumRelids(root, clause) == 1) &&
(is_pseudo_constant_clause(lsecond(expr->args)) ||
(varonleft = false,
is_pseudo_constant_clause(linitial(expr->args))));
@@ -609,7 +609,7 @@ bms_is_subset_singleton(const Bitmapset *s, int x)
* restriction or join estimator. Subroutine for clause_selectivity().
*/
static inline bool
-treat_as_join_clause(Node *clause, RestrictInfo *rinfo,
+treat_as_join_clause(PlannerInfo *root, Node *clause, RestrictInfo *rinfo,
int varRelid, SpecialJoinInfo *sjinfo)
{
if (varRelid != 0)
@@ -643,7 +643,7 @@ treat_as_join_clause(Node *clause, RestrictInfo *rinfo,
if (rinfo)
return (bms_membership(rinfo->clause_relids) == BMS_MULTIPLE);
else
- return (NumRelids(clause) > 1);
+ return (NumRelids(root, clause) > 1);
}
}
@@ -860,7 +860,7 @@ clause_selectivity_ext(PlannerInfo *root,
OpExpr *opclause = (OpExpr *) clause;
Oid opno = opclause->opno;
- if (treat_as_join_clause(clause, rinfo, varRelid, sjinfo))
+ if (treat_as_join_clause(root, clause, rinfo, varRelid, sjinfo))
{
/* Estimate selectivity for a join clause. */
s1 = join_selectivity(root, opno,
@@ -896,7 +896,7 @@ clause_selectivity_ext(PlannerInfo *root,
funcclause->funcid,
funcclause->args,
funcclause->inputcollid,
- treat_as_join_clause(clause, rinfo,
+ treat_as_join_clause(root, clause, rinfo,
varRelid, sjinfo),
varRelid,
jointype,
@@ -907,7 +907,7 @@ clause_selectivity_ext(PlannerInfo *root,
/* Use node specific selectivity calculation function */
s1 = scalararraysel(root,
(ScalarArrayOpExpr *) clause,
- treat_as_join_clause(clause, rinfo,
+ treat_as_join_clause(root, clause, rinfo,
varRelid, sjinfo),
varRelid,
jointype,
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 380336518f..aab06c7d21 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -1858,7 +1858,7 @@ cost_incremental_sort(Path *path,
* Check if the expression contains Var with "varno 0" so that we
* don't call estimate_num_groups in that case.
*/
- if (bms_is_member(0, pull_varnos((Node *) member->em_expr)))
+ if (bms_is_member(0, pull_varnos(root, (Node *) member->em_expr)))
{
unknown_varno = true;
break;
diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index ab6eaaead1..0188c1e9a1 100644
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -196,7 +196,8 @@ process_equivalence(PlannerInfo *root,
ntest->location = -1;
*p_restrictinfo =
- make_restrictinfo((Expr *) ntest,
+ make_restrictinfo(root,
+ (Expr *) ntest,
restrictinfo->is_pushed_down,
restrictinfo->outerjoin_delayed,
restrictinfo->pseudoconstant,
@@ -716,7 +717,7 @@ get_eclass_for_sort_expr(PlannerInfo *root,
/*
* Get the precise set of nullable relids appearing in the expression.
*/
- expr_relids = pull_varnos((Node *) expr);
+ expr_relids = pull_varnos(root, (Node *) expr);
nullable_relids = bms_intersect(nullable_relids, expr_relids);
newem = add_eq_member(newec, copyObject(expr), expr_relids,
@@ -1696,7 +1697,8 @@ create_join_clause(PlannerInfo *root,
*/
oldcontext = MemoryContextSwitchTo(root->planner_cxt);
- rinfo = build_implied_join_equality(opno,
+ rinfo = build_implied_join_equality(root,
+ opno,
ec->ec_collation,
leftem->em_expr,
rightem->em_expr,
@@ -1996,7 +1998,8 @@ reconsider_outer_join_clause(PlannerInfo *root, RestrictInfo *rinfo,
cur_em->em_datatype);
if (!OidIsValid(eq_op))
continue; /* can't generate equality */
- newrinfo = build_implied_join_equality(eq_op,
+ newrinfo = build_implied_join_equality(root,
+ eq_op,
cur_ec->ec_collation,
innervar,
cur_em->em_expr,
@@ -2141,7 +2144,8 @@ reconsider_full_join_clause(PlannerInfo *root, RestrictInfo *rinfo)
cur_em->em_datatype);
if (OidIsValid(eq_op))
{
- newrinfo = build_implied_join_equality(eq_op,
+ newrinfo = build_implied_join_equality(root,
+ eq_op,
cur_ec->ec_collation,
leftvar,
cur_em->em_expr,
@@ -2156,7 +2160,8 @@ reconsider_full_join_clause(PlannerInfo *root, RestrictInfo *rinfo)
cur_em->em_datatype);
if (OidIsValid(eq_op))
{
- newrinfo = build_implied_join_equality(eq_op,
+ newrinfo = build_implied_join_equality(root,
+ eq_op,
cur_ec->ec_collation,
rightvar,
cur_em->em_expr,
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 9c069a213f..ff536e6b24 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -153,7 +153,8 @@ static IndexClause *match_clause_to_indexcol(PlannerInfo *root,
RestrictInfo *rinfo,
int indexcol,
IndexOptInfo *index);
-static IndexClause *match_boolean_index_clause(RestrictInfo *rinfo,
+static IndexClause *match_boolean_index_clause(PlannerInfo *root,
+ RestrictInfo *rinfo,
int indexcol, IndexOptInfo *index);
static IndexClause *match_opclause_to_indexcol(PlannerInfo *root,
RestrictInfo *rinfo,
@@ -169,13 +170,16 @@ static IndexClause *get_index_clause_from_support(PlannerInfo *root,
int indexarg,
int indexcol,
IndexOptInfo *index);
-static IndexClause *match_saopclause_to_indexcol(RestrictInfo *rinfo,
+static IndexClause *match_saopclause_to_indexcol(PlannerInfo *root,
+ RestrictInfo *rinfo,
int indexcol,
IndexOptInfo *index);
-static IndexClause *match_rowcompare_to_indexcol(RestrictInfo *rinfo,
+static IndexClause *match_rowcompare_to_indexcol(PlannerInfo *root,
+ RestrictInfo *rinfo,
int indexcol,
IndexOptInfo *index);
-static IndexClause *expand_indexqual_rowcompare(RestrictInfo *rinfo,
+static IndexClause *expand_indexqual_rowcompare(PlannerInfo *root,
+ RestrictInfo *rinfo,
int indexcol,
IndexOptInfo *index,
Oid expr_op,
@@ -2305,7 +2309,7 @@ match_clause_to_indexcol(PlannerInfo *root,
opfamily = index->opfamily[indexcol];
if (IsBooleanOpfamily(opfamily))
{
- iclause = match_boolean_index_clause(rinfo, indexcol, index);
+ iclause = match_boolean_index_clause(root, rinfo, indexcol, index);
if (iclause)
return iclause;
}
@@ -2325,11 +2329,11 @@ match_clause_to_indexcol(PlannerInfo *root,
}
else if (IsA(clause, ScalarArrayOpExpr))
{
- return match_saopclause_to_indexcol(rinfo, indexcol, index);
+ return match_saopclause_to_indexcol(root, rinfo, indexcol, index);
}
else if (IsA(clause, RowCompareExpr))
{
- return match_rowcompare_to_indexcol(rinfo, indexcol, index);
+ return match_rowcompare_to_indexcol(root, rinfo, indexcol, index);
}
else if (index->amsearchnulls && IsA(clause, NullTest))
{
@@ -2368,7 +2372,8 @@ match_clause_to_indexcol(PlannerInfo *root,
* index's key, and if so, build a suitable IndexClause.
*/
static IndexClause *
-match_boolean_index_clause(RestrictInfo *rinfo,
+match_boolean_index_clause(PlannerInfo *root,
+ RestrictInfo *rinfo,
int indexcol,
IndexOptInfo *index)
{
@@ -2438,7 +2443,7 @@ match_boolean_index_clause(RestrictInfo *rinfo,
IndexClause *iclause = makeNode(IndexClause);
iclause->rinfo = rinfo;
- iclause->indexquals = list_make1(make_simple_restrictinfo(op));
+ iclause->indexquals = list_make1(make_simple_restrictinfo(root, op));
iclause->lossy = false;
iclause->indexcol = indexcol;
iclause->indexcols = NIL;
@@ -2663,7 +2668,8 @@ get_index_clause_from_support(PlannerInfo *root,
{
Expr *clause = (Expr *) lfirst(lc);
- indexquals = lappend(indexquals, make_simple_restrictinfo(clause));
+ indexquals = lappend(indexquals,
+ make_simple_restrictinfo(root, clause));
}
iclause->rinfo = rinfo;
@@ -2684,7 +2690,8 @@ get_index_clause_from_support(PlannerInfo *root,
* which see for comments.
*/
static IndexClause *
-match_saopclause_to_indexcol(RestrictInfo *rinfo,
+match_saopclause_to_indexcol(PlannerInfo *root,
+ RestrictInfo *rinfo,
int indexcol,
IndexOptInfo *index)
{
@@ -2703,7 +2710,7 @@ match_saopclause_to_indexcol(RestrictInfo *rinfo,
return NULL;
leftop = (Node *) linitial(saop->args);
rightop = (Node *) lsecond(saop->args);
- right_relids = pull_varnos(rightop);
+ right_relids = pull_varnos(root, rightop);
expr_op = saop->opno;
expr_coll = saop->inputcollid;
@@ -2751,7 +2758,8 @@ match_saopclause_to_indexcol(RestrictInfo *rinfo,
* is handled by expand_indexqual_rowcompare().
*/
static IndexClause *
-match_rowcompare_to_indexcol(RestrictInfo *rinfo,
+match_rowcompare_to_indexcol(PlannerInfo *root,
+ RestrictInfo *rinfo,
int indexcol,
IndexOptInfo *index)
{
@@ -2796,14 +2804,14 @@ match_rowcompare_to_indexcol(RestrictInfo *rinfo,
* These syntactic tests are the same as in match_opclause_to_indexcol()
*/
if (match_index_to_operand(leftop, indexcol, index) &&
- !bms_is_member(index_relid, pull_varnos(rightop)) &&
+ !bms_is_member(index_relid, pull_varnos(root, rightop)) &&
!contain_volatile_functions(rightop))
{
/* OK, indexkey is on left */
var_on_left = true;
}
else if (match_index_to_operand(rightop, indexcol, index) &&
- !bms_is_member(index_relid, pull_varnos(leftop)) &&
+ !bms_is_member(index_relid, pull_varnos(root, leftop)) &&
!contain_volatile_functions(leftop))
{
/* indexkey is on right, so commute the operator */
@@ -2822,7 +2830,8 @@ match_rowcompare_to_indexcol(RestrictInfo *rinfo,
case BTLessEqualStrategyNumber:
case BTGreaterEqualStrategyNumber:
case BTGreaterStrategyNumber:
- return expand_indexqual_rowcompare(rinfo,
+ return expand_indexqual_rowcompare(root,
+ rinfo,
indexcol,
index,
expr_op,
@@ -2856,7 +2865,8 @@ match_rowcompare_to_indexcol(RestrictInfo *rinfo,
* but we split it out for comprehensibility.
*/
static IndexClause *
-expand_indexqual_rowcompare(RestrictInfo *rinfo,
+expand_indexqual_rowcompare(PlannerInfo *root,
+ RestrictInfo *rinfo,
int indexcol,
IndexOptInfo *index,
Oid expr_op,
@@ -2926,7 +2936,7 @@ expand_indexqual_rowcompare(RestrictInfo *rinfo,
if (expr_op == InvalidOid)
break; /* operator is not usable */
}
- if (bms_is_member(index->rel->relid, pull_varnos(constop)))
+ if (bms_is_member(index->rel->relid, pull_varnos(root, constop)))
break; /* no good, Var on wrong side */
if (contain_volatile_functions(constop))
break; /* no good, volatile comparison value */
@@ -3036,7 +3046,8 @@ expand_indexqual_rowcompare(RestrictInfo *rinfo,
matching_cols);
rc->rargs = list_truncate(copyObject(non_var_args),
matching_cols);
- iclause->indexquals = list_make1(make_simple_restrictinfo((Expr *) rc));
+ iclause->indexquals = list_make1(make_simple_restrictinfo(root,
+ (Expr *) rc));
}
else
{
@@ -3050,7 +3061,7 @@ expand_indexqual_rowcompare(RestrictInfo *rinfo,
copyObject(linitial(non_var_args)),
InvalidOid,
linitial_oid(clause->inputcollids));
- iclause->indexquals = list_make1(make_simple_restrictinfo(op));
+ iclause->indexquals = list_make1(make_simple_restrictinfo(root, op));
}
}
@@ -3667,7 +3678,9 @@ relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel,
* specified index column matches a boolean restriction clause.
*/
bool
-indexcol_is_bool_constant_for_query(IndexOptInfo *index, int indexcol)
+indexcol_is_bool_constant_for_query(PlannerInfo *root,
+ IndexOptInfo *index,
+ int indexcol)
{
ListCell *lc;
@@ -3689,7 +3702,7 @@ indexcol_is_bool_constant_for_query(IndexOptInfo *index, int indexcol)
continue;
/* See if we can match the clause's expression to the index column */
- if (match_boolean_index_clause(rinfo, indexcol, index))
+ if (match_boolean_index_clause(root, rinfo, indexcol, index))
return true;
}
@@ -3801,10 +3814,10 @@ match_index_to_operand(Node *operand,
* index: the index of interest
*/
bool
-is_pseudo_constant_for_index(Node *expr, IndexOptInfo *index)
+is_pseudo_constant_for_index(PlannerInfo *root, Node *expr, IndexOptInfo *index)
{
/* pull_varnos is cheaper than volatility check, so do that first */
- if (bms_is_member(index->rel->relid, pull_varnos(expr)))
+ if (bms_is_member(index->rel->relid, pull_varnos(root, expr)))
return false; /* no good, contains Var of table */
if (contain_volatile_functions(expr))
return false; /* no good, volatile comparison value */
diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c
index 3ebc57a154..bd9a176d7d 100644
--- a/src/backend/optimizer/path/pathkeys.c
+++ b/src/backend/optimizer/path/pathkeys.c
@@ -598,7 +598,7 @@ build_index_pathkeys(PlannerInfo *root,
* should stop considering index columns; any lower-order sort
* keys won't be useful either.
*/
- if (!indexcol_is_bool_constant_for_query(index, i))
+ if (!indexcol_is_bool_constant_for_query(root, index, i))
break;
}
diff --git a/src/backend/optimizer/path/tidpath.c b/src/backend/optimizer/path/tidpath.c
index 8ef0406057..0845b460e2 100644
--- a/src/backend/optimizer/path/tidpath.c
+++ b/src/backend/optimizer/path/tidpath.c
@@ -123,7 +123,7 @@ IsTidEqualClause(RestrictInfo *rinfo, RelOptInfo *rel)
* other side of the clause does.
*/
static bool
-IsTidEqualAnyClause(RestrictInfo *rinfo, RelOptInfo *rel)
+IsTidEqualAnyClause(PlannerInfo *root, RestrictInfo *rinfo, RelOptInfo *rel)
{
ScalarArrayOpExpr *node;
Node *arg1,
@@ -148,7 +148,7 @@ IsTidEqualAnyClause(RestrictInfo *rinfo, RelOptInfo *rel)
IsCTIDVar((Var *) arg1, rel))
{
/* The other argument must be a pseudoconstant */
- if (bms_is_member(rel->relid, pull_varnos(arg2)) ||
+ if (bms_is_member(rel->relid, pull_varnos(root, arg2)) ||
contain_volatile_functions(arg2))
return false;
@@ -190,7 +190,7 @@ IsCurrentOfClause(RestrictInfo *rinfo, RelOptInfo *rel)
* (Using a List may seem a bit weird, but it simplifies the caller.)
*/
static List *
-TidQualFromRestrictInfo(RestrictInfo *rinfo, RelOptInfo *rel)
+TidQualFromRestrictInfo(PlannerInfo *root, RestrictInfo *rinfo, RelOptInfo *rel)
{
/*
* We may ignore pseudoconstant clauses (they can't contain Vars, so could
@@ -210,7 +210,7 @@ TidQualFromRestrictInfo(RestrictInfo *rinfo, RelOptInfo *rel)
* Check all base cases. If we get a match, return the clause.
*/
if (IsTidEqualClause(rinfo, rel) ||
- IsTidEqualAnyClause(rinfo, rel) ||
+ IsTidEqualAnyClause(root, rinfo, rel) ||
IsCurrentOfClause(rinfo, rel))
return list_make1(rinfo);
@@ -227,7 +227,7 @@ TidQualFromRestrictInfo(RestrictInfo *rinfo, RelOptInfo *rel)
* This function is just concerned with handling AND/OR recursion.
*/
static List *
-TidQualFromRestrictInfoList(List *rlist, RelOptInfo *rel)
+TidQualFromRestrictInfoList(PlannerInfo *root, List *rlist, RelOptInfo *rel)
{
List *rlst = NIL;
ListCell *l;
@@ -255,14 +255,14 @@ TidQualFromRestrictInfoList(List *rlist, RelOptInfo *rel)
List *andargs = ((BoolExpr *) orarg)->args;
/* Recurse in case there are sub-ORs */
- sublist = TidQualFromRestrictInfoList(andargs, rel);
+ sublist = TidQualFromRestrictInfoList(root, andargs, rel);
}
else
{
RestrictInfo *rinfo = castNode(RestrictInfo, orarg);
Assert(!restriction_is_or_clause(rinfo));
- sublist = TidQualFromRestrictInfo(rinfo, rel);
+ sublist = TidQualFromRestrictInfo(root, rinfo, rel);
}
/*
@@ -284,7 +284,7 @@ TidQualFromRestrictInfoList(List *rlist, RelOptInfo *rel)
else
{
/* Not an OR clause, so handle base cases */
- rlst = TidQualFromRestrictInfo(rinfo, rel);
+ rlst = TidQualFromRestrictInfo(root, rinfo, rel);
}
/*
@@ -390,7 +390,7 @@ create_tidscan_paths(PlannerInfo *root, RelOptInfo *rel)
* If any suitable quals exist in the rel's baserestrict list, generate a
* plain (unparameterized) TidPath with them.
*/
- tidquals = TidQualFromRestrictInfoList(rel->baserestrictinfo, rel);
+ tidquals = TidQualFromRestrictInfoList(root, rel->baserestrictinfo, rel);
if (tidquals)
{
diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index 90460a69bd..37eb64bcef 100644
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -231,7 +231,7 @@ join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo)
continue; /* it definitely doesn't reference innerrel */
if (bms_is_subset(phinfo->ph_eval_at, innerrel->relids))
return false; /* there isn't any other place to eval PHV */
- if (bms_overlap(pull_varnos((Node *) phinfo->ph_var->phexpr),
+ if (bms_overlap(pull_varnos(root, (Node *) phinfo->ph_var->phexpr),
innerrel->relids))
return false; /* it does reference innerrel */
}
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index ac7dd5d4c8..02f813cebd 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -60,7 +60,8 @@ static SpecialJoinInfo *make_outerjoininfo(PlannerInfo *root,
Relids left_rels, Relids right_rels,
Relids inner_join_rels,
JoinType jointype, List *clause);
-static void compute_semijoin_info(SpecialJoinInfo *sjinfo, List *clause);
+static void compute_semijoin_info(PlannerInfo *root, SpecialJoinInfo *sjinfo,
+ List *clause);
static void distribute_qual_to_rels(PlannerInfo *root, Node *clause,
bool below_outer_join,
JoinType jointype,
@@ -1196,7 +1197,7 @@ make_outerjoininfo(PlannerInfo *root,
/* this always starts out false */
sjinfo->delay_upper_joins = false;
- compute_semijoin_info(sjinfo, clause);
+ compute_semijoin_info(root, sjinfo, clause);
/* If it's a full join, no need to be very smart */
if (jointype == JOIN_FULL)
@@ -1210,7 +1211,7 @@ make_outerjoininfo(PlannerInfo *root,
/*
* Retrieve all relids mentioned within the join clause.
*/
- clause_relids = pull_varnos((Node *) clause);
+ clause_relids = pull_varnos(root, (Node *) clause);
/*
* For which relids is the clause strict, ie, it cannot succeed if the
@@ -1390,7 +1391,7 @@ make_outerjoininfo(PlannerInfo *root,
* SpecialJoinInfo; the rest may not be set yet.
*/
static void
-compute_semijoin_info(SpecialJoinInfo *sjinfo, List *clause)
+compute_semijoin_info(PlannerInfo *root, SpecialJoinInfo *sjinfo, List *clause)
{
List *semi_operators;
List *semi_rhs_exprs;
@@ -1454,7 +1455,7 @@ compute_semijoin_info(SpecialJoinInfo *sjinfo, List *clause)
list_length(op->args) != 2)
{
/* No, but does it reference both sides? */
- all_varnos = pull_varnos((Node *) op);
+ all_varnos = pull_varnos(root, (Node *) op);
if (!bms_overlap(all_varnos, sjinfo->syn_righthand) ||
bms_is_subset(all_varnos, sjinfo->syn_righthand))
{
@@ -1475,8 +1476,8 @@ compute_semijoin_info(SpecialJoinInfo *sjinfo, List *clause)
opno = op->opno;
left_expr = linitial(op->args);
right_expr = lsecond(op->args);
- left_varnos = pull_varnos(left_expr);
- right_varnos = pull_varnos(right_expr);
+ left_varnos = pull_varnos(root, left_expr);
+ right_varnos = pull_varnos(root, right_expr);
all_varnos = bms_union(left_varnos, right_varnos);
opinputtype = exprType(left_expr);
@@ -1621,7 +1622,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
/*
* Retrieve all relids mentioned within the clause.
*/
- relids = pull_varnos(clause);
+ relids = pull_varnos(root, clause);
/*
* In ordinary SQL, a WHERE or JOIN/ON clause can't reference any rels
@@ -1835,7 +1836,8 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
/*
* Build the RestrictInfo node itself.
*/
- restrictinfo = make_restrictinfo((Expr *) clause,
+ restrictinfo = make_restrictinfo(root,
+ (Expr *) clause,
is_pushed_down,
outerjoin_delayed,
pseudoconstant,
@@ -2309,7 +2311,7 @@ process_implied_equality(PlannerInfo *root,
*
* Retrieve all relids mentioned within the possibly-simplified clause.
*/
- relids = pull_varnos(clause);
+ relids = pull_varnos(root, clause);
Assert(bms_is_subset(relids, qualscope));
/*
@@ -2341,7 +2343,8 @@ process_implied_equality(PlannerInfo *root,
/*
* Build the RestrictInfo node itself.
*/
- restrictinfo = make_restrictinfo((Expr *) clause,
+ restrictinfo = make_restrictinfo(root,
+ (Expr *) clause,
true, /* is_pushed_down */
false, /* outerjoin_delayed */
pseudoconstant,
@@ -2407,7 +2410,8 @@ process_implied_equality(PlannerInfo *root,
* caller's responsibility that left_ec/right_ec be set as necessary.
*/
RestrictInfo *
-build_implied_join_equality(Oid opno,
+build_implied_join_equality(PlannerInfo *root,
+ Oid opno,
Oid collation,
Expr *item1,
Expr *item2,
@@ -2433,7 +2437,8 @@ build_implied_join_equality(Oid opno,
/*
* Build the RestrictInfo node itself.
*/
- restrictinfo = make_restrictinfo(clause,
+ restrictinfo = make_restrictinfo(root,
+ clause,
true, /* is_pushed_down */
false, /* outerjoin_delayed */
false, /* pseudoconstant */
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index 6d4cc1bcce..54ef61bfb3 100644
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -1302,7 +1302,7 @@ convert_ANY_sublink_to_join(PlannerInfo *root, SubLink *sublink,
* it's not gonna be a join. (Note that it won't have Vars referring to
* the subquery, rather Params.)
*/
- upper_varnos = pull_varnos(sublink->testexpr);
+ upper_varnos = pull_varnos(root, sublink->testexpr);
if (bms_is_empty(upper_varnos))
return NULL;
@@ -1486,7 +1486,7 @@ convert_EXISTS_sublink_to_join(PlannerInfo *root, SubLink *sublink,
* The ones <= rtoffset belong to the upper query; the ones > rtoffset do
* not.
*/
- clause_varnos = pull_varnos(whereClause);
+ clause_varnos = pull_varnos(root, whereClause);
upper_varnos = NULL;
while ((varno = bms_first_member(clause_varnos)) >= 0)
{
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index f9ef96991d..d961592e01 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -82,7 +82,8 @@ static void pull_up_union_leaf_queries(Node *setOp, PlannerInfo *root,
int childRToffset);
static void make_setop_translation_list(Query *query, Index newvarno,
AppendRelInfo *appinfo);
-static bool is_simple_subquery(Query *subquery, RangeTblEntry *rte,
+static bool is_simple_subquery(PlannerInfo *root, Query *subquery,
+ RangeTblEntry *rte,
JoinExpr *lowest_outer_join);
static Node *pull_up_simple_values(PlannerInfo *root, Node *jtnode,
RangeTblEntry *rte);
@@ -95,7 +96,8 @@ static bool is_simple_union_all(Query *subquery);
static bool is_simple_union_all_recurse(Node *setOp, Query *setOpQuery,
List *colTypes);
static bool is_safe_append_member(Query *subquery);
-static bool jointree_contains_lateral_outer_refs(Node *jtnode, bool restricted,
+static bool jointree_contains_lateral_outer_refs(PlannerInfo *root,
+ Node *jtnode, bool restricted,
Relids safe_upper_varnos);
static void perform_pullup_replace_vars(PlannerInfo *root,
pullup_replace_vars_context *rvcontext,
@@ -744,7 +746,7 @@ pull_up_subqueries_recurse(PlannerInfo *root, Node *jtnode,
* unless is_safe_append_member says so.
*/
if (rte->rtekind == RTE_SUBQUERY &&
- is_simple_subquery(rte->subquery, rte, lowest_outer_join) &&
+ is_simple_subquery(root, rte->subquery, rte, lowest_outer_join) &&
(containing_appendrel == NULL ||
is_safe_append_member(rte->subquery)))
return pull_up_simple_subquery(root, jtnode, rte,
@@ -973,7 +975,7 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
* easier just to keep this "if" looking the same as the one in
* pull_up_subqueries_recurse.
*/
- if (is_simple_subquery(subquery, rte, lowest_outer_join) &&
+ if (is_simple_subquery(root, subquery, rte, lowest_outer_join) &&
(containing_appendrel == NULL || is_safe_append_member(subquery)))
{
/* good to go */
@@ -1398,7 +1400,7 @@ make_setop_translation_list(Query *query, Index newvarno,
* lowest_outer_join is the lowest outer join above the subquery, or NULL.
*/
static bool
-is_simple_subquery(Query *subquery, RangeTblEntry *rte,
+is_simple_subquery(PlannerInfo *root, Query *subquery, RangeTblEntry *rte,
JoinExpr *lowest_outer_join)
{
/*
@@ -1477,7 +1479,8 @@ is_simple_subquery(Query *subquery, RangeTblEntry *rte,
safe_upper_varnos = NULL; /* doesn't matter */
}
- if (jointree_contains_lateral_outer_refs((Node *) subquery->jointree,
+ if (jointree_contains_lateral_outer_refs(root,
+ (Node *) subquery->jointree,
restricted, safe_upper_varnos))
return false;
@@ -1496,7 +1499,9 @@ is_simple_subquery(Query *subquery, RangeTblEntry *rte,
*/
if (lowest_outer_join != NULL)
{
- Relids lvarnos = pull_varnos_of_level((Node *) subquery->targetList, 1);
+ Relids lvarnos = pull_varnos_of_level(root,
+ (Node *) subquery->targetList,
+ 1);
if (!bms_is_subset(lvarnos, safe_upper_varnos))
return false;
@@ -1929,7 +1934,8 @@ is_safe_append_member(Query *subquery)
* in safe_upper_varnos.
*/
static bool
-jointree_contains_lateral_outer_refs(Node *jtnode, bool restricted,
+jointree_contains_lateral_outer_refs(PlannerInfo *root, Node *jtnode,
+ bool restricted,
Relids safe_upper_varnos)
{
if (jtnode == NULL)
@@ -1944,7 +1950,8 @@ jointree_contains_lateral_outer_refs(Node *jtnode, bool restricted,
/* First, recurse to check child joins */
foreach(l, f->fromlist)
{
- if (jointree_contains_lateral_outer_refs(lfirst(l),
+ if (jointree_contains_lateral_outer_refs(root,
+ lfirst(l),
restricted,
safe_upper_varnos))
return true;
@@ -1952,7 +1959,7 @@ jointree_contains_lateral_outer_refs(Node *jtnode, bool restricted,
/* Then check the top-level quals */
if (restricted &&
- !bms_is_subset(pull_varnos_of_level(f->quals, 1),
+ !bms_is_subset(pull_varnos_of_level(root, f->quals, 1),
safe_upper_varnos))
return true;
}
@@ -1971,18 +1978,20 @@ jointree_contains_lateral_outer_refs(Node *jtnode, bool restricted,
}
/* Check the child joins */
- if (jointree_contains_lateral_outer_refs(j->larg,
+ if (jointree_contains_lateral_outer_refs(root,
+ j->larg,
restricted,
safe_upper_varnos))
return true;
- if (jointree_contains_lateral_outer_refs(j->rarg,
+ if (jointree_contains_lateral_outer_refs(root,
+ j->rarg,
restricted,
safe_upper_varnos))
return true;
/* Check the JOIN's qual clauses */
if (restricted &&
- !bms_is_subset(pull_varnos_of_level(j->quals, 1),
+ !bms_is_subset(pull_varnos_of_level(root, j->quals, 1),
safe_upper_varnos))
return true;
}
@@ -2366,7 +2375,8 @@ pullup_replace_vars_callback(Var *var,
* level-zero var must belong to the subquery.
*/
if ((rcon->target_rte->lateral ?
- bms_overlap(pull_varnos((Node *) newnode), rcon->relids) :
+ bms_overlap(pull_varnos(rcon->root, (Node *) newnode),
+ rcon->relids) :
contain_vars_of_level((Node *) newnode, 0)) &&
!contain_nonstrict_functions((Node *) newnode))
{
@@ -2804,7 +2814,7 @@ reduce_outer_joins_pass2(Node *jtnode,
overlap = list_intersection(local_nonnullable_vars,
forced_null_vars);
if (overlap != NIL &&
- bms_overlap(pull_varnos((Node *) overlap),
+ bms_overlap(pull_varnos(root, (Node *) overlap),
right_state->relids))
jointype = JOIN_ANTI;
}
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 51d26a0691..d2470b7c6a 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -1897,9 +1897,9 @@ is_pseudo_constant_clause_relids(Node *clause, Relids relids)
* Returns the number of different relations referenced in 'clause'.
*/
int
-NumRelids(Node *clause)
+NumRelids(PlannerInfo *root, Node *clause)
{
- Relids varnos = pull_varnos(clause);
+ Relids varnos = pull_varnos(root, clause);
int result = bms_num_members(varnos);
bms_free(varnos);
diff --git a/src/backend/optimizer/util/inherit.c b/src/backend/optimizer/util/inherit.c
index 36d248beb5..be1c9ddd96 100644
--- a/src/backend/optimizer/util/inherit.c
+++ b/src/backend/optimizer/util/inherit.c
@@ -748,7 +748,8 @@ apply_child_basequals(PlannerInfo *root, RelOptInfo *parentrel,
}
/* reconstitute RestrictInfo with appropriate properties */
childquals = lappend(childquals,
- make_restrictinfo((Expr *) onecq,
+ make_restrictinfo(root,
+ (Expr *) onecq,
rinfo->is_pushed_down,
rinfo->outerjoin_delayed,
pseudoconstant,
@@ -785,7 +786,7 @@ apply_child_basequals(PlannerInfo *root, RelOptInfo *parentrel,
/* not likely that we'd see constants here, so no check */
childquals = lappend(childquals,
- make_restrictinfo(qual,
+ make_restrictinfo(root, qual,
true, false, false,
security_level,
NULL, NULL, NULL));
diff --git a/src/backend/optimizer/util/orclauses.c b/src/backend/optimizer/util/orclauses.c
index a4de576fd8..d559f33826 100644
--- a/src/backend/optimizer/util/orclauses.c
+++ b/src/backend/optimizer/util/orclauses.c
@@ -268,7 +268,8 @@ consider_new_or_clause(PlannerInfo *root, RelOptInfo *rel,
* Build a RestrictInfo from the new OR clause. We can assume it's valid
* as a base restriction clause.
*/
- or_rinfo = make_restrictinfo(orclause,
+ or_rinfo = make_restrictinfo(root,
+ orclause,
true,
false,
false,
diff --git a/src/backend/optimizer/util/placeholder.c b/src/backend/optimizer/util/placeholder.c
index 2f21455c7b..1c4202d864 100644
--- a/src/backend/optimizer/util/placeholder.c
+++ b/src/backend/optimizer/util/placeholder.c
@@ -98,7 +98,7 @@ find_placeholder_info(PlannerInfo *root, PlaceHolderVar *phv,
* ph_eval_at. If no referenced rels are within the syntactic scope,
* force evaluation at the syntactic location.
*/
- rels_used = pull_varnos((Node *) phv->phexpr);
+ rels_used = pull_varnos(root, (Node *) phv->phexpr);
phinfo->ph_lateral = bms_difference(rels_used, phv->phrels);
if (bms_is_empty(phinfo->ph_lateral))
phinfo->ph_lateral = NULL; /* make it exactly NULL if empty */
diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c
index 221e1caa68..eb113d94c1 100644
--- a/src/backend/optimizer/util/restrictinfo.c
+++ b/src/backend/optimizer/util/restrictinfo.c
@@ -21,7 +21,8 @@
#include "optimizer/restrictinfo.h"
-static RestrictInfo *make_restrictinfo_internal(Expr *clause,
+static RestrictInfo *make_restrictinfo_internal(PlannerInfo *root,
+ Expr *clause,
Expr *orclause,
bool is_pushed_down,
bool outerjoin_delayed,
@@ -30,7 +31,8 @@ static RestrictInfo *make_restrictinfo_internal(Expr *clause,
Relids required_relids,
Relids outer_relids,
Relids nullable_relids);
-static Expr *make_sub_restrictinfos(Expr *clause,
+static Expr *make_sub_restrictinfos(PlannerInfo *root,
+ Expr *clause,
bool is_pushed_down,
bool outerjoin_delayed,
bool pseudoconstant,
@@ -56,7 +58,8 @@ static Expr *make_sub_restrictinfos(Expr *clause,
* later.
*/
RestrictInfo *
-make_restrictinfo(Expr *clause,
+make_restrictinfo(PlannerInfo *root,
+ Expr *clause,
bool is_pushed_down,
bool outerjoin_delayed,
bool pseudoconstant,
@@ -70,7 +73,8 @@ make_restrictinfo(Expr *clause,
* above each subclause of the top-level AND/OR structure.
*/
if (is_orclause(clause))
- return (RestrictInfo *) make_sub_restrictinfos(clause,
+ return (RestrictInfo *) make_sub_restrictinfos(root,
+ clause,
is_pushed_down,
outerjoin_delayed,
pseudoconstant,
@@ -82,7 +86,8 @@ make_restrictinfo(Expr *clause,
/* Shouldn't be an AND clause, else AND/OR flattening messed up */
Assert(!is_andclause(clause));
- return make_restrictinfo_internal(clause,
+ return make_restrictinfo_internal(root,
+ clause,
NULL,
is_pushed_down,
outerjoin_delayed,
@@ -99,7 +104,8 @@ make_restrictinfo(Expr *clause,
* Common code for the main entry points and the recursive cases.
*/
static RestrictInfo *
-make_restrictinfo_internal(Expr *clause,
+make_restrictinfo_internal(PlannerInfo *root,
+ Expr *clause,
Expr *orclause,
bool is_pushed_down,
bool outerjoin_delayed,
@@ -137,8 +143,8 @@ make_restrictinfo_internal(Expr *clause,
*/
if (is_opclause(clause) && list_length(((OpExpr *) clause)->args) == 2)
{
- restrictinfo->left_relids = pull_varnos(get_leftop(clause));
- restrictinfo->right_relids = pull_varnos(get_rightop(clause));
+ restrictinfo->left_relids = pull_varnos(root, get_leftop(clause));
+ restrictinfo->right_relids = pull_varnos(root, get_rightop(clause));
restrictinfo->clause_relids = bms_union(restrictinfo->left_relids,
restrictinfo->right_relids);
@@ -165,7 +171,7 @@ make_restrictinfo_internal(Expr *clause,
restrictinfo->left_relids = NULL;
restrictinfo->right_relids = NULL;
/* and get the total relid set the hard way */
- restrictinfo->clause_relids = pull_varnos((Node *) clause);
+ restrictinfo->clause_relids = pull_varnos(root, (Node *) clause);
}
/* required_relids defaults to clause_relids */
@@ -225,7 +231,8 @@ make_restrictinfo_internal(Expr *clause,
* contained rels.
*/
static Expr *
-make_sub_restrictinfos(Expr *clause,
+make_sub_restrictinfos(PlannerInfo *root,
+ Expr *clause,
bool is_pushed_down,
bool outerjoin_delayed,
bool pseudoconstant,
@@ -241,7 +248,8 @@ make_sub_restrictinfos(Expr *clause,
foreach(temp, ((BoolExpr *) clause)->args)
orlist = lappend(orlist,
- make_sub_restrictinfos(lfirst(temp),
+ make_sub_restrictinfos(root,
+ lfirst(temp),
is_pushed_down,
outerjoin_delayed,
pseudoconstant,
@@ -249,7 +257,8 @@ make_sub_restrictinfos(Expr *clause,
NULL,
outer_relids,
nullable_relids));
- return (Expr *) make_restrictinfo_internal(clause,
+ return (Expr *) make_restrictinfo_internal(root,
+ clause,
make_orclause(orlist),
is_pushed_down,
outerjoin_delayed,
@@ -266,7 +275,8 @@ make_sub_restrictinfos(Expr *clause,
foreach(temp, ((BoolExpr *) clause)->args)
andlist = lappend(andlist,
- make_sub_restrictinfos(lfirst(temp),
+ make_sub_restrictinfos(root,
+ lfirst(temp),
is_pushed_down,
outerjoin_delayed,
pseudoconstant,
@@ -277,7 +287,8 @@ make_sub_restrictinfos(Expr *clause,
return make_andclause(andlist);
}
else
- return (Expr *) make_restrictinfo_internal(clause,
+ return (Expr *) make_restrictinfo_internal(root,
+ clause,
NULL,
is_pushed_down,
outerjoin_delayed,
diff --git a/src/backend/optimizer/util/var.c b/src/backend/optimizer/util/var.c
index 19b2bba707..e307d6fbb0 100644
--- a/src/backend/optimizer/util/var.c
+++ b/src/backend/optimizer/util/var.c
@@ -23,6 +23,7 @@
#include "access/sysattr.h"
#include "nodes/nodeFuncs.h"
#include "optimizer/optimizer.h"
+#include "optimizer/placeholder.h"
#include "optimizer/prep.h"
#include "parser/parsetree.h"
#include "rewrite/rewriteManip.h"
@@ -31,6 +32,7 @@
typedef struct
{
Relids varnos;
+ PlannerInfo *root;
int sublevels_up;
} pull_varnos_context;
@@ -92,11 +94,12 @@ static Relids alias_relid_set(Query *query, Relids relids);
* SubPlan, we only need to look at the parameters passed to the subplan.
*/
Relids
-pull_varnos(Node *node)
+pull_varnos(PlannerInfo *root, Node *node)
{
pull_varnos_context context;
context.varnos = NULL;
+ context.root = root;
context.sublevels_up = 0;
/*
@@ -117,11 +120,12 @@ pull_varnos(Node *node)
* Only Vars of the specified level are considered.
*/
Relids
-pull_varnos_of_level(Node *node, int levelsup)
+pull_varnos_of_level(PlannerInfo *root, Node *node, int levelsup)
{
pull_varnos_context context;
context.varnos = NULL;
+ context.root = root;
context.sublevels_up = levelsup;
/*
@@ -159,33 +163,56 @@ pull_varnos_walker(Node *node, pull_varnos_context *context)
}
if (IsA(node, PlaceHolderVar))
{
- /*
- * A PlaceHolderVar acts as a variable of its syntactic scope, or
- * lower than that if it references only a subset of the rels in its
- * syntactic scope. It might also contain lateral references, but we
- * should ignore such references when computing the set of varnos in
- * an expression tree. Also, if the PHV contains no variables within
- * its syntactic scope, it will be forced to be evaluated exactly at
- * the syntactic scope, so take that as the relid set.
- */
PlaceHolderVar *phv = (PlaceHolderVar *) node;
- pull_varnos_context subcontext;
- subcontext.varnos = NULL;
- subcontext.sublevels_up = context->sublevels_up;
- (void) pull_varnos_walker((Node *) phv->phexpr, &subcontext);
+ /*
+ * If a PlaceHolderVar is not of the target query level, ignore it,
+ * instead recursing into its expression to see if it contains any
+ * vars that are of the target level.
+ */
if (phv->phlevelsup == context->sublevels_up)
{
- subcontext.varnos = bms_int_members(subcontext.varnos,
- phv->phrels);
- if (bms_is_empty(subcontext.varnos))
+ /*
+ * Ideally, the PHV's contribution to context->varnos is its
+ * ph_eval_at set. However, this code can be invoked before
+ * that's been computed. If we cannot find a PlaceHolderInfo,
+ * fall back to the conservative assumption that the PHV will be
+ * evaluated at its syntactic level (phv->phrels).
+ *
+ * There is a second hazard: this code is also used to examine
+ * qual clauses during deconstruct_jointree, when we may have a
+ * PlaceHolderInfo but its ph_eval_at value is not yet final, so
+ * that theoretically we could obtain a relid set that's smaller
+ * than we'd see later on. That should never happen though,
+ * because we deconstruct the jointree working upwards. Any outer
+ * join that forces delay of evaluation of a given qual clause
+ * will be processed before we examine that clause here, so the
+ * ph_eval_at value should have been updated to include it.
+ */
+ PlaceHolderInfo *phinfo = NULL;
+
+ if (phv->phlevelsup == 0)
+ {
+ ListCell *lc;
+
+ foreach(lc, context->root->placeholder_list)
+ {
+ phinfo = (PlaceHolderInfo *) lfirst(lc);
+ if (phinfo->phid == phv->phid)
+ break;
+ phinfo = NULL;
+ }
+ }
+ if (phinfo != NULL)
+ context->varnos = bms_add_members(context->varnos,
+ phinfo->ph_eval_at);
+ else
context->varnos = bms_add_members(context->varnos,
phv->phrels);
+ return false; /* don't recurse into expression */
}
- context->varnos = bms_join(context->varnos, subcontext.varnos);
- return false;
}
- if (IsA(node, Query))
+ else if (IsA(node, Query))
{
/* Recurse into RTE subquery or not-yet-planned sublink subquery */
bool result;
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index d5e61664bc..47ca4ddbb5 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -2206,7 +2206,7 @@ rowcomparesel(PlannerInfo *root,
/*
* Otherwise, it's a join if there's more than one relation used.
*/
- is_join_clause = (NumRelids((Node *) opargs) > 1);
+ is_join_clause = (NumRelids(root, (Node *) opargs) > 1);
}
if (is_join_clause)
@@ -4771,7 +4771,7 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
* membership. Note that when varRelid isn't zero, only vars of that
* relation are considered "real" vars.
*/
- varnos = pull_varnos(basenode);
+ varnos = pull_varnos(root, basenode);
onerel = NULL;
diff --git a/src/include/optimizer/clauses.h b/src/include/optimizer/clauses.h
index da3fc4df10..0673887a85 100644
--- a/src/include/optimizer/clauses.h
+++ b/src/include/optimizer/clauses.h
@@ -46,7 +46,7 @@ extern Var *find_forced_null_var(Node *clause);
extern bool is_pseudo_constant_clause(Node *clause);
extern bool is_pseudo_constant_clause_relids(Node *clause, Relids relids);
-extern int NumRelids(Node *clause);
+extern int NumRelids(PlannerInfo *root, Node *clause);
extern void CommuteOpExpr(OpExpr *clause);
diff --git a/src/include/optimizer/optimizer.h b/src/include/optimizer/optimizer.h
index 6235933ca4..d587952b7d 100644
--- a/src/include/optimizer/optimizer.h
+++ b/src/include/optimizer/optimizer.h
@@ -97,7 +97,8 @@ extern double clamp_row_est(double nrows);
/* in path/indxpath.c: */
-extern bool is_pseudo_constant_for_index(Node *expr, IndexOptInfo *index);
+extern bool is_pseudo_constant_for_index(PlannerInfo *root, Node *expr,
+ IndexOptInfo *index);
/* in plan/planner.c: */
@@ -188,8 +189,8 @@ extern SortGroupClause *get_sortgroupref_clause_noerr(Index sortref,
#define PVC_RECURSE_PLACEHOLDERS 0x0020 /* recurse into PlaceHolderVar
* arguments */
-extern Bitmapset *pull_varnos(Node *node);
-extern Bitmapset *pull_varnos_of_level(Node *node, int levelsup);
+extern Bitmapset *pull_varnos(PlannerInfo *root, Node *node);
+extern Bitmapset *pull_varnos_of_level(PlannerInfo *root, Node *node, int levelsup);
extern void pull_varattnos(Node *node, Index varno, Bitmapset **varattnos);
extern List *pull_vars_of_level(Node *node, int levelsup);
extern bool contain_var_clause(Node *node);
diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h
index 2d51cbecaa..035d3e1206 100644
--- a/src/include/optimizer/paths.h
+++ b/src/include/optimizer/paths.h
@@ -75,7 +75,8 @@ extern void create_index_paths(PlannerInfo *root, RelOptInfo *rel);
extern bool relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel,
List *restrictlist,
List *exprlist, List *oprlist);
-extern bool indexcol_is_bool_constant_for_query(IndexOptInfo *index,
+extern bool indexcol_is_bool_constant_for_query(PlannerInfo *root,
+ IndexOptInfo *index,
int indexcol);
extern bool match_index_to_operand(Node *operand, int indexcol,
IndexOptInfo *index);
diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.h
index 777655210b..bf1adfc52a 100644
--- a/src/include/optimizer/planmain.h
+++ b/src/include/optimizer/planmain.h
@@ -87,7 +87,8 @@ extern RestrictInfo *process_implied_equality(PlannerInfo *root,
Index security_level,
bool below_outer_join,
bool both_const);
-extern RestrictInfo *build_implied_join_equality(Oid opno,
+extern RestrictInfo *build_implied_join_equality(PlannerInfo *root,
+ Oid opno,
Oid collation,
Expr *item1,
Expr *item2,
diff --git a/src/include/optimizer/restrictinfo.h b/src/include/optimizer/restrictinfo.h
index 266faaf07c..0165ffde37 100644
--- a/src/include/optimizer/restrictinfo.h
+++ b/src/include/optimizer/restrictinfo.h
@@ -18,10 +18,11 @@
/* Convenience macro for the common case of a valid-everywhere qual */
-#define make_simple_restrictinfo(clause) \
- make_restrictinfo(clause, true, false, false, 0, NULL, NULL, NULL)
+#define make_simple_restrictinfo(root, clause) \
+ make_restrictinfo(root, clause, true, false, false, 0, NULL, NULL, NULL)
-extern RestrictInfo *make_restrictinfo(Expr *clause,
+extern RestrictInfo *make_restrictinfo(PlannerInfo *root,
+ Expr *clause,
bool is_pushed_down,
bool outerjoin_delayed,
bool pseudoconstant,
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 81b42c601b..5c7528c029 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -4756,6 +4756,42 @@ where ss.stringu2 !~* ss.case1;
(1 row)
rollback;
+-- test case to expose miscomputation of required relid set for a PHV
+explain (verbose, costs off)
+select i8.*, ss.v, t.unique2
+ from int8_tbl i8
+ left join int4_tbl i4 on i4.f1 = 1
+ left join lateral (select i4.f1 + 1 as v) as ss on true
+ left join tenk1 t on t.unique2 = ss.v
+where q2 = 456;
+ QUERY PLAN
+-------------------------------------------------------------
+ Nested Loop Left Join
+ Output: i8.q1, i8.q2, ((i4.f1 + 1)), t.unique2
+ -> Nested Loop Left Join
+ Output: i8.q1, i8.q2, (i4.f1 + 1)
+ -> Seq Scan on public.int8_tbl i8
+ Output: i8.q1, i8.q2
+ Filter: (i8.q2 = 456)
+ -> Seq Scan on public.int4_tbl i4
+ Output: i4.f1
+ Filter: (i4.f1 = 1)
+ -> Index Only Scan using tenk1_unique2 on public.tenk1 t
+ Output: t.unique2
+ Index Cond: (t.unique2 = ((i4.f1 + 1)))
+(13 rows)
+
+select i8.*, ss.v, t.unique2
+ from int8_tbl i8
+ left join int4_tbl i4 on i4.f1 = 1
+ left join lateral (select i4.f1 + 1 as v) as ss on true
+ left join tenk1 t on t.unique2 = ss.v
+where q2 = 456;
+ q1 | q2 | v | unique2
+-----+-----+---+---------
+ 123 | 456 | |
+(1 row)
+
-- bug #8444: we've historically allowed duplicate aliases within aliased JOINs
select * from
int8_tbl x join (int4_tbl x cross join int4_tbl y) j on q1 = f1; -- error
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 9887fe0c0b..6a209a27aa 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -1696,6 +1696,22 @@ where ss.stringu2 !~* ss.case1;
rollback;
+-- test case to expose miscomputation of required relid set for a PHV
+explain (verbose, costs off)
+select i8.*, ss.v, t.unique2
+ from int8_tbl i8
+ left join int4_tbl i4 on i4.f1 = 1
+ left join lateral (select i4.f1 + 1 as v) as ss on true
+ left join tenk1 t on t.unique2 = ss.v
+where q2 = 456;
+
+select i8.*, ss.v, t.unique2
+ from int8_tbl i8
+ left join int4_tbl i4 on i4.f1 = 1
+ left join lateral (select i4.f1 + 1 as v) as ss on true
+ left join tenk1 t on t.unique2 = ss.v
+where q2 = 456;
+
-- bug #8444: we've historically allowed duplicate aliases within aliased JOINs
select * from
diff --git a/src/backend/optimizer/util/var.c b/src/backend/optimizer/util/var.c
index 19b2bba707..ad275cba3a 100644
--- a/src/backend/optimizer/util/var.c
+++ b/src/backend/optimizer/util/var.c
@@ -159,33 +159,28 @@ pull_varnos_walker(Node *node, pull_varnos_context *context)
}
if (IsA(node, PlaceHolderVar))
{
- /*
- * A PlaceHolderVar acts as a variable of its syntactic scope, or
- * lower than that if it references only a subset of the rels in its
- * syntactic scope. It might also contain lateral references, but we
- * should ignore such references when computing the set of varnos in
- * an expression tree. Also, if the PHV contains no variables within
- * its syntactic scope, it will be forced to be evaluated exactly at
- * the syntactic scope, so take that as the relid set.
- */
PlaceHolderVar *phv = (PlaceHolderVar *) node;
- pull_varnos_context subcontext;
- subcontext.varnos = NULL;
- subcontext.sublevels_up = context->sublevels_up;
- (void) pull_varnos_walker((Node *) phv->phexpr, &subcontext);
+ /*
+ * If a PlaceHolderVar is not of the target query level, ignore it,
+ * instead recursing into its expression to see if it contains any
+ * vars that are of the target level.
+ */
if (phv->phlevelsup == context->sublevels_up)
{
- subcontext.varnos = bms_int_members(subcontext.varnos,
- phv->phrels);
- if (bms_is_empty(subcontext.varnos))
- context->varnos = bms_add_members(context->varnos,
- phv->phrels);
+ /*
+ * Ideally, the PHV's contribution to context->varnos is its
+ * ph_eval_at set. However, this code can be invoked before
+ * that's been computed, and we lack easy access to it anyway.
+ * Instead make the conservative assumption that the PHV will be
+ * evaluated at its syntactic level (phv->phrels).
+ */
+ context->varnos = bms_add_members(context->varnos,
+ phv->phrels);
+ return false; /* don't recurse into expression */
}
- context->varnos = bms_join(context->varnos, subcontext.varnos);
- return false;
}
- if (IsA(node, Query))
+ else if (IsA(node, Query))
{
/* Recurse into RTE subquery or not-yet-planned sublink subquery */
bool result;
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 81b42c601b..43d684a11a 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -4726,18 +4726,22 @@ from
left join uniquetbl u1 ON u1.f1 = t1.string4) ss
on t0.f1 = ss.case1
where ss.stringu2 !~* ss.case1;
- QUERY PLAN
---------------------------------------------------------------------------------------------
+ QUERY PLAN
+----------------------------------------------------------------------------------------------
Nested Loop
Join Filter: (CASE t1.ten WHEN 0 THEN 'doh!'::text ELSE NULL::text END = t0.f1)
- -> Nested Loop
- -> Seq Scan on int4_tbl i4
- -> Index Scan using tenk1_unique2 on tenk1 t1
- Index Cond: (unique2 = i4.f1)
- Filter: (stringu2 !~* CASE ten WHEN 0 THEN 'doh!'::text ELSE NULL::text END)
+ -> Hash Right Join
+ Hash Cond: (u1.f1 = t1.string4)
+ Filter: (t1.stringu2 !~* (CASE t1.ten WHEN 0 THEN 'doh!'::text ELSE NULL::text END))
+ -> Seq Scan on uniquetbl u1
+ -> Hash
+ -> Nested Loop
+ -> Seq Scan on int4_tbl i4
+ -> Index Scan using tenk1_unique2 on tenk1 t1
+ Index Cond: (unique2 = i4.f1)
-> Materialize
-> Seq Scan on text_tbl t0
-(9 rows)
+(13 rows)
select t0.*
from
@@ -4756,6 +4760,42 @@ where ss.stringu2 !~* ss.case1;
(1 row)
rollback;
+-- test case to expose miscomputation of required relid set for a PHV
+explain (verbose, costs off)
+select i8.*, ss.v, t.unique2
+ from int8_tbl i8
+ left join int4_tbl i4 on i4.f1 = 1
+ left join lateral (select i4.f1 + 1 as v) as ss on true
+ left join tenk1 t on t.unique2 = ss.v
+where q2 = 456;
+ QUERY PLAN
+-------------------------------------------------------------
+ Nested Loop Left Join
+ Output: i8.q1, i8.q2, ((i4.f1 + 1)), t.unique2
+ -> Nested Loop Left Join
+ Output: i8.q1, i8.q2, (i4.f1 + 1)
+ -> Seq Scan on public.int8_tbl i8
+ Output: i8.q1, i8.q2
+ Filter: (i8.q2 = 456)
+ -> Seq Scan on public.int4_tbl i4
+ Output: i4.f1
+ Filter: (i4.f1 = 1)
+ -> Index Only Scan using tenk1_unique2 on public.tenk1 t
+ Output: t.unique2
+ Index Cond: (t.unique2 = ((i4.f1 + 1)))
+(13 rows)
+
+select i8.*, ss.v, t.unique2
+ from int8_tbl i8
+ left join int4_tbl i4 on i4.f1 = 1
+ left join lateral (select i4.f1 + 1 as v) as ss on true
+ left join tenk1 t on t.unique2 = ss.v
+where q2 = 456;
+ q1 | q2 | v | unique2
+-----+-----+---+---------
+ 123 | 456 | |
+(1 row)
+
-- bug #8444: we've historically allowed duplicate aliases within aliased JOINs
select * from
int8_tbl x join (int4_tbl x cross join int4_tbl y) j on q1 = f1; -- error
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 9887fe0c0b..6a209a27aa 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -1696,6 +1696,22 @@ where ss.stringu2 !~* ss.case1;
rollback;
+-- test case to expose miscomputation of required relid set for a PHV
+explain (verbose, costs off)
+select i8.*, ss.v, t.unique2
+ from int8_tbl i8
+ left join int4_tbl i4 on i4.f1 = 1
+ left join lateral (select i4.f1 + 1 as v) as ss on true
+ left join tenk1 t on t.unique2 = ss.v
+where q2 = 456;
+
+select i8.*, ss.v, t.unique2
+ from int8_tbl i8
+ left join int4_tbl i4 on i4.f1 = 1
+ left join lateral (select i4.f1 + 1 as v) as ss on true
+ left join tenk1 t on t.unique2 = ss.v
+where q2 = 456;
+
-- bug #8444: we've historically allowed duplicate aliases within aliased JOINs
select * from
pgsql-hackers by date: